FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Allow to explicitly define a build as (not) cross-compiled by piranna · Pull Request #10287 · nodejs/node · GitHub

/ node Public

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

Closed
piranna wants to merge 1 commit into
nodejs:masterfrom
piranna:master
ConversationCommits (1)ChecksFiles changed
Closed

Allow to explicitly define a build as (not) cross-compiled#10287
piranna wants to merge 1 commit into
nodejs:masterfrom
piranna:master

Conversation

piranna commented Dec 15, 2016

Copy link
Copy Markdown
Contributor
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 added build Issues and PRs related to build files or the CI. lts-watch-v6.x labels Dec 15, 2016
addaleax approved these changes Dec 15, 2016
View reviewed changes

addaleax left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Dec 15, 2016

Copy link
Copy Markdown
Contributor Author

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

Done, thank you for the advice :-)

piranna mentioned this pull request Dec 16, 2016
Closed

jasnell commented Dec 23, 2016

Copy link
Copy Markdown
Member

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

jasnell self-assigned this Dec 23, 2016
bnoordhuis approved these changes Dec 23, 2016
View reviewed changes

bnoordhuis left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

gibfahn commented Dec 23, 2016

Copy link
Copy Markdown
Member

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

build: add (not) cross-compiled configure flags
b09d14e 
Adds --cross-compiling and --no-cross-compiling flags

Fixes: nodejs#10271

piranna commented Dec 26, 2016

Copy link
Copy Markdown
Contributor Author

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
build: add (not) cross-compiled configure flags
8e60e0f 
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 commented Dec 27, 2016

Copy link
Copy Markdown
Member

Landed in 8e60e0f

jasnell closed this Dec 27, 2016

piranna commented Dec 27, 2016

Copy link
Copy Markdown
Contributor Author

Thank you :-)

evanlucas mentioned this pull request Jan 3, 2017
Merged
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
build: add (not) cross-compiled configure flags
be4e2bc 
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
build: add (not) cross-compiled configure flags
cff57be 
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
build: add (not) cross-compiled configure flags
b7279bc 
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
build: add (not) cross-compiled configure flags
a289b05 
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
build: add (not) cross-compiled configure flags
418d5ce 
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
build: add (not) cross-compiled configure flags
4052c02 
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
build: add (not) cross-compiled configure flags
0742272 
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
Merged
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
build: add (not) cross-compiled configure flags
7d519fa 
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
build: add (not) cross-compiled configure flags
0bb77f2 
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
jasnell jasnell approved these changes
addaleax addaleax approved these changes

Assignees

jasnell

Labels

build Issues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL