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

RF: Dedicated method to perform a local annex sync #4243

Merged
merged 4 commits into from Mar 6, 2020

Conversation

mih
Copy link
Member

@mih mih commented Mar 5, 2020

Per request in #4206 (comment)

To consolidate corresponding branches and sync 'git-annex' branches of
one or more given, or all remotes. Does not leave behind 'synced/...'
branches, unless they existed prior calling the method.

@mih mih mentioned this pull request Mar 5, 2020
datalad/support/annexrepo.py Outdated Show resolved Hide resolved
@codecov
Copy link

@codecov codecov bot commented Mar 6, 2020

Codecov Report

Merging #4243 into master will increase coverage by 0.35%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4243      +/-   ##
==========================================
+ Coverage   89.45%   89.81%   +0.35%     
==========================================
  Files         275      275              
  Lines       36328    40014    +3686     
==========================================
+ Hits        32498    35938    +3440     
- Misses       3830     4076     +246     
Impacted Files Coverage Δ
datalad/distribution/tests/test_update.py 90.36% <0.00%> (-7.37%) ⬇️
datalad/interface/ls.py 57.51% <0.00%> (-5.10%) ⬇️
datalad/interface/tests/test_unlock.py 95.65% <0.00%> (-2.18%) ⬇️
datalad/customremotes/base.py 83.49% <0.00%> (-0.65%) ⬇️
datalad/core/local/tests/test_save.py 96.98% <0.00%> (-0.51%) ⬇️
datalad/distribution/tests/test_uninstall.py 99.25% <0.00%> (-0.42%) ⬇️
datalad/support/tests/test_gitrepo.py 99.59% <0.00%> (-0.17%) ⬇️
datalad/support/tests/test_annexrepo.py 95.07% <0.00%> (-0.06%) ⬇️
datalad/distribution/publish.py 89.59% <0.00%> (+0.62%) ⬆️
datalad/plugin/check_dates.py 96.90% <0.00%> (+1.19%) ⬆️
... and 8 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 8a174f5...2a83eb8. Read the comment docs.

@mih mih changed the title RF: Dedicated method to perform a local sync with a corresponding branch RF: Dedicated method to perform a local annex sync Mar 6, 2020
To consolidate corresponding branches and sync 'git-annex' branches of
one or more given, or all remotes. Does not leave behind 'synced/...'
branches, unless they existed prior calling the method.
@@ -3476,24 +3465,56 @@ def _save_post(self, message, status, partial_commit):
# first do standard GitRepo business
super(AnnexRepo, self)._save_post(
message, status, partial_commit)
# check if we have to deal with adjusted branch
# then sync potential managed branches
self.localsync()
Copy link
Contributor

@kyleam kyleam Mar 6, 2020

Choose a reason for hiding this comment

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

The "is corresponding branch" conditional is now removed from localsync, so shouldn't _save_post do it to avoid unnecessary sync calls?

Copy link
Member Author

@mih mih Mar 6, 2020

Choose a reason for hiding this comment

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

Makes sense. Although I spent a day trying graps all the combinations, and I am still not done. So....we'll see....

Copy link
Member Author

@mih mih Mar 6, 2020

Choose a reason for hiding this comment

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

I will introduce a managed_only=False parameter that allows for triggering the test from the outside.

Copy link
Contributor

@kyleam kyleam Mar 6, 2020

Choose a reason for hiding this comment

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

Sounds good. And it looks like being able to avoid annex sync is necessary to prevent merge conflict failures like the one now happening in test_rerun. Here's a reduced script that will trigger such a conflict without datalad:

conflict demo
#!/bin/sh

set -eux

cd "$(mktemp -d --tmpdir gx-XXXXXXX)"
git init
git annex init

echo foo >foo
git annex add foo
git commit -mfoo

git annex unlock foo
echo more >>foo
git annex add foo
git commit -m"foo more"

git annex sync --no-pull --no-push --no-commit --no-resolvemerge

git reset --hard HEAD~1
git annex unlock foo
echo different more >>foo
git annex add foo
git commit -m"foo different more"

git annex sync --no-pull --no-push --no-commit --no-resolvemerge

Copy link
Member Author

@mih mih Mar 6, 2020

Choose a reason for hiding this comment

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

I pushed that change. Need to detach now till some time next week.

datalad/distribution/update.py Outdated Show resolved Hide resolved
mih added 2 commits Mar 6, 2020
Otherwise it would not consolidate git-annex branches across remotes.
If localsync() is called for syncing managed branches only, this
should be tested upfront and outside (e.g. is_managed_branch()).
kyleam
kyleam approved these changes Mar 6, 2020
Copy link
Contributor

@kyleam kyleam left a comment

LGTM (and sorry my suggestion in gh-4206 led to so much work :/)

@mih
Copy link
Member Author

@mih mih commented Mar 6, 2020

LGTM (and sorry my suggestion in gh-4206 led to so much work :/)

I think this is now in a better place, so keep those coming! ;-)

@kyleam kyleam merged commit 4266470 into datalad:master Mar 6, 2020
14 of 17 checks passed
@mih mih deleted the rf-sync branch Mar 6, 2020
@mih mih mentioned this pull request Mar 6, 2020
21 tasks
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.

None yet

2 participants