Conan: Redesign SystemPackageTool interface

[memsharded]

From the addition of Chocolatey to SystemPackageTool (https://github.com/conan-io/conan/pull/1657), which could break existing recipes, I think there is something a bit weird in SystemPackageTool

The thing is that it can be used as:

pack_name = "some_pkg"
installer = SystemPackageTool()
installer.install(pack_name)

That will try to use a different system package manager in every platform, like apt in debian, yum, pkgutil.... And will fall back to the NullTool if not existing. For example in Windows, and the above code will be a no-op in Windows.

So adding Chocolatey as another tool for Windows had potential to break things.

Typically the complexity of the decission to install a package is given on the previous package name selection:

pack_name = None
    if os_info.linux_distro == "ubuntu":
        if os_info.os_version > "12":
            pack_name = "package_name_in_ubuntu_10"
        else:
            pack_name = "package_name_in_ubuntu_12"
    elif 

Maybe it makes more sense something like:

def system_requirements(self):
      apt.install(<some pkgs>)
      yum.install(<some pkgs>)
      choco.install(<some pkgs>)

So extensibility is under user control. The above will be no-ops if the system it is running doesn't support that package managers.

Wdyt? @uilianries @sixten-hilborn @lasote

[uilianries]

@memsharded Thanks for think about redesign SystemPackageTool. I agree, we have a big number of variations by platform, package manager, os version ... The explicit name by helper should solve a future case involving an new optional package manager. Also, only some them are native (e.g. apt), but we don't have complete sure about if is installed, is a out of edge case, but is possible to use a tidy distro, without the package manager installed.

What will happen with the current interface? I mean, if you chance SystemPackageTool to don't solve the package manager, you could break many recipes. My question is, how to preserve the current behavior and extend to use a explicit mechanism?

For now we have:

choco = SystemPackageTool(runner=self.runner, os_info=OSInfo(). tool=ChocolateyTool())
choco.install(<package>)

OSInfo and runner are not really required, but is just for the example. Do you think about to create a new simple interface? Maybe like this:

choco = ChocoPackageTool(runner=self.runner)
choco.install(<package>)

Again, runner could be optional, if it is null, then a new ConanRunner is created.

Regards.

[lasote]

I agree with @memsharded and about keeping the old interface compatibility @uilianries . And of course, changing the docs to only show the new way

[sixten-hilborn]

I think the problem is that SystemPackageTool doesn't fail if no package manager is found (e.g. it falls back to the NullTool). Removing NullTool and simply fail if no package manager is found would allow extending SystemPackageTool for new package managers/distros etc. in the future, without requiring Conan package developers to know about every possible future/current package manager for every system.

As a transition a new flag can be added and deprecate setting it to True:

installer = SystemPackageTool(allowMissingTool=True/False)

[memsharded]

No, I am not saying about removing NullTool, nor failing. Also, maybe it is very optimistic that the code will be so general that new system package managers can be plugged and existing recipes would work. In fact, from what I see, the SystemPackageTool calling is preceded by something like:

 if os_info.linux_distro == "ubuntu":
        if os_info.os_version > "12":
            pack_name = "package_name_in_ubuntu_10"
        else:
            pack_name = "package_name_in_ubuntu_12"
    elif 

So maybe there is no way this would work when new system package managers are added in the conan internal code. I feel that the abstraction would be good only if it also abstracted the mappings from packages to the specific names in concrete package managers, but for that a very complete database would be necessary, and it is out of scope.

[sixten-hilborn]

Yes it's hard/not feasible to map a dependency to all possible names in all package managers, so it's better to implement the dependency as a Conan package (if possible). :)
Using installer.install(["package_name_in_ubuntu_12", "package_name_in_ubuntu_10"]) may increase the chances that it will work with a new manager in the future though.

I don't like that .install can fail without an error when the recipe obviously requires that dependency. Failing during build because of the missing dependency can take longer to troubleshoot or (worst case) the package is silently built without it.

[SSE4]

that's reasonable addition, because anyway packages have different naming across distribution, and you already shall have code like that:

if os_info.linux_distro == "debian":
    installer.install("libfreetype-dev")
elif os_info.linux_distro == "centos":
    installer.install("freetype-devel")

and it will be much more explicit to use then:

if os_info.linux_distro == "debian":
    apt.install("libfreetype-dev")
elif os_info.linux_distro == "centos":
    yum.install("freetype-devel")

[weatherhead99]

I've been using the SystemPackageManager tool in anger recently, and I think that at least in the specific case of debian based vs redhat based package naming (or e.g. fedora or suse which have specific shared library package naming conventions) it would be possible and useful to automate some of this, e.g. have a "devel_package_suffix" field in OSInfo or similar as a start.

My feeling is that I would rather see more generic rather than specific solutions. Cross platform packages are already high friction, and this is the kind of thing where a sensible default can cover a very large portion of cases I think.

[memsharded]

During this re-design, https://github.com/conan-io/conan/issues/506 could be considered.

[yangcha]

From the addition of Chocolatey to SystemPackageTool (#1657), which could break existing recipes, I think there is something a bit weird in SystemPackageTool

The thing is that it can be used as:

pack_name = "some_pkg"
installer = SystemPackageTool()
installer.install(pack_name)

That will try to use a different system package manager in every platform, like apt in debian, yum, pkgutil.... And will fall back to the NullTool if not existing. For example in Windows, and the above code will be a no-op in Windows.

So adding Chocolatey as another tool for Windows had potential to break things.

Typically the complexity of the decission to install a package is given on the previous package name selection:

pack_name = None
    if os_info.linux_distro == "ubuntu":
        if os_info.os_version > "12":
            pack_name = "package_name_in_ubuntu_10"
        else:
            pack_name = "package_name_in_ubuntu_12"
    elif 

Maybe it makes more sense something like:

def system_requirements(self):
      apt.install(<some pkgs>)
      yum.install(<some pkgs>)
      choco.install(<some pkgs>)

So extensibility is under user control. The above will be no-ops if the system it is running doesn't support that package managers.

Wdyt? @uilianries @sixten-hilborn @lasote

@memsharded, I think we could use a dict to map the OS(with version) to the system packages to install, such as:

install({'ubuntu_x64:12', [<some pkgs>], 'centos_x64:6', [<some pkgs>]})

One thing I am not comfortable about conan is that there is no unified or it is hard to get the information of OS. It would be nice if there is a conan command "conan system inspect", which show the system information, and first item shows the system OS and version in a string such as 'ubuntu_x64:12'. This string will be used as the key of the dict.

[brianthelion]

As a conan user and outside observer to the design process, I can't help but feel that the very idea of the SystemPackageTool is wrongheaded. The specter of a giant, hand-coded if ... else tree involving distros, package names, versions, and other terrible things is the sign of a broken model. Plus, it opens the door to the user wanting -- and then "needing" -- to marshal general system configuration from within the conanfile. This is bad for conan.

The alternative is to offload this burden to an existing tool that's designed to handle precisely this kind of terribleness. I, personally, would love to see conan just call out to salt or ansible and be done with it. The artifacts necessary to make that work (which also happen to be called "recipes" in the case of salt) can be easily stored and versioned alongside their conan counterparts. For simple system requirements, though, a multi-line yaml string in the conanfile will probably get the job done.

Will the naive conan user be annoyed that they have to learn some opaque DSL to add simple system requirements? Sure, but they'll thank you later.

[lasote]

Approved a behavior change for 2.0:

It should by default, if it needs to install a system requirement: fail with a message "missing package XXX, run 'apt install....'".
Only optin by the conan.conf, enable the automatic installation

[tvogel]

def system_requirements(self):
      apt.install(<some pkgs>)
      yum.install(<some pkgs>)
      choco.install(<some pkgs>)

So extensibility is under user control. The above will be no-ops if the system it is running doesn't support that package managers.

Be aware that there is the possibility that multiple package managers are available on a given system. Yes, there might be a primary one but there could be both zypper and apt on an openSUSE system (https://software.opensuse.org/package/apt).
So, to me, explicit dependency on the distribution makes sense.