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

[feature] Accept list for SystemPackageTool #6961

Closed
1 task done
uilianries opened this issue May 6, 2020 · 2 comments · Fixed by #7779
Closed
1 task done

[feature] Accept list for SystemPackageTool #6961

uilianries opened this issue May 6, 2020 · 2 comments · Fixed by #7779

Comments

@uilianries
Copy link
Member

uilianries commented May 6, 2020

The SystemPackageTool class offers the method install which only supports a string with all packages to be installed. However, even a list of packages must be passed as a string, it's a bit confuse, and get worst if you need to check if it's installed or not before: #6960 (comment)

It would be great accepting both string (backward) and list:

installer = SystemPackageTool()
installer.install(["pkg1", "pkg2", "pkg3"])
@jgsogo
Copy link
Contributor

jgsogo commented May 7, 2020

The documentation states it can use a list (in a single string) and the implementation tries to work with a list too:

    def install(self, packages, update=True, force=False, arch_names=None):
        """ Get the system package tool install command.

        :param packages: String with all package to be installed e.g. "libusb-dev libfoobar-dev"
        
        """
        packages = [packages] if isinstance(packages, str) else list(packages)

Probably this is not a feature, but a bug as it is not working as expected (and it has already the ability to work with a list/string-list). Here we need to fix a couple of things and probably fixing them will cover this feature.

I'm closing this one in favor of #6960

@jgsogo jgsogo closed this as completed May 7, 2020
@uilianries
Copy link
Member Author

The documentation says :

Note: This list of packages is intended for providing alternative names for the same package, to account for small variations of the name for the same package in different distros. To install different packages, one call to install() per package is necessary.

This feature request is about installing different packages at same time: e.g.

def system_requirements(self):
    installer = tools.SystemPackageTool()
    installer.install(["curl", "vim", "cowsay"])

Using this code, with the current implementation, Conan install will Curl, and end the execution.

However, if we pass to accept a list of packages, we would break the current behavior, so we need an alternative, maybe a new parameter:

def system_requirements(self):
    installer = tools.SystemPackageTool(all=True)
    installer.install(["curl", "vim", "cowsay"])

The parameter all represents, install all packages listed, they are not alternatives.

uilianries added a commit to uilianries/conan that referenced this issue Sep 30, 2020
Support different packages to be installed by a single list

Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 22, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 23, 2020
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 23, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 23, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 27, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Oct 30, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
memsharded pushed a commit that referenced this issue Nov 29, 2020
* #6961 Install all system packages

Support different packages to be installed by a single list

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* restart ci

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Separate command for installing all packages

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Rename unit test name for install system packages

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

* Rename install packages method name

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #6961 Accept variants and packages at same input

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #6961 Fix single system package install

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

* #6961 Install only what is needed

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #6961 Update install_packages docstring

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Sort package list

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #6961 Do not accept more than 1 pkg per str

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Update conans/client/tools/system_pm.py

* #6961 Improve install steps

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Remove env var

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
@memsharded memsharded added this to the 1.32 milestone Nov 29, 2020
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.

3 participants