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

Support for ~ (home dir) in path configurations #3756

Merged
merged 1 commit into from
Jun 29, 2017

Conversation

vbfox
Copy link
Contributor

@vbfox vbfox commented Jun 29, 2017

Summary

This fixes the handling of path that are present in npm configuration (#2930). What's now handled :

  • Paths starting with ~/ (Also ~\ on windows) get expanded
  • Path that aren't rooted are considered relative to the current directory
  • All path are fully resolved

All of this is from npm ( https://github.com/npm/npm/blob/latest/lib/config/core.js#L394 ).
I'm not sure that I like the fact that relative paths are relative to cwd instead of the configuration file where they are found but it's what's npm is doing 😉

Motivation

My original motivation was to stop having errors when i'm installing node-sass on node 8. As versions of node-sass that aren't the latest don't include pre-built binaries for node 8 the installer try to use node-gyp to build a fresh copy.

This build would work (I have the necessary dev tools) except that node-gyp since recently read the cafile property and open it. As the cwd for install script is their package, it fail and break most yarn commands.

Test plan

I added some unit tests tests.

Also verified on the output of an npm module dumping it's environment in it's install script (during yarn install --verbose) :

2017-06-29 16_05_09-vs 2015

With .npmrc:

cafile=cafile
prefix=~/.npm-global

@arcanis
Copy link
Member

arcanis commented Jun 29, 2017

Thanks!

@arcanis arcanis merged commit 35ee7de into yarnpkg:master Jun 29, 2017
bestander added a commit that referenced this pull request Jun 30, 2017
bestander added a commit that referenced this pull request Jun 30, 2017
* Revert "Fix: emoji should default to true on darwin platforms (#3766)"

This reverts commit 69574f6.

* Revert "Support for ~ (home dir) in path configurations (#3756)"

This reverts commit 35ee7de.

* Revert "Update: add regression tests for Git.spawn env issues (#3759)"

This reverts commit cd26fec.
BYK pushed a commit that referenced this pull request Oct 4, 2017
**Summary**

Looking at two solutions introduced in #3393 and #3756, the first one doesn't support win32, while the second does, sticking with the second one more beneficial and supports a wider range of OS.

Removed the stuff introduced in #3393 keeping only #3756.

#3756 also introduced config file normalization, so probably second argument to getOption is obsolete, will discover that and submit another PR if that's the case.

**Test plan**

Modified tests appropriately.
joaolucasl pushed a commit to joaolucasl/yarn that referenced this pull request Oct 27, 2017
…kg#4605)

**Summary**

Looking at two solutions introduced in yarnpkg#3393 and yarnpkg#3756, the first one doesn't support win32, while the second does, sticking with the second one more beneficial and supports a wider range of OS.

Removed the stuff introduced in yarnpkg#3393 keeping only yarnpkg#3756.

yarnpkg#3756 also introduced config file normalization, so probably second argument to getOption is obsolete, will discover that and submit another PR if that's the case.

**Test plan**

Modified tests appropriately.
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.

2 participants