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

Re: [PATCH v2 0/9] submodule: tests, cleanup to prepare for built-in

From
Glen Choo <chooglen@google.com>
Date
Nov 8, 2022, 18:30 UTC
Message-ID
<kl6lh6z947ud.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To
<cover-v2-0.9-00000000000-20221108T140501Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 14 quoted lines
> Changes since v1:
>
> * Noted the history of when "submodule--helper config" was last used,
>   per Glen's comment.
> * Re-titled the tests htat no longer use the now-removed
>   "submodule--helper config", per Glen's comment.
> * I removed _() from a string in what's left of "submodule--helper
>   config", which is now a test helper. We should not be translating
>   test helper strings.
> * Fixed a buggy for-loop in the t7422-submodule-output.sh test I'm
>   adding.
> * Fixed a trivial typo in a comment in that test & in another nearby
>   comment.
>
[...]
Show 118 quoted lines
> Range-diff against v1:
>  1:  b2649f96715 !  1:  ca8f8b4bf63 submodule--helper: move "config" to a test-tool
>     @@ Commit message
>          'ab/submodule-helper-prep', 2022-09-13) the "config" sub-command was
>          only used by our own tests.
>      
>     +    It was last used by "git submodule" itself in code that went away with
>     +    a6226fd772b (submodule--helper: convert the bulk of cmd_add() to C,
>     +    2021-08-10).
>     +
>          Let's move it over, and while doing so make it easier to reason about
>          by splitting up the various uses for it into separate sub-commands, so
>          that we don't need to count arguments to see what it does.
>      
>     +    This also has the advantage that we stop wasting future translator
>     +    time on this command, currently the usage information for this
>     +    internal-only tool has been translated into several languages. The use
>     +    of the "_" function has also been removed from the "please make
>     +    sure..." message.
>     +
>          Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>      
>       ## builtin/submodule--helper.c ##
>     @@ t/helper/test-submodule.c: static int cmd__submodule_resolve_relative_url(int ar
>      +	/* Equivalent to ACTION_SET in builtin/config.c */
>      +	if (argc == 3) {
>      +		if (!is_writing_gitmodules_ok())
>     -+			die(_("please make sure that the .gitmodules file is in the working tree"));
>     ++			die("please make sure that the .gitmodules file is in the working tree");
>      +
>      +		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
>      +	}
>     @@ t/helper/test-submodule.c: static int cmd__submodule_resolve_relative_url(int ar
>      +
>      +	if (argc == 2) {
>      +		if (!is_writing_gitmodules_ok())
>     -+			die(_("please make sure that the .gitmodules file is in the working tree"));
>     ++			die("please make sure that the .gitmodules file is in the working tree");
>      +		return config_set_in_gitmodules_file_gently(argv[1], NULL);
>      +	}
>      +	usage_with_options(usage, options);
>     @@ t/helper/test-submodule.c: static int cmd__submodule_resolve_relative_url(int ar
>      
>       ## t/t7411-submodule-config.sh ##
>      @@ t/t7411-submodule-config.sh: test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
>     - test_expect_success 'reading submodules config from the working tree with "submodule--helper config"' '
>     + 	)
>     + '
>     + 
>     +-test_expect_success 'reading submodules config from the working tree with "submodule--helper config"' '
>     ++test_expect_success 'reading submodules config from the working tree' '
>       	(cd super &&
>       		echo "../submodule" >expect &&
>      -		git submodule--helper config submodule.submodule.url >actual &&
>     @@ t/t7411-submodule-config.sh: test_expect_success 'error in history in fetchrecur
>       	)
>       '
>       
>     - test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
>     +-test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
>     ++test_expect_success 'unsetting submodules config from the working tree' '
>       	(cd super &&
>      -		git submodule--helper config --unset submodule.submodule.url &&
>      -		git submodule--helper config submodule.submodule.url >actual &&
>     @@ t/t7411-submodule-config.sh: test_expect_success 'error in history in fetchrecur
>       		test_must_be_empty actual
>       	)
>       '
>     -@@ t/t7411-submodule-config.sh: test_expect_success 'unsetting submodules config from the working tree with "sub
>     - test_expect_success 'writing submodules config with "submodule--helper config"' '
>     + 
>     + 
>     +-test_expect_success 'writing submodules config with "submodule--helper config"' '
>     ++test_expect_success 'writing submodules config' '
>       	(cd super &&
>       		echo "new_url" >expect &&
>      -		git submodule--helper config submodule.submodule.url "new_url" &&
>     @@ t/t7411-submodule-config.sh: test_expect_success 'unsetting submodules config fr
>       		test_cmp expect actual
>       	)
>       '
>     -@@ t/t7411-submodule-config.sh: test_expect_success 'overwriting unstaged submodules config with "submodule--hel
>     + 
>     +-test_expect_success 'overwriting unstaged submodules config with "submodule--helper config"' '
>     ++test_expect_success 'overwriting unstaged submodules config' '
>       	test_when_finished "git -C super checkout .gitmodules" &&
>       	(cd super &&
>       		echo "newer_url" >expect &&
>  2:  cda36b5b6e0 !  2:  5508c27f653 submodule tests: add tests for top-level flag output
>     @@ t/t7422-submodule-output.sh (new)
>      +	git -C X pull &&
>      +	GIT_ALLOW_PROTOCOL=file git -C X submodule update --init &&
>      +
>     -+	# dirty p
>     ++	# dirty
>      +	for d in S.D X/S.D
>      +	do
>     -+		echo dirty >S.D/A.t || return 1
>     ++		echo dirty >"$d"/A.t || return 1
>      +	done &&
>      +
>      +	# commit (for --cached)
>     @@ t/t7422-submodule-output.sh (new)
>      +
>      +	for ref in A B C
>      +	do
>     -+		# Not different with SHA-1 and SHA-256, just (ab)usign
>     ++		# Not different with SHA-1 and SHA-256, just (ab)using
>      +		# test_oid_cache as a variable bag to avoid using
>      +		# $(git rev-parse ...).
>      +		oid=$(git rev-parse $ref) &&
>  -:  ----------- >  3:  a3529d7f9e0 submodule--helper: fix  a memory leak in "status"
>  3:  0ed1fc7fdf8 =  4:  c14cc0e14b8 submodule tests: test for a "foreach" blind-spot
>  4:  f7adfbc13ae =  5:  459ea25125b submodule.c: refactor recursive block out of absorb function
>  5:  2b8afd73b9b =  6:  322a02c30fc submodule API & "absorbgitdirs": remove "----recursive" option
>  6:  91208241070 =  7:  d1f4ac20a4f submodule--helper: remove --prefix from "absorbgitdirs"
>  7:  77d4d5a6c09 =  8:  ac9ff05ef68 submodule--helper: drop "update --prefix <pfx>" for "-C <pfx> update"
>  8:  105853cd358 =  9:  cccd92a829c submodule--helper: use OPT_SUBCOMMAND() API

Thanks! This addresses all of my comments from v1, and I didn't spot any other issues during a quick 2nd pass.

  Reviewed-by: Glen Choo <chooglen@google.com>
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 33 of 35 in “submodule: tests, cleanup to prepare for built-in”
  1. 0/8 submodule: tests, cleanup to prepare for built-inÆvar Arnfjörð Bjarmason, Nov 2, 2022
  2. 1/8 submodule--helper: move "config" to a test-toolÆvar Arnfjörð Bjarmason, Nov 2, 2022
  3. Glen ChooNov 3, 2022
  4. 2/8 submodule tests: add tests for top-level flag outputÆvar Arnfjörð Bjarmason, Nov 2, 2022
  5. Glen ChooNov 3, 2022
  6. 3/8 submodule tests: test for a "foreach" blind-spotÆvar Arnfjörð Bjarmason, Nov 2, 2022
  7. 4/8 submodule.c: refactor recursive block out of absorb functionÆvar Arnfjörð Bjarmason, Nov 2, 2022
  8. 5/8 submodule API & "absorbgitdirs": remove "----recursive" optionÆvar Arnfjörð Bjarmason, Nov 2, 2022
  9. Glen ChooNov 3, 2022
  10. Ævar Arnfjörð BjarmasonNov 4, 2022
  11. FW: [PATCH 5/8] submodule API & "absorbgitdirs": remove "----recursive" optionSimpson, Phyllis, Nov 4, 2022
  12. Glen ChooNov 4, 2022
  13. 6/8 submodule--helper: remove --prefix from "absorbgitdirs"Ævar Arnfjörð Bjarmason, Nov 2, 2022
  14. 7/8 submodule--helper: drop "update --prefix <pfx>" for "-C <pfx> update"Ævar Arnfjörð Bjarmason, Nov 2, 2022
  15. 8/8 submodule--helper: use OPT_SUBCOMMAND() APIÆvar Arnfjörð Bjarmason, Nov 2, 2022
  16. Glen ChooNov 3, 2022
  17. Ævar Arnfjörð BjarmasonNov 4, 2022
  18. Glen ChooNov 4, 2022
  19. Ævar Arnfjörð BjarmasonNov 5, 2022
  20. Glen ChooNov 7, 2022
  21. Glen ChooNov 4, 2022
  22. Taylor BlauNov 4, 2022
  23. 0/9 submodule: tests, cleanup to prepare for built-inÆvar Arnfjörð Bjarmason, Nov 8, 2022
  24. 1/9 submodule--helper: move "config" to a test-toolÆvar Arnfjörð Bjarmason, Nov 8, 2022
  25. 2/9 submodule tests: add tests for top-level flag outputÆvar Arnfjörð Bjarmason, Nov 8, 2022
  26. 3/9 submodule--helper: fix a memory leak in "status"Ævar Arnfjörð Bjarmason, Nov 8, 2022
  27. 5/9 submodule.c: refactor recursive block out of absorb functionÆvar Arnfjörð Bjarmason, Nov 8, 2022
  28. 6/9 submodule API & "absorbgitdirs": remove "----recursive" optionÆvar Arnfjörð Bjarmason, Nov 8, 2022
  29. 7/9 submodule--helper: remove --prefix from "absorbgitdirs"Ævar Arnfjörð Bjarmason, Nov 8, 2022
  30. 4/9 submodule tests: test for a "foreach" blind-spotÆvar Arnfjörð Bjarmason, Nov 8, 2022
  31. 8/9 submodule--helper: drop "update --prefix <pfx>" for "-C <pfx> update"Ævar Arnfjörð Bjarmason, Nov 8, 2022
  32. 9/9 submodule--helper: use OPT_SUBCOMMAND() APIÆvar Arnfjörð Bjarmason, Nov 8, 2022
  33. Glen ChooNov 8, 2022
  34. Ævar Arnfjörð BjarmasonNov 8, 2022
  35. Glen ChooNov 8, 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.