OpenShadingLanguage icon indicating copy to clipboard operation
OpenShadingLanguage copied to clipboard

Use cmake find_package to find LLVM rather than a custom setup

Open debaetsd opened this issue 4 years ago • 3 comments

Description

This PR changes how CMake searches for LLVM. Rather then having a custom FindLLVM setup, it is possible to rely on importable targets setup by the LLVM project.

This was proposed/discussed in LLVM importable CMake targets on the mailing list (initial post on Oct 16 2020).

Please note that I renamed the clang-format target into osl-clang-format to prevent name clashes with the "real" clang-format.

Tests

Existing CI still passes. Windows build verified locally. Updated CI to look for osl-clang-format

Checklist:

  • [x] I have read the contribution guidelines.
  • [x] I have previously submitted a Contributor License Agreement.
  • [ ] I have updated the documentation, if applicable.
  • [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • [x] My code follows the prevailing code style of this project.

debaetsd avatar Oct 23 '20 20:10 debaetsd

FYI, I'm fiddling around with this to get it totally working with the GPU mode and on my Mac. There are some minor edits I'll need to push to amend it.

lgritz avatar Oct 28 '20 20:10 lgritz

@lgritz I hacked up a test to use target_link_libaries directly: https://github.com/debaetsd/OpenShadingLanguage/commit/003168887f523f578c5d928233f17ae22ff0d202 All in all pretty clean imho. This will work for any target (installed in the linked LLVM that is). Note though that this has issues with llvm7, either we need to special case it or we drop support for it.

debaetsd avatar Oct 29 '20 09:10 debaetsd

I think we let this languish for a while because it didn't work for llvm 7, which we were still supporting at the time. Since then, we have pulled (in main anyway) the minimum llvm to 9, so we should see if we can revive this.

lgritz avatar Jan 05 '22 01:01 lgritz