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

Fix recent test_copy_file_prevent_dotgit_placement failure #5258

Merged
merged 4 commits into from Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion datalad/local/copy_file.py
Expand Up @@ -227,7 +227,12 @@ def __call__(
if not specs_from:
raise ValueError("Neither `paths` nor `specs_from` given.")

if not target_dir and ds:
if target_dir:
if ".git" in target_dir.parts:
raise ValueError(
"Target directory should not contain a .git directory: {}"
.format(target_dir))
elif ds:
# no specific target set, but we have to write into a dataset,
# and one was given. It seems to make sense to use this dataset
# as a target. it is already to reference for any path resolution.
Expand Down
28 changes: 27 additions & 1 deletion datalad/local/tests/test_copy_file.py
Expand Up @@ -27,6 +27,7 @@
chpwd,
eq_,
nok_,
ok_,
ok_file_has_content,
serve_path_via_http,
slow,
Expand Down Expand Up @@ -337,6 +338,31 @@ def test_copy_file_prevent_dotgit_placement(srcpath, destpath):
assert_in_results(
dest.copy_file(
[sub.pathobj / '.git' / 'config',
dest.pathobj / 'some', '.git'], on_failure='ignore'),
dest.pathobj / 'some' / '.git'], on_failure='ignore'),
status='impossible',
action='copy_file')

# The last path above wasn't treated as a target directory because it
# wasn't an existing directory. We also guard against a '.git' in the
# target directory code path, though the handling is different.
with assert_raises(ValueError):
dest.copy_file([sub.pathobj / '.git' / 'config',
dest.pathobj / '.git'])

# A source path can have a leading .git/ if the destination is outside of
# .git/.
nok_((dest.pathobj / "config").exists())
dest.copy_file(sub.pathobj / '.git' / 'config')
ok_((dest.pathobj / "config").exists())

target = dest.pathobj / 'some'
nok_(target.exists())
dest.copy_file([sub.pathobj / '.git' / 'config', target])
ok_(target.exists())

# But we only waste so many cycles trying to prevent foot shooting. This
# next one sneaks by because only .name, not all upstream parts, is checked
# for each destination that comes out of _yield_specs().
badobj = dest.pathobj / '.git' / 'objects' / 'i-do-not-exist'
dest.copy_file([sub.pathobj / '.git' / 'config', badobj])
ok_(badobj.exists())
8 changes: 8 additions & 0 deletions datalad/support/gitrepo.py
Expand Up @@ -3440,6 +3440,14 @@ def _get_content_info_line_helper(self, ref, info, lines,
inf = {}
props = props_re.match(line)
if not props:
# Kludge: Filter out paths starting with .git/ to work around
# an `ls-files -o` bug that was fixed in Git 2.25.
#
# TODO: Drop this condition when GIT_MIN_VERSION is at least
# 2.25.
if line.startswith(".git/"):
lgr.debug("Filtering out .git/ file: %s", line)
continue
# not known to Git, but Git always reports POSIX
path = ut.PurePosixPath(line)
inf['gitshasum'] = None
Expand Down
9 changes: 9 additions & 0 deletions datalad/support/tests/test_fileinfo.py
Expand Up @@ -14,6 +14,7 @@
from datalad.tests.utils import (
assert_dict_equal,
assert_equal,
assert_false,
assert_in,
assert_not_in,
assert_raises,
Expand Down Expand Up @@ -271,3 +272,11 @@ def test_info_path_inside_submodule(path):
cinfo = ds.repo.get_content_info(
ref="HEAD", paths=[foo.relative_to(ds.pathobj)])
assert_in("gitshasum", cinfo[subds.pathobj])


@with_tempfile
def test_get_content_info_dotgit(path):
ds = Dataset(path).create()
# Files in .git/ won't be reported, though this takes a kludge on our side
# before Git 2.25.
assert_false(ds.repo.get_content_info(paths=[op.join(".git", "config")]))