Skip to content

openSUSE package query and install enhancements #729

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.

Sign up for GitHub

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

Merged
nuclearsandwich merged 2 commits into from
Apr 2, 2020
Merged

openSUSE package query and install enhancements #729

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2305bf9
Enable PIP installer for openSUSE
gleichdick Nov 12, 2019
ec74a9c
openSUSE package detection with RPM capabilities
gleichdick Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/rosdep2/platforms/opensuse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

from rospkg.os_detect import OS_OPENSUSE

from .pip import PIP_INSTALLER
from .source import SOURCE_INSTALLER
from ..installers import PackageManagerInstaller

Expand All @@ -44,14 +45,15 @@ def register_installers(context):

def register_platforms(context):
context.add_os_installer_key(OS_OPENSUSE, SOURCE_INSTALLER)
context.add_os_installer_key(OS_OPENSUSE, PIP_INSTALLER)
context.add_os_installer_key(OS_OPENSUSE, ZYPPER_INSTALLER)
context.set_default_os_installer_key(OS_OPENSUSE, lambda self: ZYPPER_INSTALLER)


def rpm_detect(packages):
installed = []
for p in packages:
if not subprocess.call(['rpm', '-q', p]):
if not subprocess.call(['rpm', '-q', '--whatprovides', p]):
Copy link
Copy Markdown
Contributor

@nuclearsandwich nuclearsandwich Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the rpm facilities for the redhat platform are a bit more sophisticated. Is it worth centralizing and sharing some of this logic or is there too much divergence between them? Also asking @cottsay as my local RedHat expert and probable contributor for the redhat RPM handling.

Copy link
Copy Markdown
Member

@cottsay cottsay Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a similar approach to the one taken in the RedHat installer:

rosdep/src/rosdep2/platforms/redhat.py

Lines 110 to 118 in cfa21c8

cmd = ['rpm', '-q', '--whatprovides', '--qf', '[%{PROVIDES}\n]']
cmd.extend(packages)
std_out = exec_fn(cmd)
out_lines = std_out.split('\n')
for index, package in enumerate(packages):
if package in out_lines:
ret_list.append(raw_packages[index])
return ret_list

Invoking the rpm executable repetitively on a large workspace was pretty slow, so a single command is invoked for all of the packages and the output is parsed to ensure availability since the return value isn't useful anymore.

This change, however, shouldn't make the openSUSE installer any less performant than it currently is, and uses an already proven approach.

Copy link
Copy Markdown
Member

@cottsay cottsay Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth centralizing and sharing some of this logic...?

Maybe. The RedHat installer tries to use the RPM python module if it is available, so I think we'd need to do some testing to ensure it works right, but I expect that the RPM solution in redhat.py would work here as-is.

I don't think that work should block this PR though.

installed.append(p)
return installed

Expand Down