NF: create-sibling-ria command

[mih]

This is the code that was used to push the HCP datasets to store.datalad.org -- written by @bpoldrack (see datalad/git-annex-ria-remote#38). It is one of the last pieces of RIA functionality to move into -core (the actual special remote implementation being the very last one). It is being proposed as a separate PR, because it can function without it (HCP doesn't use the ria-special remote), and to keep the complexity of the PR more manageable.

Feedback is very much welcome! Thanks!

Related:

• NF: Command abstractions #4068 consolidation on common operations with create_sibling
• UX issues:
  • create-sibling-ria fails when prev attempt failed due to login permission error git-annex-ria-remote#44
  • create-sibling-ria should allow specification of trust-level git-annex-ria-remote#45

Update by @bpoldrack :

From my point of view, this is ready. Above mentioned UX issues are addressed and test failures fixed (includes a fix for #4151). A long the way I doscovered #4152 and #4153. However, those are neither fixed herein nor are fixes for that actually required for this PR.

Tests currently depend on installing https://github.com/datalad/git-annex-ria-remote@transition-to-core, which removes the conflicting implementation of create-sibling-ria therein. While this seems somewhat ugly, it should be a quite short lived approach. Rational: We are moving the special remote to core. I'm currently working on PR #4068 to integrate execution via a persistent remote shell. When this is done, git-annex-ria-remote can go into core completely. If we had an actual dependency on the ria_remote package via setup.py, this would lead to people installing it, when shortly afterwards it will become at least unnecessary, if not conflicting to have it installed, which is why I consider this approach superior.

Edit by @bpoldrack :

Plans changed. Since there's some time pressure and a new idea re special remote configuration ( #4124 (comment) ), updated plan for ria-remote's move to core:

• move ria-remote as is with remote execution factored out for easier RF later on.
• once it's merged:
  • this PR (create-sibling-ria) can remove Travis dependency it properly reference/import within core and be merged
  • then special remote's initremote can be adapted to store the annex UUID in the bare repo's config
  • and clone then needs to be aware to make use of it

Note, that integration with #4068 is thereby postponed.

So, 3 PRs:

• the main move to core
• this very PR with adaptions
• change RIARemote.initremote() and datalad-clone

[mih]

@bpoldrack can you have a look at the test failures please, it seems we are missing a few of their idiosyncracies in datalad-core (which should preferably by removed). Thx!

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@2bbcf92). Click here to learn what that means.
The diff coverage is 93.86%.

@@            Coverage Diff            @@
##             master    #4124   +/-   ##
=========================================
  Coverage          ?   89.54%           
=========================================
  Files             ?      279           
  Lines             ?    36707           
  Branches          ?        0           
=========================================
  Hits              ?    32868           
  Misses            ?     3839           
  Partials          ?        0

Impacted Files	Coverage Δ
datalad/interface/common_opts.py	100% <ø> (ø)
datalad/distributed/create_sibling_gitlab.py	74.67% <ø> (ø)
datalad/core/local/tests/test_run.py	99.55% <ø> (ø)
datalad/local/tests/test_subdataset.py	100% <ø> (ø)
datalad/customremotes/tests/test_archives.py	89.54% <ø> (ø)
datalad/core/local/tests/test_diff.py	99.48% <ø> (ø)
datalad/core/local/tests/test_resulthooks.py	100% <ø> (ø)
datalad/support/tests/test_fileinfo.py	100% <ø> (ø)
datalad/core/local/tests/test_status.py	98.85% <ø> (ø)
datalad/tests/test_utils_testrepos.py	100% <ø> (ø)
... and 89 more

---

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 2bbcf92...e790b9b. Read the comment docs.

[bpoldrack]

Added trust-level option. @mih: You might want to have a quick look at ff095ff

[mih]

@bpoldrack That last patch tripped the tests

[bpoldrack]

Note, that datalad/git-annex-ria-remote#44 and datalad/git-annex-ria-remote#45 are addressed here.

Re test: That's strange. unexpected keyword argument should happen locally as well if legit. But doesn't. Hmm.

[bpoldrack]

FTR: ATM I have no idea waht's going on on Travis. Can't reproduce and looks like it has nothing to do with my last commit.

However, meanwhile fixing an issue wrt --existing. Therefor removed merge label for now.

[bpoldrack]

Finally able to reproduce issue on Travis. Reason: Happens only on the merge commit with master. At least twofold:

1. publish tries to fetch special remote #4151 (easy fix)
2. Somehow a url...insteadOf config gets lost when cloning. No matter whether I store it global or use override with datalad.cfg. Having a breakpoint in the test, creating proper global config manually succeeds.

Edit:
Just noting it down here. May turn into a fix right here or into an issue otherwise.

As suspected (but not traced down) before: We shoot our feet with the fake $HOME we set up for tests:

(Pdb) cfg.__dict__
{'_store': {'user.name': 'Benjamin Poldrack', 'user.email': 'benjaminpoldrack@gmail.com', 'url.ria+file:///tmp/datalad_temp__test_create_storele6bc8ij.insteadof': 'ria+ssh://test-store:', 'datalad.datasets.topurl': 'http://datasets-tests.datalad.org/', 'datalad.log.level': '100'}, '_cfgfiles': {'/home/ben/.gitconfig'}, '_cfgmtimes': {'/home/ben/.gitconfig': 1581683659.070651}, '_dataset_path': None, '_dataset_cfgfname': None, '_repo_cfgfname': None, '_config_cmd': ['git', '--git-dir=', 'config'], 'overrides': {}, '_src_mode': 'any', '_runner': <datalad.cmd.GitRunner object at 0x7f86b2795520>, '_gitconfig_has_showorgin': True}

Note, that it reads /home/ben/.gitconfig.
But:

(Pdb) os.environ['HOME']
'/tmp/datalad_temp_ft_s2hz0'

Edit 2:

And number 3:
clone is buggy. Looks for RIA special remote, before annex-init, which is why it's not yet enabled and therefore not found.

Edit 3:

Above mentioned wrong order brought some headache, since it should only lead to not setting the publication dependency. Instead, special remote wasn't enabled at all. How so? Turns out, that the search for such a special remote results in annex-calls like git-annex info --json .... Problem with that? It doesn't actually run git-annex-init (no respective message and no special remote enabling), but it creates .git/annex. Later on, we check whether we need to annex-init by calling AnnexRepo.is_initialized(). And this does check for the existence of .git/annex. Bang - no init, no autoenabled special remote. Apart from doing things in the right order to begin with, this leads to the question whether this is an annex bug. If not, then I don't see a way of detecting whether it really is initialized. The respective entry in .git/config (annex.uuid) was created by git-annex-info. So, looks like an annex bug to me, that annex-init is only partially executed in that case.

[mih]

I have lost track of all the moving pieces here. Could you please leave a note in the top comment on the status of this PR. It looks ready function-wise, but there is still the test dependency issue on that branch in ria-remote. What is the general plan?

[bpoldrack]

@mih: Updated top-level comment.

[mih]

Idea:

• when create-sibling-ria has call initremote we record the UUID of the special remote in the config of the bare repo in the store
• when we datalad clone from a store and the special remote is not functional, we check that store config, and enableremote the special remote with the matching UUID to use the store URL and access protocol that has just led to a successful clone

This avoids the need for any url...insteadof config, and always results in an appropriate (clone-matched) access to the RIA remote.

[mih]

% datalad create-sibling-ria -s datastore THISISIT
[INFO   ] create siblings 'datastore' and 'datastore-ria' ... 
[INFO   ] Fetching updates for <Dataset path=/tmp/test> 
CommandError: command '['git', 'fetch', '--verbose', '--progress', '--prune', '--recurse-submodules=no', 'datastore']' failed with exitcode 128
Failed to run ['git', 'fetch', '--verbose', '--progress', '--prune', '--recurse-submodules=no', 'datastore'] at '/tmp/test'.
stdout=
stderr=fatal: unable to find remote helper for 'ria+ssh'

fatal: unable to find remote helper for 'ria+ssh'

% git config -l |grep THISIS
url.ria+ssh://mih@XXX.YYY.de/tmp/tmpstore.insteadof=THISISIT

The failure doesn't happen with a URL that doesn't have a user specified. It is likely a URL parsing issue.

[bpoldrack]

The failure doesn't happen with a URL that doesn't have a user specified. It is likely a URL parsing issue.

Addressed in b4533ea

[bpoldrack]

As for the annex UUID in bare repo's config: Will move ria-remote to core first and only then implement it in its initremote. This leads to updated plans - see toplevel.

[yarikoptic]

Minor comments on names and wording

[yarikoptic]

This name is confusing given the have of the command. May be make it --no-ria-archive if that what it is?

[bpoldrack]

Potential confusion - true. But it's not about an archive either. It's about whether or not to have a special remote, since it can be useful to have that ID based tree of bare repos without any annex object.

[mih]

I agree with @bpoldrack

[bpoldrack]

@yarikoptic : Addressed your comments (and pushed) except for the one answered. Don't see a better name for --no-ria-remote ATM.

[bpoldrack]

FTR: Windows failure is in datalad.tests.test_utils.test_get_open_files and looks like usual path resolution issue with short paths. No idea how this would be related to this PR.

Note, that this is about github Win2019 tests. appveyor fails with the known coverage issue only.

Restarted build after failure didn't happen in master. Now it passes. So - whatever.