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.

Sign up for GitHub

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

Jump to bottom

Fix configure script to work with Apple Clang 11 #28071

Merged
merged 1 commit into from
Jun 12, 2019
Merged

Fix configure script to work with Apple Clang 11 #28071

merged 1 commit into from
Jun 12, 2019

Conversation

saagarjha
Copy link
Contributor

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

Xcode 11's clang doesn't match our regex:

$ clang --version
Apple clang version 11.0.0 (clang-1100.0.20.17)
Target: x86_64-apple-darwin19.0.0
Thread model: posix
InstalledDir: /Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I've updated the regex in the configure script so that it will try to match the newer one if the old one fails.

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jun 5, 2019
bnoordhuis
bnoordhuis approved these changes Jun 5, 2019
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion.

configure.py Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/23796/

@lpinca
Copy link
Member

lpinca commented Jun 11, 2019

@saagarjha can you please fix the commit title?

build: fix configure script to work with Apple Clang 11

@saagarjha
Copy link
Contributor Author

Fixed, thanks.

refack
refack approved these changes Jun 12, 2019
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

RSLGTM

@saagarjha @refack
build: fix configure script to work with Apple Clang 11
1f143b8 
PR-URL: nodejs#28071
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
@refack refack added the python PRs and issues that require attention from people who are familiar with Python. label Jun 12, 2019
@refack refack merged commit 1f143b8 into nodejs:master Jun 12, 2019
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
@saagarjha @BridgeAR
build: fix configure script to work with Apple Clang 11
d320791 
PR-URL: #28071
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
Merged
@AshCripps
Copy link
Member

I suggest this gets backported to v10.x as without this change im unable to build v10 on catalina with current xcode versions.

@BethGriggs @nodejs/lts

@sam-github
Copy link
Contributor

sam-github commented Nov 12, 2019 •
edited
Loading

@nodejs/lts Doesn't affect our CI, but catalina users won't be able to build v10.x without it.

Picks clean onto v10.x-staging.

BethGriggs pushed a commit that referenced this pull request Nov 19, 2019
@saagarjha @BethGriggs
build: fix configure script to work with Apple Clang 11
61d6ac7 
PR-URL: #28071
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
@saagarjha @MylesBorins
build: fix configure script to work with Apple Clang 11
a80c591 
PR-URL: #28071
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Jan 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@lpinca lpinca lpinca approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. python PRs and issues that require attention from people who are familiar with Python.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@saagarjha @nodejs-github-bot @lpinca @AshCripps @sam-github @refack @bnoordhuis @richardlau @BethGriggs