-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[gcc] Model internal libraries #15128
Conversation
I detected other pull requests that are modifying gcc/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit d8a6e91gcc/11.3.0
|
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 0f5e082gcc/12.2.0
gcc/11.3.0
gcc/10.2.0
|
@@ -1,12 +1,14 @@ | |||
from conan import ConanFile | |||
from conan.tools.gnu import Autotools, AutotoolsToolchain | |||
# from conan.tools.gnu.get_gnu_triplet import _get_gnu_triplet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jcar87 here's a really good triplets
# | libgcc_eh.a # libgcc exception handling. User by -static-libgcc | ||
# | libgcov.a # test coverage library | ||
|
||
self.cpp_info.set_property("cmake_target_name", "gcc::gcc_all") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the targets are the dame ad the default and I am pretty sure they aren't from upstream so you can save some complexity and remove them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate what you mean? I've modelled the libraries as components mostly to represent the relationships between them so that they can be included in the link command when necessary and linked in the right order - my understanding is that collect_libs
wouldn't satisfy this.
This specific line I felt necessary to put in because the default catchall target gcc::gcc
was overridden to refer to libgcc.a
|
||
self.cpp_info.components["gcov"].set_property("cmake_target_name", "gcc::gcov") | ||
self.cpp_info.components["gcov"].libdirs = [os.path.join("lib","gcc", triplet, self.version)] | ||
self.cpp_info.components["gcov"].libs = ["gcov"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be super cool to call this one in the the twst package!
I can see a bunch of people doing coverage with it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll see if I can put something together
from conan.errors import ConanInvalidConfiguration | ||
from conan.tools.layout import basic_layout | ||
from conan.tools.apple import XCRun | ||
from conan.tools.files import copy, get, replace_in_file, rmdir, rm | ||
from conan.tools.build import cross_building | ||
from conan.tools.env import VirtualBuildEnv | ||
from conan.tools.microsoft import is_msvc | ||
# from conan.tools.build.cross_building import get_cross_building_settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will - I've left this here for the first review by @jcar87 as it relates to conan-io/conan#12789. Perhaps there's a way of packaging this library that means the gnu triplet isn't required, as it's presently hardcoded: https://github.com/conan-io/conan-center-index/pull/15128/files#diff-91c51d260a5e804807092c0510143447d89f4fc2d42ac14a2203c0d71cd6abb8R166
* Add legacy env_info environment variable definitions for conan v1 compatibility
* Define cpp_info.libdirs and cpp_info.libs to enable linkage against gnu libraries * Populate tools.build:compiler_executables with exported compilers using conf_info
* Add model for libraries that ship with gcc to enable individual libraries to be linked against and included in the right order in command line arguments
* Remove automatic library discovery * Remove conf propagation of tools.build:compiler_executables as this conflicts with profiles when used
* Removed the conf_info definition for compiler executables as this breaks builds when the profile doesn't match the gcc version. This change has been deferred to a separate PR.
* Add description of all libraries to package_info * Add static libraries to package model. This is an initial representation limited by a lack of knowledge about the interdependent relationships between these libraries.
* Add comments describing future triplet functionality and the imports that will be required
Rebasing on the changes made for conan 2.0 compatibility changed the path to liblto_plugin.so from <package_folder>/bin/libexec/gcc/<triplet>/<version> to <package_folder/libexec/gcc/<triplet>/<version>
0f5e082
to
39f5c23
Compare
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 39f5c23gcc/10.2.0
|
@uilianries @prince-chrismc I'm not sure it's going to be possible to split gcc out into static and shared libs - is it possible to add an exception to KB-H076 for gcc? |
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
@prince-chrismc can you open this again? looks like the |
Conan v1 pipeline ❌Failure in build 1 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. Conan v2 pipeline ❌
The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future. See details:Failure in build 1 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
Hooks produced the following warnings for commit 39f5c23gcc/11.3.0
|
Also relates to conan-io/conan#13533. Some thought needs to be put into how compiler packages are consumed, given that they impose both build time and run time requirements, which aren't always available on the host system. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
keep alive post |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@samuel-emrys bump? |
this is still waiting on review by @jcar87 as he indicated some objection to these changes |
@jcar87 ? @uilianries @SSE4 ? |
what are the potential issues? are they blocking us from merging the PR? can we just add FIXME and proceed for now? |
@samuel-emrys I've read, and he says as a summary:
so, are there specific issues we can reproduce? |
Yes, I raised this PR to demonstrate the benefit from a runtime gfortran requirement perspective: #15556 From a reproducibility perspective, my opinion is that you would want to ensure you're using the same libgfortran version on any system a resulting lapack is built with, rather than relying on whatever is installed on the system. I'm not sure this is a perspective that @jcar87 shares, and perhaps this could be circumvented to some degree with a static libgfortran compilation. Other utility might be found in exposing gcov. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
This PR provides an initial model of gcc's internal libraries to allow the recipe to be used as a runtime requirement rather than only as a tool requirement. This is necessary for packages such as lapack, which provides fortran libraries liblapack and libblas, which have a runtime dependency on libgfortran.
I've made an initial attempt to model the relationships between these libraries so that they are included and in the right order in any linker commands. This implementation is likely only reasonably accurate for the shared libraries since
ldd
is available to show the relationships the shared libraries have. The static libraries are somewhat harder to model and has been excluded as out of scope for this pull request, though targets are still provided.Closes #15099