Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
build: fix DESTCPU detection on non-Intel platforms#6310
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
richardlau commented Apr 20, 2016
cc @mhdawson |
mhdawson commented Apr 20, 2016
Just wondering which case this covers: +ifeq ($(findstring powerpc,$(shell uname -p)),powerpc) +DESTCPU ?= ppc +else otherwise LGTM |
richardlau commented Apr 20, 2016
That case covers AIX. |
jbergstroem commented Apr 21, 2016
@nodejs/build |
richardlau commented Apr 21, 2016
Note about the implementation: With regards to host architecture detection there are two other places this is done in the build scripts:
|
dec85d5 to fde7310Comparerichardlau commented Apr 21, 2016
@mhdawson I've updated the commit so on AIX it defaults to 64-bit as we discussed on the phone. |
fde7310 to 3afb621Comparemhdawson commented Apr 21, 2016
LGTM |
mhdawson commented Apr 21, 2016
@jbergstroem just wondering if you can take a quick look and let me know if you have any concerns with this. |
7da4fd4 to c7066fbComparejoaocgreis commented Apr 26, 2016
I can't claim to know enough of this to stamp it, but since it uses only However, at least ARM64 is missing: $ uname -m aarch64 |
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture.
3afb621 to 972cb47Comparerichardlau commented Apr 26, 2016
Thanks, rebased onto latest master and added aarch64. |
MylesBorins commented May 12, 2016
ci: https://ci.nodejs.org/job/node-test-pull-request/2614/ overall the changes to the makefile LGTM How was ppc + arm being detected and built correctly before? |
rvagg commented May 13, 2016
sgtm might be time to pull out all of this ARCH and DESTCPU stuff into a script (bash or perl) in tools |
jbergstroem commented May 13, 2016
@rvagg we could use python's |
rvagg commented May 13, 2016
@jbergstroem sounds good. To clarify though, that shouldn't hold up this PR if the IBM folks agree. @thealphanerd we manually specify both |
jbergstroem commented May 13, 2016
@rvagg we don't manually specify it on all our machines; mainly release-related stuff -- I think we should avoid it where possible since we'd be testing the configure process as well. With that said, in cases we have to, we should obviously override (esp |
jbergstroem commented May 13, 2016
LGTM from me as well btw. Just haven't tried all variations of |
mhdawson commented May 13, 2016
Looks like we have enough LGTMs to land, will do that now |
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture. PR-URL: #6310 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
mhdawson commented May 13, 2016
Landed as 830a726 |
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture. PR-URL: #6310 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture. PR-URL: #6310 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture. PR-URL: #6310 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
`make binary` attempts to auto detect DESTCPU if not set, but was assuming being on an Intel architecture. PR-URL: #6310 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Checklist
Affected core subsystem(s)
build
Description of change
make binaryattempts to auto detectDESTCPUif not set, but wasassuming being on an Intel architecture (i.e. it was checking for x64
and assuming x86 otherwise).