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

clang10 is unsupported #29536

Closed
krytarowski opened this issue Sep 12, 2019 · 4 comments
Closed

clang10 is unsupported #29536

krytarowski opened this issue Sep 12, 2019 · 4 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@krytarowski
Copy link
Contributor

  • Version: Snapshot as of Thu Sep 12 17:55:54 CEST 2019
  • Platform: NetBSD
  • Subsystem: ?

clang10 (svn snapshot) is unsupported

ERROR: Did not find a new enough assembler, install one or build with
       --openssl-no-asm.
       Please refer to BUILDING.md
*** Error code 1

Culrpit code in configure.py:

def get_llvm_version(cc):
  return get_version_helper(
cc, r"(^(?:FreeBSD )?clang version|based on LLVM) ([3-9]\.[0-9]+)")

This patch fixes the problem for me:

-    cc, r"(^(?:FreeBSD )?clang version|based on LLVM) ([3-9]\.[0-9]+)")
+    cc, r"(^(?:FreeBSD )?clang version|based on LLVM) ((\d{2}|[3-9])\.[0-9]+)")

Maybe it would be reasonable to accept all LLVM versions now.

@bnoordhuis bnoordhuis added the build Issues and PRs related to build files or the CI. label Sep 12, 2019
@bnoordhuis
Copy link
Member

PR welcome. Your suggested change seems reasonable but I'd mirror the regex from get_xcode_version():

node/configure.py

Lines 729 to 731 in b354d12

def get_xcode_version(cc):
return get_version_helper(
cc, r"(^Apple (?:clang|LLVM) version) ([0-9]+\.[0-9]+)")

krytarowski added a commit to krytarowski/node that referenced this issue Sep 13, 2019
Detected on NetBSD/amd64.

Fixes: nodejs#29536
@Trott Trott closed this as completed in 233cdb6 Sep 15, 2019
targos pushed a commit that referenced this issue Sep 20, 2019
Detected on NetBSD/amd64.

Fixes: #29536

PR-URL: #29541
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR pushed a commit that referenced this issue Sep 25, 2019
Detected on NetBSD/amd64.

Fixes: #29536

PR-URL: #29541
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@sam-github sam-github reopened this Apr 13, 2020
@sam-github
Copy link
Contributor

sam-github commented Apr 13, 2020

This issue is still present on v10.x, the fix needs backporting: EDIT: "cherrypicking"

Your branch is up to date with 'upstream/v10.x-staging'.
core/lts (v10.x-staging $% u=) % ../Config

export CC=clang-11 CXX=clang++-11 LD=clang++-11

exec ./configure --ninja --verbose
ERROR: Did not find a new enough assembler, install one or build with
       --openssl-no-asm.
       Please refer to BUILDING.md
core/lts (v10.x-staging $% u=) % git l1 | head
ef2df6986b (HEAD -> v10.x-staging, upstream/v10.x-staging, upstream/v10.x) Working on v10.20.2
f08441edac (tag: v10.20.1) 2020-04-12 Node.js v10.20.1 'Dubnium' (LTS) Release
670f3dbecd Working on v10.20.1
246eedebec (tag: v10.20.0) 2020-04-08, Version 10.20.0 'Dubnium' (LTS)
017909b847 test: fix tool path in test-doctool-versions.js
a175b8d3a7 tools: only fetch previous versions when necessary
3756be8511 tools: add NODE_TEST_NO_INTERNET to the doc builder
ac1ea7312a tools: make doctool work if no internet available
1ea70d641d test: fix flaky doctool and test
0177464b0e doc,tools: get altDocs versions from CHANGELOG.md

richardlau pushed a commit to richardlau/node-1 that referenced this issue Apr 17, 2020
Detected on NetBSD/amd64.

Fixes: nodejs#29536

PR-URL: nodejs#29541
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this issue Jun 2, 2020
Detected on NetBSD/amd64.

Fixes: #29536

PR-URL: #29541
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@nathanblair
Copy link
Contributor

nathanblair commented Jun 11, 2020

It appears that a patch has made its way onto the master branch but did not make it to the 14.4.0 release. Does anyone know if its going to ported to that release line?

Edit: Jk apparently it is there. I am still having issues getting alpine with clang to build though. Should not the assembler be set to llvm-as for clang/llvm?

Here's been my experience on alpine using its clang's compiled package:

clang -v reports:

Alpine clang version 10.0.0 (git://git.alpinelinux.org/aports ae0b46db0cf7920d8fc9232c22abb6684aaea539)
Target: x86_64-alpine-linux-musl
Thread model: posix
InstalledDir: /usr/bin

But according to the get_version_helper method, this is not matching with the regexp (^(?:FreeBSD )?clang version|based on LLVM) ([0-9]+\.[0-9]+)

As such, get_llvm_version always returns '0.0', and the assembler check fails.

I'm unsure what the output of other clang-compiled packages report on other systems for clang -v. I'll check a void-linux install as well.

EDIT: Here's what void-linux reports for clang -v:

clang version 10.0.0 
Target: x86_64-unknown-linux-musl
Thread model: posix
InstalledDir: /bin

It seems like maybe the regexp is getting messed up in alpine perhaps because of its looking for clang at the beginning of the version output? My regexp is a little rusty but is that what the '^' operator is for?

In any case, this seems like it probably is boiling down to package maintainers at this point. If someone could confirm that I'm right and which version output is the desired one I would be happy to report this up to the alpine linux folks.

@nathanblair
Copy link
Contributor

https://gitlab.alpinelinux.org/alpine/aports/-/issues/11640

Here's a thread/issue I started RE: the compilation on an alpine system.

I think perhaps there is an even more optimal approach to take with the regexp used in the get_llvm_version call. Given the compile-time CLANG_VENDOR variable that appears to set the prefix in the returned version string for the entity during compilation, perhaps its better to replace the ^(?:FreeBSD)? sub-expression with something less hard-coded for BSD? Could it perhaps be replaced with something like ^(?:.+)?? I can try testing it out on some of the clang --version strings I have examples of.

I can also add that Apple adds a clang vendor to their version, it seems; though I don't know how nodejs is compiled to run on OSX:

Apple clang version 11.0.3 (clang-1103.0.32.62)
Target: x86_64-apple-darwin19.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

codebytere pushed a commit that referenced this issue Jun 27, 2020
Fixes: #29536

PR-URL: #33860
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
codebytere pushed a commit that referenced this issue Jun 30, 2020
Fixes: #29536

PR-URL: #33860
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
codebytere pushed a commit that referenced this issue Jul 10, 2020
Fixes: #29536

PR-URL: #33860
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
codebytere pushed a commit that referenced this issue Jul 12, 2020
Fixes: #29536

PR-URL: #33860
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
codebytere pushed a commit that referenced this issue Jul 14, 2020
Fixes: #29536

PR-URL: #33860
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
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 a pull request may close this issue.

4 participants