Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Adds flag to patch npm install for iojs compatibility #36357

Closed
wants to merge 6 commits into from
Closed

Adds flag to patch npm install for iojs compatibility #36357

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
464a6be
Patched npm for iojs compatability
bcomnes Jan 29, 2015
ad94f07
Restored relavant iojs caveats
bcomnes Jan 29, 2015
89ba7e0
Put npm patch behind a flag.
bcomnes Jan 29, 2015
7351df5
Added npm iojs support caveat
bcomnes Jan 29, 2015
9c535d4
Single lined bash-completion install
bcomnes Jan 29, 2015
b765478
One-lined variable declaration
bcomnes Jan 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions Library/Formula/iojs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,15 @@ def install
end

def caveats; <<-EOS.undent
iojs was installed without npm.
iojs was installed without npm. To install npm either:
brew install node
or follow:
https://github.com/npm/npm#fancy-install-unix

iojs currently requires a patched npm (i.e. not the npm installed by node).
To prepend iojs to your PATH add to your ~/.bashrc:
export PATH="#{Formula["iojs"].opt_bin}:$PATH"

This will also e.g. make npm use iojs's node.
EOS
end

Expand Down
27 changes: 27 additions & 0 deletions Library/Formula/node.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ def install
ENV["NPM_CONFIG_LOGLEVEL"] = "verbose"

cd buildpath/"npm_install" do
# Patch node-gyp until github.com/TooTallNate/node-gyp/pull/564 is resolved
patch :DATA
Copy link
Member

Choose a reason for hiding this comment

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

Don't think this will actually apply the patch here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shit, your right. Sorry, my test must have misled me. Fixing for discussions sake.

system "./configure", "--prefix=#{libexec}/npm"
system "make", "install"
end
Expand Down Expand Up @@ -162,3 +164,28 @@ def caveats
end
end
end

__END__
diff --git a/node_modules/node-gyp/lib/install.js b/node_modules/node-gyp/lib/install.js
index 6f72e6a..ebc4e57 100644
--- a/node_modules/node-gyp/lib/install.js
+++ b/node_modules/node-gyp/lib/install.js
@@ -39,7 +39,7 @@ function install (gyp, argv, callback) {
}
}

- var distUrl = gyp.opts['dist-url'] || gyp.opts.disturl || 'http://nodejs.org/dist'
+ var distUrl = gyp.opts['dist-url'] || gyp.opts.disturl || 'https://iojs.org/dist'


// Determine which node dev files version we are installing
@@ -185,7 +185,7 @@ function install (gyp, argv, callback) {

// now download the node tarball
var tarPath = gyp.opts['tarball']
- var tarballUrl = tarPath ? tarPath : distUrl + '/v' + version + '/node-v' + version + '.tar.gz'
+ var tarballUrl = tarPath ? tarPath : distUrl + '/v' + version + '/iojs-v' + version + '.tar.gz'
Copy link
Member

Choose a reason for hiding this comment

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

This patch looks way too iojs specific; it's literally just overriding node urls with iojs ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be only temporary until node-gyp can decide where to download from on its own. This only touches the npm portion of the install and this is how npm devs are currently dealing with iojs compatibility. It also doesn't break joyent node compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather wait until this patch is accepted for this reason. It looks like it could cause issues if that node fallback is just removed.

, badDownload = false
, extractCount = 0
, gunzip = zlib.createGunzip()