Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
Junio C Hamano writes: >> So, this SQUASH looks like the correct way forward. Hopefully, Junio >> can just squash it so I don't have to flood the list again with this >> long series. > > The topic already has another dependent topic so rerolling or > squashing would be a bit cumbersome. I'll see what I could do but > it may not be until tomorrow's pushout. OK, there was only one topic that forked fro this one, so squash with rebase are all in the 'pu'. Thanks, all.
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
Hi Eric, On Mon, 16 Jul 2018, Eric Sunshine wrote: > On Mon, Jul 16, 2018 at 02:37:32PM -0700, Junio C Hamano wrote: > > Eric Sunshine writes: > > > On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin > > > wrote: > > >> On Mon, 16 Jul 2018, Johannes Schindelin wrote: > > >> > > - git submodule add --force bogus-url submod && > > >> > > + git submodule add --force /bogus-url submod && > > >> > This breaks on Windows because of the absolute Unix-y path having to be > > >> > translated to a Windows path: > > >> > I could imagine that using "$(pwd)/bogus-url" (which will generate a > > >> > Windows-y absolute path on Windows) would work, though. > > >> > > >> Yes, this works indeed, see the patch below. Could you please squash it > > >> in > > >> if you send another iteration of your patch series? Junio, could you > > >> please add this as a SQUASH??? commit so that `pu` is fixed on Windows? > > > > > > So, this SQUASH looks like the correct way forward. Hopefully, Junio > > > can just squash it so I don't have to flood the list again with this > > > long series. > > > > The topic already has another dependent topic so rerolling or > > squashing would be a bit cumbersome. I'll see what I could do but > > it may not be until tomorrow's pushout. > > No problem. Here's Dscho's fix in the form of a proper patch if > that makes it easier for you (it just needs his sign-off): > > --- >8 --- > From: Johannes Schindelin > Subject: [PATCH] t7400: make "submodule add/reconfigure --force" work on > Windows > > On Windows, the "Unixy" path /bogus-url gets translated automatically to > a Windows-style path (such as C:/git-sdk/bogus_url). This is normally > not problem, since it's still a valid and usable path in that form, s/not problem/not a problem/ > however, this test wants to do a comparison against the original path. > $(pwd) was invented exactly for this case, so use it to make the path > comparison work. > > [es: commit message] > Signed-off-by: Johannes Schindelin > Signed-off-by: Eric Sunshine > --- > t/t7400-submodule-basic.sh | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index 76cf522a08..bfb09dd566 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path > with --force' ' > test_expect_success 'submodule add to reconfigure existing submodule with > --force' ' > ( > cd addtest-ignore && > - git submodule add --force /bogus-url submod && > + bogus_url="$(pwd)/bogus-url" && > + git submodule add --force "$bogus_url" submod && > git submodule add --force -b initial "$submodurl" submod-branch > && > - test "/bogus-url" = "$(git config -f .gitmodules > submodule.submod.url)" && > - test "/bogus-url" = "$(git config submodule.submod.url)" && > + test "$bogus_url" = "$(git config -f .gitmodules > submodule.submod.url)" && > + test "$bogus_url" = "$(git config submodule.submod.url)" && > # Restore the url > git submodule add --force "$submodurl" submod && > test "$submodurl" = "$(git config -f .gitmodules > submodule.submod.url)" && > -- > 2.18.0.233.g985f88cf7e Thanks! Dscho
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
On Mon, Jul 16, 2018 at 02:37:32PM -0700, Junio C Hamano wrote: > Eric Sunshine writes: > > On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin > > wrote: > >> On Mon, 16 Jul 2018, Johannes Schindelin wrote: > >> > > - git submodule add --force bogus-url submod && > >> > > + git submodule add --force /bogus-url submod && > >> > This breaks on Windows because of the absolute Unix-y path having to be > >> > translated to a Windows path: > >> > I could imagine that using "$(pwd)/bogus-url" (which will generate a > >> > Windows-y absolute path on Windows) would work, though. > >> > >> Yes, this works indeed, see the patch below. Could you please squash it in > >> if you send another iteration of your patch series? Junio, could you > >> please add this as a SQUASH??? commit so that `pu` is fixed on Windows? > > > > So, this SQUASH looks like the correct way forward. Hopefully, Junio > > can just squash it so I don't have to flood the list again with this > > long series. > > The topic already has another dependent topic so rerolling or > squashing would be a bit cumbersome. I'll see what I could do but > it may not be until tomorrow's pushout. No problem. Here's Dscho's fix in the form of a proper patch if that makes it easier for you (it just needs his sign-off): --- >8 --- From: Johannes Schindelin Subject: [PATCH] t7400: make "submodule add/reconfigure --force" work on Windows On Windows, the "Unixy" path /bogus-url gets translated automatically to a Windows-style path (such as C:/git-sdk/bogus_url). This is normally not problem, since it's still a valid and usable path in that form, however, this test wants to do a comparison against the original path. $(pwd) was invented exactly for this case, so use it to make the path comparison work. [es: commit message] Signed-off-by: Eric Sunshine --- t/t7400-submodule-basic.sh | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 76cf522a08..bfb09dd566 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path with --force' ' test_expect_success 'submodule add to reconfigure existing submodule with --force' ' ( cd addtest-ignore && - git submodule add --force /bogus-url submod && + bogus_url="$(pwd)/bogus-url" && + git submodule add --force "$bogus_url" submod && git submodule add --force -b initial "$submodurl" submod-branch && - test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" && - test "/bogus-url" = "$(git config submodule.submod.url)" && + test "$bogus_url" = "$(git config -f .gitmodules submodule.submod.url)" && + test "$bogus_url" = "$(git config submodule.submod.url)" && # Restore the url git submodule add --force "$submodurl" submod && test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" && -- 2.18.0.233.g985f88cf7e
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
Eric Sunshine writes: > On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin > wrote: >> On Mon, 16 Jul 2018, Johannes Schindelin wrote: >> > > - git submodule add --force bogus-url submod && >> > > + git submodule add --force /bogus-url submod && >> > >> > This breaks on Windows because of the absolute Unix-y path having to be >> > translated to a Windows path: >> > >> > >> > https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365=logs >> > >> > I could imagine that using "$(pwd)/bogus-url" (which will generate a >> > Windows-y absolute path on Windows) would work, though. >> >> Yes, this works indeed, see the patch below. Could you please squash it in >> if you send another iteration of your patch series? Junio, could you >> please add this as a SQUASH??? commit so that `pu` is fixed on Windows? > > Thanks for reporting and diagnosing. I wondered briefly if we could > get by with simply "./bogus-url" instead of having to use $(pwd), > however, "./bogus-url" gets normalized internally into an absolute > path, so $(pwd) is needed anyhow to make the test '"$bogus_url" = > "$(git config ...)"' check work. > > So, this SQUASH looks like the correct way forward. Hopefully, Junio > can just squash it so I don't have to flood the list again with this > long series. The topic already has another dependent topic so rerolling or squashing would be a bit cumbersome. I'll see what I could do but it may not be until tomorrow's pushout.
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin wrote: > On Mon, 16 Jul 2018, Johannes Schindelin wrote: > > > - git submodule add --force bogus-url submod && > > > + git submodule add --force /bogus-url submod && > > > > This breaks on Windows because of the absolute Unix-y path having to be > > translated to a Windows path: > > > > > > https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365=logs > > > > I could imagine that using "$(pwd)/bogus-url" (which will generate a > > Windows-y absolute path on Windows) would work, though. > > Yes, this works indeed, see the patch below. Could you please squash it in > if you send another iteration of your patch series? Junio, could you > please add this as a SQUASH??? commit so that `pu` is fixed on Windows? Thanks for reporting and diagnosing. I wondered briefly if we could get by with simply "./bogus-url" instead of having to use $(pwd), however, "./bogus-url" gets normalized internally into an absolute path, so $(pwd) is needed anyhow to make the test '"$bogus_url" = "$(git config ...)"' check work. So, this SQUASH looks like the correct way forward. Hopefully, Junio can just squash it so I don't have to flood the list again with this long series.
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
Hi Eric, On Mon, 16 Jul 2018, Johannes Schindelin wrote: > On Sun, 1 Jul 2018, Eric Sunshine wrote: > > > This test has been dysfunctional since it was added by 619acfc78c > > (submodule add: extend force flag to add existing repos, 2016-10-06), > > however, two problems early in the test went unnoticed due to a broken > > &&-chain later in the test. > > > > First, it tries configuring the submodule with repository "bogus-url", > > however, "git submodule add" insists that the repository be either an > > absolute URL or a relative pathname requiring prefix "./" or "../" (this > > is true even with --force), but "bogus-url" does not meet those > > criteria, thus the command fails. > > > > Second, it then tries configuring a submodule with a path which is > > .gitignore'd, which is disallowed. This restriction can be overridden > > with --force, but the test neglects to use that option. > > > > Fix both problems, as well as the broken &&-chain behind which they hid. > > > > Reviewed-by: Stefan Beller > > Signed-off-by: Eric Sunshine > > --- > > t/t7400-submodule-basic.sh | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > > index 812db137b8..401adaed32 100755 > > --- a/t/t7400-submodule-basic.sh > > +++ b/t/t7400-submodule-basic.sh > > @@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored > > path with --force' ' > > test_expect_success 'submodule add to reconfigure existing submodule with > > --force' ' > > ( > > cd addtest-ignore && > > - git submodule add --force bogus-url submod && > > - git submodule add -b initial "$submodurl" submod-branch && > > - test "bogus-url" = "$(git config -f .gitmodules > > submodule.submod.url)" && > > - test "bogus-url" = "$(git config submodule.submod.url)" && > > + git submodule add --force /bogus-url submod && > > + git submodule add --force -b initial "$submodurl" submod-branch > > && > > + test "/bogus-url" = "$(git config -f .gitmodules > > submodule.submod.url)" && > > + test "/bogus-url" = "$(git config submodule.submod.url)" && > > # Restore the url > > - git submodule add --force "$submodurl" submod > > + git submodule add --force "$submodurl" submod && > > test "$submodurl" = "$(git config -f .gitmodules > > submodule.submod.url)" && > > test "$submodurl" = "$(git config submodule.submod.url)" > > ) > > This breaks on Windows because of the absolute Unix-y path having to be > translated to a Windows path: > > > https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365=logs > > (In this case, it is prefixed with `C:/git-sdk-64-ci` because that is the > pseudo root when running in a Git for Windows SDK that is installed into > that directory.) > > I could imagine that using "$(pwd)/bogus-url" (which will generate a > Windows-y absolute path on Windows) would work, though. Yes, this works indeed, see the patch below. Could you please squash it in if you send another iteration of your patch series? Junio, could you please add this as a SQUASH??? commit so that `pu` is fixed on Windows? Thanks. -- snipsnap -- diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 5ffc2726aad..2c2c97e1441 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path with --force' ' test_expect_success 'submodule add to reconfigure existing submodule with --force' ' ( cd addtest-ignore && - git submodule add --force /bogus-url submod && + bogus_url="$(pwd)/bogus-url" && + git submodule add --force "$bogus_url" submod && git submodule add --force -b initial "$submodurl" submod-branch && - test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" && - test "/bogus-url" = "$(git config submodule.submod.url)" && + test "$bogus_url" = "$(git config -f .gitmodules submodule.submod.url)" && + test "$bogus_url" = "$(git config submodule.submod.url)" && # Restore the url git submodule add --force "$submodurl" submod && test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
Hi Eric, On Sun, 1 Jul 2018, Eric Sunshine wrote: > This test has been dysfunctional since it was added by 619acfc78c > (submodule add: extend force flag to add existing repos, 2016-10-06), > however, two problems early in the test went unnoticed due to a broken > &&-chain later in the test. > > First, it tries configuring the submodule with repository "bogus-url", > however, "git submodule add" insists that the repository be either an > absolute URL or a relative pathname requiring prefix "./" or "../" (this > is true even with --force), but "bogus-url" does not meet those > criteria, thus the command fails. > > Second, it then tries configuring a submodule with a path which is > .gitignore'd, which is disallowed. This restriction can be overridden > with --force, but the test neglects to use that option. > > Fix both problems, as well as the broken &&-chain behind which they hid. > > Reviewed-by: Stefan Beller > Signed-off-by: Eric Sunshine > --- > t/t7400-submodule-basic.sh | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index 812db137b8..401adaed32 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored path > with --force' ' > test_expect_success 'submodule add to reconfigure existing submodule with > --force' ' > ( > cd addtest-ignore && > - git submodule add --force bogus-url submod && > - git submodule add -b initial "$submodurl" submod-branch && > - test "bogus-url" = "$(git config -f .gitmodules > submodule.submod.url)" && > - test "bogus-url" = "$(git config submodule.submod.url)" && > + git submodule add --force /bogus-url submod && > + git submodule add --force -b initial "$submodurl" submod-branch > && > + test "/bogus-url" = "$(git config -f .gitmodules > submodule.submod.url)" && > + test "/bogus-url" = "$(git config submodule.submod.url)" && > # Restore the url > - git submodule add --force "$submodurl" submod > + git submodule add --force "$submodurl" submod && > test "$submodurl" = "$(git config -f .gitmodules > submodule.submod.url)" && > test "$submodurl" = "$(git config submodule.submod.url)" > ) This breaks on Windows because of the absolute Unix-y path having to be translated to a Windows path: https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365=logs (In this case, it is prefixed with `C:/git-sdk-64-ci` because that is the pseudo root when running in a Git for Windows SDK that is installed into that directory.) I could imagine that using "$(pwd)/bogus-url" (which will generate a Windows-y absolute path on Windows) would work, though. Ciao, Dscho
