-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
language/node: add setup_yarn_environment #2962
Conversation
Library/Homebrew/language/node.rb
Outdated
yarnrc = Pathname.new("#{ENV["HOME"]}/.yarnrc") | ||
# only run setup_yarn_environment once per formula | ||
return if yarnrc.exist? | ||
# Writing a temporary `.yarnrc` to `.brew_home` to tell yarn to set it's global prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/it's/its/
Can you please elaborate on the problem? Given this isn't a released stable version yet, it might be possible to work with upstream so that we don't need a back-end workaround here.
Can we make the installation less "odd" so that this isn't necessary? It seems weird if we're installing node in such a way that the modules cannot be found by yarn without back-end changes to brew. |
Fixed comment type and fixed test to not write to |
Does that mean requiring a specific version of yarn?
would it work to put a link to node in libexec/bin? |
It means requiring a specific version of node (sry if I missed to explain that). For example metabase requires node to be exactly v4.4.7 and yarn will throw an error in yarn v0.28+ (instead of only warning about it) if the build tool is run with a different version (even if it works fine with other node versions too). BTW: Npm did move in the exact opposite direction and deprecated the engine filed and does not even warn anymore about it in recent versions.
Added a link to the actual code above. We would need to link Also this piece of code is a optional performance optimization. Because we've set yarns global prefix to something sandbox writeable and added it to the PATH we could also simply rely on yarn's fallback to temporary download Edit: If I'm going to upstream something to yarn anyway, maybe I can get this fix in too: chrmoritz/yarn@fb440c9 (which adds support for detecting |
I can confirm that I've also added setting yarn's cache to |
As a general note I think pretty much all the Honestly if we have to write all this code to support |
The only thing which is required to support yarn 0.28+ is the |
If we're going to install |
And how should this be done in case of |
It can be done
|
Another idea about prefix: this seems like something |
I thought the point of having these language helpers here is to avoid such duplication for language specific common task. In theory we could remove all language helpers and duplicate their functionality in all formulae as needed. But this would quickly end up in a maintenance nightmare, because every time we would need to make only the slightest change to such an helper method, we would need to make it to a lot of formulas at the same time and make sure to not forget a formula (which has failed in the past, see Homebrew/homebrew-core#16074 for example).
You mean writing a build specific env script wrapper to buildpath and prepending it to the PATH before the actual yarn bin from cellar/bin? This would work with a lot of code duplication (see 1), but wouldn't fix ignore-engines because there is no env var for this.
This would fix ignore-engines, but not prefix.
It does, |
It could set |
i.e. we could adjust the env script wrapper to first check whether |
But I thought the plan is to get rid of the formulae env script wrappers again once we update to a yarn version, which includes a fix for the yarn global prefix != HOMEBREW_PREFIX issue. |
I think a goal of having no env script wrappers would be preferable to having env script wrappers which would be preferable to temporary inreplaces which would be preferable to permanent inreplaces which would be preferable to Homebrew/brew changes we have to support pretty much forever. |
So you would prefer:
over:
? |
@chrmoritz I've expressed many times my preference is working with upstream so that we don't need custom code to handle all this stuff, honestly. |
And how should yarn upstream be able to solve the issue that:
|
The same way all software does: just write to the Cellar and resolve symlinks accordingly and, if really needed (which most software doesn't, notably) allow that directory to be specified in another argument to a
The same way Ruby/Python do: allow configuration of where things can be installed which doesn't infer needing write permissions there at install time.
So I'm not sure what the question/problem is? |
I mean, let's be clear here: |
@MikeMcQuaid I think you misunderstood my last comment. I was talking about using yarn as a build tool inside another formula (the case this PR tries to fix) and not about installing yarn itself. |
@chrmoritz I understand that. We don't need this level of handholding for most buildsystems, even. |
Ok, but than:
This would need to be handled by e.g.
It's about strict engine checks, which is not an issue with yarn, but either an issue with upstream being way to strict/conservative about supported node version or an issue with the homebrew formula not using the exact node version upstream wants (I'm not saying that doing this is a good idea, we have enough pain with it for |
Again this increasingly seems to point towards the "we stop packaging software that can't play nicely with Homebrew" as the solution. |
I don't see Given we currently have a grand total of three formulae that build using |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
@chrmoritz What's the status of this? |
@chrmoritz Closing for now but happy to reopen when you let us know the status. |
brew tests
with your changes locally?This PR adds a
setup_yarn_environment
helper method tolanguage/node
, which will be needed for the upcoming yarn v0.28 (currently inbrew install yarn --devel
) to be able to build some yarn depending formulae.What it does:
.yarnrc
to.brew_home
to tell yarn too:.brew_home/.yarn/bin
so that global build tool installations won't fail because of brew sandbox restrictions (like auto installingnode-gyp
on demand)HOMEBREW_CACHE/yarn_cache
(from.brew_home/Library/Caches/Yarn
) similar to what we do for npmpackage.json
engine filed would fail otherwise (warning => error)node-gyp
copy of the homebrew managed npm to the $PATH as a workaround preventing yarn from having to globally install a temporary copy ofnode-gyp
on demand (yarn default detectingnode-gyp
from npm code does not work because of homebrews-npm-installation-structure-oddness)