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

[PATCH v3] doc-diff: don't `cd_to_toplevel`

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Feb 4, 2019, 20:50 UTC
Message-ID
<20190204205037.32143-1-martin.agren@gmail.com>
In-Reply-To
<20190203230152.GA25608@sigill.intra.peff.net>

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

A general fix might 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).

An early version of this patch moved `cd_to_toplevel` back to where it was before ad51743007 and taught the "--clean" code to cd on its own. But let's try instead to get rid of the cd-ing entirely. We don't really need it and we can work with absolute paths instead. There's just one use of $PWD that we need to adjust by simply dropping it.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Thanks Peff for the suggestions. Trying not to cd at all seems sane to
 me. That it allows `./Documentation/doc-diff` is a bonus, I guess, but
 you're right that it's probably nothing anyone will use.
 I've verified that diffs produced by `./Documentation/doc-diff foo bar`
 and `./doc-diff foo bar` are identical, FWIW.
 Martin
 Documentation/doc-diff | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index dfd9418778..32c83dd26f 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -39,8 +39,7 @@ do
 	shift
 done
 
-cd_to_toplevel
-tmp=Documentation/tmp-doc-diff
+tmp="$(git rev-parse --show-toplevel)/Documentation/tmp-doc-diff" || exit 1
 
 if test -n "$clean"
 then
@@ -109,7 +108,7 @@ render_tree () {
 		make -j$parallel -C "$tmp/worktree" \
 			GIT_VERSION=omitted \
 			SOURCE_DATE_EPOCH=0 \
-			DESTDIR="$PWD/$tmp/installed/$1+" \
+			DESTDIR="$tmp/installed/$1+" \
 			install-man &&
 		mv "$tmp/installed/$1+" "$tmp/installed/$1"
 	fi &&
-- 
2.20.1.390.gb5101f9297
Previous: Jeff KingNext: Jeff King
Message 8 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.