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.

Sign up for GitHub

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

Jump to bottom

Recognize electron as a valid runtime #175

Merged
merged 2 commits into from
Oct 5, 2015
Merged

Recognize electron as a valid runtime #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da292e7
Recognize electron as a valid runtime
zcbenz Sep 24, 2015
ce53480
Use g++ 4.8 for building on CI
zcbenz Sep 24, 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
5 changes: 2 additions & 3 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5' ]
# for testing node-webkit
packages: ['clang-3.5', 'xvfb','libasound2','libx11-6','libglib2.0-0','libgtk2.0-0','libatk1.0-0','libgdk-pixbuf2.0-0','libcairo2','libfreetype6','libfontconfig1','libxcomposite1','libasound2','libxdamage1','libxext6','libxfixes3','libnss3','libnspr4','libgconf-2-4','libexpat1','libdbus-1-3','libudev0']
packages: ['g++-4.8', 'xvfb','libasound2','libx11-6','libglib2.0-0','libgtk2.0-0','libatk1.0-0','libgdk-pixbuf2.0-0','libcairo2','libfreetype6','libfontconfig1','libxcomposite1','libasound2','libxdamage1','libxext6','libxfixes3','libnss3','libnspr4','libgconf-2-4','libexpat1','libdbus-1-3','libudev0']


env:
Expand All @@ -28,8 +28,7 @@ env:

before_install:
- if [[ $(uname -s) == 'Linux' ]]; then
export CXX="clang++-3.5";
export CC="clang-3.5";
export CXX="g++-4.8";
fi;
- source ./scripts/install_node.sh ${NODE_VERSION}
- npm config set loglevel=error
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Options include:

- `-C/--directory`: run the command in this directory
- `--build-from-source`: build from source instead of using pre-built binary
- `--runtime=node-webkit`: customize the runtime: `node` and `node-webkit` are the valid options
- `--runtime=node-webkit`: customize the runtime: `node`, `electron` and `node-webkit` are the valid options
- `--fallback-to-build`: fallback to building from source if pre-built binary is not available
- `--target=0.10.25`: Pass the target node or node-webkit version to compile against
- `--target_arch=ia32`: Pass the target arch and override the host `arch`. Valid values are 'ia32','x64', or `arm`.
Expand Down
16 changes: 16 additions & 0 deletions lib/util/versioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ if (process.env.NODE_PRE_GYP_ABI_CROSSWALK) {
abi_crosswalk = require('./abi_crosswalk.json');
}

function get_electron_abi(runtime, target_version) {
if (!runtime) {
throw new Error("get_electron_abi requires valid runtime arg");
}
if (typeof target_version === 'undefined') {
// erroneous CLI call
throw new Error("Empty target version is not supported if electron is the target.");
}
// Electron guarentees that patch version update won't break native modules.
var sem_ver = semver.parse(target_version);
return runtime + '-v' + sem_ver.major + '.' + sem_ver.minor;
}
module.exports.get_electron_abi = get_electron_abi;

function get_node_webkit_abi(runtime, target_version) {
if (!runtime) {
throw new Error("get_node_webkit_abi requires valid runtime arg");
Expand Down Expand Up @@ -55,6 +69,8 @@ function get_runtime_abi(runtime, target_version) {
}
if (runtime === 'node-webkit') {
return get_node_webkit_abi(runtime, target_version || process.versions['node-webkit']);
} else if (runtime === 'electron') {
return get_electron_abi(runtime, target_version || process.versions.electron);
Copy link

Choose a reason for hiding this comment

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

what's the BKM for getting my runtime & target_version in, if I have a project that uses a package which consumes node-pre-gyp?

Copy link

Choose a reason for hiding this comment

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

sorry, best-known-method. preferred method, really*

Copy link

Choose a reason for hiding this comment

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

So, the policy that https://github.com/electronjs/electron-rebuild uses is, assume your package is a sibling package of electron-prebuilt, then use https://github.com/electronjs/electron-rebuild/blob/master/src/main.js#L42 to get the version info.

This isn't perfect, but it makes the Easy case Easy, and if you let devs also specify explicitly, you have an escape hatch

Copy link

Choose a reason for hiding this comment

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

hmm. so if I'm on electron-rebuild@1.0.1 and running it on postinstall, it should set it for me, eh? curious why I still get the Error: Empty target version is not supported if electron... error from above

Copy link

Choose a reason for hiding this comment

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

It will if electron-prebuilt is in the same folder as electron-rebuild in your node_modules folder. If not, nbd, you can use the cmd line flags to work around it

Copy link

Choose a reason for hiding this comment

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

thanks @paulcbetts, really appreciate it. perhaps you'd be willing to help me ID what I've missed. electron-prebuilt and electron-rebuild are good to go, it would seem.

cdieringer@Snapper-osx:~/node/coinstac$ ll node_modules/ | grep electron
drwxr-xr-x  15 cdieringer  staff   510 Oct 12 13:36 electron-prebuilt/
drwxr-xr-x  10 cdieringer  staff   340 Oct 12 14:03 electron-rebuild/

cdieringer@Snapper-osx:~/node/coinstac$ cat package.json | grep electron
    "electron-prebuilt": "^0.33.7",
    "electron-rebuild": "^1.0.1",

however, it still yields the following, against expectation:

...
> fsevents@1.0.2 install /Users/cdieringer/node/coinstac/node_modules/babel/node_modules/chokidar/node_modules/fsevents
> node-pre-gyp install --fallback-to-build

node-pre-gyp ERR! install error
node-pre-gyp ERR! stack Error: Empty target version is not supported if electron is the target.

Copy link

Choose a reason for hiding this comment

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

Oh this is a different error than I thought it was - I'll look into this. Can you file a bug on electronjs/electron-rebuild?

Copy link

Choose a reason for hiding this comment

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

ya, id be happy to. i might spend a little time looking into it tonight. i'd really like to give back somehow to a project that has helped me so much!

} else {
if (runtime != 'node') {
throw new Error("Unknown Runtime: '" + runtime + "'");
Expand Down