From: Peter Kästle Date: Tue, 08 Dec 2020 14:58:39 GMT Subject: Re: [PATCH v2] submodules: fix of regression on fetching of non-init subsub-repo Message-ID: In-Reply-To: On 07.12.20 20:22, Junio C Hamano wrote: > Peter Kaestle writes: > >> +add_commit_push () { >> + dir="$1" && >> + msg="$2" && >> + shift 2 && >> + git -C "$dir" add "$@" && >> + git -C "$dir" commit -a -m "$msg" && >> + git -C "$dir" push >> +} >> + >> +compare_refs_in_dir () { >> + fail= && >> + if test "x$1" = 'x!' >> + then >> + fail='!' && >> + shift >> + fi && >> + git -C "$1" rev-parse --verify "$2" >expect && >> + git -C "$3" rev-parse --verify "$4" >actual && >> + eval $fail test_cmp expect actual >> +} > > > >> +test_expect_success 'fetching a superproject containing an uninitialized sub/sub project' ' >> + # depends on previous test for setup >> + >> + git -C B/ fetch && >> + compare_refs_in_dir A origin/master B origin/master > > Can we do this without relying on the name of the default branch? > Perhaps when outer, middle and inner are prepared, they can be > forced to be on the 'sample' (not 'master' nor 'main') branch, or > something like that? Using origin/HEAD for compare_refs_in_dir should be fine without additional setup, as for the regression the "git -C B/ fetch" will fail and return with false (see description of the patch). This compare_refs_in_dir is just for additional checking as you proposed in the mail: https://public-inbox.org/git/xmqqk0uuct94.fsf@gitster.c.googlers.com/ ------------8<------------- > And from B that was an original copy of A with only the top and > middle layer instantiated, you run "git fetch". Are you happy as > long as "git fetch" does not exit with non-zero status? That is > hard to believe---it may be a necessary condition for the command to > exit with zero status, but you have other expectations, like what > commit the remote tracking branch refs/remotes/origin/HEAD ought to > be pointing at. I think we should check that, too. ----------->8------------- > >> +test_expect_success 'setup recursive fetch with uninit submodule' ' >> + # does not depend on any previous test setups >> + >> + git init main && >> + git init sub && > > "super vs sub" would give us a better contrast than "main vs sub", > and it would help reduce mistakes in the mechanical conversion of > "master" to "main" happening in another topic. > ok. >> + # In a regression the following git call will run into infinite recursion. >> + # To handle that, we connect the grep command to the git call by a pipe >> + # so that grep can kill the infinite recusion when detected. >> + # The recursion creates git output like: >> + # Fetching submodule sub >> + # Fetching submodule sub/sub <-- [1] >> + # Fetching submodule sub/sub/sub >> + # ... >> + # [1] grep will trigger here and kill git by exiting and closing its stdin > > "trigger here and kill..." -> "stop reading and cause git to > eventually stop and die" > > But we probably cannot use 'grep -m1' so it is a moot point. > >> + >> + ! git -C main fetch --recurse-submodules 2>&1 | >> + grep -v -m1 "Fetching submodule sub$" && > > Unfortunately, "grep -m" is not even in POSIX, I would think. > > What do we expect to happen in the correct case? sigh, we can't use grep -m1. Too bad, it was such a nice solution. > > - A line "Fetching submodule sub" and nothing else is given? That > feels a bit brittle (how are we making sure, in the presence of > "2>&1", that we will not get any other output, like progress?) > > - "sub" is the only thing that appears on lines that begin with > "Fetching submodule" (i.e. "Fetching submodule $something" where > $something is not 'sub' is an error), and we allow other garbage > in the output? That would be a bit more robust than the above. > > As you seem to be comfortable using "sed" below, perhaps use it to > extract the first few lines that say "^Fetching submodule " from the > output and stop, and check that the output has only one such line > about 'sub' and nothing else? According to [1] posix sed offers equal possibility to quit like grep -m1 and I'll adopt: $> yes posixgrepisnogoodforus | sed "/posix/q" [1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html Looking at the other mails I think I processed over all open comments and will prepare for v3. Thanks again. -- kind regards --peter;