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

Win/BF: don't invoke 'git-annex' but 'git annex' #5001

Merged
merged 2 commits into from Oct 12, 2020

Conversation

adswa
Copy link
Member

@adswa adswa commented Oct 9, 2020

On a Windows machine, while git-annex and git annex invocations work fine from an Anaconda prompt, a "git-annex" command invoked via run_annex_command fails to find the git-annex executable.
When using 'git annex', though, commands seem to work just fine. This change fixes #4892. I lack an in-depth understanding of why that is, yet, but I also figured that this change doesn't do harm to anything else.

@adswa adswa mentioned this pull request Oct 9, 2020
@codecov
Copy link

codecov bot commented Oct 9, 2020

Codecov Report

Merging #5001 into maint will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##            maint    #5001   +/-   ##
=======================================
  Coverage   89.91%   89.91%           
=======================================
  Files         292      292           
  Lines       40736    40736           
=======================================
  Hits        36629    36629           
  Misses       4107     4107           
Impacted Files Coverage Δ
datalad/customremotes/base.py 87.45% <ø> (ø)
datalad/customremotes/archives.py 91.95% <100.00%> (ø)
datalad/support/annexrepo.py 86.84% <100.00%> (ø)
datalad/tests/test_cmd.py 97.48% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d071bcd...9bfbe7f. Read the comment docs.

Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this should do not harm. But I also lack insight on why it would make a difference.

Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you and @mih say, I don't see how this would be problematic, and there are places in the code (including a couple of non-test spots) that already use the separated variant that this PR switches to.

A few comments:

  • Grepping for ["']git-annex["'] is noisy, but I see at least a few other instances that might trigger the same issue (e.g., a ["git-annex", "get", ...] in datalad/customremotes/archives.py).

  • Since it's not obvious what's going on here, it'd be nice to add a code comment in _run_annex_command that mentions the importantance of the separated form.

  • I think this could go to maint.

@adswa adswa changed the base branch from master to maint October 11, 2020 09:02
On a Windows machine, while git-annex and git annex work fine from an Anaconda prompt, a
"git-annex" command invoked via run_annex_command fails to find the git-annex executable.
When using 'git annex', though, commands seem to work just fine.
@adswa
Copy link
Member Author

adswa commented Oct 11, 2020

Thanks much for your comments, Kyle! I think I have applied them, and I hope I caught all git-annex command calls.I rebased onto maint.

Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates.

I'm going to restart the failing macOS build to see if that failure goes away. (If it does, I'll open a separate issue for the apparently intermittent failure.)

https://github.com/datalad/datalad/pull/5001/checks?check_run_id=1237765992

edit: restarted build passed

https://github.com/datalad/datalad/pull/5001/checks?check_run_id=1242140615

@kyleam kyleam merged commit f74a4c3 into datalad:maint Oct 12, 2020
@adswa adswa deleted the win-gitannex branch October 12, 2020 13:34
@kyleam
Copy link
Contributor

kyleam commented Oct 13, 2020

(If it does, I'll open a separate issue for the apparently intermittent failure.)

Just to follow up on this: I was about to do that now, but I don't recall seeing it elsewhere, and it looks to be an internal git-annex locking issue, so I'll wait to see if it pops up again before opening an issue that nobody will be able to act on.

@yarikoptic yarikoptic added this to the 0.13.5 milestone Dec 12, 2020
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 this pull request may close these issues.

error with datalad install
4 participants