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.

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

package.json: Make llnode installable via npm #60

Merged
merged 5 commits into from Jan 10, 2017
Merged

Conversation

hhellyer
Copy link
Collaborator

@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
Collaborator 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
Collaborator 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
Collaborator 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
Collaborator 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
Collaborator 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
Collaborator 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
Collaborator 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
Collaborator 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.

None yet

8 participants