git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Peter Kaestle <peter.kaestle@nokia.com>
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.us>,
	Ralf Thielow <ralf.thielow@gmail.com>
Subject: Re: [PATCH v2] submodules: fix of regression on fetching of non-init subsub-repo
Date: Mon, 07 Dec 2020 11:22:42 -0800	[thread overview]
Message-ID: <xmqq360hbev1.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <1607348819-61355-1-git-send-email-peter.kaestle@nokia.com> (Peter Kaestle's message of "Mon, 7 Dec 2020 14:46:59 +0100")

Peter Kaestle <peter.kaestle@nokia.com> 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?

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

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

  parent reply	other threads:[~2020-12-07 19:23 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02 15:56 BUG in fetching non-checked out submodule Ralf Thielow
2020-12-02 17:19 ` Philippe Blain
2020-12-02 23:06   ` Junio C Hamano
2020-12-03  7:54     ` Peter Kästle
2020-12-03 15:25       ` Philippe Blain
2020-12-03 15:33         ` Peter Kästle
2020-12-03 18:12           ` Junio C Hamano
2020-12-04 15:23           ` [PATCH] submodules: fix of regression on fetching of non-init subsub-repo Peter Kaestle
2020-12-04 18:06             ` Eric Sunshine
2020-12-07  8:28               ` Peter Kästle
2020-12-07  8:40                 ` Eric Sunshine
2020-12-07 13:46                   ` [PATCH v2] " Peter Kaestle
2020-12-07 18:42                     ` Philippe Blain
2020-12-07 19:43                       ` Junio C Hamano
2020-12-08  8:46                         ` Peter Kästle
2020-12-07 19:56                       ` Junio C Hamano
2020-12-08 14:06                       ` Peter Kästle
2020-12-07 19:22                     ` Junio C Hamano [this message]
2020-12-07 20:44                       ` Philippe Blain
2020-12-07 21:02                         ` Junio C Hamano
2020-12-07 21:10                           ` Junio C Hamano
2020-12-08 14:58                       ` Peter Kästle
2020-12-08 15:42                         ` [PATCH v3] " Peter Kaestle
2020-12-08 15:51                           ` Peter Kästle
2020-12-08 20:46                           ` Junio C Hamano
2020-12-08 23:25                           ` Philippe Blain
2020-12-09  9:58                             ` Peter Kästle
2020-12-09 10:58                               ` [PATCH v4] " Peter Kaestle
2020-12-09 14:00                                 ` Philippe Blain
2020-12-03  7:45   ` BUG in fetching non-checked out submodule Ralf Thielow
2020-12-03  8:20     ` Peter Kästle
2020-12-03  9:38       ` Ralf Thielow
2020-12-03  9:43         ` Peter Kästle
2020-12-03 12:30           ` Ralf Thielow
2020-12-03 15:10             ` Peter Kästle
2020-12-03 16:45               ` Ralf Thielow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq360hbev1.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=peter.kaestle@nokia.com \
    --cc=ralf.thielow@gmail.com \
    --cc=sunshine@sunshineco.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).