{"thread":{"id":"26167","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","startedAt":"2010-12-31T13:00:32Z","lastAt":"2011-01-11T17:47:06Z","messageCount":12,"participants":["Nguyen Thai Ngoc Duy","Johannes Sixt","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"158779","messageId":"AANLkTi=uCCfBFBpC=+V9RpaXRXiiPYp-tZXBxAVNB7+e@mail.gmail.com","threadId":"26167","inReplyTo":"201012302205.13728.j6t@kdbg.org","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-12-31T13:00:32Z","receivedAt":"2010-12-31T13:00:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/12/31 Johannes Sixt <j6t@kdbg.org>:\n> On Windows, bash stores absolute path names in shell variables in POSIX\n> format that begins with a slash, rather than in drive-letter format; such\n> a value is converted to the latter format when it is passed to a non-MSYS\n> program such as git.\n\nHmm.. from test-lib.sh:\n\nTEST_DIRECTORY=$(pwd)\ntest=\"trash directory.$(basename \"$0\" .sh)\"\nTRASH_DIRECTORY=\"$TEST_DIRECTORY/$test\"\n\nI'm just curious how these lines make $TRASH_DIRECTORY in POSIX format, while\n\nhere=$(pwd)\n\nin your patch does not. Does bash auto convert value in\nTRASH_DIRECTORY=\"$TE...\" line?\n-- \nDuy\n"},{"id":"158787","messageId":"201012311711.06989.j6t@kdbg.org","threadId":"26167","inReplyTo":"AANLkTi=uCCfBFBpC=+V9RpaXRXiiPYp-tZXBxAVNB7+e@mail.gmail.com","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-12-31T16:11:06Z","receivedAt":"2010-12-31T16:11:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Freitag, 31. Dezember 2010, Nguyen Thai Ngoc Duy wrote:\n> 2010/12/31 Johannes Sixt <j6t@kdbg.org>:\n> > On Windows, bash stores absolute path names in shell variables in POSIX\n> > format that begins with a slash, rather than in drive-letter format; such\n> > a value is converted to the latter format when it is passed to a non-MSYS\n> > program such as git.\n>\n> Hmm.. from test-lib.sh:\n>\n> TEST_DIRECTORY=$(pwd)\n> test=\"trash directory.$(basename \"$0\" .sh)\"\n> TRASH_DIRECTORY=\"$TEST_DIRECTORY/$test\"\n>\n> I'm just curious how these lines make $TRASH_DIRECTORY in POSIX format,\n> while\n>\n> here=$(pwd)\n>\n> in your patch does not. Does bash auto convert value in\n> TRASH_DIRECTORY=\"$TE...\" line?\n\nNo. When this line is executed:\n\nTEST_DIRECTORY=$(pwd)\n\n$(pwd) still has its default behavior to return the POSIX style path. pwd is \nredefined to pwd -W only later.\n\nI'm hesitant to redefine pwd earlier in test-lib.sh, though, because we would \nhave to audit all uses of TEST_DIRECTORY for whether POSIX style paths or \ndrive-letter paths are needed.\n\n-- Hannes\n"},{"id":"158794","messageId":"20101231203019.GC5898@burratino","threadId":"26167","inReplyTo":"201012311711.06989.j6t@kdbg.org","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-31T20:30:20Z","receivedAt":"2010-12-31T20:30:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n> On Freitag, 31. Dezember 2010, Nguyen Thai Ngoc Duy wrote:\n\n>> in your patch does not. Does bash auto convert value in\n>> TRASH_DIRECTORY=\"$TE...\" line?\n>\n> No. When this line is executed:\n> \n> TEST_DIRECTORY=$(pwd)\n> \n> $(pwd) still has its default behavior to return the POSIX style path. pwd is \n> redefined to pwd -W only later.\n\nWould it make sense to change it to\n\n TEST_DIRECTORY=$PWD\n\nfor clarity and robustness against code movement, then?\n"},{"id":"158795","messageId":"201012312321.31294.j6t@kdbg.org","threadId":"26167","inReplyTo":"20101231203019.GC5898@burratino","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-12-31T22:21:31Z","receivedAt":"2010-12-31T22:21:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Freitag, 31. Dezember 2010, Jonathan Nieder wrote:\n> Johannes Sixt wrote:\n> > On Freitag, 31. Dezember 2010, Nguyen Thai Ngoc Duy wrote:\n> >> in your patch does not. Does bash auto convert value in\n> >> TRASH_DIRECTORY=\"$TE...\" line?\n> >\n> > No. When this line is executed:\n> >\n> > TEST_DIRECTORY=$(pwd)\n> >\n> > $(pwd) still has its default behavior to return the POSIX style path. pwd\n> > is redefined to pwd -W only later.\n>\n> Would it make sense to change it to\n>\n>  TEST_DIRECTORY=$PWD\n>\n> for clarity and robustness against code movement, then?\n\nYes, that would make sense.\n\n-- Hannes\n"},{"id":"158798","messageId":"AANLkTinWdGDtxPk24OBQnJP8fdHW9gYNDn46YKRGTWoB@mail.gmail.com","threadId":"26167","inReplyTo":"201012311711.06989.j6t@kdbg.org","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-01-01T03:46:54Z","receivedAt":"2011-01-01T03:46:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Dec 31, 2010 at 11:11 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> in your patch does not. Does bash auto convert value in\n>> TRASH_DIRECTORY=\"$TE...\" line?\n>\n> No. When this line is executed:\n>\n> TEST_DIRECTORY=$(pwd)\n>\n> $(pwd) still has its default behavior to return the POSIX style path. pwd is\n> redefined to pwd -W only later.\n>\n> I'm hesitant to redefine pwd earlier in test-lib.sh, though, because we would\n> have to audit all uses of TEST_DIRECTORY for whether POSIX style paths or\n> drive-letter paths are needed.\n\nAh I missed that. Thanks. Ack from me.\n-- \nDuy\n"},{"id":"158804","messageId":"7v62u8hz01.fsf@alter.siamese.dyndns.org","threadId":"26167","inReplyTo":"201012312321.31294.j6t@kdbg.org","subject":"Re: [PATCH] Fix expected values of setup tests on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-02T01:31:58Z","receivedAt":"2011-01-02T01:31:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> On Freitag, 31. Dezember 2010, Jonathan Nieder wrote:\n>> Johannes Sixt wrote:\n>> > On Freitag, 31. Dezember 2010, Nguyen Thai Ngoc Duy wrote:\n>> >> in your patch does not. Does bash auto convert value in\n>> >> TRASH_DIRECTORY=\"$TE...\" line?\n>> >\n>> > No. When this line is executed:\n>> >\n>> > TEST_DIRECTORY=$(pwd)\n>> >\n>> > $(pwd) still has its default behavior to return the POSIX style path. pwd\n>> > is redefined to pwd -W only later.\n>>\n>> Would it make sense to change it to\n>>\n>>  TEST_DIRECTORY=$PWD\n>>\n>> for clarity and robustness against code movement, then?\n>\n> Yes, that would make sense.\n\nIt will be very much appreciated to add a few sentences to clarify this to\n\"Do's and don'ts\" section of t/README if you are re-rolling this.  Thanks.\n"},{"id":"159307","messageId":"4D2C0A5E.7090708@viscovery.net","threadId":"26167","inReplyTo":"4D2C09D7.3070700@viscovery.net","subject":"[PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-01-11T07:44:30Z","receivedAt":"2011-01-11T07:44:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt <j6t@kdbg.org>\n\nThis adds just a \"do it this way\" instruction without a lot of explanation,\nbecause the details are too complex to be explained at this point.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n t/README |    6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 90718b4..f98ebb3 100644\n--- a/t/README\n+++ b/t/README\n@@ -283,6 +283,12 @@ Do:\n    Tests that are likely to smoke out future regressions are better\n    than tests that just inflate the coverage metrics.\n \n+ - When a test checks for an absolute path that a git command generated,\n+   construct the expected value using $(pwd) rather than $PWD,\n+   $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on\n+   Windows, where the shell (MSYS bash) mangles absolute path names.\n+   For details, see the commit message of 4114156ae9.\n+\n Don't:\n \n  - exit() within a <script> part.\n-- \n1.7.4.rc1.1258.g84aa\n"},{"id":"159309","messageId":"20110111075441.GB9445@burratino","threadId":"26167","inReplyTo":"4D2C0A5E.7090708@viscovery.net","subject":"Re: [PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-11T07:54:41Z","receivedAt":"2011-01-11T07:54:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> This adds just a \"do it this way\" instruction without a lot of explanation,\n> because the details are too complex to be explained at this point.\n\nThanks, looks very useful.\n\n> --- a/t/README\n> +++ b/t/README\n> @@ -283,6 +283,12 @@ Do:\n>     Tests that are likely to smoke out future regressions are better\n>     than tests that just inflate the coverage metrics.\n>  \n> + - When a test checks for an absolute path that a git command generated,\n> +   construct the expected value using $(pwd) rather than $PWD,\n> +   $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on\n> +   Windows, where the shell (MSYS bash) mangles absolute path names.\n> +   For details, see the commit message of 4114156ae9.\n> +\n\nPerhaps it is also worth explaining the cases where $PWD is needed?\n\n\tBy contrast, when a passing a path to git or constructing a URL,\n\tuse $PWD.  It makes a difference on Windows, where\n\n\t - $(pwd) is a Windows-style path such as git might output, and\n\t - $PWD is a Unix-style path that the shell (MSYS bash) will\n\t   mangle before passing to native apps like git.\n"},{"id":"159310","messageId":"4D2C11A5.4050709@viscovery.net","threadId":"26167","inReplyTo":"20110111075441.GB9445@burratino","subject":"Re: [PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-01-11T08:15:33Z","receivedAt":"2011-01-11T08:15:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 1/11/2011 8:54, schrieb Jonathan Nieder:\n> Perhaps it is also worth explaining the cases where $PWD is needed?\n>\n> \tBy contrast, when a passing a path to git or constructing a URL,\n> \tuse $PWD.\n\nThe first part of the \"or\" is not true: you can pass the result of $(pwd)\nto a command, and it means the same as $PWD; I would even recommend\nagainst $PWD so that a reader does not have to wonder \"why pass $PWD, but\ncheck for $(pwd)?\"\n\nThe second part I don't know whether it is true: I haven't noticed a\npattern where people did it the wrong way, therefore, I'don't even know\nwhether $PWD is really *always* required. Do *you* know?\n\n>       It makes a difference on Windows, where\n> \n> \t - $(pwd) is a Windows-style path such as git might output, and\n> \t - $PWD is a Unix-style path that the shell (MSYS bash) will\n> \t   mangle before passing to native apps like git.\n\nThis information is already included by reference to 4114156ae9.\n\n-- Hannes\n"},{"id":"159311","messageId":"20110111083729.GD9445@burratino","threadId":"26167","inReplyTo":"4D2C11A5.4050709@viscovery.net","subject":"Re: [PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-11T08:37:29Z","receivedAt":"2011-01-11T08:37:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n> Am 1/11/2011 8:54, schrieb Jonathan Nieder:\n\n>> Perhaps it is also worth explaining the cases where $PWD is needed?\n>>\n>> \tBy contrast, when a passing a path to git or constructing a URL,\n>> \tuse $PWD.\n>\n> The first part of the \"or\" is not true: you can pass the result of $(pwd)\n> to a command, and it means the same as $PWD; I would even recommend\n> against $PWD so that a reader does not have to wonder \"why pass $PWD, but\n> check for $(pwd)?\"\n\nI _think_ that passing $PWD always gives the right result.  By\ncontrast, constructions like\n\n\tPATH=$(pwd)/bin:$PATH\n\nbreak iirc.\n\nI suspect that the reader will end up wondering \"why does this have to\nbe so complicated\" no matter what.\n\n> The second part I don't know whether it is true: I haven't noticed a\n> pattern where people did it the wrong way, therefore, I'don't even know\n> whether $PWD is really *always* required. Do *you* know?\n\n24f1136 is one example.  I don't know of any utility that treats\n\n\tfile://c:/foo/bar/baz\n\nas a URL representing a resource on localhost (and msys bash has no\nrewriting rule for it), so in that particular case\n(file://$directory), $PWD really does seem to be always required.\n\n>>       It makes a difference on Windows, where\n>>\n>> \t - $(pwd) is a Windows-style path such as git might output, and\n>> \t - $PWD is a Unix-style path that the shell (MSYS bash) will\n>> \t   mangle before passing to native apps like git.\n>\n> This information is already included by reference to 4114156ae9.\n\n... but if we can summarize it nicely, we can save the reader a\nstep, no?\n\nAnyway, what you have already written is useful; clearing up these\ndetails would just be icing on the top.\n"},{"id":"159312","messageId":"4D2C1AAE.1040002@viscovery.net","threadId":"26167","inReplyTo":"20110111083729.GD9445@burratino","subject":"Re: [PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-01-11T08:54:06Z","receivedAt":"2011-01-11T08:54:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 1/11/2011 9:37, schrieb Jonathan Nieder:\n> I suspect that the reader will end up wondering \"why does this have to\n> be so complicated\" no matter what.\n\nUnfortunately, yes. Therefore, I'd like to keep the paragraph minimal,\nfocused on how expected values should be constructed, which is where\nerrors will happen primarily.\n\n>>>       It makes a difference on Windows, where\n>>>\n>>> \t - $(pwd) is a Windows-style path such as git might output, and\n>>> \t - $PWD is a Unix-style path that the shell (MSYS bash) will\n>>> \t   mangle before passing to native apps like git.\n>>\n>> This information is already included by reference to 4114156ae9.\n> \n> ... but if we can summarize it nicely, we can save the reader a\n> step, no?\n\nI don't think so: it's not complete enough. Readers will ask: \"So what?\"\nDigging archives or a three paragraph follow-up explanation on the ML will\nbe required anyway.\n\n> Anyway, what you have already written is useful; clearing up these\n> details would just be icing on the top.\n\nOK, thanks for a review.\n\n-- Hannes\n"},{"id":"159351","messageId":"7vmxn749k5.fsf@alter.siamese.dyndns.org","threadId":"26167","inReplyTo":"4D2C1AAE.1040002@viscovery.net","subject":"Re: [PATCH 2/2] t/README: hint about using $(pwd) rather than $PWD in tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-11T17:47:06Z","receivedAt":"2011-01-11T17:47:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Am 1/11/2011 9:37, schrieb Jonathan Nieder:\n>> I suspect that the reader will end up wondering \"why does this have to\n>> be so complicated\" no matter what.\n>\n> Unfortunately, yes. Therefore, I'd like to keep the paragraph minimal,\n> focused on how expected values should be constructed, which is where\n> errors will happen primarily.\n\nI really was hoping for something simple like \"never use $PWD\" myself,\nthough X-<.\n\nAnyway, will queue for 1.7.4.  Thanks.\n"}]}