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

[PATCH v2] doc-diff: don't `cd_to_toplevel` before calling `usage`

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Feb 3, 2019, 11:08 UTC
Message-ID
<20190203110817.17242-1-martin.agren@gmail.com>
In-Reply-To
<CAPig+cT32oi5HLe7tokghrCmaiECwAXxON8U7DrpMYb3GCSNyQ@mail.gmail.com>

`usage` tries to call $0, which might very well be "./doc-diff", so if we `cd_to_toplevel` before calling `usage`, we'll end with an error to the effect of "./doc-diff: not found" rather than a friendly `doc-diff -h` output. Granted, all of these `usage` calls are in error paths, so we're about to exit anyway, but the user experience of something like `(cd Documentation && ./doc-diff)` could be a bit better than "./doc-diff: not found".

This regressed in ad51743007 ("doc-diff: add --clean mode to remove temporary working gunk", 2018-08-31) where we moved the call to `cd_to_toplevel` to much earlier. Move it back to where it was, and teach the "--clean" code to cd on its own. This way, we only cd once we've verified the arguments.

A more general fix would be to teach git-sh-setup to save away the absolute path for $0 and then use that, instead. I'm not aware of any portable way of doing that, see, e.g., d2addc3b96 ("t7800: readlink may not be available", 2016-05-31), so let's just fix this user instead.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 > Punting and extending the commit message like that sounds reasonable.
 So here's a v2 doing exactly that.
 Documentation/doc-diff | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index dfd9418778..f820febf8f 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -39,12 +39,11 @@ do
 	shift
 done
 
-cd_to_toplevel
-tmp=Documentation/tmp-doc-diff
-
 if test -n "$clean"
 then
 	test $# -eq 0 || usage
+	cd_to_toplevel
+	tmp=Documentation/tmp-doc-diff
 	git worktree remove --force "$tmp/worktree" 2>/dev/null
 	rm -rf "$tmp"
 	exit 0
@@ -66,6 +65,9 @@ to=$1; shift
 from_oid=$(git rev-parse --verify "$from") || exit 1
 to_oid=$(git rev-parse --verify "$to") || exit 1
 
+cd_to_toplevel
+tmp=Documentation/tmp-doc-diff
+
 if test -n "$force"
 then
 	rm -rf "$tmp"
-- 
2.20.1.309.g16a465bc01
Previous: Eric SunshineNext: Jeff King
Message 6 of 27 in “doc-diff: don't `cd_to_toplevel` before calling `usage`”
  1. doc-diff: don't `cd_to_toplevel` before calling `usage`Martin Ågren, Feb 3, 2019
  2. Eric SunshineFeb 3, 2019
  3. Eric SunshineFeb 3, 2019
  4. Martin ÅgrenFeb 3, 2019
  5. Eric SunshineFeb 3, 2019
  6. doc-diff: don't `cd_to_toplevel` before calling `usage`Martin Ågren, Feb 3, 2019
  7. Jeff KingFeb 3, 2019
  8. doc-diff: don't `cd_to_toplevel`Martin Ågren, Feb 4, 2019
  9. Jeff KingFeb 4, 2019
  10. Johannes SchindelinFeb 5, 2019
  11. Junio C HamanoFeb 5, 2019
  12. Johannes SchindelinFeb 6, 2019
  13. Junio C HamanoFeb 6, 2019
  14. Jeff KingFeb 6, 2019
  15. Johannes SchindelinFeb 7, 2019
  16. Junio C HamanoFeb 7, 2019
  17. Johannes SchindelinFeb 7, 2019
  18. Jeff KingFeb 7, 2019
  19. Johannes SchindelinFeb 7, 2019
  20. Jeff KingFeb 6, 2019
  21. Johannes SchindelinFeb 7, 2019
  22. Jeff KingFeb 7, 2019
  23. Johannes SchindelinFeb 7, 2019
  24. Jeff KingFeb 8, 2019
  25. Jeff KingFeb 8, 2019
  26. SZEDER GáborFeb 8, 2019
  27. Jeff KingFeb 8, 2019

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.