This repository was archived by the owner on Jan 16, 2025. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Graduate iojs out of keg-only #36369
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
module Language | ||
module JS | ||
|
||
NPM_URL = "https://registry.npmjs.org/npm/-/npm-2.9.0.tgz" | ||
NPM_SHA256 = "a4d1434e934eaf08d985dbe3b70e0d456fb6866828afc91e99b3aac286fca055" | ||
|
||
def install_npm(path) | ||
# make sure npm can find node | ||
ENV.prepend_path "PATH", bin | ||
# make sure user prefix settings in $HOME are ignored | ||
ENV["HOME"] = buildpath/"home" | ||
# set log level temporarily for npm's `make install` | ||
ENV["NPM_CONFIG_LOGLEVEL"] = "verbose" | ||
|
||
cd path do | ||
system "./configure", "--prefix=#{libexec}/npm" | ||
system "make", "install" | ||
end | ||
end | ||
|
||
def install_npm_bash_completion (path) | ||
bash_completion.install \ | ||
path/"lib/utils/completion.sh" => "npm" | ||
end | ||
|
||
def npm_post_install(libexec) | ||
node_modules = HOMEBREW_PREFIX/"lib/node_modules" | ||
node_modules.mkpath | ||
npm_exec = node_modules/"npm/bin/npm-cli.js" | ||
# Kill npm but preserve all other modules across node updates/upgrades. | ||
rm_rf node_modules/"npm" | ||
|
||
cp_r libexec/"npm/lib/node_modules/npm", node_modules | ||
# This symlink doesn't hop into homebrew_prefix/bin automatically so | ||
# remove it and make our own. This is a small consequence of our bottle | ||
# npm make install workaround. All other installs **do** symlink to | ||
# homebrew_prefix/bin correctly. We ln rather than cp this because doing | ||
# so mimics npm's normal install. | ||
ln_sf npm_exec, "#{HOMEBREW_PREFIX}/bin/npm" | ||
|
||
# Let's do the manpage dance. It's just a jump to the left. | ||
# And then a step to the right, with your hand on rm_f. | ||
["man1", "man3", "man5", "man7"].each do |man| | ||
# Dirs must exist first: https://github.com/Homebrew/homebrew/issues/35969 | ||
mkdir_p HOMEBREW_PREFIX/"share/man/#{man}" | ||
rm_f Dir[HOMEBREW_PREFIX/"share/man/#{man}/{npm.,npm-,npmrc.}*"] | ||
ln_sf Dir[libexec/"npm/share/man/#{man}/npm*"], HOMEBREW_PREFIX/"share/man/#{man}" | ||
end | ||
|
||
npm_root = node_modules/"npm" | ||
npmrc = npm_root/"npmrc" | ||
npmrc.atomic_write("prefix = #{HOMEBREW_PREFIX}\n") | ||
end | ||
|
||
def npm_caveats | ||
s = "" | ||
|
||
if build.without? "npm" | ||
s += <<-EOS.undent | ||
Homebrew has NOT installed npm. If you later install it, you should supplement | ||
your NODE_PATH with the npm module folder: | ||
#{HOMEBREW_PREFIX}/lib/node_modules | ||
EOS | ||
else | ||
s += <<-EOS.undent | ||
npm has been installed. To update run | ||
npm install -g npm@latest | ||
|
||
You can install global npm packages with | ||
npm install -g <package> | ||
|
||
Packages will install into the global node_modiles directory | ||
#{HOMEBREW_PREFIX}/lib/node_modules | ||
EOS | ||
end | ||
return s | ||
end | ||
|
||
def npm_test_install | ||
assert (HOMEBREW_PREFIX/"bin/npm").exist?, "npm must exist" | ||
assert (HOMEBREW_PREFIX/"bin/npm").executable?, "npm must be executable" | ||
system "#{HOMEBREW_PREFIX}/bin/npm", "--verbose", "install", "npm@latest" | ||
# Test if node-gyp is working by building a native addon | ||
system "#{HOMEBREW_PREFIX}/bin/npm", "--verbose", "install", "buffertools" | ||
end | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just call these method with the full paths instead.
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.
I lazily took advantage of the extended scope. I'll have to sort out passing the correct methods and variable to get them to work. I'll peek at it tomorrow.
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.
@MikeMcQuaid Can you give me an example of what you were thinking? My knowledge of ruby's class system is failing me. I gave it a shot here: bcomnes@53b7c47
Is there a way to more explicitly call mixin functions, or did you not want to use a mixin in this situation?
It works, but it seems like this way loses the scope of the formula its called in and I have to pass in all local variables and utility functions. npm's verbose output starts to leak to stdout as well. Any ideas?
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.
Yeh, bcomnes@53b7c47 makes sense to me. I think losing the scope is a feature there (stops being as coupled to the
Formula
API). @xu-cheng previously handled thestdout
leaking case but I can't quite remember how; I'm sure he'll be able to elaborate.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.
This
stdout
leaking is caused by losing the scope. You are actually callingKernel.system
instead ofFormula.system
.The right solution is leaving
include Language::JS
as it is. We did the same thing in Haskell(e.g.pandoc.rb
).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.
I remember we talked about exposing
Formula.system
's behaviour elsewhere? Did we ever do that? If not: maybe this is a good time to do so?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.
I don't know. @jacknagel should know more than I do.
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.
I think we didn't. Yeh, can leave it as an
include
for now and I'll add that to my TODO list.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.
I'd be happy to go back and update it to decouple from the
Formula
API in the future once the correct stdout containment ingredients are in place.Leaving it as a mixin for now.