# [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X 8 messages from 2008-12-30 to 2009-01-06. Participants: Marcel M. Cary, Marcel Koeppen, Junio C Hamano, Wincent Colaiuta. Thread: https://gitlist.dev/t/16924 ## Marcel M. Cary, 2008-12-30 15:10 Subject: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <1230649824-1893-1-git-send-email-marcel@oak.homeunix.org> URL: https://gitlist.dev/e/1230649824-1893-1-git-send-email-marcel%40oak.homeunix.org In-Reply-To: ``` On Mac OS X and possibly BSDs, /bin/pwd reads PWD from the environment if available and shows the logical path by default rather than the physical one. Unset PWD before running /bin/pwd in both cd_to_toplevel and its test. Still use the external /bin/pwd because in my Bash on Linux, the builtin pwd prints the same result whether or not PWD is set. --- Brian Gernhardt wrote: > I didn't pay attention to this at the time, but I just tried to build a > new version of git and noticed this... > >> + ..|../*|*/..|*/../*) >> + # Interpret $cdup relative to the physical, not logical, >> cwd. >> + # Probably /bin/pwd is more portable than passing -P to >> cd or pwd. >> + phys="$(/bin/pwd)/$cdup" >> + ;; > > This is a non-portable construct. Notably, on OS X (and possibly some > BSDs) /bin/pwd does not give the physical path, but $(/bin/pwd -P) > does. Ouch! Junio C Hamano wrote: > Having said that, I think it would probably be better to bite the > bullet and start using "cd -P" soon after 1.6.1 goes final, and at the > same time existing places that use "cd `pwd`" as a workaround if there > are some. Perhaps it's time to start using "cd -P" with the recent release of 1.6.1. But maybe it's also worth finding a fix that doesn't rely on it for any maintenance release of 1.6 that might happen? Brian Gernhardt wrote: > We may have to build this string ourselves with a --show-cd-absolute > for portability. Some options I considered: (1) We could implement --show-cd-absolute in the short term, with the expectation of removing it we switch to "cd -P". Not sure it would really work to try to remove a switch, even if it were undocumented, or documented as deprecated. (2) We could check the output of /bin/pwd for ".." and if they are still present, add the "-P". But I suppose there's no guarantee "-P" would work, and I shy away from additional analysis of the path because I don't want to miss an edge case. (3) We could unset PWD before running /bin/pwd. (Note that in my Bash, the pwd *builtin* prints the same result whether or not I unset PWD first.) I suppose it's possible some /bin/pwd dies when PWD is unset or something, but that seems unlikely. From http://developer.apple.com/DOCUMENTATION/Darwin/Reference/ManPages/man1/pwd.1.html > The -L option does not work unless the PWD environment variable is > exported by the shell. I like option 3 best. Any thoughts? I sent the first rev of this patch to just Brian. It didn't have either of the unit test changes. He said it fixed all but t2300.3, where cd_to_toplevel doesn't actually "cd", so I made the same change to the unit test itself. Can someone with OS X try running the test suite with v2 of this patch? I don't have OS X readily available. Marcel git-sh-setup.sh | 2 +- t/t2300-cd-to-toplevel.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index f07d96b..2142308 100755 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -96,7 +96,7 @@ cd_to_toplevel () { ..|../*|*/..|*/../*) # Interpret $cdup relative to the physical, not logical, cwd. # Probably /bin/pwd is more portable than passing -P to cd or pwd. - phys="$(/bin/pwd)/$cdup" + phys="$(unset PWD; /bin/pwd)/$cdup" ;; *) # There's no "..", so no need to make things absolute. diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh index beddb4e..e42cbfe 100755 --- a/t/t2300-cd-to-toplevel.sh +++ b/t/t2300-cd-to-toplevel.sh @@ -10,12 +10,12 @@ test_cd_to_toplevel () { cd '"'$1'"' && . git-sh-setup && cd_to_toplevel && - [ "$(/bin/pwd)" = "$TOPLEVEL" ] + [ "$(unset PWD; /bin/pwd)" = "$TOPLEVEL" ] ) ' } -TOPLEVEL="$(/bin/pwd)/repo" +TOPLEVEL="$(unset PWD; /bin/pwd)/repo" mkdir -p repo/sub/dir mv .git repo/ SUBDIRECTORY_OK=1 -- 1.6.0.3 ``` ## Marcel Koeppen, 2009-01-02 22:53 Subject: Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: URL: https://gitlist.dev/e/AC726FD4-AE7F-4EC0-82E5-62C6D03C4E5A%40marzelpan.de In-Reply-To: <1230649824-1893-1-git-send-email-marcel@oak.homeunix.org> ``` Hi, Am 30.12.2008 um 16:10 schrieb Marcel M. Cary: > I sent the first rev of this patch to just Brian. It didn't have > either of the unit test changes. He said it fixed all but t2300.3, > where cd_to_toplevel doesn't actually "cd", so I made the same change > to the unit test itself. Can someone with OS X try running the test > suite with v2 of this patch? I don't have OS X readily available. the patch fixes t2300-cd-to-toplevel and t5521-pull-symlink for me. Marcel [I don't know why my replies get lost, so I dropped all individual recipients on this third try...] ``` ## Junio C Hamano, 2009-01-03 22:01 Subject: Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <7v8wps59ss.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8wps59ss.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1230649824-1893-1-git-send-email-marcel@oak.homeunix.org> ``` "Marcel M. Cary" writes: > I sent the first rev of this patch to just Brian. It didn't have > either of the unit test changes. He said it fixed all but t2300.3, > where cd_to_toplevel doesn't actually "cd", so I made the same change > to the unit test itself. Can someone with OS X try running the test > suite with v2 of this patch? I don't have OS X readily available. I think I saw a success report on the list. Care to resend it with Sign-off (by you) and Tested-by: tester (on PLATFORM) lines as you see necessary for application? Thanks. ``` ## Wincent Colaiuta, 2009-01-04 13:58 Subject: Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: URL: https://gitlist.dev/e/D9F420FC-12C5-4262-833B-0820D8C80AC9%40wincent.com In-Reply-To: <7v8wps59ss.fsf@gitster.siamese.dyndns.org> ``` El 3/1/2009, a las 23:01, Junio C Hamano escribió: > "Marcel M. Cary" writes: > >> I sent the first rev of this patch to just Brian. It didn't have >> either of the unit test changes. He said it fixed all but t2300.3, >> where cd_to_toplevel doesn't actually "cd", so I made the same change >> to the unit test itself. Can someone with OS X try running the test >> suite with v2 of this patch? I don't have OS X readily available. > > I think I saw a success report on the list. Care to resend it with > Sign-off (by you) and > > Tested-by: tester (on PLATFORM) > > lines as you see necessary for application? > > Thanks. I also tested it and can confirm that it fixes the failures on Mac OS X 10.5.5. So feel free to add: Tested-by: Wincent Colaiuta (on Mac OS X 10.5.5) Cheers, Wincent ``` ## Marcel Koeppen, 2009-01-04 18:49 Subject: Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <8C7E36D0-C037-427D-B6E2-4050CC767CD0@marzelpan.de> URL: https://gitlist.dev/e/8C7E36D0-C037-427D-B6E2-4050CC767CD0%40marzelpan.de In-Reply-To: <7v8wps59ss.fsf@gitster.siamese.dyndns.org> ``` Am 03.01.2009 um 23:01 schrieb Junio C Hamano: > "Marcel M. Cary" writes: > >> I sent the first rev of this patch to just Brian. It didn't have >> either of the unit test changes. He said it fixed all but t2300.3, >> where cd_to_toplevel doesn't actually "cd", so I made the same change >> to the unit test itself. Can someone with OS X try running the test >> suite with v2 of this patch? I don't have OS X readily available. > > I think I saw a success report on the list. Care to resend it with > Sign-off (by you) and > > Tested-by: tester (on PLATFORM) > > lines as you see necessary for application? > > Thanks. Hi, please add Tested-by: Marcel Koeppen (on Mac OS X 10.5.6) Marcel ``` ## Marcel M. Cary, 2009-01-04 21:27 Subject: [PATCH v2 tested] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <1231104445-12531-1-git-send-email-marcel@oak.homeunix.org> URL: https://gitlist.dev/e/1231104445-12531-1-git-send-email-marcel%40oak.homeunix.org In-Reply-To: ``` On Mac OS X and possibly BSDs, /bin/pwd reads PWD from the environment if available and shows the logical path by default rather than the physical one. Unset PWD before running /bin/pwd in both cd_to_toplevel and its test. Still use the external /bin/pwd because in my Bash on Linux, the builtin pwd prints the same result whether or not PWD is set. Signed-off-by: Marcel M. Cary Tested-by: Marcel Koeppen --- Junio C Hamano wrote: > I think I saw a success report on the list. Care to resend it with > Sign-off (by you) and > > Tested-by: tester (on PLATFORM) > > lines as you see necessary for application? Same as before but with S-o-b/T-b lines. Marcel Koeppen wrote: > [I don't know why my replies get lost, so I dropped all individual > recipients on this third try...] I noticed that Brian Gernhardt's message also didn't make it to the list, even though it was addressed to the list. I'm not sure why. git-sh-setup.sh | 2 +- t/t2300-cd-to-toplevel.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index f07d96b..2142308 100755 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -96,7 +96,7 @@ cd_to_toplevel () { ..|../*|*/..|*/../*) # Interpret $cdup relative to the physical, not logical, cwd. # Probably /bin/pwd is more portable than passing -P to cd or pwd. - phys="$(/bin/pwd)/$cdup" + phys="$(unset PWD; /bin/pwd)/$cdup" ;; *) # There's no "..", so no need to make things absolute. diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh index beddb4e..e42cbfe 100755 --- a/t/t2300-cd-to-toplevel.sh +++ b/t/t2300-cd-to-toplevel.sh @@ -10,12 +10,12 @@ test_cd_to_toplevel () { cd '"'$1'"' && . git-sh-setup && cd_to_toplevel && - [ "$(/bin/pwd)" = "$TOPLEVEL" ] + [ "$(unset PWD; /bin/pwd)" = "$TOPLEVEL" ] ) ' } -TOPLEVEL="$(/bin/pwd)/repo" +TOPLEVEL="$(unset PWD; /bin/pwd)/repo" mkdir -p repo/sub/dir mv .git repo/ SUBDIRECTORY_OK=1 -- 1.6.1 ``` ## Marcel M. Cary, 2009-01-04 21:47 Subject: [PATCH v2 tested-v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <1231105649-12998-1-git-send-email-marcel@oak.homeunix.org> URL: https://gitlist.dev/e/1231105649-12998-1-git-send-email-marcel%40oak.homeunix.org In-Reply-To: <8C7E36D0-C037-427D-B6E2-4050CC767CD0@marzelpan.de> ``` On Mac OS X and possibly BSDs, /bin/pwd reads PWD from the environment if available and shows the logical path by default rather than the physical one. Unset PWD before running /bin/pwd in both cd_to_toplevel and its test. Still use the external /bin/pwd because in my Bash on Linux, the builtin pwd prints the same result whether or not PWD is set. Signed-off-by: Marcel M. Cary Tested-by: Marcel Koeppen (on Mac OS X 10.5.6) --- > please add > > Tested-by: Marcel Koeppen (on Mac OS X 10.5.6) Now with the OS, in detail. git-sh-setup.sh | 2 +- t/t2300-cd-to-toplevel.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index f07d96b..2142308 100755 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -96,7 +96,7 @@ cd_to_toplevel () { ..|../*|*/..|*/../*) # Interpret $cdup relative to the physical, not logical, cwd. # Probably /bin/pwd is more portable than passing -P to cd or pwd. - phys="$(/bin/pwd)/$cdup" + phys="$(unset PWD; /bin/pwd)/$cdup" ;; *) # There's no "..", so no need to make things absolute. diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh index beddb4e..e42cbfe 100755 --- a/t/t2300-cd-to-toplevel.sh +++ b/t/t2300-cd-to-toplevel.sh @@ -10,12 +10,12 @@ test_cd_to_toplevel () { cd '"'$1'"' && . git-sh-setup && cd_to_toplevel && - [ "$(/bin/pwd)" = "$TOPLEVEL" ] + [ "$(unset PWD; /bin/pwd)" = "$TOPLEVEL" ] ) ' } -TOPLEVEL="$(/bin/pwd)/repo" +TOPLEVEL="$(unset PWD; /bin/pwd)/repo" mkdir -p repo/sub/dir mv .git repo/ SUBDIRECTORY_OK=1 -- 1.6.1 ``` ## Junio C Hamano, 2009-01-06 08:18 Subject: Re: [PATCH v2 tested-v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X Message-ID: <7v63ksluei.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63ksluei.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1231105649-12998-1-git-send-email-marcel@oak.homeunix.org> ``` Thanks; queued. ```