Skip to content

make build tool independent - #7

Closed
juliangruber wants to merge 1 commit into
prebuild:masterfrom
juliangruber:make-build-tool-independent
Closed

make build tool independent#7
juliangruber wants to merge 1 commit into
prebuild:masterfrom
juliangruber:make-build-tool-independent

Conversation

@juliangruber

Copy link
Copy Markdown

no need to be limited to node-gyp :) I came up with this because a project I want to use this on uses cmake-js.

I tested this in https://github.com/voltraco/node-taglib2 so far, but it should work everywhere as node-gyp reads all its config also from the environment variables npm sets.

@juliangruber

juliangruber commented Jun 23, 2017

Copy link
Copy Markdown
Author

This PR goes hand in hand with prebuild/node-gyp-build#4.

@mafintosh

Copy link
Copy Markdown
Collaborator

Very cool. Did you test that this works on windows?

@juliangruber

Copy link
Copy Markdown
Author

not yet, will do

@mafintosh

Copy link
Copy Markdown
Collaborator

I don't see anything that wouldnt work but you never know ... windows

@juliangruber

Copy link
Copy Markdown
Author

I tested this with taglib2 and it works :)

@bcomnes

bcomnes commented Aug 29, 2019

Copy link
Copy Markdown
Contributor

@juliangruber So in order to use this, you would land this patch and run https://github.com/juliangruber/prebuildify-load ?

@vweevers

Copy link
Copy Markdown
Member

@bcomnes That is outdated by now. We can instead add the missing functionality to node-gyp-build, provided that it doesn't need new dependencies.

@bcomnes

bcomnes commented Aug 29, 2019

Copy link
Copy Markdown
Contributor

Sounds good

@vweevers

Copy link
Copy Markdown
Member

Maybe a package rename is in order, but we can table that for now.

PR(s) welcome!

@bcomnes

bcomnes commented Aug 30, 2019

Copy link
Copy Markdown
Contributor

Don't quite have a vision of what you mean, maybe if we write it up?

@vweevers

Copy link
Copy Markdown
Member

To clarify, I thought it was okay to add functionality to the existing modules and that we could later choose to rename node-gyp-build to something more generic.

But on second thought, there's a fundamental difference between prebuildify-load and node-gyp-build that may in fact warrant the existence of prebuildify-load (which has a better name too). While node-gyp-build wraps node-gyp, prebuildify-load externalizes that. Compare:

"scripts": {
  "install": "node-gyp-build"
}
"scripts": {
  "install": "prebuildify-load || node-gyp build"
}

The latter approach avoids needing integration code specific to e.g. cmake-js.

If we can agree (@mafintosh @ralphtheninja) that this is the preferred approach, we could move prebuildify-load into the prebuild org and revive it, phasing out node-gyp-build.

@mafintosh

Copy link
Copy Markdown
Collaborator

node-gyp-build is the only production dep needed to run the prebuildify stack, so we need to keep that extremely light (all the install speed comes from that package having no deps), so adding cmake there is prob a nogo. Making a new package for that use-case is fine

@vweevers

Copy link
Copy Markdown
Member

@mafintosh My point was, with the approach of prebuildify-load, there's no need to add cmake.

@bcomnes

bcomnes commented Aug 30, 2019

Copy link
Copy Markdown
Contributor

I don’t personally have a need or a valid perspective on this so I’ll abstain

@mafintosh

Copy link
Copy Markdown
Collaborator

Old issue, but I think this makes sense actually. @vweevers should we revisit this?

@vweevers

Copy link
Copy Markdown
Member

It's a good feature and doesn't add complexity. I'm for it, as long as it's semver-major because it will break prebuildify-cross.

@mafintosh

Copy link
Copy Markdown
Collaborator

@vweevers i just registered the @prebuild org on npm (you should have gotten an invite). Then we could make a prebuild loader tool at @prebuild/load (i see julian's is archived)

@juliangruber

Copy link
Copy Markdown
Author

we could revive mine as well, I'm open to hand anything over

Comment thread index.js
var args = [
'rebuild',
'--target=' + target
'install',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we consider npm run install instead of npm install? The latter will slow builds down (doing unnecessary work), especially with prebuildify-cross

Comment thread index.js
'--target=' + target
'install',
'--target=' + target,
'--abi=' + abi.getAbi(target, runtime)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might need to add --build-from-source, if the install script is node-gyp-build.

@aminya

aminya commented Jan 6, 2021

Copy link
Copy Markdown
Contributor

@juliangruber Could you register your fork on npm so we can use it?

This fixes #49

@juliangruber

Copy link
Copy Markdown
Author

@juliangruber Could you register your fork on npm so we can use it?

You can also publish it :) Or use the npm github syntax

@juliangruber
juliangruber deleted the make-build-tool-independent branch June 13, 2023 08:48
@vweevers vweevers mentioned this pull request Dec 3, 2023
@vweevers vweevers mentioned this pull request Feb 22, 2026
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.

6 participants