Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git submodules are not cloned #5536

Closed
bradrn opened this issue May 10, 2021 · 31 comments
Closed

Git submodules are not cloned #5536

bradrn opened this issue May 10, 2021 · 31 comments

Comments

@bradrn
Copy link
Contributor

bradrn commented May 10, 2021

General summary/comments

I was testing haskell-game/dear-imgui.hs. However, I was not able to build this package as detailed in haskell-game/dear-imgui.hs#58. Upon closer examination, the problem turns out to be that this package relies on the availability of the dear-imgui C++ library in a known location; this library was distributed as a Git submodule, which Stack had not cloned when cloning dear-imgui.hs.

Steps to reproduce

Add dear-imgui as an extra-dep:

extra-deps:
    - git: https://github.com/haskell-game/dear-imgui.hs
      commit: 73eee5fc9e6bec591dfa90e1909609dfea1e9536

Then run stack build.

Expected

Git submodules are cloned allowing package to build successfully.

Actual

Git submodules are not cloned

Stack version

> stack --version
Version 2.7.1, Git revision 8afe0c2932716b0441cf4440d6942c59568b6b19 x86_64 hpack-0.34.4

Method of installation

Official binary, downloaded from stackage.org or fpcomplete's package repository

@jorpic
Copy link
Contributor

jorpic commented Jan 20, 2023

Just tried to reproduce this with a recent version of stack and everything works as expected — git submodule is cloned and dear-imgui.hs is built successfully.

Don't have any ideas why it was broken. Seems like v2.7.1 stack depends on pantry >=0.5.1.3 and pantry at that version had the same code to update submodules as in the latest one.

$ stack --version
Version 2.9.1, Git revision 409d56031b4240221d656db09b2ba476fe6bb5b1 x86_64 hpack-0.35.0

stack.yaml

resolver: lts-20.0
packages:
- .

extra-deps:
    - git: https://github.com/haskell-game/dear-imgui.hs
      commit: 8697aa3a0a084754d36798db1423b7a9393cf147

@jship
Copy link
Contributor

jship commented Feb 21, 2024

This seems to be a Windows-specific bug. On Mac stack, the submodule will be pulled in, but on Windows stack the submodule doesn't get pulled in and I get the same error described in haskell-game/dear-imgui.hs#58.

Stack Version

  • Windows: Version 2.13.1, Git revision 8102bb8afce90fc954f48efae38b87f37cabc988 x86_64 hpack-0.36.0
  • Mac: Version 2.13.1, Git revision 8102bb8afce90fc954f48efae38b87f37cabc988 (9949 commits) aarch64 hpack-0.36.0

Method of installation

Via ghcup on both Mac and Windows.

@mpilgrem
Copy link
Member

@jship, that is a puzzle because I am pretty confident that the code in the pantry library is the same on both operating systems. I am wondering if it could be something to do with the git executable on Windows. What version of git would Stack be using?

@jship
Copy link
Contributor

jship commented Feb 22, 2024

@mpilgrem I dug into this some more and I might have some clues on the problem. Running stack -v setup in a clean environment, the log snippet relevant to cloning dear-imgui.hs.git is:

2024-02-22 09:23:12.175578: [info] Cloning dbf8d42721eae75db0426f3504b823d0355423a6 from https://github.com/jship/dear-imgui.hs.git
2024-02-22 09:23:12.176578: [debug] Run process: C:\Program Files\Git\mingw64\bin\git.EXE clone https://github.com/jship/dear-imgui.hs.git C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned
2024-02-22 09:23:13.302577: [debug] Process finished in 1126ms: C:\Program Files\Git\mingw64\bin\git.EXE clone https://github.com/jship/dear-imgui.hs.git C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned
2024-02-22 09:23:13.304576: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE reset --hard dbf8d42721eae75db0426f3504b823d0355423a6
2024-02-22 09:23:13.408578: [debug] Process finished in 105ms: C:\Program Files\Git\mingw64\bin\git.EXE reset --hard dbf8d42721eae75db0426f3504b823d0355423a6
2024-02-22 09:23:13.409577: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE submodule update --init --recursive
2024-02-22 09:23:31.652667: [debug] Process finished in 18243ms: C:\Program Files\Git\mingw64\bin\git.EXE submodule update --init --recursive
2024-02-22 09:23:31.653667: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE -c core.autocrlf=false archive -o C:\Users\jason\AppData\Local\Temp\with-repo-archive36944\foo.tar HEAD
2024-02-22 09:23:31.706666: [debug] Process finished in 52ms: C:\Program Files\Git\mingw64\bin\git.EXE -c core.autocrlf=false archive -o C:\Users\jason\AppData\Local\Temp\with-repo-archive36944\foo.tar HEAD
2024-02-22 09:23:31.706666: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE --version
2024-02-22 09:23:31.723667: [debug] Process finished in 18ms: C:\Program Files\Git\mingw64\bin\git.EXE --version
2024-02-22 09:23:31.724666: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE submodule foreach --recursive "git -c core.autocrlf=false archive --prefix=$displaypath/ -o bar.tar HEAD; tar --force-local  -Af C:\\Users\\jason\\AppData\\Local\\Temp\\with-repo-archive36944\\foo.tar bar.tar"
2024-02-22 09:23:32.452667: [debug] Process finished in 728ms: C:\Program Files\Git\mingw64\bin\git.EXE submodule foreach --recursive "git -c core.autocrlf=false archive --prefix=$displaypath/ -o bar.tar HEAD; tar --force-local  -Af C:\\Users\\jason\\AppData\\Local\\Temp\\with-repo-archive36944\\foo.tar bar.tar"
2024-02-22 09:23:32.551670: [debug] parseArchive of GZIP-ed tar file: ZlibException (-3)

The ZLibException at the bottom is spurious as far as I can tell, as that's reported for any other cloned repos specified in extra-deps too, and no issues on those other deps.

I converted the steps stack is executing into powershell so I could run it line-by-line and check each result: script.txt. The script includes comments where tar archive contents are listed in full.

The problem seems to be this command, where for each submodule, a bar.tar archive is created then concatenated with the main foo.tar archive:

2024-02-22 09:23:31.724666: [debug] Run process within C:\Users\jason\AppData\Local\Temp\with-repo36945\cloned: C:\Program Files\Git\mingw64\bin\git.EXE submodule foreach --recursive "git -c core.autocrlf=false archive --prefix=$displaypath/ -o bar.tar HEAD; tar --force-local  -Af C:\\Users\\jason\\AppData\\Local\\Temp\\with-repo-archive36944\\foo.tar bar.tar"

The gist is that when stack creates an auxiliary bar.tar archive containing the submodule's contents, the files in the tar listing are all prefixed with C:/Program Files/Git and they should instead be prefixed with imgui:

tar --list --file .\imgui\bar.tar
# ... bunch of files ...
C:/Program Files/Git/backends/
C:/Program Files/Git/backends/imgui_impl_allegro5.cpp
C:/Program Files/Git/backends/imgui_impl_allegro5.h
C:/Program Files/Git/backends/imgui_impl_android.cpp
# ... bunch of files ...

The main foo.tar containing the Haskell repo's contents has correct paths for the non-submodule bits:

# > tar --list --file ..\..\with-repo-archive36944\foo.tar
# ... bunch of files ...
cabal.project
dear-imgui.cabal
default.nix
examples/
examples/Readme.hs
examples/fonts/
examples/fonts/Main.hs
examples/fonts/NotoSansJP-Regular.otf
# ... bunch of files ...

bar.tar does successfully get included into foo.tar, so the submodule was technically cloned and included, it's just the prefixes on all entries in bar.tar are invalid.

It looks like the issue is regarding quoting/variable expansion. When I run this command to produce a test.tar for the imgui submodule:

git submodule foreach --recursive "git -c core.autocrlf=false archive --prefix=$displaypath/ -o test.tar HEAD"

$displaypath is expanded too early because of the git command being in double-quotes. It's equivalent to running this (which also produces the C:/Program Files/Git/ prefixes):

git submodule foreach --recursive "git -c core.autocrlf=false archive --prefix=/ -o test.tar HEAD"

Changing double-quotes in the command to single-quotes produces a correct imgui prefix:

git submodule foreach --recursive 'git -c core.autocrlf=false archive --prefix=$displaypath/ -o test.tar HEAD'

This manual testing is not totally apples-to-apples with stack though since I'm running at the shell (where I'm definitely subject to early variable expansion) and stack is executing subprocesses. I see that stack's "Run process..." argument output comes from RIO's showProcessArgDebug, which is show-ing arg strings containing spaces (producing the double-quotes in the log output), so it's not clear to me exactly what command arg format the underlying process/typed-process calls are receiving or if variable expansion is even involved there at all. 🤔

Assuming early-expansion is a possibility, perhaps this Pantry code needs explicit single quotes?

@mpilgrem mpilgrem reopened this Feb 22, 2024
@mpilgrem
Copy link
Member

@jship, thanks for the investigation. I've reopened the issue while I look into what you have identified.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 22, 2024

For my own reference: Link to foreach documentation: https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-foreach--recursiveltcommandgt

$displaypath contains the relative path from the current working directory to the submodules root directory

What shell is git submodule foreach using? I can't see that git documents that, but this 2020 post suggests that it is sh on Windows: https://stackoverflow.com/questions/65057512/set-a-different-shell-for-git-submodule-foreach.

Quoting rules in PowerShell: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-7.4

@jship
Copy link
Contributor

jship commented Feb 22, 2024

What shell is git submodule foreach using? I can't see that git documents that, but this 2020 post suggests that it is sh on Windows: https://stackoverflow.com/questions/65057512/set-a-different-shell-for-git-submodule-foreach.

Nice find! So early variable expansion may actually be in play here.

Quoting rules in PowerShell: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-7.4

As another data point, I've tried the dear-imgui.hs build both in PowerShell and Git Bash and the result is the same. Both shells use the same installed git version at C:\Program Files\Git\bin\git.exe on my system, though Git Bash gives a more unix-y shell environment to work in. The build failing in both cases tracks too, considering it sounds like the issue is more with the git binary's shell invocation for the submodule foreach.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 22, 2024

@jship, I am looking at Pantry.Repo.archiveSubmodules and wondering if RIO.Process.proc puts quotation marks around arguments that contain spaces. I'm looking at: https://hackage.haskell.org/package/process-1.6.18.0/src/System/Process/Windows.hsc. I think the answer is 'Yes'.

@jship
Copy link
Contributor

jship commented Feb 22, 2024

Yeah, it looks like Window.hsc's commandToProcess puts double-quotes around each argument, regardless of spaces or no spaces. This is the ghci fragment I used to verify:

ghci> :{
ghci| translateInternal :: String -> String
ghci| translateInternal xs = '"' : snd (foldr escape (True,"\"") xs)
ghci|   where escape '"'  (_,     str) = (True,  '\\' : '"'  : str)
ghci|         escape '\\' (True,  str) = (True,  '\\' : '\\' : str)
ghci|         escape '\\' (False, str) = (False, '\\' : str)
ghci|         escape c    (_,     str) = (False, c : str)
ghci| :}
ghci>
ghci> :{
ghci| data RawCommand = RawCommand FilePath [String]
ghci| :}
ghci>
ghci> :{
ghci| test :: RawCommand -> String
ghci| test (RawCommand cmd args) =
ghci|   translateInternal cmd ++ concatMap ((' ':) . translateInternal) args
ghci| :}
ghci>
ghci> :{
ghci| putStrLn $ test $ RawCommand "git"
ghci|   [ "submodule"
ghci|   , "foreach"
ghci|   , "--recursive"
ghci|   , "git -c core.autocrlf=false archive --prefix=$displaypath/ -o bar.tar HEAD"
ghci|   ]
ghci| :}
"git" "submodule" "foreach" "--recursive" "git -c core.autocrlf=false archive --prefix=$displaypath/ -o bar.tar HEAD"

@mpilgrem
Copy link
Member

mpilgrem commented Feb 22, 2024

The problem Stack/Pantry has is that it does not know the shell that is being used on Windows to run the git command - Command Prompt (unlikely these days, but cannot be ruled out), PowerShell (most likely), something else (possible). Perhaps Pantry should run the git command in sh on Windows. That would not solve the double quotation marks problem.

EDIT: If Pantry could be confident of PowerShell, it could put a backtick character before the $displaypath to stop it being expanded within the double quotation marks.

@jship
Copy link
Contributor

jship commented Feb 22, 2024

Another angle as food for thought: I wonder if we could sidestep this shell complexity by using git ls-files --recurse-submodules to get all files in the repo including submodule files, and skip the whole bit of creating individual bar.tar archives for each submodule. The --recurse-submodules argument has been around since v2.11 of git released in 11/2016, so it seems likely we could assume the user's git version is 2.11+.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 22, 2024

I think it is acceptable for Pantry/Stack to specify a minimum version of git as a pre-requisite, so I'll look into your suggestion.

EDIT: For my own reference: https://git-scm.com/docs/git-ls-files#Documentation/git-ls-files.txt---recurse-submodules

@mpilgrem
Copy link
Member

@jship, I've opened a corresponding issue at the pantry repository.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 24, 2024

@jship, I am starting to question whether it is actually premature variable expansion by PowerShell before git submodule foreach receives the command argument, because I can't seem to replicate variable expansion with a small test program. If I use:

module Main (main) where

import System.Process.Typed ( proc, readProcess )

main :: IO ()
main = do
  (_, output, err) <- readProcess $ proc "cmd" ["/c", "--prefix=$displaypath/"]
  print output
  print err

I get (no expansion):

""
"'\"--prefix=$displaypath/' is not recognized as an internal or external command,\r\noperable program or batch file.\r\n"

At a PowerShell command line, the same cmd c/ --prefix=$displaypath/ yields:

'--prefix' is not recognized as an internal or external command,
operable program or batch file.

EDIT: That is, perhaps it is premature variable expansion while the command argument is in the hands of git itself, before foreach gets to apply it.

@mpilgrem
Copy link
Member

After some experimentation, if I command (on Windows, in PowerShell):

stack exec -- git submodule foreach --recursive 'echo $SHELL'

it yields (and the same without the stack exec --, outside of the Stack environment):

Entering 'imgui'
/usr/bin/bash

and git submodule foreach --recursive 'echo $Host.Name' yields:

Entering 'imgui'
.Name

so, the PowerShell variable $Host is not recognised in the foreach environment.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 24, 2024

@jship, I created a small executable from the code below, and it worked fine when executed in a clone of the repository containing the submodule. I think working through Stack's steps manually in PowerShell may have been misleading.

module Main (main) where

import System.Process.Typed ( proc, readProcess )

main :: IO ()
main = do
  (_, output, err) <- 
    readProcess $ proc
      "git"
      [ "submodule"
      , "foreach"
      , "--recursive"
      , "git archive --prefix=$displaypath/ -o bar.tar HEAD"
      ]
  print output
  print err

I am going to start again, from the beginning. First step is to see if I can reproduce the problem you are experiencing. It would help me if you could be more precise about what you were doing.

EDIT: I tried to build the 'Hello World' example at https://github.com/haskell-game/dear-imgui.hs but sdl2-2.5.5.0 will not build; it complains about undefined symbols __stack_chk_fail and __stack_chk_guard. Perhaps that is something to do with sdl2-2.5.5.0 requiring SDL 2.0.18 and the MSYS2 project providing version 2.30.0-1.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

I'll shoot for getting a repo put together with a minimal repro later today. Thanks for digging into this!

Yeah the SDL bit is a known thing where newer versions of SDL in msys are giving us that issue. It's been reported upstream with the msys packaging maintainers as far as I'm aware, but for now we're kind of stuck on Windows installing an older version (2.24 iirc) via stack's bundled pacman. I'd link to that issue but don't have the easiest access to it at the moment.

@mpilgrem
Copy link
Member

@jship, with that pointer, I had no difficulty in building the ImGui 'Hello World' executable on Windows 11 with Stack 2.15.1. I encountered no problem with submodule cloning.

My Stack configuration was:

snapshot: lts-22.11 # GHC 9.6.4
extra-deps:
- git: https://github.com/haskell-game/dear-imgui.hs.git
  commit: bab4d769eaacba2de5da342e853f18746db0e12c
- megaparsec-9.2.2@sha256:c306a135ec25d91d252032c6128f03598a00e87ea12fcf5fc4878fdffc75c768,3219
flags:
  dear-imgui:
    sdl: true
    opengl3: true

My package.yaml included:

dependencies:
- base >= 4.7 && < 5
- dear-imgui
- gl
- managed
- sdl2

Using stack exec -- pacman, I had to install:

-S mingw-w64-x86_64-pkgconf
-U mingw-w64-x86_64-SDL2-2.24.1-1-any.pkg.tar.zst # From archive at https://repo.msys2.org/mingw/mingw64/
-S mingw-w64-x86_64-glew

@jship
Copy link
Contributor

jship commented Feb 24, 2024

Hmm, very interesting. 🤔 I made a repro where it consistently fails for me on Windows 10 in both Powershell and Git Bash: https://github.com/jship/stack-repro-num5536/. The repro uses dear-imgui.hs's build flags so that it doesn't require SDL or anything extra.

powershell
git-bash

I'm on stack Version 2.13.1, Git revision 8102bb8afce90fc954f48efae38b87f37cabc988 x86_64 hpack-0.36.0 and git version 2.42.0.windows.2.

I'll try upgrading stack to 2.15.1 and see if the behavior's any different.

@mpilgrem
Copy link
Member

In case it is relevant, I have Git for Windows on my path. git --version is git version 2.40.0.windows.1.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

Some differences between our setups that are possibly worth calling out:

  • My repro uses GHC 9.4.8 (via lts-21.25) vs your successful test using GHC 9.6.4 (via lts-22.11)
  • My repro uses dear-imgui.hs HEAD (b48ef7904b10fe467b07088c452b6a64c1791409) vs your successful test which uses the commit for v2.2.0. I need to use HEAD because it includes Link against system-cxx-std-lib for GHC 9.4+ haskell-game/dear-imgui.hs#197, which was a different issue I encountered when attempting Windows builds. It's surprising that separate issue also didn't bite you in your test.

EDIT: For the second point, it's no longer surprising because that one required both dear-imgui and file-embed being used in the same project.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

Just tried on stack Version 2.15.1, Git revision 427cc21fc723a039e2eefad02fa92e54f5e414f7 x86_64 hpack-0.36.0 and no dice with that version either unfortunately

@jship
Copy link
Contributor

jship commented Feb 24, 2024

Browsing the Git for Windows release notes and the Git release notes for our versions and in-between, and nothing's jumping out at me on that angle.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

Also gave building the same version of dear-imgui as you did in the successful test (bab4d769eaacba2de5da342e853f18746db0e12c) a shot, but that also failed. This makes sense that the behavior wouldn't be any different based on the changes between 2.2.0 and HEAD, but was worth a shot.

@mpilgrem
Copy link
Member

@jship, I am wondering if the local pantry database is corrupt. I note that the screen shots have:

Warning: 'include-dirs: imgui' specifies a directory which does not exist.

However, that directory would have existed once the repository was cloned (ignoring the submodule fetch).

Can you try deleting the pantry directory in the Stack root? Stack will recreate what it needs.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

I've been deleting the whole stack root as well as the project's individual .stack-work directories before attempting each test, so each test has been starting from scratch stack-wise.

@jship
Copy link
Contributor

jship commented Feb 24, 2024

However, that directory would have existed once the repository was cloned (ignoring the submodule fetch).

I think with the build of dear-imgui happening at a ...\Temp\stack-<uuid> directory and not the location where the clone happened (which is of the form ...\Temp\with-repo<XXXXX>\cloned), that suggests that even though the repo clone + git submodule update --init --recursive were performed, the submodule didn't make it into the tar file that was eventually unpacked into ...\Temp\stack-<uuid>.

@mpilgrem
Copy link
Member

mpilgrem commented Feb 25, 2024

@jship, I deleted my Stack root, and now I am experiencing the same problem as you! Perhaps I was not building with Stack 2.15.1 but with my not-quite-fixed Stack. EDIT: That now looks to be the explanation. Sorry for the confusion.

@mpilgrem
Copy link
Member

@jorpic, I have identified the problem. In the foreach environment, an absolute path for tarball like C:\Users\mike\AppData\Local\Tempwith-repo-archive6276\foo.tar is being interpreted as C:UsersmikeAppDataLocalTempwith-repo-archive6276foo.tar. The fix is, I think, simple. I hope to have pull requests available shortly.

mpilgrem added a commit that referenced this issue Feb 25, 2024
mpilgrem added a commit that referenced this issue Feb 25, 2024
@mpilgrem
Copy link
Member

@jship, this should be fixed in the master branch version of Stack (if you are not using GHCup to manage versions of Stack, stack upgrade --source-only --git).

@jship
Copy link
Contributor

jship commented Feb 25, 2024

@mpilgrem Thank you! I tried the updated stack using both PowerShell and Git Bash and it worked like a charm.

mpilgrem added a commit that referenced this issue Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants