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

Fix MinGW toolchain install with 'install-llvm-toolchain' #543

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stephanosio
Copy link

When MinGW LLVM Embedded Toolchain for Arm is installed using the install-llvm-toolchain target, the MinGW-built toolchain executables are not installed under the prefix because llvm-toolchain-mingw component is added to the LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL variable, which is never referenced.

Fix this by enumerating LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL as intended.

When MinGW LLVM Embedded Toolchain for Arm is installed using the
`install-llvm-toolchain` target, the MinGW-built toolchain executables
are not installed under the prefix because `llvm-toolchain-mingw`
component is added to the `LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL`
variable, which is never referenced.

Fix this by enumerating `LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL` as
intended.

Signed-off-by: Stephanos Ioannidis <[email protected]>
@@ -2195,7 +2195,7 @@ set(LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL ${LLVM_TOOLCHAIN_DISTRIBUTION_COM
if(LLVM_TOOLCHAIN_CROSS_BUILD_MINGW)
list(APPEND LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL llvm-toolchain-mingw)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for spotting this.

Looking again at it, I can't see any reason for LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS_ALL to exist it all, and it would make more sense for llvm-toolchain-mingw to be included in LLVM_TOOLCHAIN_DISTRIBUTION_COMPONENTS instead of creating a duplicate list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants