FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix configure script to work with Apple Clang 11 by saagarjha · Pull Request #28071 · nodejs/node · GitHub

/ node Public

Fix configure script to work with Apple Clang 11 - #28071

Merged
refack merged 1 commit into
nodejs:masterfrom
saagarjha:master
Jun 12, 2019
ConversationCommits (1)ChecksFiles changed
Merged

Fix configure script to work with Apple Clang 11#28071
refack merged 1 commit into
nodejs:masterfrom
saagarjha:master

Conversation

saagarjha commented Jun 5, 2019

Copy link
Copy Markdown
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 added the build Issues and PRs related to build files or the CI. label Jun 5, 2019
bnoordhuis approved these changes Jun 5, 2019
View reviewed changes

bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with a suggestion.

Comment thread configure.py Outdated

nodejs-github-bot commented Jun 11, 2019

Copy link
Copy Markdown
Collaborator

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

lpinca commented Jun 11, 2019

Copy link
Copy Markdown
Member

@saagarjha can you please fix the commit title?

build: fix configure script to work with Apple Clang 11

saagarjha commented Jun 11, 2019

Copy link
Copy Markdown
Contributor Author

Fixed, thanks.

refack approved these changes Jun 12, 2019
View reviewed changes

refack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

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 added the python PRs and issues that require attention from people who are familiar with Python. label Jun 12, 2019
refack merged commit 1f143b8 into nodejs:master Jun 12, 2019
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
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 mentioned this pull request Jun 17, 2019
Merged

AshCripps commented Nov 12, 2019

Copy link
Copy Markdown
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 commented Nov 12, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@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
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 mentioned this pull request Dec 4, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
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 mentioned this pull request Jan 7, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
lpinca lpinca approved these changes
richardlau richardlau approved these changes
+1 more reviewer
refack refack approved these changes
Reviewers whose approvals may not affect merge requirements

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

Back | FazBrowse Home | New Git URL