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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2020, 19:22 UTC
Message-ID: <xmqq360hbev1.fsf@gitster.c.googlers.com>
In-Reply-To: <1607348819-61355-1-git-send-email-peter.kaestle@nokia.com>

Peter Kaestle <peter.kaestle@nokia.com> writes:

Show 20 quoted lines

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

Show 5 quoted lines

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

Show 5 quoted lines

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

Show 9 quoted lines

> +	# 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<count>" is not even in POSIX, I would think.

What do we expect to happen in the correct case?

 - 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?

> +	git -C main submodule status >out &&
> +	sed -e "s/^-//" -e "s/ sub$//" out >actual &&
> +	test_cmp expect actual