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
Detect System package name by recipe settings (#5026) #5215
Detect System package name by recipe settings (#5026) #5215
Conversation
- Each package manages has a specific prefix/suffix to deal with architectures. - Some package managers (e.g brew) do not provide multi-arch support, only an universal option. Signed-off-by: Uilian Ries <uilianries@gmail.com>
- Validate yum and apt when installing x86 Signed-off-by: Uilian Ries <uilianries@gmail.com>
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.
Thanks for your contribution!
Although I have to say that this is the typical thing that never hammers all the nails at once 🔨 🔨
hi @uilianries |
Probably we need an |
Yes, you are right. I totally forget the exotic repositories.
The initial idea was including an option called platforms = {"x86_64": "all", "x86": "all"}
installer = SystemPackageTool(tool=AptTool(), settings=self.settings, platforms=platforms)
installer.install("foobar") OUTPUT: WDYT? |
conans/client/tools/system_pm.py
Outdated
@@ -114,6 +116,17 @@ def install(self, packages, update=True, force=False): | |||
self.update() | |||
self._install_any(packages) | |||
|
|||
def _parse_packages_arch(self, packages): | |||
if self._settings and cross_building(self._settings): | |||
_, _, _, host_arch = get_cross_building_settings(self._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.
I do not fully understand that packages will be installed for the host_arch always, as by definition, the apt-get will be running in the build-machine, not the host. Will this break everything for users with build_requires
packages that use SystemPackageTool
?
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.
Thinking again, you are right. I thought in the case where someone wants to cross-building some package and need a library from system. But sometimes users need a tool rather a library, which means the arch build.
We could detect if arch is not declared and use build_arch
instead. WDYT?
- User is able to specify prefix/suffix according the arch Signed-off-by: Uilian Ries <uilianries@gmail.com>
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 is more confusing that it should, because the idea is very simple. I suggested some changes.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
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.
Just change settings
to conanfile
in the constructor (and docs), probably it is a better pattern for every tool. It is a problem we already have in the MSBuild
class.
Good catch @jgsogo . I agree. Please, @uilianries change it so we can merge this. |
@jgsogo yeah, it sounds better, we are using such approach for CMake helper. |
Signed-off-by: Uilian Ries <uilianries@gmail.com>
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 promise to you this is the last change I request 🤞
Co-Authored-By: Javier G. Sogo <jgsogo@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
🎉 |
Changelog: Feature: SystemPackageTool platform detection (#5026)
Docs: conan-io/docs#1291
closes #5026
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.