Fix recent test_copy_file_prevent_dotgit_placement failure #5258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
test_copy_file_prevent_dotgit_placement() has a copy_file() call that
is supposed to return an impossible result because the destination
ends with .git/. This doesn't work as intended, though. Due to a
typo, it is actually a list of three items, the last a plain .git
string [*]. The impossible results comes from the "some" source file
not being found.
Fix the typo so that the intended .git/ guard is triggered.
[*] This is treated as a target directory, but the target directory
code path does not have the same ".git" as explicit destinations.
The next commit will address that.
copy-file already does some .git safety checks. One is that the destination path that comes out of _yield_specs() doesn't end in .git. But when there is not an explicit destination path, the target directory is used, bypassing this check. Abort if any part of the target directory is ".git". The handling is a bit different than the destination paths. A ValueError is raised instead of yielding impossible results, because this check can be done up front on a single value. Also, with one value, it seems worth the cost to check all the parts rather than just the final name.
Codecov Report
@@ Coverage Diff @@
## maint #5258 +/- ##
==========================================
+ Coverage 90.43% 90.44% +0.01%
==========================================
Files 294 294
Lines 40936 40986 +50
==========================================
+ Hits 37021 37071 +50
Misses 3915 3915
Continue to review full report at Codecov.
|
It appears that copying items _from_ underneath .git/ to a destination outside of .git/ is intentionally supported, so add two explicit cases for this. Also, document a known case where the destination path checks aren't sufficient to prevent a file from being copied under the destination .git/.
`git ls-files -o .git/` incorrectly traverses the .git/ directory
before Git 2.25, specifically b9670c1f5e (dir: fix checks on common
prefix directory, 2019-12-19). To get consistent behavior with older
versions of Git that we support, filter out ls-files results that
start with ".git/".
This change fixes a recent test_copy_file_prevent_dotgit_placement()
failure on master with Git versions before 2.25. Despite the .git/
checks, there are cases where a file can get copied to a destination
in .git/. Due to the `ls-files -o` bug above, save() considers
.git/config as a file of interest, leading to `git annex add` being
called with a file in .git/. The `git annex add` call fails,
presumably due to the ls-files bug also affecting it internal logic:
{"command":"add","note":"non-large file; adding content to git repository",
"success":true,"input":[".git/config"],"error-messages":[],"file":".git/config"}
error: invalid path '.git/config'
error: unable to add '.git/config' to index
fatal: adding files failed
git-annex: user error
(xargs ["-0","git","--git-dir=.git","--work-tree=.","--literal-pathspecs","add","--"]
exited 123)
The reason we now see these failures on master is that, with the
switch to _call_annex_records() in 46d78a8 (RF: Replace all usage
of AnnexRepo._run_annex_command_json(), 2020-11-18), a CommandError is
raised for the `git annex add` call. Before _run_annex_command_json()
returned the successful looking record above.
With a Git after 2.25, this failure doesn't happen because `ls-files
-o` doesn't report the file, and so the save() call doesn't result in
the .git/ file being passed to `git annex add`. Even if it were
passed to `git annex add`, git-annex would ignore it rather than fail.
Fixes datalad#5248.
kyleam
force-pushed
the
contentinfo-filter-dotgit
branch
from
a754d55
to
6b3bbe0
Compare
|
@mih Thanks for the review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This last commit in this series fixes the git-version-dependent
test_copy_file_prevent_dotgit_placementfailure reported in gh-5248. The first three commits are changes related tocopy-files.git/guards.