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

Do not add architecture to system packages by default, only when cross-compiling #12349

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

czoido
Copy link
Contributor

@czoido czoido commented Oct 20, 2022

Changelog: Feature: Do not add architecture to system packages by default, only when cross-compiling.
Docs: omit

The most common situation when installing system packages is that you don't use the architecture in the package name unless you are cross-compiling, so this defaults to that behaviour.
Also it's testing how to install archless packages even with cross-compilation, but that's something we probably want to change as the code would look a bit ugly. You can workaround that with an empty dictionary as arch_names in classes that accept the argument:

Apt(self).install(["libudev-dev"], update=True, check=True)
Apt(self, arch_names={}).install(["archless-package"], update=True, check=True)

Closes: #12320

@czoido czoido added this to the 1.54 milestone Oct 20, 2022
@ericLemanissier
Copy link
Contributor

I don't think this closes #12320, because the situation stays the same during cross compilation: for a given recipe either:

  • all system packages have an arch suffix (if the recipe has an arch setting) or
  • no system package has an arch suffix (if the recipe does not have an arch setting)

To fix the issue, we would need to have a method which installs the system packages it receives as argument, with noarch/all/any architecture suffix, depending on the package manager.
Just to be clear, there is no need for this feature currently in CCI. The offending case in CCI (xorg/system) was solved by removing all the arch-less system packages from xorg/system, and putting them in a dedicated recipe without arch setting

@czoido
Copy link
Contributor Author

czoido commented Oct 21, 2022

I don't think this closes #12320, because the situation stays the same during cross compilation: for a given recipe either:

  • all system packages have an arch suffix (if the recipe has an arch setting) or
  • no system package has an arch suffix (if the recipe does not have an arch setting)

To fix the issue, we would need to have a method which installs the system packages it receives as argument, with noarch/all/any architecture suffix, depending on the package manager. Just to be clear, there is no need for this feature currently in CCI. The offending case in CCI (xorg/system) was solved by removing all the arch-less system packages from xorg/system, and putting them in a dedicated recipe without arch setting

Hi @ericLemanissier, thanks for the comments. The intention of this PR is to:

  • Change the default when invoking system package managers and don't add the architecture to the package names for the most common case that is not cross-compiling. As you say this would not close the issue.
  • Provide a way of forcing installing packages without the architecture appended to the package name even if you are cross-compiling. This is not yet covered in the PR with a solution, I commented above that this can be workarounded with something like this:
Apt(self, arch_names={}).install(["archless-package"], update=True, check=True)

Passing arch_names as an empty dictionary in the constructor will make that the name resolution skips adding the architecture for all cases (cross-compiling and not). But that is ugly and probably we want to pass a new argument to cover this case. That workaround I think would close #12320 as then you also could mix archless packages with packages with arch in the same recipe without having to write a new recipe for the archless ones which I think would be more convenient. Am I missing something and maybe this would not be enough to cover the case?

Thanks a lot.

@memsharded memsharded marked this pull request as ready for review November 2, 2022 11:29
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.

Looks like a good default with the possibility of adding fine-grained control in the future 👍

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.

[feature] install system packages without architecture
4 participants