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 build with Clang 12 and install the native lib #244

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

zeule
Copy link
Contributor

@zeule zeule commented Jun 25, 2021

Closes #241.

@ghost
Copy link

ghost commented Jun 25, 2021

CLA assistant check
All CLA requirements met.

clangAST
clangFrontend
libclang
)
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide more details about how you built such that you didn't have clangAST or clangFrontend?

These exist in both the official release binaries (https://github.com/llvm/llvm-project/releases/tag/llvmorg-12.0.0, see `lib/libclangAST.a and friends) and in the binaries produced if built locally.

Copy link
Member

Choose a reason for hiding this comment

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

This CMake was also used to succesfully build libClangSharp for 12.0.0 on all supported platforms, so things were resolving correctly.

Copy link
Contributor Author

@zeule zeule Jun 25, 2021

Choose a reason for hiding this comment

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

Can you provide more details about how you built such that you didn't have clangAST or clangFrontend?

sys-devel/clang version 12.0.0 from the Gentoo main tree. Neither it nor the clang package from Arch provide those libraries. You can view the file list for the Arch package here.

The clang package here installs no static (.a) archives at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This looks like its going to break windows as there is no clang-cpp produced.

I'm fine with taking it for Unix given it looks like the official binaries do create and ship clang-cpp by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated adding library check.

@zeule zeule force-pushed the feature/install-native-lib branch 2 times, most recently from 56e1701 to 7901a36 Compare June 25, 2021 18:04
@zeule
Copy link
Contributor Author

zeule commented Jun 25, 2021

Added target_link_directories(ClangSharp PRIVATE ${LLVM_LIBRARY_DIRS}) to be on the safe side.

@zeule zeule force-pushed the feature/install-native-lib branch from 7901a36 to 2764e96 Compare June 25, 2021 18:07
@zeule
Copy link
Contributor Author

zeule commented Jun 25, 2021

Added target_link_directories(ClangSharp PRIVATE ${LLVM_LIBRARY_DIRS}) to be on the safe side.

No, that's unneeded, the import library path is set correctly by Clang's Targets.cmake.

SOVERSION ${PROJECT_VERSION}
)

include(GNUInstallDirs)
Copy link
Member

Choose a reason for hiding this comment

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

What's the behavior of this on Windows and is it going to do the "right" thing where Windows considers *.dll a bin file but *.lib a library file (in contrast to Unix which considers both *.so and *.a to be library files)?

Copy link
Contributor Author

@zeule zeule Jun 28, 2021

Choose a reason for hiding this comment

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

On Windows _BINDIR is "bin", _LIBDIR is "lib", .dll files are RUNTIME, .lib files are ARCHIVE. CMake documents that here.

@tannergooding tannergooding merged commit a4e94b5 into dotnet:main Jun 29, 2021
@zeule zeule deleted the feature/install-native-lib branch June 29, 2021 09:18
@zeule zeule mentioned this pull request Apr 15, 2022
@Perksey Perksey mentioned this pull request Jul 7, 2022
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.

Does not build with Clang 12
2 participants