★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/30099Nouvelle recherche | Portail wanayoo
Skip to content

/ node

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

Don't use -latomic on macOS #30099

Closed
wants to merge 1 commit into from
Closed

Don't use -latomic on macOS #30099

wants to merge 1 commit into from

Conversation

@ryandesign
Copy link
Contributor

ryandesign commented Oct 23, 2019

Fixes build when using open-source clang. (Builds using Apple clang from Xcode
7 and later were already working, due to the fact that node's build system
misidentifies their llvm_version as "0.0" and thus the flag wasn't being added.)

Closes #30093

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
@ryandesign
Don't use -latomic on macOS
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
dcfe2b2 
Fixes build when using open-source clang. (Builds using Apple clang from Xcode
7 and later were already working, due to the fact that node's build system
misidentifies their llvm_version as "0.0" and thus the flag wasn't being added.)

Closes #30093
@richardlau
richardlau approved these changes Oct 23, 2019
View changes
Copy link
Member

richardlau left a comment

Looks okay to me but I don't have a mac so can't test this.

cc @nodejs/platform-macos @nodejs/build-files
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Oct 24, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/26177/
@marsam marsam mentioned this pull request Nov 19, 2019
Closed
1 of 4 tasks complete
@BridgeAR

This comment has been minimized.

Sign in to view
Copy link
Member

BridgeAR commented Dec 25, 2019

Ping @nodejs/platform-macos @nodejs/build-files
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
Copy link

nodejs-github-bot commented Dec 25, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27946/
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019 •
edited by BridgeAR

CI: https://ci.nodejs.org/job/node-test-pull-request/27955/ ✔️
@BridgeAR BridgeAR requested a review from devsnek Jan 1, 2020
@devsnek devsnek removed their request for review Jan 1, 2020
@devsnek

This comment has been minimized.

Sign in to view
Copy link
Member

devsnek commented Jan 1, 2020

I don't have a mac anymore, so I can't test or verify this.
@ljharb

This comment has been minimized.

Sign in to view
Copy link
Member

ljharb commented Jan 1, 2020 •
edited

./configure && make passed on my Mac with an unset CXX, and CXX=g++. I'll leave it to someone else to test homebrew's clang++.
@BridgeAR
BridgeAR approved these changes Jan 2, 2020
View changes
Copy link
Member

BridgeAR left a comment

RSLGTM
BridgeAR added a commit that referenced this pull request Jan 2, 2020
@ryandesign @BridgeAR
build: don't use -latomic on macOS
Unverified
The committer email address is not verified.
Learn about signing commits
Loading status checks…
4bd0d8c 
Fixes build when using open-source clang.

Builds using Apple clang from Xcode v7 and later were already working,
due to the fact that node's build system misidentifies their
llvm_version as "0.0" and thus the flag wasn't being added.

PR-URL: #30099
Fixes: #30093
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR

This comment has been minimized.

Sign in to view
Copy link
Member

BridgeAR commented Jan 2, 2020

Landed in 4bd0d8c 🎉
@BridgeAR BridgeAR closed this Jan 2, 2020
@BridgeAR

This comment has been minimized.

Sign in to view
Copy link
Member

BridgeAR commented Jan 2, 2020

@ryandesign congratulations on your first commit to Node.js! 🎉
@ryandesign ryandesign deleted the ryandesign:patch-1 branch Jan 3, 2020
BridgeAR added a commit that referenced this pull request Jan 3, 2020
@ryandesign @BridgeAR
build: don't use -latomic on macOS
Unverified
The committer email address is not verified.
Learn about signing commits
9b7cf09 
Fixes build when using open-source clang.

Builds using Apple clang from Xcode v7 and later were already working,
due to the fact that node's build system misidentifies their
llvm_version as "0.0" and thus the flag wasn't being added.

PR-URL: #30099
Fixes: #30093
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
Merged
targos added a commit that referenced this pull request Jan 14, 2020
@ryandesign @targos
build: don't use -latomic on macOS
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
8ab748e 
Fixes build when using open-source clang.

Builds using Apple clang from Xcode v7 and later were already working,
due to the fact that node's build system misidentifies their
llvm_version as "0.0" and thus the flag wasn't being added.

PR-URL: #30099
Fixes: #30093
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targos targos mentioned this pull request Jan 15, 2020
Closed
@gireeshpunathil gireeshpunathil mentioned this pull request Jan 20, 2020
Open
BethGriggs added a commit that referenced this pull request Feb 6, 2020
@ryandesign @BethGriggs
build: don't use -latomic on macOS
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
cb89fbc 
Fixes build when using open-source clang.

Builds using Apple clang from Xcode v7 and later were already working,
due to the fact that node's build system misidentifies their
llvm_version as "0.0" and thus the flag wasn't being added.

PR-URL: #30099
Fixes: #30093
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Merged
saitonakamura pushed a commit to saitonakamura/node that referenced this pull request Apr 7, 2020
@ryandesign Michael Bashurov
build: don't use -latomic on macOS
420ae94 
Fixes build when using open-source clang.

Builds using Apple clang from Xcode v7 and later were already working,
due to the fact that node's build system misidentifies their
llvm_version as "0.0" and thus the flag wasn't being added.

PR-URL: nodejs#30099
Fixes: nodejs#30093
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau

@BridgeAR BridgeAR

Assignees
No one assigned
Labels
author ready build
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Wrong condition for deciding when to add -latomic
6 participants
@ryandesign @nodejs-github-bot @BridgeAR @devsnek @ljharb @richardlau
You can’t perform that action at this time.