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

[compiler-rt] Don't redefine LLVM_COMMON_CMAKE_UTILS if it's defined #66761

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

xry111
Copy link
Contributor

@xry111 xry111 commented Sep 19, 2023

If we are building compiler-rt inside llvm-x.y.z.src directory, we should not redefine LLVM_COMMON_CMAKE_UTILS.

If we are building compiler-rt inside llvm-x.y.z.src directory, we
should not redefine LLVM_COMMON_CMAKE_UTILS.
@xry111 xry111 closed this Sep 24, 2023
@xry111 xry111 reopened this Sep 24, 2023
@xry111
Copy link
Contributor Author

xry111 commented Sep 24, 2023

@MaskRay Manually request for a review. It seems the bot does not @ a group for this change automatically.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM, we do this everywhere else.

(I've encountered this as well, and ended up symlinking the directory to the right place. Thanks for actually fixing it...)

@MaskRay
Copy link
Member

MaskRay commented Sep 24, 2023

If we are building compiler-rt inside llvm-x.y.z.src directory, we should not redefine LLVM_COMMON_CMAKE_UTILS.

Can you add more instructions?

I can see that some subprojects checks DEFINED while libcxx/ doesn't check whether LLVM_COMMON_CMAKE_UTILS is defined

@xry111
Copy link
Contributor Author

xry111 commented Sep 25, 2023

If we are building compiler-rt inside llvm-x.y.z.src directory, we should not redefine LLVM_COMMON_CMAKE_UTILS.

Can you add more instructions?

I can see that some subprojects checks DEFINED while libcxx/ doesn't check whether LLVM_COMMON_CMAKE_UTILS is defined

Extracting compiler-rt into llvm/projects/ and build:

tar xf llvm-17.0.1.src.tar.xz
tar xf cmake-17.0.1.src.tar.xz
mv cmake-17.0.1.src cmake
tar xf third-party-17.0.1.src.tar.xz
mv third-party-17.0.1.src third-party
tar xf compiler-rt-17.0.1.src.tar.xz
mv compiler-rt-17.0.1.src llvm-17.0.1.src/projects/compiler-rt
mkdir llvm-17.0.1.src/build
cd llvm-17.0.1.src/build
cmake ..

@xry111
Copy link
Contributor Author

xry111 commented Oct 4, 2023

Ping. Do I need more explanation or some change?

@nikic nikic merged commit 0cf0bc7 into llvm:main Oct 4, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants