{"thread":{"id":"44463","subject":"[PATCH] mingw: hot-fix t5615","startedAt":"2016-11-11T16:29:55Z","lastAt":"2016-11-16T21:51:14Z","messageCount":17,"participants":["Johannes Schindelin","Junio C Hamano","Johannes Sixt","Jeff King","Lars Schneider","Torsten Bögershausen","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"305790","messageId":"f4336a7fa3df8c0b426c3d01ada8d7d8f38e588a.1478881741.git.johannes.schindelin@gmx.de","threadId":"44463","inReplyTo":null,"subject":"[PATCH] mingw: hot-fix t5615","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-11T16:29:33Z","receivedAt":"2016-11-11T16:29:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"That test made the incorrect assumption that the path separator character\nis always a colon. On Windows, it is a semicolon instead.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\nPublished-As: https://github.com/dscho/git/releases/tag/t5615-path-separator-v1\nFetch-It-Via: git fetch https://github.com/dscho/git t5615-path-separator-v1\n\n\tThis is required, but not sufficient, to fix `master` on Windows.\n\n t/t5615-alternate-env.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh\nindex 22d9d81..3aeffb6 100755\n--- a/t/t5615-alternate-env.sh\n+++ b/t/t5615-alternate-env.sh\n@@ -37,8 +37,10 @@ test_expect_success 'access alternate via absolute path' '\n \tEOF\n '\n \n+sep=:\n+test_have_prereq !MINGW || sep=\\;\n test_expect_success 'access multiple alternates' '\n-\tcheck_obj \"$(pwd)/one.git/objects:$(pwd)/two.git/objects\" <<-EOF\n+\tcheck_obj \"$(pwd)/one.git/objects$sep$(pwd)/two.git/objects\" <<-EOF\n \t$one blob\n \t$two blob\n \tEOF\n\nbase-commit: 0538b84027a8aba7e8b805e3ec8fceb3990023e5\n-- \n2.10.1.583.g721a9e0\n"},{"id":"305795","messageId":"xmqq37iy3q0s.fsf@gitster.mtv.corp.google.com","threadId":"44463","inReplyTo":"f4336a7fa3df8c0b426c3d01ada8d7d8f38e588a.1478881741.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] mingw: hot-fix t5615","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-11T17:06:11Z","receivedAt":"2016-11-11T17:06:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> That test made the incorrect assumption that the path separator character\n> is always a colon. On Windows, it is a semicolon instead.\n\nDocumentation/git.txt says that GIT_ALTERNATE_OBJECT_DIRECTORIES is\nseparated with \";\" on Windows fairly clearly, and we should have\ncaught that.  \n\nFor the upcoming release there is no need for any further tweak on\nyour fix I am responding to, but in the longer term we would want to\nturn this to path_sep=\";\" (or \":\") and define it in the global\nt/test-lib.sh, as it is plausible that we may want to prepend or\nappend to $PATH in the tests and that also needs \";\" on Windows, no?\n\nAre there other variables that is a list of paths that we care in\nour tests?  I notice GIT_CEILING_DIRECTORIES does not have the\ncorresponding \": separated (on windows ; separated) list\" in its\ndescription in Documentation/git.txt but the documentation may need\nto be fixed there as well?\n\nThanks for a quick fix.  Will apply on jk/alt-odb-cleanup and merge\ndown.\n\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> Published-As: https://github.com/dscho/git/releases/tag/t5615-path-separator-v1\n> Fetch-It-Via: git fetch https://github.com/dscho/git t5615-path-separator-v1\n>\n> \tThis is required, but not sufficient, to fix `master` on Windows.\n>\n>  t/t5615-alternate-env.sh | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh\n> index 22d9d81..3aeffb6 100755\n> --- a/t/t5615-alternate-env.sh\n> +++ b/t/t5615-alternate-env.sh\n> @@ -37,8 +37,10 @@ test_expect_success 'access alternate via absolute path' '\n>  \tEOF\n>  '\n>  \n> +sep=:\n> +test_have_prereq !MINGW || sep=\\;\n>  test_expect_success 'access multiple alternates' '\n> -\tcheck_obj \"$(pwd)/one.git/objects:$(pwd)/two.git/objects\" <<-EOF\n> +\tcheck_obj \"$(pwd)/one.git/objects$sep$(pwd)/two.git/objects\" <<-EOF\n>  \t$one blob\n>  \t$two blob\n>  \tEOF\n>\n> base-commit: 0538b84027a8aba7e8b805e3ec8fceb3990023e5\n"},{"id":"305796","messageId":"fa11def5-a878-a5c8-6e7b-627ea8cda7e2@kdbg.org","threadId":"44463","inReplyTo":"xmqq37iy3q0s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] mingw: hot-fix t5615","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-11T17:11:02Z","receivedAt":"2016-11-11T17:11:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.11.2016 um 18:06 schrieb Junio C Hamano:\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n>\n>> That test made the incorrect assumption that the path separator character\n>> is always a colon. On Windows, it is a semicolon instead.\n>\n> Documentation/git.txt says that GIT_ALTERNATE_OBJECT_DIRECTORIES is\n> separated with \";\" on Windows fairly clearly, and we should have\n> caught that.\n>\n> For the upcoming release there is no need for any further tweak on\n> your fix I am responding to, but in the longer term we would want to\n> turn this to path_sep=\";\" (or \":\") and define it in the global\n> t/test-lib.sh, as it is plausible that we may want to prepend or\n> append to $PATH in the tests and that also needs \";\" on Windows, no?\n>\n> Are there other variables that is a list of paths that we care in\n> our tests?  I notice GIT_CEILING_DIRECTORIES does not have the\n> corresponding \": separated (on windows ; separated) list\" in its\n> description in Documentation/git.txt but the documentation may need\n> to be fixed there as well?\n>\n> Thanks for a quick fix.  Will apply on jk/alt-odb-cleanup and merge\n> down.\n\nA simpler fix is to use $PWD instead of $(pwd). I'll submit a patch in a \nmoment.\n\n-- Hannes\n\n"},{"id":"305800","messageId":"2b69d098-92ef-77b0-367a-516e9edbe257@kdbg.org","threadId":"44463","inReplyTo":"fa11def5-a878-a5c8-6e7b-627ea8cda7e2@kdbg.org","subject":"[PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-11T17:31:48Z","receivedAt":"2016-11-11T17:31:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"We have to use $PWD instead of $(pwd) because on Windows the latter\nwould add a C: style path to bash's Unix-style $PATH variable, which\nbecomes confused by the colon after the drive letter. ($PWD is a\nUnix-style path.)\n\nIn the case of GIT_ALTERNATE_OBJECT_DIRECTORIES, bash on Windows\nassembles a Unix-style path list with the colon as separators. It\nconverts the value to a Windows-style path list with the semicolon as\npath separator when it forwards the variable to git.exe. The same\nconfusion happens when bash's original value is contaminated with\nWindows style paths.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\nAm 11.11.2016 um 18:11 schrieb Johannes Sixt:\n> Am 11.11.2016 um 18:06 schrieb Junio C Hamano:\n>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n>>\n>>> That test made the incorrect assumption that the path separator\n>>> character\n>>> is always a colon. On Windows, it is a semicolon instead.\n>>\n>> Documentation/git.txt says that GIT_ALTERNATE_OBJECT_DIRECTORIES is\n>> separated with \";\" on Windows fairly clearly, and we should have\n>> caught that.\n>>\n>> For the upcoming release there is no need for any further tweak on\n>> your fix I am responding to, but in the longer term we would want to\n>> turn this to path_sep=\";\" (or \":\") and define it in the global\n>> t/test-lib.sh, as it is plausible that we may want to prepend or\n>> append to $PATH in the tests and that also needs \";\" on Windows, no?\n\nWhen the MSYS program such as bash invokes a non-MSYS program, it\ntranslates the Unix-style paths in arguments and environment variables\nto Windows stlye. We only have to ensure that we inject only Unix-style\npaths in these places so as not to confuse the conversion algorithm.\nMost of the time, we do not have to worry.\n\nOn the other hand, when we write a path to a file that git.exe consumes\nor receive a path from git.exe, i.e., when the path travels through\nstdout and stdin, no automatic translation happens (which is quite\nunderstandable), and we have do the translation explicitly. An example\nfor such a case is when we write a .git/info/alternates file via the\nshell.\n\n> A simpler fix is to use $PWD instead of $(pwd). I'll submit a patch in a\n> moment.\n\nHere it is. I had proposed the t0021 part earlier, but it fell through\nthe cracks during the temporary maintainer change.\n\n t/t0021-conversion.sh    | 2 +-\n t/t5615-alternate-env.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 9ff502773d..b93cd44546 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -4,7 +4,7 @@ test_description='blob conversion via gitattributes'\n \n . ./test-lib.sh\n \n-TEST_ROOT=\"$(pwd)\"\n+TEST_ROOT=\"$PWD\"\n PATH=$TEST_ROOT:$PATH\n \n write_script <<\\EOF \"$TEST_ROOT/rot13.sh\"\ndiff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh\nindex 22d9d8178b..eec4137ca5 100755\n--- a/t/t5615-alternate-env.sh\n+++ b/t/t5615-alternate-env.sh\n@@ -31,14 +31,14 @@ test_expect_success 'objects inaccessible without alternates' '\n '\n \n test_expect_success 'access alternate via absolute path' '\n-\tcheck_obj \"$(pwd)/one.git/objects\" <<-EOF\n+\tcheck_obj \"$PWD/one.git/objects\" <<-EOF\n \t$one blob\n \t$two missing\n \tEOF\n '\n \n test_expect_success 'access multiple alternates' '\n-\tcheck_obj \"$(pwd)/one.git/objects:$(pwd)/two.git/objects\" <<-EOF\n+\tcheck_obj \"$PWD/one.git/objects:$PWD/two.git/objects\" <<-EOF\n \t$one blob\n \t$two blob\n \tEOF\n-- \n2.11.0.rc0.55.gd967357\n\n\n"},{"id":"305803","messageId":"20161111181256.gl5xu6jxqrd5cbg7@sigill.intra.peff.net","threadId":"44463","inReplyTo":"f4336a7fa3df8c0b426c3d01ada8d7d8f38e588a.1478881741.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] mingw: hot-fix t5615","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-11T18:12:56Z","receivedAt":"2016-11-11T18:14:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 11, 2016 at 05:29:33PM +0100, Johannes Schindelin wrote:\n\n> That test made the incorrect assumption that the path separator character\n> is always a colon. On Windows, it is a semicolon instead.\n\nOof, sorry about that. I remember being careful about the \";\" while\ndoing the original alt-odb work, but forgot about it while making the\nrecent fix in 37a95862c.\n\nThe patch itself looks like obviously correct (or at least obviously in\nthe right direction, since it sounds like more is needed).\n\n-Peff\n"},{"id":"305804","messageId":"20161111181658.bwclcxedsrs4wcu6@sigill.intra.peff.net","threadId":"44463","inReplyTo":"2b69d098-92ef-77b0-367a-516e9edbe257@kdbg.org","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-11T18:16:59Z","receivedAt":"2016-11-11T18:18:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 11, 2016 at 06:31:48PM +0100, Johannes Sixt wrote:\n\n> We have to use $PWD instead of $(pwd) because on Windows the latter\n> would add a C: style path to bash's Unix-style $PATH variable, which\n> becomes confused by the colon after the drive letter. ($PWD is a\n> Unix-style path.)\n> \n> In the case of GIT_ALTERNATE_OBJECT_DIRECTORIES, bash on Windows\n> assembles a Unix-style path list with the colon as separators. It\n> converts the value to a Windows-style path list with the semicolon as\n> path separator when it forwards the variable to git.exe. The same\n> confusion happens when bash's original value is contaminated with\n> Windows style paths.\n\nSo on reading your original I wondered why you did not need to use the\n\";\", and that explains it. But wow, it's subtle.\n\nI don't know what people typically write in the wild, and if it's worth\nactually testing explicitly the \";\" case. I'll leave that to Windows\npeople to debate.\n\n-Peff\n"},{"id":"305807","messageId":"xmqqa8d53ky9.fsf@gitster.mtv.corp.google.com","threadId":"44463","inReplyTo":"2b69d098-92ef-77b0-367a-516e9edbe257@kdbg.org","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-11T18:55:42Z","receivedAt":"2016-11-11T18:55:56Z","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> We have to use $PWD instead of $(pwd) because on Windows the latter\n> would add a C: style path to bash's Unix-style $PATH variable, which\n> becomes confused by the colon after the drive letter. ($PWD is a\n> Unix-style path.)\n>\n> In the case of GIT_ALTERNATE_OBJECT_DIRECTORIES, bash on Windows\n> assembles a Unix-style path list with the colon as separators. It\n> converts the value to a Windows-style path list with the semicolon as\n> path separator when it forwards the variable to git.exe. The same\n> confusion happens when bash's original value is contaminated with\n> Windows style paths.\n>\n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> ---\n> Am 11.11.2016 um 18:11 schrieb Johannes Sixt:\n>> Am 11.11.2016 um 18:06 schrieb Junio C Hamano:\n>>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n>>> ...\n>\n> When the MSYS program such as bash invokes a non-MSYS program, it\n> translates the Unix-style paths in arguments and environment variables\n> to Windows stlye. We only have to ensure that we inject only Unix-style\n> paths in these places so as not to confuse the conversion algorithm.\n> Most of the time, we do not have to worry.\n>\n> On the other hand, when we write a path to a file that git.exe consumes\n> or receive a path from git.exe, i.e., when the path travels through\n> stdout and stdin, no automatic translation happens (which is quite\n> understandable), and we have do the translation explicitly. An example\n> for such a case is when we write a .git/info/alternates file via the\n> shell.\n>\n>> A simpler fix is to use $PWD instead of $(pwd). I'll submit a patch in a\n>> moment.\n>\n> Here it is. I had proposed the t0021 part earlier, but it fell through\n> the cracks during the temporary maintainer change.\n\nThanks.  Dscho, does this fix both of these issues to you?\n\n>  t/t0021-conversion.sh    | 2 +-\n>  t/t5615-alternate-env.sh | 4 ++--\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index 9ff502773d..b93cd44546 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -4,7 +4,7 @@ test_description='blob conversion via gitattributes'\n>  \n>  . ./test-lib.sh\n>  \n> -TEST_ROOT=\"$(pwd)\"\n> +TEST_ROOT=\"$PWD\"\n>  PATH=$TEST_ROOT:$PATH\n>  \n>  write_script <<\\EOF \"$TEST_ROOT/rot13.sh\"\n> diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh\n> index 22d9d8178b..eec4137ca5 100755\n> --- a/t/t5615-alternate-env.sh\n> +++ b/t/t5615-alternate-env.sh\n> @@ -31,14 +31,14 @@ test_expect_success 'objects inaccessible without alternates' '\n>  '\n>  \n>  test_expect_success 'access alternate via absolute path' '\n> -\tcheck_obj \"$(pwd)/one.git/objects\" <<-EOF\n> +\tcheck_obj \"$PWD/one.git/objects\" <<-EOF\n>  \t$one blob\n>  \t$two missing\n>  \tEOF\n>  '\n>  \n>  test_expect_success 'access multiple alternates' '\n> -\tcheck_obj \"$(pwd)/one.git/objects:$(pwd)/two.git/objects\" <<-EOF\n> +\tcheck_obj \"$PWD/one.git/objects:$PWD/two.git/objects\" <<-EOF\n>  \t$one blob\n>  \t$two blob\n>  \tEOF\n"},{"id":"305857","messageId":"alpine.DEB.2.20.1611121237230.3746@virtualbox","threadId":"44463","inReplyTo":"xmqqa8d53ky9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-12T11:40:14Z","receivedAt":"2016-11-12T12:50:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 11 Nov 2016, Junio C Hamano wrote:\n\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n> > We have to use $PWD instead of $(pwd) because on Windows the latter\n> > would add a C: style path to bash's Unix-style $PATH variable, which\n> > becomes confused by the colon after the drive letter. ($PWD is a\n> > Unix-style path.)\n> >\n> > In the case of GIT_ALTERNATE_OBJECT_DIRECTORIES, bash on Windows\n> > assembles a Unix-style path list with the colon as separators. It\n> > converts the value to a Windows-style path list with the semicolon as\n> > path separator when it forwards the variable to git.exe. The same\n> > confusion happens when bash's original value is contaminated with\n> > Windows style paths.\n> >\n> > Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> > ---\n> > Am 11.11.2016 um 18:11 schrieb Johannes Sixt:\n> >> Am 11.11.2016 um 18:06 schrieb Junio C Hamano:\n> >>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> >>> ...\n> >\n> > When the MSYS program such as bash invokes a non-MSYS program, it\n> > translates the Unix-style paths in arguments and environment variables\n> > to Windows stlye. We only have to ensure that we inject only Unix-style\n> > paths in these places so as not to confuse the conversion algorithm.\n> > Most of the time, we do not have to worry.\n> >\n> > On the other hand, when we write a path to a file that git.exe consumes\n> > or receive a path from git.exe, i.e., when the path travels through\n> > stdout and stdin, no automatic translation happens (which is quite\n> > understandable), and we have do the translation explicitly. An example\n> > for such a case is when we write a .git/info/alternates file via the\n> > shell.\n> >\n> >> A simpler fix is to use $PWD instead of $(pwd). I'll submit a patch in a\n> >> moment.\n> >\n> > Here it is. I had proposed the t0021 part earlier, but it fell through\n> > the cracks during the temporary maintainer change.\n> \n> Thanks.  Dscho, does this fix both of these issues to you?\n\nApparently it does because the CI jobs for `master` and for `next` pass.\nThe one for `pu` still times out, of course.\n\nCiao,\nDscho\n"},{"id":"305867","messageId":"xmqqtwbcyyfe.fsf@gitster.mtv.corp.google.com","threadId":"44463","inReplyTo":"alpine.DEB.2.20.1611121237230.3746@virtualbox","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-13T01:13:25Z","receivedAt":"2016-11-13T01:13:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Thanks.  Dscho, does this fix both of these issues to you?\n>\n> Apparently it does because the CI jobs for `master` and for `next` pass.\n\nOK, thanks for a quick confirmation.\n\n> The one for `pu` still times out, of course.\n\nEarlier you said 'pu' did even not build, but do we know where this\n\"still times out\" comes from?  As long as I don't merge anything\nprematurely, which I need to be careful about, it shouldn't impact\nthe upcoming release, but we'd need to figure it out before moving\nthings forward post release.\n\nThanks.\n"},{"id":"305883","messageId":"0BEC2674-20B5-4AD1-851A-97CA34C0CE7F@gmail.com","threadId":"44463","inReplyTo":"xmqqtwbcyyfe.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-14T09:11:12Z","receivedAt":"2016-11-14T09:12:05Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n>>> Thanks.  Dscho, does this fix both of these issues to you?\n>> \n>> Apparently it does because the CI jobs for `master` and for `next` pass.\n> \n> OK, thanks for a quick confirmation.\n> \n>> The one for `pu` still times out, of course.\n> \n> Earlier you said 'pu' did even not build, but do we know where this\n> \"still times out\" comes from?  As long as I don't merge anything\n> prematurely, which I need to be careful about, it shouldn't impact\n> the upcoming release, but we'd need to figure it out before moving\n> things forward post release.\n\nWhat is the goal for 'pu'?\n\n(1) Builds clean on all platforms + passes all tests\n(2) Builds clean on all platforms\n(3) Builds clean on Linux\n(4) Just makes new topics easily available to a broader audience\n\nMy understanding was always (4) but the discussion above sounds \nmore like (1) or (2)?\n\n--\n\nGit 'pu' does not compile on macOS right now:\nbuiltin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized \nwhenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]\n...\n\nMore info here:\nhttps://api.travis-ci.org/jobs/175417712/log.txt?deansi=true\n\n--\n\nCheers,\nLars"},{"id":"305890","messageId":"35d3a07d-5d2f-aedd-94bc-4d92e5aa4661@web.de","threadId":"44463","inReplyTo":"0BEC2674-20B5-4AD1-851A-97CA34C0CE7F@gmail.com","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-11-14T16:35:56Z","receivedAt":"2016-11-14T16:36:23Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 14.11.16 10:11, Lars Schneider wrote:\n> \n>> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>\n>>>> Thanks.  Dscho, does this fix both of these issues to you?\n>>>\n>>> Apparently it does because the CI jobs for `master` and for `next` pass.\n>>\n>> OK, thanks for a quick confirmation.\n>>\n>>> The one for `pu` still times out, of course.\n>>\n>> Earlier you said 'pu' did even not build, but do we know where this\n>> \"still times out\" comes from?  As long as I don't merge anything\n>> prematurely, which I need to be careful about, it shouldn't impact\n>> the upcoming release, but we'd need to figure it out before moving\n>> things forward post release.\n> \n> What is the goal for 'pu'?\n> \n\n> (1) Builds clean on all platforms + passes all tests\nYes\n> (2) Builds clean on all platforms\nYes\n> (3) Builds clean on Linux\nYes\n> (4) Just makes new topics easily available to a broader audience\nYes\n\n> \n> My understanding was always (4) but the discussion above sounds \n> more like (1) or (2)?\nAll commits should work on all platforms - in the ideal world there is no problem.\n\nFrom time to time things sneak in, which are not portable.\n(in the sense that not all \"supported\" compile/run tests without breakages)\n\nAnd if everybody reports breakages and problems found on the pu\nbranch, there is a good chance that they don't reach next or master.\n\nDoes this make sense ?\n\n> \n> --\n> \n> Git 'pu' does not compile on macOS right now:\n> builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized \n> whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]\n> ...\n> \n> More info here:\n> https://api.travis-ci.org/jobs/175417712/log.txt?deansi=true\n> \n> --\n> \n> Cheers,\n> Lars\n> \n\n"},{"id":"305892","messageId":"20161114170105.btnohk2777ddaiul@sigill.intra.peff.net","threadId":"44463","inReplyTo":"35d3a07d-5d2f-aedd-94bc-4d92e5aa4661@web.de","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-14T17:01:06Z","receivedAt":"2016-11-14T17:01:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 14, 2016 at 05:35:56PM +0100, Torsten Bögershausen wrote:\n\n> > What is the goal for 'pu'?\n> \n> > (1) Builds clean on all platforms + passes all tests\n> Yes\n> > (2) Builds clean on all platforms\n> Yes\n> > (3) Builds clean on Linux\n> Yes\n> > (4) Just makes new topics easily available to a broader audience\n> Yes\n\nI'd have answered differently, though I think in the end we agree on the\noutcome.\n\nI think the only thing that matters is (4). It _usually_ builds and\npasses all tests, but not always. But the point is that nobody should\ncare in particular about \"pu\". What we care about is whether the\nindividual topics will build and pass before they are merged to master\n(or even \"next\").\n\nSo \"pu\" is a tool, because you can test all of the topics at once and\nfind out early if there are any problems. And it's good to investigate\nproblems there before topics hit next (though they are also often caught\nin review, or by people trying the broken topic on their various\nplatforms, or sometimes Junio even pushes out a known-broken state in pu\nand mentions it in \"What's cooking\").\n\nSo yes, it should do all of those things, but we don't necessarily\nexpect that it will never be broken. That's expected to happen from time\nto time, and the purpose of the branch. With respect to Lars' original\npoint:\n\n> > Git 'pu' does not compile on macOS right now:\n> > builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized \n> > whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]\n\nThe next step is to make sure that the topic author is aware (in this\ncase, one assumes it's pb/bisect).\n\nBetter still is to make a patch that can either be applied on top, or\nsquashed as appropriate. I know that Ramsay Jones does this, for\nexample, with some of his sparse-related checks, and I'm pretty sure\nfrom the turnaround-time that he runs it against \"pu\".\n\n-Peff\n"},{"id":"305893","messageId":"xmqqshqux9il.fsf@gitster.mtv.corp.google.com","threadId":"44463","inReplyTo":"0BEC2674-20B5-4AD1-851A-97CA34C0CE7F@gmail.com","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T17:21:22Z","receivedAt":"2016-11-14T17:21:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n>> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:\n>> ...\n>> Earlier you said 'pu' did even not build, but do we know where this\n>> \"still times out\" comes from?  As long as I don't merge anything\n>> prematurely, which I need to be careful about, it shouldn't impact\n>> the upcoming release, but we'd need to figure it out before moving\n>> things forward post release.\n>\n> What is the goal for 'pu'?\n>\n> (1) Builds clean on all platforms + passes all tests\n> (2) Builds clean on all platforms\n> (3) Builds clean on Linux\n> (4) Just makes new topics easily available to a broader audience\n>\n> My understanding was always (4) but the discussion above sounds \n> more like (1) or (2)?\n\nThe purpose of 'pu' is none of the above, but its intended effect\nfor people other than me is (4).  It is primarily meant as a way for\nme to avoid having to go back to the mailing list archive to find\ntopics that I felt that were potentially interesting but not yet\nready and may want to get commented on later.  When queued on 'pu',\nit is not unusual that I haven't really looked at the patches yet,\nand it is not surprising if it does not build on any platform.\n\nWhen queued to 'pu', the reason of the initial \"not yet ready\"\nassessment may not have anything to do with the quality of the\npatches but based on the phase of the development (e.g. during a\nfeature-freeze, it is less efficient use of our time to take new\ntopics to 'next'), so what was queued on 'pu' may get merged to\n'next' without any update, after getting looked at by me or by\nother people and deemed to be worth trying out.\n\nDscho's mention of 'still times out' may be an indiciation that\nsomething unspecified on 'pu' is not ready to be merged to 'next',\nbut blocking all of 'pu' with a blanket statement is not useful,\nand that was where my response comes from.  We need to know more\nto say \"this particular topic is not ready\", so that we can unblock\nother innocent topics.\n"},{"id":"305913","messageId":"27949dcc-e25b-3a4a-15e5-71245670c0f4@ramsayjones.plus.com","threadId":"44463","inReplyTo":"20161114170105.btnohk2777ddaiul@sigill.intra.peff.net","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-11-14T19:45:14Z","receivedAt":"2016-11-14T19:45:25Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 14/11/16 17:01, Jeff King wrote:\n> On Mon, Nov 14, 2016 at 05:35:56PM +0100, Torsten Bögershausen wrote:\n> \n>>> Git 'pu' does not compile on macOS right now:\n>>> builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized \n>>> whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]\n> \n> The next step is to make sure that the topic author is aware (in this\n> case, one assumes it's pb/bisect).\n\n[+cc Pranit]\n\nYep, I had a quick squint, and it looks like the compiler is correct.\nIt should be complaining about the 'bad_syn' variable for exactly the\nsame reason: namely, whenever the if condition is true, the only exit\nfrom that block is via 'goto finish' which bypasses the initialisation\nof 'good_syn' and 'bad_syn'.\n\n> Better still is to make a patch that can either be applied on top, or\n> squashed as appropriate. \n\nNo patch this time, but it simply requires those variables to be\ninitialised to NULL in their declarations. :-D\n\n>                            I know that Ramsay Jones does this, for\n> example, with some of his sparse-related checks, and I'm pretty sure\n> from the turnaround-time that he runs it against \"pu\".\n\nYep, the idea being to catch these simple problems before the topic\nreaches 'next'.\n\nATB,\nRamsay Jones\n\n\n"},{"id":"305961","messageId":"alpine.DEB.2.20.1611151753300.3746@virtualbox","threadId":"44463","inReplyTo":"xmqqshqux9il.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-15T16:54:17Z","receivedAt":"2016-11-15T16:54:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 14 Nov 2016, Junio C Hamano wrote:\n\n> Dscho's mention of 'still times out' may be an indiciation that\n> something unspecified on 'pu' is not ready to be merged to 'next',\n> but blocking all of 'pu' with a blanket statement is not useful,\n> and that was where my response comes from.\n\nUntil the time when the test suite takes less than the insane three hours\nto run, I am afraid that a blanket statement on `pu` is the best I can do.\n\nCiao,\nJohannes\n"},{"id":"306038","messageId":"alpine.DEB.2.20.1611161041040.3746@virtualbox","threadId":"44463","inReplyTo":"alpine.DEB.2.20.1611151753300.3746@virtualbox","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-16T09:47:08Z","receivedAt":"2016-11-16T09:47:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 15 Nov 2016, Johannes Schindelin wrote:\n\n> On Mon, 14 Nov 2016, Junio C Hamano wrote:\n> \n> > Dscho's mention of 'still times out' may be an indiciation that\n> > something unspecified on 'pu' is not ready to be merged to 'next',\n> > but blocking all of 'pu' with a blanket statement is not useful,\n> > and that was where my response comes from.\n> \n> Until the time when the test suite takes less than the insane three hours\n> to run, I am afraid that a blanket statement on `pu` is the best I can do.\n\nWell, I should add that the test suite does not take 3 hours to run for\n`pu` these days.\n\nIt used to time out after four hours until two days ago (I think; I was a\nbit too busy with other CI work to pay close attention to the constantly\nfailing `pu` job, with quite a few failing `next`s and even a couple of\nfailing `master`s thrown in).\n\nAs of two days ago, the test suite takes no time at all. The build already\nfails (which makes me wonder why a couple of patch series I contributed\nhad such a hard time getting into `pu` when they compiled and tested\njust fine, whereas some obviously non-building stuff gets into `pu`\nalready, makes no sense to me).\n\nThis is the offending part from last night's build:\n\n-- snipsnap --\n2016-11-16T00:31:57.5321220Z copy.c: In function 'copy_dir_1':\n2016-11-16T00:31:57.5321220Z copy.c:369:8: error: implicit declaration of function 'lchown' [-Werror=implicit-function-declaration]\n2016-11-16T00:31:57.5321220Z     if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)\n2016-11-16T00:31:57.5321220Z         ^~~~~~\n2016-11-16T00:31:57.5321220Z copy.c:391:7: error: implicit declaration of function 'mknod' [-Werror=implicit-function-declaration]\n2016-11-16T00:31:57.5321220Z    if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0)\n2016-11-16T00:31:57.5321220Z        ^~~~~\n2016-11-16T00:31:57.5321220Z copy.c:405:7: error: implicit declaration of function 'utimes' [-Werror=implicit-function-declaration]\n2016-11-16T00:31:57.5321220Z    if (utimes(dest, times) < 0)\n2016-11-16T00:31:57.5321220Z        ^~~~~~\n2016-11-16T00:31:57.5321220Z copy.c:407:7: error: implicit declaration of function 'chown' [-Werror=implicit-function-declaration]\n2016-11-16T00:31:57.5321220Z    if (chown(dest, source_stat.st_uid, source_stat.st_gid) < 0) {\n2016-11-16T00:31:57.5321220Z        ^~~~~\n2016-11-16T00:31:57.7982432Z     CC ctype.o\n2016-11-16T00:31:58.1418929Z cc1.exe: all warnings being treated as errors\n2016-11-16T00:31:58.6368128Z make: *** [Makefile:1988: copy.o] Error 1\n\n"},{"id":"306082","messageId":"xmqqfumrozzs.fsf@gitster.mtv.corp.google.com","threadId":"44463","inReplyTo":"alpine.DEB.2.20.1611161041040.3746@virtualbox","subject":"Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T21:51:03Z","receivedAt":"2016-11-16T21:51:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> This is the offending part from last night's build:\n>\n> -- snipsnap --\n> 2016-11-16T00:31:57.5321220Z copy.c: In function 'copy_dir_1':\n> 2016-11-16T00:31:57.5321220Z copy.c:369:8: error: implicit declaration of function 'lchown' [-Werror=implicit-function-declaration]\n> 2016-11-16T00:31:57.5321220Z     if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)\n> 2016-11-16T00:31:57.5321220Z         ^~~~~~\n> 2016-11-16T00:31:57.5321220Z copy.c:391:7: error: implicit declaration of function 'mknod' [-Werror=implicit-function-declaration]\n> 2016-11-16T00:31:57.5321220Z    if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0)\n> 2016-11-16T00:31:57.5321220Z        ^~~~~\n> 2016-11-16T00:31:57.5321220Z copy.c:405:7: error: implicit declaration of function 'utimes' [-Werror=implicit-function-declaration]\n> 2016-11-16T00:31:57.5321220Z    if (utimes(dest, times) < 0)\n> 2016-11-16T00:31:57.5321220Z        ^~~~~~\n> 2016-11-16T00:31:57.5321220Z copy.c:407:7: error: implicit declaration of function 'chown' [-Werror=implicit-function-declaration]\n> 2016-11-16T00:31:57.5321220Z    if (chown(dest, source_stat.st_uid, source_stat.st_gid) < 0) {\n> 2016-11-16T00:31:57.5321220Z        ^~~~~\n> 2016-11-16T00:31:57.7982432Z     CC ctype.o\n> 2016-11-16T00:31:58.1418929Z cc1.exe: all warnings being treated as errors\n> 2016-11-16T00:31:58.6368128Z make: *** [Makefile:1988: copy.o] Error 1\n\nThat looks like a part of the new 'instead of run_command(\"cp -R\"),\nlet's borrow code from somewhere and do that ourselves' in the\nnd/worktree-move topic.\n\nThe offending part is this:\n\n+\tif (S_ISBLK(source_stat.st_mode) ||\n+\t    S_ISCHR(source_stat.st_mode) ||\n+\t    S_ISSOCK(source_stat.st_mode) ||\n+\t    S_ISFIFO(source_stat.st_mode)) {\n+\t\tif (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0)\n+\t\t\treturn error_errno(_(\"can't create '%s'\"), dest);\n+\t} else\n+\t\treturn error(_(\"unrecognized file '%s' with mode %x\"),\n+\t\t\t     source, source_stat.st_mode);\n\nI think all of this is meant to be used to copy what is in the\nworking tree, and what is in the working tree is meant to be tracked\nby Git, none of the four types that triggers mknod() would be\nrelevant for our purpose.  The simplest and cleanest would be to\nmake the above to return error(\"unsupported filetype\").\n\nI do not mind run_command(\"cp -R\") and get rid of this code\naltogether; that might be a more portable and sensible approach.\n\n"}]}