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

fix(linker): use lockfileFolder when creating bin links #4730

Merged
merged 2 commits into from Oct 19, 2017

Conversation

jgoz
Copy link
Contributor

@jgoz jgoz commented Oct 17, 2017

Summary

Fixes #4706, fixes #4359, refs #4513. this.config.cwd was being used as the root for bin link paths, rather than this.config.lockfileFolder.

Test plan

@edorivai
Copy link

@jgoz, I just checked #4513, and according to my test, the issue still exists.

@jgoz
Copy link
Contributor Author

jgoz commented Oct 18, 2017

@edorivai OK, thanks for checking. I think that issue is probably related and will have a similar solution.

@edorivai
Copy link

@jgoz 👍 looking forward. With the exception of some minor issues, workspaces are very awesome until now!

@edmorley
Copy link
Contributor

edmorley commented Oct 19, 2017

Maybe fixes #4359 (I didn't test against the repro - @ohana54, could you test this branch?)

I can confirm that using the STR in #4359 (comment) this no longer reproduces for me using a build from this PR branch, when it did before under yarn 1.2.1 😺 🎉

@jgoz
Copy link
Contributor Author

jgoz commented Oct 19, 2017

@edmorley Great! Thanks for testing that. I added a unit test covering #4359 to this branch.

Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

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

Much appreciated, thanks!

@BYK
Copy link
Member

BYK commented Oct 19, 2017

Wow, this is great thank you! I'll also check #4289 with this patch.

@BYK BYK changed the title fix(linker): use lockfileFolder when creating bin links (#4706) fix(linker): use lockfileFolder when creating bin links Oct 19, 2017
@BYK BYK merged commit 58ae45e into yarnpkg:master Oct 19, 2017
@jgoz jgoz deleted the fix-root-bin-links branch October 19, 2017 23:58
joaolucasl pushed a commit to joaolucasl/yarn that referenced this pull request Oct 27, 2017
**Summary**

Fixes yarnpkg#4706, fixes yarnpkg#4359, refs yarnpkg#4513. `this.config.cwd` was being used as the root for bin link paths, rather than `this.config.lockfileFolder`.

**Test plan**

- Added tests for `add` and `remove` commands (yarnpkg#4706)
- Added test for `install` command (yarnpkg#4359)
BYK pushed a commit that referenced this pull request Nov 20, 2017
**Summary**

Partial resolution for #4543.

Previously, when running `yarn run` the env PATH would be set to look in node_modules/.bin, however, in workspaces the root workspace .bin path was not being included.

This PR adds the workspace root
node_modules/.bin path after the individual package's path.

This is generally needed because #4730 ensures bin links in a workspace will be at the workspace root. With this PR, you can now `yarn run` commands in an individual package again.

**Test plan**

Manually tested by adding a script that runs `echo $PATH`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants