Skip to content
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

Jump to bottom

Undesired submodule url #3538

Open
mih opened this issue Jul 17, 2019 · 1 comment · Fixed by #3966
Open

Undesired submodule url #3538

mih opened this issue Jul 17, 2019 · 1 comment · Fixed by #3966
Labels
corpse-in-basement fix-implemented

Comments

@mih
Copy link
Member

@mih mih commented Jul 17, 2019

Originally at #2601

datalad install -d . -s ../raw sourcedata --reckless

leads to

--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "sourcedata"]
+       path = sourcedata
+       url = /home/data/psyinf/scratch/multires3t/bids/../raw
@mih mih mentioned this issue Jul 17, 2019
Closed
@mih mih mentioned this issue Jan 10, 2020
Merged
18 tasks
mih added a commit to mih/datalad that referenced this issue Jan 10, 2020
@mih
BF: Avoid relpath mangling for submodule url configuration
02e2b4c 
Git turns these into absolute paths, but relpath specification might
actually be useful for discovery, e.g. when datasets are still stored
in the same relative locations, but were jointly moved from an original
to a new absolute path.

Fixes dataladgh-3538
@mih mih added the fix-implemented label Jan 10, 2020
@mih mih closed this as completed in #3966 Jan 11, 2020
kyleam added a commit to kyleam/datalad that referenced this issue Jan 15, 2020
@kyleam
BF: clone: Revert incorrect relative path adjustment to URLs
604b90b 
When git clone is called with a local relative path as the URL, it
will prepend the current working directory to the URL to get an
absolute path.  "../" components are retained.  In common DataLad
scenarios (see dataladgh-3538), installing a subdataset using a local path
source can lead to .git/config URLs such as

    url = /tmp/super/../source

A downside of these absolute paths is that they become invalid for
URLs that point within a dataset hierarchy (e.g., "../../source") if
the hierarchy is moved to a different location.  With relative URLs,
the URLs would remain valid as long the relative relationships are
retained.  (On the other hand, absolute paths provide a better
stability when moving datasets if the URLs point outside of the
hierarchy.)

Two recent commits from dataladgh-3966, 02e2b4c (BF: Avoid relpath mangling
for submodule url configuration, 2020-01-10) followed up by 0a80bb4
(BF: submodule relpath must start with ./ or ../, 2020-01-10), tried
to avoid these absolute URLs.  The combined result says "if the URL
given is a local path and is relative, overwrite the configured URL
with that path (prepending ./ if the path doesn't start with ../)".

However, this logic points the URL to the wrong location.  Here are
some examples (similar to the cases the added test):

  * For `git clone a b`, the correct location from _within_ repo b
    would "../a", not "./a".

  * Similarly, for `git clone ../a b`, the correct location would be
    "../../a", not "../a"

  * Calling `git clone a ../b` from $DIR, the correct location would
    "$DIR/a", not "./a".

An alternative approach is needed, but it seems unlikely to have a
quick and obviously correct fix, so let's revert the changes from
02e2b4c and 0a80bb4 to avoid these invalid URLs.

Re-opens dataladgh-3538.
kyleam added a commit to kyleam/datalad that referenced this issue Jan 15, 2020
@kyleam
BF: clone: Revert incorrect relative path adjustment to URLs
e8c5d70 
When git clone is called with a local relative path as the URL, it
will prepend the current working directory to the URL to get an
absolute path.  "../" components are retained.  In common DataLad
scenarios (see dataladgh-3538), installing a subdataset using a local path
source can lead to .git/config URLs such as

    url = /tmp/super/../source

A downside of these absolute paths is that they become invalid for
URLs that point within a dataset hierarchy (e.g., "../../source") if
the hierarchy is moved to a different location.  With relative URLs,
the URLs would remain valid as long the relative relationships are
retained.  (On the other hand, absolute paths provide a better
stability when moving datasets if the URLs point outside of the
hierarchy.)

Two recent commits from dataladgh-3966, 02e2b4c (BF: Avoid relpath mangling
for submodule url configuration, 2020-01-10) followed up by 0a80bb4
(BF: submodule relpath must start with ./ or ../, 2020-01-10), tried
to avoid these absolute URLs.  The combined result says "if the URL
given is a local path and is relative, overwrite the configured URL
with that path (prepending ./ if the path doesn't start with ../)".

However, this logic points the URL to the wrong location.  Here are
some examples (similar to the cases the added test):

  * For `git clone a b`, the correct location from _within_ repo b
    would "../a", not "./a".

  * Similarly, for `git clone ../a b`, the correct location would be
    "../../a", not "../a"

  * Calling `git clone a ../b` from $DIR, the correct location would
    "$DIR/a", not "./a".

An alternative approach is needed, but it seems unlikely to have a
quick and obviously correct fix, so let's revert the changes from
02e2b4c and 0a80bb4 to avoid these invalid URLs.

Re-opens dataladgh-3538.
@kyleam kyleam mentioned this issue Jan 15, 2020
Merged
@kyleam
Copy link
Contributor

@kyleam kyleam commented Jan 15, 2020

Reverted in 0.12.1, but a new approach has been proposed in gh-4026.

@kyleam kyleam reopened this Jan 15, 2020
bpoldrack pushed a commit to bpoldrack/datalad that referenced this issue Jan 24, 2020
@kyleam @bpoldrack
BF: clone: Revert incorrect relative path adjustment to URLs
d5b4930 
When git clone is called with a local relative path as the URL, it
will prepend the current working directory to the URL to get an
absolute path.  "../" components are retained.  In common DataLad
scenarios (see dataladgh-3538), installing a subdataset using a local path
source can lead to .git/config URLs such as

    url = /tmp/super/../source

A downside of these absolute paths is that they become invalid for
URLs that point within a dataset hierarchy (e.g., "../../source") if
the hierarchy is moved to a different location.  With relative URLs,
the URLs would remain valid as long the relative relationships are
retained.  (On the other hand, absolute paths provide a better
stability when moving datasets if the URLs point outside of the
hierarchy.)

Two recent commits from dataladgh-3966, 02e2b4c (BF: Avoid relpath mangling
for submodule url configuration, 2020-01-10) followed up by 0a80bb4
(BF: submodule relpath must start with ./ or ../, 2020-01-10), tried
to avoid these absolute URLs.  The combined result says "if the URL
given is a local path and is relative, overwrite the configured URL
with that path (prepending ./ if the path doesn't start with ../)".

However, this logic points the URL to the wrong location.  Here are
some examples (similar to the cases the added test):

  * For `git clone a b`, the correct location from _within_ repo b
    would "../a", not "./a".

  * Similarly, for `git clone ../a b`, the correct location would be
    "../../a", not "../a"

  * Calling `git clone a ../b` from $DIR, the correct location would
    "$DIR/a", not "./a".

An alternative approach is needed, but it seems unlikely to have a
quick and obviously correct fix, so let's revert the changes from
02e2b4c and 0a80bb4 to avoid these invalid URLs.

Re-opens dataladgh-3538.
@mih mih mentioned this issue Sep 1, 2020
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
corpse-in-basement fix-implemented
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Transition clone() to core API mih/datalad
2 participants
@mih @kyleam