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

BF: publish: Tell git-fetch to not recurse into submodules #4560

Merged
merged 1 commit into from May 20, 2020

Conversation

kyleam
Copy link
Contributor

@kyleam kyleam commented May 20, 2020

publish() fetches the sibling before pushing, and has done so for a
long time (f07fc51, 2017-08-26). If the fetch brings down a
recorded submodule commit that is not yet present locally, by default
git also fetch in the submodule with the missing commit.

This is problematic because, as explained in dbd84e5 (ENH: update:
Try to fetch submodule commit explicitly if needed, 2020-02-07),
submodule.c hard codes 'origin' as a fallback to fetch from, so the
fetch will fail unless there's a remote named 'origin'. Also,
conceptually, the goal of this fetch seems to be to obtain up-to-date
information on the current repository, not its submodules, which
publish() will handle itself if called with --recursive.

Fetch with --recurse-submodules=no.

publish() fetches the sibling before pushing, and has done so for a
long time (f07fc51, 2017-08-26).  If the fetch brings down a
recorded submodule commit that is not yet present locally, by default
git also fetch in the submodule with the missing commit.

This is problematic because, as explained in dbd84e5 (ENH: update:
Try to fetch submodule commit explicitly if needed, 2020-02-07),
submodule.c hard codes 'origin' as a fallback to fetch from, so the
fetch will fail unless there's a remote named 'origin'.  Also,
conceptually, the goal of this fetch seems to be to obtain up-to-date
information on the current repository, not its submodules, which
publish() will handle itself if called with --recursive.

Fetch with --recurse-submodules=no.
@kyleam
Copy link
Contributor Author

@kyleam kyleam commented May 20, 2020

Note: I've checked push. It doesn't seem to have this issue.

@codecov
Copy link

@codecov codecov bot commented May 20, 2020

Codecov Report

Merging #4560 into maint will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##            maint    #4560   +/-   ##
=======================================
  Coverage   90.10%   90.11%           
=======================================
  Files         275      275           
  Lines       37118    37133   +15     
=======================================
+ Hits        33447    33462   +15     
  Misses       3671     3671           
Impacted Files Coverage Δ
datalad/distribution/publish.py 87.93% <100.00%> (ø)
datalad/distribution/tests/test_publish.py 100.00% <100.00%> (ø)

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 4300660...8343899. Read the comment docs.

@yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented May 20, 2020

Thanks!

@yarikoptic yarikoptic merged commit df700e6 into datalad:maint May 20, 2020
11 checks passed
@kyleam kyleam deleted the publish-fetch-no-recurse branch May 21, 2020
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