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

Allow to explicitly define a build as (not) cross-compiled #10287

Closed
wants to merge 1 commit into from

Conversation

piranna
Copy link
Contributor

@piranna piranna commented Dec 15, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

Build and compilation

Description of change

This fixes issue #10271, allowing to explicitly set that we are doing a cross-compilation (or not, if we desire so) instead of only rely on heuristics. This is useful when you are compiling to the same target CPU of your host system by using a toolchain that uses a different C lib (like musl from glib), since mkpeephole gets compiled with the cross-toolchain and later can't be executed. This way we can force it to use the host toolchain instead. By default it still use the heuristics.

I have checked that it correctly set the GYP variables correctly and the compiled Node.js works as expected.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. lts-watch-v6.x labels Dec 15, 2016
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

This fixes issue #10271

Could you add a Fixes: https://github.com/nodejs/node/issues/10271 line at the end of the commit message? :)

CI: https://ci.nodejs.org/job/node-test-commit/6665/

@piranna
Copy link
Contributor Author

piranna commented Dec 15, 2016

Could you add a Fixes: #10271 line at the end of the commit message? :)

Done, thank you for the advice :-)

@jasnell
Copy link
Member

jasnell commented Dec 23, 2016

New CI, just in case: https://ci.nodejs.org/job/node-test-pull-request/5555/

@jasnell jasnell self-assigned this Dec 23, 2016
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM but please format the commit log according to the style guide from CONTRIBUTING.md.

@gibfahn
Copy link
Member

gibfahn commented Dec 23, 2016

Commit log suggestion:

build: add (not) cross-compiled configure flags

Adds --cross-compiling and --no-cross-compiling flags

Fixes: https://github.com/nodejs/node/issues/10271

Adds --cross-compiling and --no-cross-compiling flags

Fixes: nodejs#10271
@piranna
Copy link
Contributor Author

piranna commented Dec 26, 2016

Commit log suggestion:

build: add (not) cross-compiled configure flags

Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271

Done, thanks for the suggestion :-)

jasnell pushed a commit that referenced this pull request Dec 27, 2016
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnell
Copy link
Member

jasnell commented Dec 27, 2016

Landed in 8e60e0f

@jasnell jasnell closed this Dec 27, 2016
@piranna
Copy link
Contributor Author

piranna commented Dec 27, 2016

Thank you :-)

evanlucas pushed a commit that referenced this pull request Jan 3, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This was referenced Jan 24, 2017
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
Adds --cross-compiling and --no-cross-compiling flags

Fixes: #10271
PR-URL: #10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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.

None yet

7 participants