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: read RIA config from stdin instead of temporary file #7147

Merged
merged 2 commits into from
Nov 8, 2022

Conversation

adswa
Copy link
Member

@adswa adswa commented Nov 7, 2022

A RIA postclone config hook tried to read a special remote UUID from a stores config file by writing the config files contents to a temporary config file, and subsequently running git config -f <thatfile>. As outlined in a comment by @bpoldrack in the code, git config -f - can read from stdin:

echo '[core]\n\trepositoryformatversion = 0\n\tfilemode = true\n\tbare = true\n' | git config -f - core.filemode
true

A quick chat in the Git IRC channel hinted that this is supported by git as its a unix convention. Trial and error confirms that this patch also runs successfully in a Windows CMD. I'll rely on the tests on MacOS to check if it runs there successfully, too. By switching to a git config call with stdin as the input file, we spare the need to write a temporary RIA config file. This fix could thus fix #6514, where a user error lead to a non-existent TMP path and a subsequent failure to create the temporary config file.

as outlined in a comment by @bpoldrack in the code, git config -f - can
read from stdin. This is supported by git as its a unix convention.
Trial and error confirms that this also runs successfully in a Windows
CMD. By switching to a git config call with stdin as the input file, we
spare the need to write a temporary config file.
This fix could thus fix datalad#6514, where a user error lead to a non-existent
TMP path and a subsequent failure to create the temporary config file.
@adswa adswa added semver-patch Increment the patch version when merged CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Nov 7, 2022
@github-actions github-actions bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Nov 7, 2022
Copy link
Member

@bpoldrack bpoldrack left a comment

Choose a reason for hiding this comment

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

Great! Thanks double-checking with git, @adswa !

@bpoldrack
Copy link
Member

Note to myself: This will need some rectification in master, where the postclone routine moved into a dedicated RIA patch.

@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Base: 88.38% // Head: 88.53% // Increases project coverage by +0.15% 🎉

Coverage data is based on head (75b6b5c) compared to base (7957819).
Patch coverage: 58.33% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7147      +/-   ##
==========================================
+ Coverage   88.38%   88.53%   +0.15%     
==========================================
  Files         355      355              
  Lines       46517    49307    +2790     
  Branches        0     6232    +6232     
==========================================
+ Hits        41112    43655    +2543     
- Misses       5405     5620     +215     
- Partials        0       32      +32     
Impacted Files Coverage Δ
datalad/core/distributed/clone.py 85.26% <58.33%> (-5.99%) ⬇️
datalad/support/tests/test_parallel.py 65.43% <0.00%> (-29.93%) ⬇️
datalad/metadata/search.py 61.61% <0.00%> (-27.52%) ⬇️
datalad/metadata/metadata.py 63.91% <0.00%> (-24.45%) ⬇️
datalad/downloaders/base.py 73.25% <0.00%> (-8.72%) ⬇️
datalad/distributed/create_sibling_ria.py 78.44% <0.00%> (-7.27%) ⬇️
datalad/support/network.py 80.29% <0.00%> (-7.05%) ⬇️
datalad/support/annexrepo.py 86.43% <0.00%> (-3.97%) ⬇️
datalad/downloaders/http.py 73.89% <0.00%> (-3.54%) ⬇️
... and 47 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic
Copy link
Member

note: benchmarks error are likely unrelated -- filed #7149

@bpoldrack bpoldrack merged commit 30a8ca2 into datalad:maint Nov 8, 2022
@yarikoptic-gitmate
Copy link
Collaborator

PR released in 0.17.10

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.

clone from RIA store (kinda) fails with invalid TMPDIR
4 participants