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

build: Add VARIATION variable to binary target #4631

Closed
wants to merge 1 commit into from

Conversation

stefanmb
Copy link
Contributor

If the VARIATION variable is present, then "make binary" will produce archives named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).

This capability is useful when building variations of Node.js for the same platform/architecture. For example, when building x64 Linux with FIPS compliant crypto provider, it is useful to have a "-fips" suffix to easily identify this functionality without having to examine the contents.

Contrast: node-v6.0.0-linux-x64.tar.gz vs node-v6.0.0-linux-x64-variation.tar.gz

If the VARIATION variable is present, then make binary will produce archives
named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).
@silverwind silverwind added the build Issues and PRs related to build files or the CI. label Jan 11, 2016
@rvagg
Copy link
Member

rvagg commented Jan 12, 2016

not a bad idea, I think, /cc @nodejs/build

@jbergstroem
Copy link
Member

Sure, see no issue with it either.

@stefanmb
Copy link
Contributor Author

@mhdawson Does this look okay to you? Thanks!

@mhdawson
Copy link
Member

lgtm

@stefanmb
Copy link
Contributor Author

@jasnell In the absence of @mhdawson would you be okay with merging this? Thanks!

@jbergstroem
Copy link
Member

I can land it -- @rvagg LGTY?

@jasnell
Copy link
Member

jasnell commented Jan 21, 2016

Sorry, just saw this, yeah LGTM!
On Jan 20, 2016 3:36 PM, "Stefan Budeanu" notifications@github.com wrote:

@jasnell https://github.com/jasnell In the absence of @mhdawson
https://github.com/mhdawson would you be okay with merging this? Thanks!


Reply to this email directly or view it on GitHub
#4631 (comment).

@rvagg
Copy link
Member

rvagg commented Jan 21, 2016

lgtm pending https://ci.nodejs.org/job/node-test-pull-request/1338/

We also need to pay attention to the next nightly that comes out of this when landed to make sure it's working OK.

@jbergstroem
Copy link
Member

arm fails seem unrelated. Merging.

jbergstroem pushed a commit that referenced this pull request Jan 21, 2016
If the VARIATION variable is present, then make binary will produce archives
named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).

PR-URL: #4631
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@jbergstroem
Copy link
Member

Landed in bf12379.

@jasnell
Copy link
Member

jasnell commented Jan 23, 2016

marking this one as do not land in v4.x. It would likely be ok to land but given the change to the filename it could potentially break tooling. If anyone feels lts is appropriate, however, lemme know :-)

rvagg pushed a commit that referenced this pull request Jan 25, 2016
If the VARIATION variable is present, then make binary will produce archives
named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).

PR-URL: #4631
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
If the VARIATION variable is present, then make binary will produce archives
named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).

PR-URL: nodejs#4631
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants