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: fix handling of environment variables that contain whitespace #4481

Merged
merged 2 commits into from May 5, 2020

Conversation

christian-monch
Copy link
Contributor

@christian-monch christian-monch commented May 4, 2020

This PR encloses the shell-substitution for PATH and PYTHONPATH in the test-code target of the Makefile in double-quotes. That prevents errors where the original PATH or PYTHONPATH environment variables contain whitespaces.

Copy link
Contributor

@kyleam kyleam left a comment

Thanks.

Shouldn't another spot (a couple of lines above) receive the same treatment?

@codecov
Copy link

@codecov codecov bot commented May 4, 2020

Codecov Report

Merging #4481 into maint will decrease coverage by 0.68%.
The diff coverage is 83.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #4481      +/-   ##
==========================================
- Coverage   89.63%   88.95%   -0.69%     
==========================================
  Files         275      287      +12     
  Lines       37103    38254    +1151     
==========================================
+ Hits        33259    34028     +769     
- Misses       3844     4226     +382     
Impacted Files Coverage Δ
datalad/config.py 96.59% <ø> (ø)
datalad/core/local/run.py 98.70% <ø> (ø)
datalad/core/local/save.py 98.68% <ø> (ø)
datalad/customremotes/tests/test_archives.py 89.54% <ø> (ø)
datalad/distributed/create_sibling_gitlab.py 74.67% <ø> (ø)
...ad/distributed/tests/test_create_sibling_gitlab.py 100.00% <ø> (ø)
datalad/distribution/create_sibling_github.py 83.60% <ø> (ø)
datalad/distribution/drop.py 98.79% <ø> (ø)
datalad/distribution/get.py 96.09% <ø> (+7.30%) ⬆️
datalad/distribution/install.py 98.87% <ø> (ø)
... and 217 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 efdd9a8...367572e. Read the comment docs.

@yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented May 4, 2020

ideally should be positioned over maint branch

@christian-monch
Copy link
Contributor Author

@christian-monch christian-monch commented May 5, 2020

Thanks.

Shouldn't another spot (a couple of lines above) receive the same treatment?

Thanks, you are right. Fixed in the latest commit.

Christian Moench added 2 commits May 5, 2020
Fixes the handling of path variables in the test-code target of
the Makefile. The previous version fails if either PATH or
PYTHONPATH contain whitespaces.
@kyleam
Copy link
Contributor

@kyleam kyleam commented May 5, 2020

ideally should be positioned over maint branch

I'll change the base (though given the nature of the changes and the timing, I don't think it matters much in this case).

@kyleam kyleam changed the base branch from master to maint May 5, 2020
@kyleam kyleam force-pushed the bf-whitespace-in-path branch from e58276c to 367572e Compare May 5, 2020
@kyleam
Copy link
Contributor

@kyleam kyleam commented May 5, 2020

I've canceled the tests because none of them are relevant for this change.

@kyleam kyleam merged commit f5d9eff into datalad:maint May 5, 2020
2 of 9 checks passed
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

3 participants