build: fix llvm version detection in freebsd-10#11668
build: fix llvm version detection in freebsd-10#11668shigeki wants to merge 3 commits intonodejs:masterfrom
Conversation
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it.
configure
Outdated
There was a problem hiding this comment.
You could simply remove the anchor, I think?
There was a problem hiding this comment.
I thought it could be fixed but have no full confirmation if it has no side effects. This is very conservative fix not to break existing checks.
There was a problem hiding this comment.
I suppose a little conservatism won't hurt. Can you line up the strings?
There was a problem hiding this comment.
I'm a little coward ;-).
get_xcode_version in 4 lines below had also 4 space indents. I fixed both. Thanks.
configure
Outdated
There was a problem hiding this comment.
I suppose a little conservatism won't hurt. Can you line up the strings?
|
CI was done in https://ci.nodejs.org/job/node-test-pull-request/6682/ and all is green. |
configure
Outdated
There was a problem hiding this comment.
An alternative would be a non-capturing group, e.g.
cc, r"(^(?:FreeBSD )?clang version|based on LLVM) " +There was a problem hiding this comment.
I did not think of a non-capturing parentheses. It does not break existing check and looks better. Thanks.
@bnoordhuis I fixed in ed1d266. PTAL.
There was a problem hiding this comment.
Confirmed it works fine in both FreeBSD-10 of clang-3.4 with OS prefix in its banner, https://ci.nodejs.org/job/node-test-commit-freebsd/7474/nodes=freebsd10-64/consoleFull and FreeBSD-11 of clang-3.6 without no OS prefix ,https://ci.nodejs.org/job/node-test-commit-freebsd/7474/nodes=freebsd11-x64/consoleFull .
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it. PR-URL: #11668 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it. PR-URL: #11668 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it. PR-URL: #11668 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it. PR-URL: #11668 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In FreeBSD-10, the banner of clang version is "FreeBSD clang version". Fix regex to detect it. PR-URL: nodejs/node#11668 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In FreeBSD-10, the banner of clang version start with "FreeBSD clang version" but the current
configurechecks/^clang version/so thatllvm_versionis 0 and openssl is built with asm_obsolete.This can be seen in the CI output of https://ci.nodejs.org/job/node-test-commit-freebsd/7417/nodes=freebsd10-64/consoleFull that
'llvm_version': 0,in the current configure output.With this fix, the clang version banner can be checked properly in FreeBSD-10. The result of https://ci.nodejs.org/job/node-test-commit-freebsd/7419/nodes=freebsd10-64/consoleFull shows that
'llvm_version': '3.4',.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
build
CC: @nodejs/platform-freebsd or @nodejs/build