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

[llvm-core] bring package up to date. Support for Conan 2. #22997

Merged
merged 83 commits into from
Sep 24, 2024

Conversation

planetmarshall
Copy link
Contributor

@planetmarshall planetmarshall commented Mar 5, 2024

Specify library name and version: llvm-core/*

Attempt to bring the llvm-core recipe up to date. Includes contributions from @jusito and #17509. Fixes #20339

Note that I have opted not to add the latest version (18.1.3) as part of this PR, as the existing recipe needed updating and is already quite complex. I (or someone else) will add the latest version as a followup.


Copy link
Contributor

github-actions bot commented Mar 5, 2024

🤖 Beep Boop! This pull request is making changes to 'recipes/llvm-core//'.

👋 @Hopobcn @paulharris you might be interested. 😉

@planetmarshall planetmarshall marked this pull request as draft March 5, 2024 21:24
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@planetmarshall planetmarshall force-pushed the llvm-core-conan2 branch 3 times, most recently from b8f48e8 to 471320c Compare March 5, 2024 22:41
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@planetmarshall planetmarshall changed the title [llvm-core] bring package up to date. Initial support for 13.0.0 [llvm-core] bring package up to date. Support for Conan 2. Mar 7, 2024
@planetmarshall planetmarshall marked this pull request as ready for review March 7, 2024 22:48
@planetmarshall
Copy link
Contributor Author

@jcar87 Most recent CI build failed but i'm unable to view any logs.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.


components = defaultdict(list)
for lib, dep in deps:
if not lib.startswith('LLVM'):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be kept. Without something like this, I’m seeing shared libraries (libRemarks.so and libLTO.so) in the produced package even when shared=False.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might also want to delete *.so and *.so.* from the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libRemarks and libLTO are part of the LLVM infrastructure and not available as static libraries.

libLTO specifically is used by some out of tree linkers but not by LLVM itself - See https://discourse.llvm.org/t/how-to-use-dynamic-library-liblto-so/74541 for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand these are not available statically. However I don’t think it’s correct to include shared libs in a static package. For example, if my intention of using a static package is to produce an executable that doesn’t depend on any shared libs for ease of deployment, now I’m forced to link to these 2 shared libs when I link to llvm-core::llvm::core, and the executable will not run in the target environment as intended.

I think we should delete them, and the users of the static package should simply be aware that these 2 components are not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

llvm-core::llvm-core is a conan-generated target and would not ordinarily be available to consumers of LLVM, and I would expect users of the package to use the targets in the official docs, as the test package does.

One issue with deleting the libraries for static builds is that a shared library build is not supported (by LLVM) for Windows, so a static build is the only way to build those libraries. Deleting the libraries for all platforms apart from Windows seems to me inconsistent behaviour.

It's not a perfect solution - but currently the package works exactly as described in the official LLVM documentation. I'm sure further contributions to the recipe to improve the linking behaviour will be welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

llvm-core::llvm-core is a conan-generated target and would not ordinarily be available to consumers of LLVM

By “not available” do you just mean “not recommended”? Because I certainly have code using it, and it works fine (except for linking to the 2 shared libs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean it is a target provided by conan, not by LLVM. So if you are linking to llvm-core::llvm-core, your application will work with the conan package but it would not work against a package built directly from the LLVM sources just using cmake.

The test_package in the repository should still work whether LLVM is provided by conan or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do think that LTO and Remarks should probably be made separate components for the shared build, but IMHO this is something that can be added as a future improvement and shouldn't block this PR.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 43 (bd3f92f10901ca9fe5487ffaca59e3ed312454a9):

  • llvm-core/11.1.0:
    All packages built successfully! (All logs)

  • llvm-core/13.0.0:
    All packages built successfully! (All logs)

  • llvm-core/12.0.0:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 44 (bd3f92f10901ca9fe5487ffaca59e3ed312454a9):

  • llvm-core/13.0.0:
    All packages built successfully! (All logs)

  • llvm-core/12.0.0:
    All packages built successfully! (All logs)

  • llvm-core/11.1.0:
    All packages built successfully! (All logs)

Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

Thanks @planetmarshall and everyone involved - this was not an easy recipe to port, and hopefully this can unblock some of the usages that are required by the community.

There's still a few things to look into further down the line - the issue with libiconv in macOS, and the parsing of CMake files (it is obvious from the recipe code that this can be fragile) - however, I dont think these are an impediment to merge the recipe as proposed in this PR.

Thanks so much!

@conan-center-bot conan-center-bot merged commit d3985d3 into conan-io:master Sep 24, 2024
31 checks passed
@planetmarshall planetmarshall deleted the llvm-core-conan2 branch September 24, 2024 16:30
OMGtechy pushed a commit to OMGtechy/conan-center-index that referenced this pull request Dec 31, 2024
…nan 2.

* [llvm-core] bring package up to date. Initial support for 13.0.0

* [llvm-core] add support for job pool resource limits from @jusito

* [llvm-core] add cmake patch for 12.0.0

* [llvm-core] add cmake patch for 11.1.0

* [llvm-core] allow conan 1.x

* [llvm-core] fix zlib detection for 11.1.0

* [llvm-core] additional patches for missing includes and patch metadata

* [llvm-core] resolve v1 hooks issues

* [llvm-core] use llvm generated config file to extract components

* [llvm-core] add custom cmake build script

* [llvm-core] use run env during build and force ninja

* [llvm-core] fix patches for libxml

* [llvm-core] bump up default ram per link job

* [llvm-core] backport split-dwarf patches from 14.x

* [llvm-core] add support for cross compiling

* [llvm-core] fix patch files for split dwarf

* [llvm-core] disable cross compilation

* [llvm-core] add CCI build service specific behaviour

* [llvm-core] fix cci resource limitations

* [llvm-core] implement component dependencies

* [llvm-core] sanitize component names

* [llvm-core] bump ram per link job for cci shared debug

* [llvm-core] exclude cmake find modules

* [llvm-core] skip rpath on macos

* [llvm-core] reformat and fix linting issues

* [llvm-core] delete 18x patch (added in error)

* [llvm-core] rename references to LLVM-Config

* [llvm-core] retain LLVMConfigExtensions.cmake

* [llvm-core] add fake targets for *-gen

* [llvm-core] use the actual cmake target name as component name

* [llvm-core] fix component dependencies

* [llvm-core] add target options

* [llvm-core] add editline requirement (WIP)

* [llvm-core] support for libedit (editline) in 13.x

* [llvm-core] disable shared debug for CCI, update license identifier and refactor

* [llvm-core] require cxx std 14 for test package

* [llvm-core] handle edge case of static libiconv being linked into a dynamic library on macos

* [llvm-core] skip rpath handling for build on macos

* [llvm-core] propagate DYLD_LIBRARY_PATH to tblgen during build

* [llvm-core] propagate DYLD_LIBRARY_PATH to tblgen during build

* [llvm-core] do not build shared libs with the install rpath

* [llvm-core] use original test package

* [llvm-core] do not package static libraries with shared build

* [llvm-core] fix test_package

* [llvm-core] support linking in lib for native codegen

* [llvm-core] add windows support

* [llvm-core] remove pdb files

* [llvm-core] require cmake_path support in test package

* [llvm-core] handle legacy conan msvcrt setting

* [llvm-core] missing patch type

* Apply suggestions from code review

Co-authored-by: Michael Keck <[email protected]>

* [llvm-core] revert changes to license

* [llvm-core] apply review suggestions

* [llvm-core] add patch source for include file fix

* [llvm-core] use user-config for compiler resource settings

* [llvm-core] add link for further info on cross compilation

* [llvm-core] use PurePosixPath only

* Trying 'as_posix()' function

* More 'as_posix()' function

* Typo

* [llvm-core] revert usage of 'load' to workaround regex error

* [llvm-core] try relaxing restrictions on cross-building

* [llvm-core] use semi-colon separated string for targets instead of individual options

* [llvm-core] set default fPIC fallback to 'True'

* [llvm-core] use version range for ninja

* [llvm-core] move cci check to 'validate_build'

* [llvm-core] allow CCI to use llvm-core user config for resource limits

* [llvm-core] dont activate build environment for llvm-tblgen

* [llvm-core] add patch for link failure with lld

* [llvm-core] explicitly list shared libraries

* [llvm-core] revert to using

* Revert "[llvm-core] try relaxing restrictions on cross-building"

This reverts commit 7b2e085

* [llvm-core] workaround for conan-io#13560

* [llvm-core] increase ram available for link jobs in CCI

* [llvm-core] remove the split-dwarf patch

* [llvm-core] disable shared debug builds entirely and debug builds for linux

* [llvm-core] remove remaining split-dwarf capability

---------

Co-authored-by: Michael Keck <[email protected]>
Co-authored-by: Francisco Ramirez de Anton <[email protected]>
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.

llvm-core/13.0.0: recipe is broken (conan 2)