git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] submodule--helper: fix initialization of warn_if_uninitialized

From
Glen Choo <chooglen@google.com>
Date
Apr 27, 2022, 22:22 UTC
Message-ID
<kl6lczh2p3nv.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To
<xmqqtuagvq4n.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 23 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Is this a fix we can protect from future breakge by adding a test or
>> tweaking an existing test?  It is kind of surprising if we did not
>> have any test that runs "git submodule update" in a superproject
>> with initialized and uninitialized submodule(s) and make sure only
>> the initialized ones are updated.  It may be the matter of examining
>> the warning output that is currently ignored in such a test, if
>> there is one.
>
> Here is a quick-and-dirty one I came up with.  The superproject
> "super" has a handful of submodules ("submodule" and "rebasing"
> being two of them), so the new tests clone the superproject and
> initializes only one submodule.  Then we see how "submodule update"
> with pathspec works with these two submodules (one initialied and
> the other not).  In another test, we see how "submodule update"
> without pathspec works.
>
> I'll queue this on top of your fix for now tentatively.  If nobody
> finds flaws in them, I'll just squash it in soonish before merging
> the whole thing for the maintenance track.
>
> Thanks.
Thanks for adding the tests!
Show 47 quoted lines
>  t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git c/t/t7406-submodule-update.sh w/t/t7406-submodule-update.sh
> index 000e055811..43f779d751 100755
> --- c/t/t7406-submodule-update.sh
> +++ w/t/t7406-submodule-update.sh
> @@ -670,6 +670,39 @@ test_expect_success 'submodule update --init skips submodule with update=none' '
>  	)
>  '
>  
> +test_expect_success 'submodule update with pathspec warns against uninitialized ones' '
> +	test_when_finished "rm -fr selective" &&
> +	git clone super selective &&
> +	(
> +		cd selective &&
> +		git submodule init submodule &&
> +
> +		git submodule update submodule 2>err &&
> +		! grep "Submodule path .* not initialized" err &&
> +
> +		git submodule update rebasing 2>err &&
> +		grep "Submodule path .rebasing. not initialized" err &&
> +
> +		test_path_exists submodule/.git &&
> +		test_path_is_missing rebasing/.git
> +	)
> +
> +'
> +
> +test_expect_success 'submodule update without pathspec updates only initialized ones' '
> +	test_when_finished "rm -fr selective" &&
> +	git clone super selective &&
> +	(
> +		cd selective &&
> +		git submodule init submodule &&
> +		git submodule update 2>err &&
> +		test_path_exists submodule/.git &&
> +		test_path_is_missing rebasing/.git &&
> +		! grep "Submodule path .* not initialized" err
> +	)
> +
> +'
> +
>  test_expect_success 'submodule update continues after checkout error' '
>  	(cd super &&
>  	 git reset --hard HEAD &&

So we test that we only issue the warning when a pathspec is given, and that we ignore uninitialized submodules when no pathspec is given. I think this covers all of the cases, so this looks good, thanks!

Previous: Junio C HamanoNext: Glen Choo
Message 7 of 11 in “submodule--helper: fix initialization of warn_if_uninitialized”
  1. submodule--helper: fix initialization of warn_if_uninitializedOrgad Shaneh via GitGitGadget, Apr 24, 2022
  2. Junio C HamanoApr 25, 2022
  3. Orgad ShanehApr 25, 2022
  4. submodule--helper: fix initialization of warn_if_uninitializedOrgad Shaneh via GitGitGadget, Apr 25, 2022
  5. Junio C HamanoApr 25, 2022
  6. Junio C HamanoApr 25, 2022
  7. Glen ChooApr 27, 2022
  8. Glen ChooApr 27, 2022
  9. Glen ChooApr 27, 2022
  10. Glen ChooApr 27, 2022
  11. Junio C HamanoApr 27, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.