Skip to content

package.json: Make llnode installable via npm#60

Merged
hhellyer merged 5 commits intonodejs:masterfrom
hhellyer:npm
Jan 10, 2017
Merged

package.json: Make llnode installable via npm#60
hhellyer merged 5 commits intonodejs:masterfrom
hhellyer:npm

Conversation

@hhellyer
Copy link
Contributor

@hhellyer hhellyer commented Dec 6, 2016

This updates package.json and adds scripts to allow llnode to be
built via "npm install llnode"
It doesn't create a Node.js module you can use via require('llnode').
(This may change in the future depending on project direction.)

The npm name llnode has been reserved here:
https://www.npmjs.com/package/llnode
and Fedor also has access to that as discussed under issue #53.

This PR does leave the npm version number at 0.0.0, this should
be updated once the install process has been reviewed and at
that point the llnode package should be npm publish'd.

This updates package.json and adds scripts to allow llnode to be
built via "npm install llnode"
It doesn't create a Node.js module you can use via require('llnode').
(This may change in the future depending on project direction.)

The npm name llnode has been reserved here:
https://www.npmjs.com/package/llnode

This PR does leave the npm version number at 0.0.0, this should
once the install process has been reviewed and at that point
the llnode package should be npm publish'd.
richardlau

This comment was marked as off-topic.

@yjhjstz
Copy link
Contributor

yjhjstz commented Dec 7, 2016

cool!

- If lldb doesn't exist try lldb-3.9,3.8,3.7 etc instead.
- Allow install scripts guess to be overridden with --lldb_exe option
  e.g. npm install --lldb_exe=`which lldb-3.9`
@hhellyer
Copy link
Contributor Author

I've updated configure.js to do a better job of finding the lldb executable on Linux.
It also now allows the user to force the use of a particular build of lldb by letting the user specify --lldb_exe on the command line. So on Ubuntu to force the user of lldb-3.9 you can do:

npm install --lldb_exe=`which lldb-3.9` llnode

@hhellyer
Copy link
Contributor Author

@indutny @Fishrock123 - I'll be out from the end of this week to 3rd January. I'm happy to make changes this week if you have any comments. If you don't have time this week I don't mind if you make changes on my branch while I'm out, otherwise we can fix it up in the new year.

@Fishrock123 Fishrock123 self-requested a review December 19, 2016 16:59
Set the version to 0.0.1 to allow the install process from npmjs.org to be
tested. It can be corrected to the current version when this branch is
merged into master.
@hhellyer
Copy link
Contributor Author

hhellyer commented Jan 9, 2017

I've put this branch up on npmjs.org as version 0.0.1 so that it can get some testing and feedback when used as an actual npm.
https://www.npmjs.com/package/llnode

indutny

This comment was marked as off-topic.

@rnchamberlain
Copy link
Contributor

FYI, tried this out on Ubuntu, lldb 3.9, with a core dump from Node.js v7:

> npm install llnode
> lldb node -c core
(lldb) plugin load node_modules/llnode/llnode.so
(lldb) jsstack

Works well, this is going to be nice

@hhellyer
Copy link
Contributor Author

I copied the .eslintrc from a node build and ran that against the scripts. They should be formatted correctly now.

indutny

This comment was marked as off-topic.

@hhellyer
Copy link
Contributor Author

@indutny I'll put the package.json level back to 1.4.0, do a merge and then publish a v1.4.0 package to npm. Is that reasonable? (We might need to document the publish process the contribution docs now too.)

@indutny
Copy link
Member

indutny commented Jan 10, 2017

@hhellyer sounds reasonable to me.

@hhellyer hhellyer merged commit 2325ae3 into nodejs:master Jan 10, 2017
@hhellyer hhellyer deleted the npm branch January 10, 2017 13:40
@hhellyer
Copy link
Contributor Author

I've published to https://www.npmjs.com/package/llnode - it took an hour or so to update the npmjs.org page last time so that still shows 0.0.1 but doing npm install llnode now installs version 1.4.1.

@indutny
Copy link
Member

indutny commented Jan 10, 2017

Fantastic! Thank you!

@bnoordhuis
Copy link
Member

@hhellyer Why did you rename/symlink llnode.gyp to llnode.gyp.json? From looking at 2325ae3 it's quite clearly a deliberate move but for the life of me, I can't figure out why.

@hhellyer
Copy link
Contributor Author

@bnoordhuis - Well I'm quite prepared to be told I was being an idiot but no matter what I did I couldn't figure out why when I did npm install <path_to_folder> (during development) the npm never copied files ending in .gyp. So I renamed it llnode.gyp.json. (I've just re-tested it and calling it llnode.gyp or build.gyp both still cause it not to be copied.)

It might be an obscure npm feature or just something really silly I was doing but in the end renaming it seemed enough to solve my problem. There might be some weirdness around having .gyp files other than binding.gyp in your npm, I just added one to something else to verify the copying problem and it's existence alone broke the build.

I think I just put the symlink in so I didn't break anyone else who was referring to the file. (I was trying to make the minimum set of changes.)

@richardlau
Copy link
Member

I don't have the reference to hand but I believe npm has (had?) the surprising behaviour of renaming all *.gyp files to binding.gyp.

@hhellyer
Copy link
Contributor Author

@richardlau - That would actually fit what I saw. (And explain why now if I change binding.gyp in the npm I'm working on to iamafish.gyp it still builds!)

@richardlau
Copy link
Member

npm/npm#8411 and npm/fstream-npm#18

@bnoordhuis bnoordhuis mentioned this pull request May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants