Skip to content

Erroneous git url construction with relative Windows paths for subdatasets #7180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adswa opened this issue Nov 16, 2022 · 1 comment · Fixed by #7181
Closed

Erroneous git url construction with relative Windows paths for subdatasets #7180

adswa opened this issue Nov 16, 2022 · 1 comment · Fixed by #7181

Comments

@adswa
Copy link
Member

adswa commented Nov 16, 2022

I found a Windows issue while looking into fixing #3538.
If we clone a local relative Windows path into another dataset on Windows, the remote git url of the resulting subdataset will be butchered - also with git on its own:

C:\Users\adina\AppData\Local\Temp\datalad_temp_1szhiauj\ds>git clone ..\origin dummy
Cloning into 'dummy'...
done.

C:\Users\adina\AppData\Local\Temp\datalad_temp_1szhiauj\ds>git -C dummy config --list
[...]
remote.origin.url=C:/Users/adina/AppData/Local/Temp/datalad_temp_1szhiauj/ds/..\origin

Consulting git clone's manpage confirms that the desired format for clone urls are forward slashes only.
Because we pass urls consisting of relative paths as Windows paths (with escaped backslashes) on Windows, the result of a DataLad clone is even more messed up:

'C:/Users/adina/AppData/Local/Temp/datalad_temp_frvczceh/ds/..\\origin'
adswa added a commit to adswa/datalad that referenced this issue Nov 16, 2022
adswa added a commit to adswa/datalad that referenced this issue Nov 16, 2022
Git clone's url format does not include backslashes (see man git clone).
Therefore, this commits ensures that paths provided to clone on
Windows are compliying to posix.
When platform specific windows paths were passed on by clone as git urls,
cloning succeeded, but resulting remote urls can became disfunctional.
Specifically, in the case of cloning a subdataset from a local
relative path, git will append the relative Windows path to
an otherwise Posix-confirming absolute path in the remote url
in the subdatasets .git/config:

'C:/Users/adina/AppData/Local/Temp/datalad_temp_frvczceh/ds/..\\origin'

Fixes datalad#7180
adswa added a commit to adswa/datalad that referenced this issue Nov 16, 2022
adswa added a commit to adswa/datalad that referenced this issue Nov 16, 2022
Git clone's url format does not include backslashes (see man git clone).
Therefore, this commits ensures that paths provided to clone on
Windows are compliying to posix.
When platform specific windows paths were passed on by clone as git urls,
cloning succeeded, but resulting remote urls can became disfunctional.
Specifically, in the case of cloning a subdataset from a local
relative path, git will append the relative Windows path to
an otherwise Posix-confirming absolute path in the remote url
in the subdatasets .git/config:

'C:/Users/adina/AppData/Local/Temp/datalad_temp_frvczceh/ds/..\\origin'

Fixes datalad#7180
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
never saw (CI logs weren't preserved), but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
never saw (CI logs weren't preserved), but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
never saw (CI logs weren't preserved), but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
Git clone's url format does not include backslashes (see man git clone).
Therefore, this commits ensures that paths provided to clone on
Windows are compliying to posix.
When platform specific windows paths were passed on by clone as git urls,
cloning succeeded, but resulting remote urls can became disfunctional.
Specifically, in the case of cloning a subdataset from a local
relative path, git will append the relative Windows path to
an otherwise Posix-confirming absolute path in the remote url
in the subdatasets .git/config:

'C:/Users/adina/AppData/Local/Temp/datalad_temp_frvczceh/ds/..\\origin'

Fixes datalad#7180
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
never saw (CI logs weren't preserved), but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
never saw (CI logs weren't preserved), but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 17, 2022
This change reincarnates parts of a changeset originally proposed in
datalad#4026, that didn't make it in due
to a failing Windows test that Ivnever saw (CI logs weren't preserved),
but that I suspect to originate
in datalad#7180 (Git stitching together posix and windows paths into a
non-functional URL). Thus, this change sits on top of datalad#7181, which fixes
these URLs to be fully posix.
Based on those fixes, this change wrangles absolute URLs originating
from relative paths back into relative paths, to keep them functional.
This change also enables an old test for this problem, marked as a known
failure.

Fixes datalad#3538
adswa added a commit to adswa/datalad that referenced this issue Nov 21, 2022
use constant instead of hard coded remote name
adswa added a commit to adswa/datalad that referenced this issue Nov 21, 2022
Git clone's url format does not include backslashes (see man git clone).
Therefore, this commits ensures that paths provided to clone on
Windows are compliying to posix.
When platform specific windows paths were passed on by clone as git urls,
cloning succeeded, but resulting remote urls can became disfunctional.
Specifically, in the case of cloning a subdataset from a local
relative path, git will append the relative Windows path to
an otherwise Posix-confirming absolute path in the remote url
in the subdatasets .git/config:

'C:/Users/adina/AppData/Local/Temp/datalad_temp_frvczceh/ds/..\\origin'

Fixes datalad#7180
@yarikoptic-gitmate
Copy link
Collaborator

Issue fixed in 0.17.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants