Thread (9 messages) 9 messages, 3 authors, 2020-12-09

Re: [PATCH v2] submodules: fix of regression on fetching of non-init subsub-repo

From: Peter Kästle <hidden>
Date: 2020-12-08 14:59:46

Possibly related (same subject, not in this thread)

On 07.12.20 20:22, Junio C Hamano wrote:
Peter Kaestle [off-list ref] writes:
quoted
+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
+}

quoted
+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-------------

quoted
+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.

quoted
+	# 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.
quoted
+
+	! git -C main fetch --recurse-submodules 2>&1 |
+		grep -v -m1 "Fetching submodule sub$" &&
Unfortunately, "grep -m<count>" 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;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help