Skip to content

Commit

Permalink
Merge pull request #3797 from mih/rf-resolvepath
Browse files Browse the repository at this point in the history
  • Loading branch information
kyleam committed Oct 19, 2019
2 parents e7305d3 + 4f68b47 commit c016d3f
Show file tree
Hide file tree
Showing 14 changed files with 126 additions and 120 deletions.
4 changes: 2 additions & 2 deletions datalad/core/local/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
datasetmethod,
EnsureDataset,
rev_get_dataset_root,
rev_resolve_path,
resolve_path,
path_under_rev_dataset,
require_dataset,
)
Expand Down Expand Up @@ -200,7 +200,7 @@ def __call__(
"no annex repo.")

if path:
path = rev_resolve_path(path, dataset)
path = resolve_path(path, dataset)

path = path if path \
else getpwd() if ds is None \
Expand Down
4 changes: 2 additions & 2 deletions datalad/core/local/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
Dataset,
datasetmethod,
require_dataset,
rev_resolve_path,
resolve_path,
path_under_rev_dataset,
rev_get_dataset_root,
)
Expand Down Expand Up @@ -159,7 +159,7 @@ def _diff_cmd(
# special case is the root dataset, always report its content
# changes
orig_path = str(p)
resolved_path = rev_resolve_path(p, dataset)
resolved_path = resolve_path(p, dataset)
p = \
resolved_path, \
orig_path.endswith(op.sep) or resolved_path == ds.pathobj
Expand Down
4 changes: 2 additions & 2 deletions datalad/core/local/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
EnsureDataset,
datasetmethod,
require_dataset,
rev_resolve_path,
resolve_path,
path_under_rev_dataset,
rev_get_dataset_root,
)
Expand Down Expand Up @@ -301,7 +301,7 @@ def __call__(
# given path argument, before any normalization happens
# for further decision logic below
orig_path = str(p)
p = rev_resolve_path(p, dataset)
p = resolve_path(p, dataset)
root = rev_get_dataset_root(str(p))
if root is None:
# no root, not possibly underneath the refds
Expand Down
4 changes: 2 additions & 2 deletions datalad/distributed/create_sibling_gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
datasetmethod,
EnsureDataset,
require_dataset,
rev_resolve_path,
resolve_path,
)
from ..dochelpers import exc_str

Expand Down Expand Up @@ -216,7 +216,7 @@ def __call__(
publish_depends=None,
description=None,
dryrun=False):
path = rev_resolve_path(assure_list(path), ds=dataset) \
path = resolve_path(assure_list(path), ds=dataset) \
if path else None

if project and (recursive or (path and len(path) > 1)):
Expand Down
6 changes: 3 additions & 3 deletions datalad/distribution/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
from datalad.distribution.dataset import (
Dataset,
datasetmethod,
rev_resolve_path,
resolve_path,
require_dataset,
EnsureDataset,
)
Expand Down Expand Up @@ -158,7 +158,7 @@ def __call__(
path))

if path is not None:
path = rev_resolve_path(path, dataset)
path = resolve_path(path, dataset)

# Possibly do conversion from source into a git-friendly url
# luckily GitRepo will undo any fancy file:/// url to make use of Git's
Expand All @@ -175,7 +175,7 @@ def __call__(
# and derive the path from the source and continue
path = _get_installationpath_from_url(source)
# since this is a relative `path`, resolve it:
path = rev_resolve_path(path, dataset)
path = resolve_path(path, dataset)
lgr.debug("Determined clone target path from source")
lgr.debug("Resolved clone target path to: '%s'", path)

Expand Down
37 changes: 5 additions & 32 deletions datalad/distribution/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from os.path import normpath, isabs
from os.path import pardir
from os.path import realpath
from os.path import relpath
from weakref import WeakValueDictionary
import wrapt

Expand All @@ -41,9 +40,8 @@

import datalad.utils as ut
from datalad.utils import getpwd
from datalad.utils import optional_args, expandpath, is_explicit_path
from datalad.utils import optional_args
from datalad.utils import get_dataset_root
from datalad.utils import dlabspath
from datalad.utils import Path
from datalad.utils import PurePath
from datalad.utils import assure_list
Expand All @@ -53,34 +51,6 @@
lgr.log(5, "Importing dataset")


# TODO: use the same piece for resolving paths against Git/AnnexRepo instances
# (see normalize_path)
def resolve_path(path, ds=None):
"""Resolve a path specification (against a Dataset location)
Any explicit path (absolute or relative) is returned as an absolute path.
In case of an explicit relative path, the current working directory is
used as a reference. Any non-explicit relative path is resolved against
as dataset location, i.e. considered relative to the location of the
dataset. If no dataset is provided, the current working directory is
used.
Returns
-------
Absolute path
"""
path = expandpath(path, force_absolute=False)
if is_explicit_path(path):
# normalize path consistently between two (explicit and implicit) cases
return dlabspath(path, norm=True)

# no dataset given, use CWD as reference
# note: abspath would disregard symlink in CWD
top_path = getpwd() \
if ds is None else ds.path if isinstance(ds, Dataset) else ds
return normpath(opj(top_path, path))


class Dataset(object, metaclass=Flyweight):
"""Representation of a DataLad dataset/repository
Expand Down Expand Up @@ -608,7 +578,7 @@ def require_dataset(dataset, check_installed=True, purpose=None):
# New helpers, courtesy of datalad-revolution.


def rev_resolve_path(path, ds=None):
def resolve_path(path, ds=None):
"""Resolve a path specification (against a Dataset location)
Any path is returned as an absolute path. If, and only if, a dataset
Expand Down Expand Up @@ -697,6 +667,9 @@ def rev_resolve_path(path, ds=None):
out.append(p)
return out[0] if isinstance(path, (str, PurePath)) else out

# TODO keep this around for a while so that extensions can be updated
rev_resolve_path = resolve_path


def path_under_rev_dataset(ds, path):
ds_path = ds.pathobj
Expand Down
4 changes: 2 additions & 2 deletions datalad/distribution/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

from datalad.distribution.dataset import (
datasetmethod,
rev_resolve_path,
resolve_path,
require_dataset,
EnsureDataset,
)
Expand Down Expand Up @@ -308,7 +308,7 @@ def __call__(
# a peculiar use-case IMHO
# TODO Stringification can be removed once PY35 is no longer
# supported
path = str(rev_resolve_path(path_ri.localpath, dataset))
path = str(resolve_path(path_ri.localpath, dataset))
# any `path` argument that point to something local now
# resolved and is no longer a URL
except ValueError:
Expand Down
96 changes: 33 additions & 63 deletions datalad/distribution/tests/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,18 @@
"""

import os
import shutil
import os.path as op
from os.path import join as opj, abspath, normpath, relpath, exists
from os.path import join as opj, abspath, relpath


from ..dataset import Dataset, EnsureDataset, resolve_path, require_dataset
from ..dataset import rev_resolve_path
from ..dataset import Dataset, EnsureDataset, require_dataset
from ..dataset import resolve_path
from datalad import cfg
from datalad.api import create
from datalad.api import get
import datalad.utils as ut
from datalad.utils import chpwd, getpwd, rmtree
from datalad.utils import chpwd, rmtree
from datalad.utils import _path_
from datalad.utils import get_dataset_root
from datalad.utils import on_windows
from datalad.utils import Path
from datalad.support.gitrepo import GitRepo
Expand All @@ -32,8 +30,7 @@
from nose.tools import ok_, eq_, assert_false, assert_equal, assert_true, \
assert_is_instance, assert_is_none, assert_is_not, assert_is_not_none
from datalad.tests.utils import SkipTest
from datalad.tests.utils import with_tempfile, assert_in, with_tree, with_testrepos
from datalad.tests.utils import assert_cwd_unchanged
from datalad.tests.utils import with_tempfile, with_testrepos
from datalad.tests.utils import assert_raises
from datalad.tests.utils import known_failure_windows
from datalad.tests.utils import assert_is
Expand All @@ -42,7 +39,6 @@
from datalad.tests.utils import OBSCURE_FILENAME

from datalad.support.exceptions import InsufficientArgumentsError
from datalad.support.exceptions import PathKnownToRepositoryError


def test_EnsureDataset():
Expand All @@ -64,32 +60,6 @@ def test_EnsureDataset():
# part of the constraint itself, so not explicitly tested here.


@assert_cwd_unchanged
@with_tempfile(mkdir=True)
def test_resolve_path(somedir):

abs_path = abspath(somedir) # just to be sure
rel_path = "some"
expl_path_cur = opj(os.curdir, rel_path)
expl_path_par = opj(os.pardir, rel_path)

eq_(resolve_path(abs_path), abs_path)

current = getpwd()
# no Dataset => resolve using cwd:
eq_(resolve_path(abs_path), abs_path)
eq_(resolve_path(rel_path), opj(current, rel_path))
eq_(resolve_path(expl_path_cur), normpath(opj(current, expl_path_cur)))
eq_(resolve_path(expl_path_par), normpath(opj(current, expl_path_par)))

# now use a Dataset as reference:
ds = Dataset(abs_path)
eq_(resolve_path(abs_path, ds), abs_path)
eq_(resolve_path(rel_path, ds), opj(abs_path, rel_path))
eq_(resolve_path(expl_path_cur, ds), normpath(opj(current, expl_path_cur)))
eq_(resolve_path(expl_path_par, ds), normpath(opj(current, expl_path_par)))


# TODO: test remember/recall more extensive?


Expand Down Expand Up @@ -476,7 +446,7 @@ def test_symlinked_dataset_properties(repo1, repo2, repo3, non_repo, symlink):


@with_tempfile(mkdir=True)
def test_rev_resolve_path(path):
def test_resolve_path(path):
if op.realpath(path) != path:
raise SkipTest("Test assumptions require non-symlinked parent paths")
# initially ran into on OSX https://github.com/datalad/datalad/issues/2406
Expand All @@ -492,72 +462,72 @@ def test_rev_resolve_path(path):
for d in (opath,) if on_windows else (opath, lpath):
ds_local = Dataset(d)
# no symlink resolution
eq_(str(rev_resolve_path(d)), d)
eq_(str(resolve_path(d)), d)
# list comes out as a list
eq_(rev_resolve_path([d]), [Path(d)])
eq_(resolve_path([d]), [Path(d)])
# multiple OK
eq_(rev_resolve_path([d, d]), [Path(d), Path(d)])
eq_(resolve_path([d, d]), [Path(d), Path(d)])

with chpwd(d):
# be aware: knows about cwd, but this CWD has symlinks resolved
eq_(str(rev_resolve_path(d).cwd()), opath)
eq_(str(resolve_path(d).cwd()), opath)
# using pathlib's `resolve()` will resolve any
# symlinks
# also resolve `opath`, as on old windows systems the path might
# come in crippled (e.g. C:\Users\MIKE~1/...)
# and comparison would fails unjustified
eq_(rev_resolve_path('.').resolve(), ut.Path(opath).resolve())
eq_(resolve_path('.').resolve(), ut.Path(opath).resolve())
# no norming, but absolute paths, without resolving links
eq_(rev_resolve_path('.'), ut.Path(d))
eq_(str(rev_resolve_path('.')), d)
eq_(resolve_path('.'), ut.Path(d))
eq_(str(resolve_path('.')), d)

# there is no concept of an "explicit" relative path anymore
# relative is relative, regardless of the specific syntax
eq_(rev_resolve_path(op.join(os.curdir, 'bu'), ds=ds_global),
eq_(resolve_path(op.join(os.curdir, 'bu'), ds=ds_global),
ds_global.pathobj / 'bu')
# there is no full normpath-ing or other funky resolution of
# parent directory back-reference
eq_(str(rev_resolve_path(op.join(os.pardir, 'bu'), ds=ds_global)),
eq_(str(resolve_path(op.join(os.pardir, 'bu'), ds=ds_global)),
op.join(ds_global.path, os.pardir, 'bu'))

# resolve against a dataset given as a path/str
# (cmdline input scenario)
eq_(rev_resolve_path('bu', ds=ds_local.path), Path.cwd() / 'bu')
eq_(rev_resolve_path('bu', ds=ds_global.path), Path.cwd() / 'bu')
eq_(resolve_path('bu', ds=ds_local.path), Path.cwd() / 'bu')
eq_(resolve_path('bu', ds=ds_global.path), Path.cwd() / 'bu')
# resolve against a dataset given as a dataset instance
# (object method scenario)
eq_(rev_resolve_path('bu', ds=ds_local), ds_local.pathobj / 'bu')
eq_(rev_resolve_path('bu', ds=ds_global), ds_global.pathobj / 'bu')
eq_(resolve_path('bu', ds=ds_local), ds_local.pathobj / 'bu')
eq_(resolve_path('bu', ds=ds_global), ds_global.pathobj / 'bu')
# not being inside a dataset doesn't change the resolution result
eq_(rev_resolve_path(op.join(os.curdir, 'bu'), ds=ds_global),
eq_(resolve_path(op.join(os.curdir, 'bu'), ds=ds_global),
ds_global.pathobj / 'bu')
eq_(str(rev_resolve_path(op.join(os.pardir, 'bu'), ds=ds_global)),
eq_(str(resolve_path(op.join(os.pardir, 'bu'), ds=ds_global)),
op.join(ds_global.path, os.pardir, 'bu'))


# little brother of the test above, but actually (must) run
# under any circumstances
@with_tempfile(mkdir=True)
def test_rev_resolve_path_symlink_edition(path):
def test_resolve_path_symlink_edition(path):
deepest = ut.Path(path) / 'one' / 'two' / 'three'
deepest_str = str(deepest)
os.makedirs(deepest_str)
with chpwd(deepest_str):
# direct absolute
eq_(deepest, rev_resolve_path(deepest))
eq_(deepest, rev_resolve_path(deepest_str))
eq_(deepest, resolve_path(deepest))
eq_(deepest, resolve_path(deepest_str))
# explicit direct relative
eq_(deepest, rev_resolve_path('.'))
eq_(deepest, rev_resolve_path(op.join('.', '.')))
eq_(deepest, rev_resolve_path(op.join('..', 'three')))
eq_(deepest, rev_resolve_path(op.join('..', '..', 'two', 'three')))
eq_(deepest, rev_resolve_path(op.join('..', '..', '..',
eq_(deepest, resolve_path('.'))
eq_(deepest, resolve_path(op.join('.', '.')))
eq_(deepest, resolve_path(op.join('..', 'three')))
eq_(deepest, resolve_path(op.join('..', '..', 'two', 'three')))
eq_(deepest, resolve_path(op.join('..', '..', '..',
'one', 'two', 'three')))
# weird ones
eq_(deepest, rev_resolve_path(op.join('..', '.', 'three')))
eq_(deepest, rev_resolve_path(op.join('..', 'three', '.')))
eq_(deepest, rev_resolve_path(op.join('..', 'three', '.')))
eq_(deepest, rev_resolve_path(op.join('.', '..', 'three')))
eq_(deepest, resolve_path(op.join('..', '.', 'three')))
eq_(deepest, resolve_path(op.join('..', 'three', '.')))
eq_(deepest, resolve_path(op.join('..', 'three', '.')))
eq_(deepest, resolve_path(op.join('.', '..', 'three')))


@with_tempfile(mkdir=True)
Expand Down
Loading

0 comments on commit c016d3f

Please sign in to comment.