Skip to content

Pass branch option into recursive call within Install - for the cases whenever install is invoked with URL(s) #7463

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

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Jul 28, 2023

Happens whenever install is invoked with a URL and --branch option. Closes #7461.

TODO:

  • test that case, so far tests had explicit "source" specified and thus did not trigger this code path

@yarikoptic yarikoptic added semver-patch Increment the patch version when merged CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Jul 28, 2023
@@ -200,7 +200,6 @@ def __call__(
reckless=None,
jobs="auto",
branch=None):

Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated change, I will drop

@github-actions github-actions bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Jul 28, 2023
yarikoptic and others added 3 commits July 28, 2023 12:04
Happens whenever install is invoked with a URL and --branch option.
Closes datalad#7461.

TODO:

- [ ] test that case, so far tests had explicit  "source" specified
and thus did not trigger this code path
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.60% 🎉

Comparison is base (928498e) 90.73% compared to head (e9225f3) 91.34%.

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7463      +/-   ##
==========================================
+ Coverage   90.73%   91.34%   +0.60%     
==========================================
  Files         325      325              
  Lines       43399    43407       +8     
  Branches        0     5818    +5818     
==========================================
+ Hits        39380    39650     +270     
+ Misses       4019     3742     -277     
- Partials        0       15      +15     
Files Changed Coverage Δ
datalad/distribution/install.py 98.86% <ø> (ø)
datalad/distribution/tests/test_install.py 99.81% <100.00%> (+<0.01%) ⬆️

... and 39 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarikoptic
Copy link
Member Author

eh, travis is unhappy but seems unrelated e.g.

FAILED ../datalad/customremotes/tests/test_archives.py::test_basic_scenario - datalad.runner.exception.CommandError: CommandError: 'git -c diff.ignoreSubmodules=none -c core.quotepath=false annex get -c annex.retry=3 --json --json-error-messages --json-progress -c annex.dotfiles=true -- ' |;&%b5{}'"'"'"<>ΔЙקم๗あ .dbtc '' failed with exitcode 1 under /tmp/datalad_temp_tree_test_basic_scenario14h3468n [info keys: stdout_json]
> from datalad-archives...
Unable to access these remotes: datalad-archives
  Failed to fetch any archive containing MD5E-s3--202cb962ac59075b964b07152d234b70. Tried: ['MD5E-s142--e1f8dde8c4d4160878d6a0210869cfd6.tar.gz']
  Failed to fetch any archive containing MD5E-s3--202cb962ac59075b964b07152d234b70. Tried: ['MD5E-s142--e1f8dde8c4d4160878d6a0210869cfd6.tar.gz', 'MD5E-s142--e1f8dde8c4d4160878d6a0210869cfd6.tar.gz']
  Failed to fetch any archive containing MD5E-s3--202cb962ac59075b964b07152d234b70. Tried: ['MD5E-s142--e1f8dde8c4d4160878d6a0210869cfd6.tar.gz', 'MD5E-s142--e1f8dde8c4d4160878d6a0210869cfd6.tar.gz'] [err: 'get: 1 failed']
FAILED ../datalad/customremotes/tests/test_archives.py::test_annex_get_from_subdir - datalad.runner.exception.CommandError: CommandError: 'git annex drop -- ' |;&%b5{}'"'"'"<>ΔЙקم๗あ .datc '' failed with exitcode 1
FAILED ../datalad/tests/test_archives.py::test_ExtractedArchive - AssertionError: /tmp/datalad_temp_1e1e0ce8ee_5_370fyp/ |;&%b5{}'"<>ΔЙקم๗あ .dbtc / |;&%b5{}'"<>ΔЙקم๗あ .datc  must exist
FAILED ../datalad/tests/test_archives.py::test_decompress_file[None] - assert False
FAILED ../datalad/tests/test_archives.py::test_decompress_file[strip] - assert False
= 5 failed, 1134 passed, 52 skipped, 3 xfailed, 22 warnings in 942.58s (0:15:42) =

so I will take it out of draft.

@yarikoptic yarikoptic marked this pull request as ready for review July 30, 2023 18:30
Copy link
Contributor

@mslw mslw left a comment

Choose a reason for hiding this comment

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

Looks like a fix to me 💯

@yarikoptic yarikoptic merged commit 5b69f70 into datalad:maint Jul 31, 2023
@yarikoptic-gitmate
Copy link
Collaborator

PR released in 0.19.3

@yarikoptic yarikoptic deleted the bf-install branch November 27, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

datalad install does not respect --branch option
3 participants