Make push work with push.default=matching repos #4896
Conversation
By `push.default=matching` or another way.
Rational: in a dataset that is configured with `push.default=matching` we have no other ability to trigger the push of a (new) branch that gave the context for the PATHS that `push` might be moving. datalad#4888
ok_() will pass as long as the first get_hexsha() returns true.
test_push_matching() fails on the crippled fs run because the target repo already has an unrelated commit. mk_push_target() has a dedicated workaround for this, so use that instead of create_sibling(). Also correct the hexsha comparison to avoid comparing an adjusted branch in one repository to the corresponding branch in the other. Cf: https://github.com/datalad/datalad/pull/4896/checks?check_run_id=1080990572
There was a problem hiding this comment.
Okay, so when we repurposed the optimization in 211fead as a fix for gh-4780, we traded the gh-4780 issue/limitation for this bug. (That commit was introduced in 0.13.2.)
The fix looks good to me. As you note, the always-push-current is a change in behavior, but given how young push is and how unlikely it seems that someone would rely on the previous behavior in general, I think it's safe.
I pushed a fix for the new test on crippled fs. It fixes the run for me locally under tools/eval_under_testloopfs. I'm hoping it also fixes the appveyor failure.
datalad/core/distributed/push.py
Outdated
| if is_annex_repo: | ||
| must_have_branches.append('git-annex') | ||
| for branch in must_have_branches: | ||
| # active_branch could be None |
There was a problem hiding this comment.
Not true with the current upstream code :)
There was a problem hiding this comment.
Arrgh, true. Too many iterations if moving conditionals around. Will fix that up.
The build still had a push test failure, but I think it's one we've been seeing. The new https://github.com/datalad/datalad/pull/4896/checks?check_run_id=1081754243
It did: https://ci.appveyor.com/project/mih/datalad/builds/35069164/job/8ykc5ukd3n509wxp#L654 |
Codecov Report
@@ Coverage Diff @@
## maint #4896 +/- ##
==========================================
- Coverage 89.66% 89.62% -0.05%
==========================================
Files 288 288
Lines 40406 40424 +18
==========================================
- Hits 36230 36228 -2
- Misses 4176 4196 +20
Continue to review full report at Codecov.
|
Thx to @kyleam for noticing
|
Thx @kyleam I believe the singularity failure on the -container tests is unrelated to this PR. Will merge. |
Story is in the commit messages. Big change is the push of the active branch.
Fixes #4888