Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Don't check for the existence of a writable home directory if BUNDLE_USER_HOME is set #6885

Merged
1 commit merged into from Apr 28, 2019

Conversation

jturkel
Copy link
Contributor

@jturkel jturkel commented Jan 7, 2019

#6024 added support for specifying an alternate bundler home directory via the BUNDLE_USER_HOME environment variable. Unfortunately bundler still checks for a writable user home directory even if the BUNDLE_USER_HOME override is specified resulting in warnings like this:

`/home/appuser` is not a directory.
Bundler will use `/tmp/bundler/home/unknown' as your home directory temporarily.

The fix for this problem is to only evaluate bundler path fallbacks if environment variable overrides are not specified.

@welcome
Copy link

welcome bot commented Jan 7, 2019

Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack.

For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide

@segiddins
Copy link
Member

This looks good, can you please add a test for this?

@jturkel
Copy link
Contributor Author

jturkel commented Jan 27, 2019

@segiddins - I believe testing is covered by the change I made to spec/bundler/bundler_spec.rb to raise an exception if the user home is requested (at least that makes the test fail without the corresponding code fix). Is there additional test coverage you think is needed?

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

This PR looks good to me as is. @segiddins do you have any additional coverage in mind?

@deivid-rodriguez
Copy link
Member

No further feedback so let's do this!

@bundlerbot r+

ghost pushed a commit that referenced this pull request Apr 28, 2019
6885: Don't check for the existence of a writable home directory if BUNDLE_USER_HOME is set r=deivid-rodriguez a=jturkel

#6024 added support for specifying an alternate bundler home directory via the `BUNDLE_USER_HOME` environment variable. Unfortunately bundler still checks for a writable user home directory even if the `BUNDLE_USER_HOME` override is specified resulting in warnings like this:
```
`/home/appuser` is not a directory.
Bundler will use `/tmp/bundler/home/unknown' as your home directory temporarily.
```
The fix for this problem is to only evaluate bundler path fallbacks if environment variable overrides are not specified.

Co-authored-by: Joel Turkel <jturkel@salsify.com>
@ghost
Copy link

ghost commented Apr 28, 2019

Build succeeded

@ghost ghost merged commit 8b0779a into rubygems:master Apr 28, 2019
@semaperepelitsa
Copy link
Contributor

Bundler 2.0.2 (released 5 days ago) does not include this change. Are you planning it for future versions?

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants