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

Re: [PATCH 1/2] push: --dry-run updates submodules when --recurse-submodules=on-demand

From
BWBrandon Williams <bmwill@google.com>
Date
Nov 15, 2016, 17:29 UTC
Message-ID
<20161115172909.GA66382@google.com>
In-Reply-To
<4a72ad14-0a8f-ede9-9f54-601fcd37740b@kdbg.org>
On 11/15, Johannes Sixt wrote:
Show 16 quoted lines
> Am 15.11.2016 um 02:18 schrieb Brandon Williams:
> >diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> >index 198ce84..e6ccc30 100755
> >--- a/t/t5531-deep-submodule-push.sh
> >+++ b/t/t5531-deep-submodule-push.sh
> >@@ -427,7 +427,31 @@ test_expect_success 'push unpushable submodule recursively fails' '
> > 		cd submodule.git &&
> > 		git rev-parse master >../actual
> > 	) &&
> >-	test_cmp expected actual
> >+	test_cmp expected actual &&
> >+	git -C work reset --hard master^
> 
> This line looks like a clean-up to be done after the test case. You
> should wrap it in test_when_finished, but outside of a sub-shell,
> which looks like it's just one line earlier, before the test_cmp.
K will do.
Show 17 quoted lines
> 
> >+'
> >+
> >+test_expect_failure 'push --dry-run does not recursively update submodules' '
> >+	(
> >+		cd work &&
> >+		(
> >+			cd gar/bage &&
> >+			git checkout master &&
> >+			git rev-parse master >../../../expected_submodule &&
> >+			> junk9 &&
> >+			git add junk9 &&
> >+			git commit -m "Ninth junk"
> >+		) &&
> 
> Could you please avoid this nested sub-shell? It is fine to cd
> around when you are in a sub-shell.

Yes I can reorganize it to avoid the nested sub-shells. I was just trying to follow the organization of the other tests in the same file.

Show 17 quoted lines
> 
> >+		git checkout master &&
> >+		git rev-parse master >../expected_pub
> 
> Broken && chain.
> 
> >+		git add gar/bage &&
> >+		git commit -m "Ninth commit for gar/bage" &&
> >+		git push --dry-run --recurse-submodules=on-demand ../pub.git master
> >+	) &&
> >+	git -C submodule.git rev-parse master >actual_submodule &&
> >+	git -C pub.git rev-parse master >actual_pub &&
> 
> All of the commands above are 'git something' that could become 'git
> -C work something' and then the sub-shell would be unnecessary. I'm
> not sure I would appreciate the verbosity of the result, though.
> (Perhaps aligning the git subcommands after -C foo would help.)

I'll play around with it and try to make it look pretty while trying to avoid sub-shells. I'm assuming the reason we want to avoid sub-shells is for performance reasons right?

-- 
Brandon Williams
Previous: Johannes SixtNext: Johannes Sixt
Message 4 of 16 in “bug fix with push --dry-run and submodules”
  1. 0/2 bug fix with push --dry-run and submodulesBrandon Williams, Nov 15, 2016
  2. 1/2 push: --dry-run updates submodules when --recurse-submodules=on-demandBrandon Williams, Nov 15, 2016
  3. Johannes SixtNov 15, 2016
  4. Brandon WilliamsNov 15, 2016
  5. Johannes SixtNov 15, 2016
  6. 2/2 push: fix --dry-run to not push submodulesBrandon Williams, Nov 15, 2016
  7. 0/2 bug fix with push --dry-run and submodulesBrandon Williams, Nov 17, 2016
  8. 1/2 push: --dry-run updates submodules when --recurse-submodules=on-demandBrandon Williams, Nov 17, 2016
  9. 2/2 push: fix --dry-run to not push submodulesBrandon Williams, Nov 17, 2016
  10. Stefan BellerNov 17, 2016
  11. Brandon WilliamsNov 17, 2016
  12. Junio C HamanoNov 22, 2016
  13. Brandon WilliamsNov 22, 2016
  14. Junio C HamanoNov 23, 2016
  15. Stefan BellerNov 17, 2016
  16. Brandon WilliamsNov 17, 2016

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.