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: use arm64 as DESTCPU for aarch64 #22548

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Aug 27, 2018

On a aarch64 system I can run the complete build with tests without
specifying the Makefile variable DESTCPU.

But when running the tar-headers target DESTCPU is passed to configure:

    $(PYTHON) ./configure \
               --prefix=/ \
               --dest-cpu=$(DESTCPU) \
               ...

The value of DESTCPU in this case will be 'aarch64' which will cause
configure to fail:

configure: error: option --dest-cpu: invalid choice: 'aarch64'
(choose from 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc',
'ppc64', 'x32', 'x64', 'x86', 'x86_64', 's390', 's390x')

In the configure script there is a matching of __aarch64__ to arm64:

$ python -c 'from configure import host_arch_cc; print host_arch_cc()'
arm64

In our case it would be nice to have consitent behaviour for both of
these cases on aarch64.

This commit changes DESTCPU to arm64 to be consistent with the
configure script. DESTCPU is used in $(TARBALL)-headers and in
$(BINARYTAR) but I'm not sure about the implications of making the
change purposed and hope others might chime in and provide some
guidance.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

On a `aarch64` system I can run the complete build with tests without
specifying the Makefile variable `DESTCPU`.

But when running the `tar-headers` target `DESTCPU` is passed to configure:
```shell
    $(PYTHON) ./configure \
               --prefix=/ \
               --dest-cpu=$(DESTCPU) \
               ...
```
The value of `DESTCPU` in this case will be 'aarch64' which will cause
configure to fail:
```shell
configure: error: option --dest-cpu: invalid choice: 'aarch64'
(choose from 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc',
'ppc64', 'x32', 'x64', 'x86', 'x86_64', 's390', 's390x')
```

In the configure script there is a matching of `__aarch64__` to `arm64`:
```shell
$ python -c 'from configure import host_arch_cc; print host_arch_cc()'
arm64
```

In our case it would be nice to have consitent behaviour for both of
these cases on `aarch64`.

This commit changes `DESTCPU` to `arm64` to be consistent with the
configure script. `DESTCPU` is used in `$(TARBALL)-headers` and in
`$(BINARYTAR)` but I'm not sure about the implications of making the
change purposed and hope others might chime in and provide some
guidance.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Aug 27, 2018
@danbev
Copy link
Contributor Author

danbev commented Aug 27, 2018

@danbev
Copy link
Contributor Author

danbev commented Aug 29, 2018

Re-run of failing node-test-commit-freebsd was successful.

@richardlau
Copy link
Member

Ref: #5175

Copy link

@yselkowitz yselkowitz left a comment

Choose a reason for hiding this comment

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

This is only half the change necessary. Further below (around line 750) you also need to:

-ifeq ($(DESTCPU),aarch64)
+ifeq ($(DESTCPU),arm64)

@danbev
Copy link
Contributor Author

danbev commented Aug 30, 2018

This is only half the change necessary. Further below (around line 750) you also need to

Thanks @yselkowitz! I'll take a closer look at this tomorrow.

@danbev
Copy link
Contributor Author

danbev commented Aug 31, 2018

@BridgeAR
Copy link
Member

@nodejs/build-files PTAL

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 31, 2018
@addaleax
Copy link
Member

@refack
Copy link
Contributor

refack commented Aug 31, 2018

I don't have enough background, but reading https://stackoverflow.com/a/47274698/27955 makes me think it should be the other way around... (map arm64 to aarch64)

  1. https://www.phoronix.com/scan.php?page=news_item&px=MTY5ODk
 With that aligned, within the LLVM code-base the Apple ARM64 back-end was then
 renamed to AArch64 to reflect the official 64-bit ARM architecture title.
  1. https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html

@yselkowitz
Copy link

v8 uses arm64 throughout, as does the kernel and golang for instance. While aarch64 is more canonical, I'm in no position to change any of those and doing so would be much more extensive than simply fixing DESTCPU.

Copy link
Member

@gdams gdams left a comment

Choose a reason for hiding this comment

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

I'm not going to block this change but IMO we should be using aarch64 instead of arm64 to be consistent with most other packages. AFAIK arm64 is something that Apple invented and never properly got adopted by anyone else.

@refack
Copy link
Contributor

refack commented Aug 31, 2018

v8 uses arm64 throughout

Since node is highly dependant on V8, let's go with arm64...

@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

@danbev
Copy link
Contributor Author

danbev commented Sep 3, 2018

Landed in d7d3bf5.

@danbev danbev closed this Sep 3, 2018
@danbev danbev deleted the build_aarch_distcpu branch September 3, 2018 07:07
danbev added a commit that referenced this pull request Sep 3, 2018
On a aarch64 system I can run the complete build with tests without
specifying the Makefile variable DESTCPU.

But when running the tar-headers target DESTCPU is passed to configure:
$(PYTHON) ./configure \
       --prefix=/ \
       --dest-cpu=$(DESTCPU) \
       ...
The value of DESTCPU in this case will be aarch64 which will cause
configure to fail:
configure: error: option --dest-cpu: invalid choice: 'aarch64'
(choose from 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc',
'ppc64', 'x32', 'x64', 'x86', 'x86_64', 's390', 's390x')

In the configure script there is a matching of __aarch64__ to arm64:
$ python -c 'from configure import host_arch_cc; print host_arch_cc()'
arm64

In our case it would be nice to have consitent behaviour for both of
these cases on aarch64.

This commit changes DESTCPU to arm64 to be consistent with the
configure script. DESTCPU is used in $(TARBALL)-headers and in
$(BINARYTAR) but I'm not sure about the implications of making the
change purposed and hope others might chime in and provide some
guidance.

PR-URL: #22548
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
On a aarch64 system I can run the complete build with tests without
specifying the Makefile variable DESTCPU.

But when running the tar-headers target DESTCPU is passed to configure:
$(PYTHON) ./configure \
       --prefix=/ \
       --dest-cpu=$(DESTCPU) \
       ...
The value of DESTCPU in this case will be aarch64 which will cause
configure to fail:
configure: error: option --dest-cpu: invalid choice: 'aarch64'
(choose from 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc',
'ppc64', 'x32', 'x64', 'x86', 'x86_64', 's390', 's390x')

In the configure script there is a matching of __aarch64__ to arm64:
$ python -c 'from configure import host_arch_cc; print host_arch_cc()'
arm64

In our case it would be nice to have consitent behaviour for both of
these cases on aarch64.

This commit changes DESTCPU to arm64 to be consistent with the
configure script. DESTCPU is used in $(TARBALL)-headers and in
$(BINARYTAR) but I'm not sure about the implications of making the
change purposed and hope others might chime in and provide some
guidance.

PR-URL: #22548
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2018
On a aarch64 system I can run the complete build with tests without
specifying the Makefile variable DESTCPU.

But when running the tar-headers target DESTCPU is passed to configure:
$(PYTHON) ./configure \
       --prefix=/ \
       --dest-cpu=$(DESTCPU) \
       ...
The value of DESTCPU in this case will be aarch64 which will cause
configure to fail:
configure: error: option --dest-cpu: invalid choice: 'aarch64'
(choose from 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc',
'ppc64', 'x32', 'x64', 'x86', 'x86_64', 's390', 's390x')

In the configure script there is a matching of __aarch64__ to arm64:
$ python -c 'from configure import host_arch_cc; print host_arch_cc()'
arm64

In our case it would be nice to have consitent behaviour for both of
these cases on aarch64.

This commit changes DESTCPU to arm64 to be consistent with the
configure script. DESTCPU is used in $(TARBALL)-headers and in
$(BINARYTAR) but I'm not sure about the implications of making the
change purposed and hope others might chime in and provide some
guidance.

PR-URL: #22548
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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

8 participants