-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Redirect "global" yarn directory #2969
Conversation
6003b25
to
11353f9
Compare
@r15ch13 any comments? |
@milang I'm just a random guy from the internet. I like Scoop and I noticed there's many PRs so I figured I could spend a few minutes helping with code reviews. I have no Scoop authority whatsoever. I think the code behind this change is good, but I'm not sure what the practical problem is with Yarn's global directory remaining in I understand that it would be neater if it's all managed in scoop's persistent storage, but it also adds complexity. Did you write this just for neatness, or is there a practical problem with Yarn managing its own global storage behind Scoop's back? |
That's a matter of choice, it's neat that redirecting it to scoop's persistent storage, but as you said, it adds complexity. I don't care about the problem that some apps do have behavior outside Scoop's folder, so just keeping yarn's default I can't force every Scoop user to do that, so making a variant manifest for yourself is a better choice, that's a highlight feature of Scoop. Make Scoop fit your need! P.S. Changing yarn's |
This change seems reasonable, but I don’t use yarn. Are there any downsides to merging this in? |
I see one downside and one risk:
Finally, while I can see that this PR has been a lot of work to make, and the code itself looks fine, I still don't know what problem it solves. If it's just the aesthetics of not having a Yarn folder outside the Scoop tree then I'm against merging this in for the above-mentioned downside and risk. But maybe there's a different problem that I fail to see. @milang can you chip in? (note: i am just a guy from the internet, my opinion should not weigh particularly heavily) |
I've made a PR in upstream (yarnpkg/yarn#7056) that add When the upstream PR is not merged, maybe a hardcoded fix to yarn is better then hardcoded fix to install script in scoop manifest. I'll try to make one (a simple -replace doesn't work). |
- Yarn's global folder can be set via `yarn config set global-folder` (yarnpkg/yarn#7056) and thus can be persisted. - ~~Since yarn's global bin creating procedure is still problematic (yarnpkg/yarn#6902, **fixed by yarnpkg/yarn#6954 The `.bin` folder in `global\node_modules` is a better path to add to env, and this can avoid the annoying problem when you install scoop in some place except `C:` (that the shims in global bin have wrong relative path pointer). - If you install yarn via `scoop install yarn`, the `Yarn` folder in `$env:LOCALAPPDATA` is useless, and when you uninstall `yarn`, the `.yarnrc` is unused, so the manifest add `uninstaller.script` to remove them when you uninstall. - For reconfiguration, please use `scoop update yarn -f` instead of `scoop reset yarn`. - Close #2969
Thank you @niheaven, your fix is much better. |
Current installer for
yarn
redirectsbin
,cache
andmirror
directories to scoop's persistent storage. However, theglobal
directory (used by packages that get installed globally viayarn global add
) is still using the default$env:LOCALAPPDATA\Yarn\Data\global
.This pull request adds a persistent directory
global
and then updates.yarnrc
to use it.Unfortunately
yarn
does not currently recognizeglobal-folder
configuration setting, only--global-folder
(source), so I added an extra post-install PowerShell fragment that adds the--
prefix toglobal-folder
in.yarnrc
file.The following screenshot illustrates the difference in
yarn global dir
output when using installing the currentyarn
bundle and when installing updated bundle (as per this PR):