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

Bump minimum git-annex version to 8.20200309 #5512

Merged
merged 10 commits into from
Mar 19, 2021

Conversation

kyleam
Copy link
Contributor

@kyleam kyleam commented Mar 18, 2021

This series increases GIT_ANNEX_MIN_VERSION to 8.20200309 and removes no longer needed kludges. The second commit explains why 8.20200309 was chosen, though please chime in with suggestions if you think another value makes more sense.


562ba4f (BF/TST: Adapting more tests to direct mode, 2017-02-07)
prevented a "repo is clean" assertion from being executed on v6+
repos, but it's not clear to me why the assertion wouldn't apply to
v6+ repos, and it passes for me locally (with git-annex 8.20200309 and
8.20210310), including under tools/eval_under_testloopfs.
Increasing GIT_ANNEX_MIN_VERSION is long overdue.  The main question
is what the new minimum should be.  Some landmarks:

  * 7.20190912 is when v5 repos started automatically upgrading (to
    v7).

  * buster-backports has 8.20200330.

  * Neurodebian currently has a recent release (8.20210223).

  * 8.20200330 is required so that subdataset changes are synced back
    to an adjusted branch (datalad#4292).  subdataset deletions require
    8.20210127 (datalad#5290).

  * 7.20200202.7 until 8.20200309 is incompatible with the ora special
    remote (dataladgh-4509).

As suggested by dataladgh-5399, we probably shouldn't increase this beyond
what's available from buster-backports.  That's 8.20200330 and would
be a good minimum due to the subdataset/adjusted branch fixes.

However, conda-forge doesn't have 8.20200330, which makes testing the
minimum on the CI a little more complicated.  It does have one release
earlier, 8.20200309.  So, go with 8.20200309 for ease of testing.

Closes datalad#5399.
GIT_ANNEX_MIN_VERSION is now above 7.20190819.  Keep the
check_direct_mode_support() method and supports_direct_mode attribute
around for at least another release just in case there are any
third-party users.  (No extensions uses these.)
With the current GIT_ANNEX_MIN_VERSION, this is always true for a
fresh repository.
When annex.version isn't set (as is the case of uninitialized repos),
supports_unlocked_pointers falls back to False.  The minimum git-annex
version now supports unlocked pointers, though, so answering True here
probably makes more sense.

Even though the minimum git-annex version always creates/upgrades
repos to v6+, the supports_unlocked_pointers() method should stay
around because annex.version=5 repos may still be encountered in the
wild.
@kyleam kyleam force-pushed the bump-min-git-annex-version branch from 182c22d to 3165389 Compare March 18, 2021 15:11
@kyleam kyleam marked this pull request as ready for review March 18, 2021 15:12
Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

Everything looks great to me. Thank you @kyleam . If tests pass, I am good with the current state

@codecov
Copy link

codecov bot commented Mar 18, 2021

Codecov Report

Merging #5512 (6f54e69) into master (3f5b53c) will decrease coverage by 0.42%.
The diff coverage is 91.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5512      +/-   ##
==========================================
- Coverage   90.22%   89.79%   -0.43%     
==========================================
  Files         296      296              
  Lines       42189    42191       +2     
==========================================
- Hits        38065    37886     -179     
- Misses       4124     4305     +181     
Impacted Files Coverage Δ
datalad/core/local/tests/test_run.py 100.00% <ø> (+0.34%) ⬆️
datalad/tests/test_direct_mode.py 76.92% <ø> (+32.02%) ⬆️
datalad/support/annexrepo.py 88.73% <71.42%> (-2.92%) ⬇️
datalad/core/local/tests/test_save.py 97.49% <100.00%> (-0.23%) ⬇️
datalad/interface/tests/test_unlock.py 100.00% <100.00%> (ø)
datalad/support/tests/test_annexrepo.py 97.53% <100.00%> (+0.08%) ⬆️
datalad/tests/utils.py 88.29% <100.00%> (-1.02%) ⬇️
datalad/ui/utils.py 56.25% <0.00%> (-21.88%) ⬇️
datalad/metadata/extractors/tests/test_base.py 87.80% <0.00%> (-12.20%) ⬇️
datalad/metadata/extractors/tests/test_xmp.py 88.46% <0.00%> (-11.54%) ⬇️
... and 56 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 3f5b53c...6f54e69. Read the comment docs.

@kyleam kyleam added the do not merge Not to be merged, will trigger WIP app "not passed" status label Mar 18, 2021
7.20190819 is no longer supported.  8.20210224 wasn't chosen for any
particular reason; it's just the most recent version on conda-forge.
@kyleam kyleam removed the do not merge Not to be merged, will trigger WIP app "not passed" status label Mar 18, 2021
@kyleam
Copy link
Contributor Author

kyleam commented Mar 18, 2021

@yarikoptic Thanks for reviewing.

If tests pass, I am good with the current state

I missed two Travis builds that used 7.20190819. Updated. Hopefully things will be green now.

@kyleam kyleam added the do not merge Not to be merged, will trigger WIP app "not passed" status label Mar 18, 2021
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.

Rational of version selection is sound. Thx!

After the previous commit switched to 8.20210224, the Travis build had
two test failures where the underlying git-annex command had an error:

  datalad.customremotes.tests.test_archives:test_annex_get_from_subdir
  datalad.customremotes.tests.test_archives:test_basic_scenario

  <https://travis-ci.com/github/datalad/datalad/builds/220534126>

With the same git-annex version built from source, the tests pass for
me locally.  I haven't tried the conda source, but the Travis logs
indicate that conda falls back to the standalone install for this
version:

  Standalone distribution of git-annex was installed, instead of the
  standard distribution, likely due to package conflicts in the target
  environment.  The standalone distribution may have issues [...]

Given that newer and older conda installs pass on Travis and that I
don't see the failures when building from source, perhaps the failures
are related to the standalone setup issues.  It looks like there is a
more recent version available on conda-forge and judging by the
"alldep" rather than "nodep" in the file name, it should avoid the
standalone fallback.  So try with that.
@kyleam
Copy link
Contributor Author

kyleam commented Mar 19, 2021

Oh, I knew it felt like too few kludges were being dropped. I grepped for GIT_ANNEX_MIN_VERSION and git_annex_version, but forgot about external_versions["cmd:annex"]!

Check external_versions['cmd:annex'] spots as well, completing the
changes from a few commits back.
@kyleam
Copy link
Contributor Author

kyleam commented Mar 19, 2021

Thanks, @mih and @yarikoptic, for the reviews. All of the test builds are green, and hopefully I've caught/removed all of the now unnecessary workarounds.

@kyleam kyleam merged commit f059eb3 into datalad:master Mar 19, 2021
@kyleam kyleam deleted the bump-min-git-annex-version branch March 19, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Not to be merged, will trigger WIP app "not passed" status
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants