Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> > expecting success:
>> > actual=$(git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' '../.') &&
>> > test "$actual" = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
>> >
>> > +++ git submodule--helper resolve-relative-url-test '(null)' '/usr/src/git/wip/t/trash directory.t0060-path-utils/.' ../.
>> > ++ actual=C:/git-sdk-64/usr/src/git/wip/t/.
>> > ++ test C:/git-sdk-64/usr/src/git/wip/t/. = 'C:/git-sdk-64/usr/src/git/wip/t/trash directory.t0060-path-utils/.'
This may well be total misunderstanding on my side, but is the
expectation of this test even correct? If it wants to take "../."
relative to "$LEAD/t/trash utils/.", should't it go one level up
with ".." to $LEAD/t and then stay there with ".", expecting
"$LEAD/t" which is what the above is giving us?
IOW, the above makes me wonder why having one of these as the base
A - path/to/dir
B - path/to/dir/
C - path/to/dir/.
to resolve the relative "../." give different results. Whether bash
on Windows removes the dot at the end of C to turn it into B, as
long as A and B give us the same result we wouldn't be hitting the
problem, no?
>> > test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
>> > test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
>> > -test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
>> > +test_submodule_relative_url "(null)" "$(pwd)/." "../." "$(pwd)/."
>> > The reasons this is ugly: we specifically test for *Unixy* paths when we
>> > use $PWD, as opposed to *Windowsy* paths when using $(pwd).
Just to ensure I am following this correctly, two tests that come
before the one you are touching above have $PWD on the input side
and $(pwd) on the expectation side. That is what you mean by the
next paragraph, right? They want to make sure that you honor the
Unixy user input on Windows and still produce Windowsy result, that
is.
>> > We do this to
>> > ensure a certain level of confidence that running things such as
>> >
>> > git clone --recurse-submodules /z/project/.
>> >
>> > work. And now that does not work anymore.
And I agree from that point of view that having to spell both sides
as $(pwd) would mean you are not testing that "Unixy input to
Windowsy output" expectation, but at the same time, I think you
would want "Windowsy input to Windowsy output" combination also does
produce correct result, which is not tested in the three tests shown
above. IOW, probably you would want to test both (at least on any
platform where $PWD and $(pwd) textually disagree) for all these
[*1*], and the pair
"../." taken relative to "$(pwd)/." must be "$(pwd)/."
"../." taken relative to "$PWD/." must be "$(pwd)/."
test, because of the limitation of your bash, cannot have the latter
half of the pair, so you'd need to comment it out with in-code
explanation, perhaps? IOW something along the lines of...
-- >8 -- snip -- >8 --
test_submodule_relative_url "(null)" "$(pwd)/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
test_submodule_relative_url "(null)" "$(pwd)/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
test_submodule_relative_url "(null)" "$(pwd)/." "../." "$(pwd)/."
if test_have_prereq MINGW
then
test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
# This does not work correctly because Win-Bash strips . at the end
# "of $PWD/."
# test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
fi
-- >8 -- snip -- >8 --
In any case, I find it more disturbing that we somehow ended up with
a system where these three things are expected to behave differently:
A - path/to/dir
B - path/to/dir/
C - path/to/dir/.
Is that something we can fix?
[Footnote]
*1* It is tempting to update the above test sequence using
a helper like:
tsru () {
test_submodule_relative_url "(null)" "$(pwd)/$1" "$2" "$(pwd)/$3"
if test_have_prereq MINGW
then
test_submodule_relative_url "(null)" "$PWD/$1" "$2" "$(pwd)/$3"
fi
}
then write the above three tests like so:
tsru subsuper_update_r ../subsubsuper_update_r subsubsuper_update_r
tsru super_update_r2 ../subsuper_update_r subsuper_update_r
tsru . ../. .
but you would want to disable the MINGW half for only the third
test, we cannot quite do that.