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

[bug] System Manager updates always when a package is installed already #11715

Closed
uilianries opened this issue Jul 27, 2022 · 5 comments · Fixed by #11716
Closed

[bug] System Manager updates always when a package is installed already #11715

uilianries opened this issue Jul 27, 2022 · 5 comments · Fixed by #11716
Assignees
Milestone

Comments

@uilianries
Copy link
Member

uilianries commented Jul 27, 2022

It's listed as a bug, because the old SystemPackageTool has a different behavior, which, had an advantage:

When a system package is installed on your machine, and you call SystemPackageTool.install(..., update=True), Conan checks first if that package is already installed, then will run update + install after that, only when is not installed. The only exception is when the parameter force=True is configured (False by default).

More information:

if not force and self._installed(packages):

The new designed of System Manager, runs update first, then it checks if it's installed or not:

https://github.com/conan-io/conan/blob/develop/conan/tools/system/package_manager.py#L106

Thus, does not make sense forcing a new update, when all package are already installed. It would be better checking first, then compare if some package is missing, then install them.

Related to conan-io/conan-center-index#11936

/cc @emzeat

Environment Details (include every applicable attribute)

  • Operating System+version: Linux, BTW I use Arch
  • Compiler+version: GCC 11
  • Conan version: 1.50.0
  • Python version: 3.10.4

Steps to reproduce (Include if Applicable)

Running on conanio/gcc10 docker image

sudo apt-get -qq update
sudo apt-get -qq install -y --no-install-recommends --no-install-suggests vim

Now, create a dummy conanfile.py with the follow section:

from conan import ConanFile
from conan.tools.system.package_manager import Apt

class Bugged(ConanFile):

    def system_requirements(self):
        apt = Apt(self)
        apt.install(["vim"], update=True, check=True)

And don't forget to update the configuration mode:

tools.system.package_manager:mode=install

Logs (Executed commands with output) (Include/Attach if Applicable)

@memsharded
Copy link
Member

Possibly closed by #11712

@uilianries
Copy link
Member Author

Possibly closed by #11712

Just check and it's not same case. The PR #11712 avoids the update command in case of the mode is check. However, for our propose, we need install mode, which will run the update command, but still need to be done only when a package is not installed.

@czoido
Copy link
Contributor

czoido commented Jul 27, 2022

@uilianries

  • I'm not sure that update=True should be set in the system package recipes in conan center index... why is that necessary?
  • Also, and beyond that, I think that setting that update in the recipes can be problematic in the future and probably we should move that to a conf, so you decide what you want to do and don't depend on the changes on recipes.

@uilianries
Copy link
Member Author

@czoido CCI uses the Conan docker images, which have no cached APT list, which means, we need to run update anyway. Of course, the same recipe is used by users, which means, they need to update too.

About the config, I like the idea, it could be tools.system.package_manager:update=True, then Apt.install(..., update=None). Here, None is the default value, because if the user pass True/False, it must be respected over the configuration.

@emzeat
Copy link

emzeat commented Jul 27, 2022

Thanks @uilianries for reporting this issue on my behalf.

As a user I would definitely prefer to have control over when conan is pulling updates (or even touching external servers to update the index) and when not. Exposing this via tools.system.package_manager:update=True sounds like a good idea. Alternatively I could also see the index update for the CCI build scripts to happen outside of conan so that when conan runs it is fully populated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants