Skip to content

openSUSE package query and install enhancements#729

Merged
nuclearsandwich merged 2 commits intoros-infrastructure:masterfrom
gleichdick:master
Apr 2, 2020
Merged

openSUSE package query and install enhancements#729
nuclearsandwich merged 2 commits intoros-infrastructure:masterfrom
gleichdick:master

Conversation

@gleichdick
Copy link
Contributor

Enable pip installer, use rpm capabilities for package lookup to prevent package renaming issues.

For example, in python.yml the python-numpy dependency is resolved to the opensuse rpm package python-numpy which is absent. But the package python2-numpy provides it as a capability as shown below:

$ rpm -q --provides python2-numpy
python-numpy = 1.16.5
python2-numpy = 1.16.5-2.1
python2-numpy(x86-64) = 1.16.5-2.1

So instead of matching the exact package name at dependency resolving the capabilities should be queried (--whatprovides flag).

$ rpm -q --whatprovides python2-numpy
python2-numpy-1.16.5-2.1.x86_64
$ rpm -q --whatprovides python-numpy
python2-numpy-1.16.5-2.1.x86_64

Of course, if none of the installed package provides the given capability the return code is still non-zero:

$ rpm -q --whatprovides anyunknowncap; echo $?
no package provides anyunknowncap
1

Packages sometimes get renamed and their old name is kept as an rpm capability (like an alias), so the additional flag `--whatprovides` is passed to the `rpm` query.
installed = []
for p in packages:
if not subprocess.call(['rpm', '-q', p]):
if not subprocess.call(['rpm', '-q', '--whatprovides', p]):
Copy link
Contributor

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
Member

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:

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
Member

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.

@nuclearsandwich nuclearsandwich self-assigned this Apr 2, 2020
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

I haven't actually tried the change myself, by it makes sense.

@nuclearsandwich nuclearsandwich merged commit 03d8d82 into ros-infrastructure:master Apr 2, 2020
gstavrinos pushed a commit to gstavrinos/rosdep that referenced this pull request Nov 12, 2020
* Add Suite3 option with Ubuntu Focal (ros-infrastructure#734)

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* [update] Allow to process single ROS distro, fix 723 (ros-infrastructure#738)

* [update] skip other distro if --rosdistro passed

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* ignore argument if specified distro doesnt exist

* address review comments

* update help message for rosdistro

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* Fix CI and reduce CI time (ros-infrastructure#739)

* test newer python
* use yaml.safe_load
* pin PyYAML version for Python 3.4
* pass user flag to pip when needed
* move slow jobs to the top to reduce CI time

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* Strip Alpine's patch version from OS codename (ros-infrastructure#716)

* Resolve Alpine os_version_type using OsDetect
* Reduced Alpine OS VERSION to Major.Minor

* improve support for null entries (ros-infrastructure#726)

* support null for entire OS and not only OS version

syntax like 'ubuntu: null' will now be raising ResolutionError with
relevant error message instead of InvalidData with obscure error message
will also cover syntax like:
"
ubuntu:
  '*': null
  bionic: [foobar]
"

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

Co-authored-by: Scott K Logan <logans@cottsay.net>

* Use DNF installer on RHEL 8 and newer (ros-infrastructure#713)

RHEL/CentOS 8 uses DNF by default.

* Updates to YUM and DNF (ros-infrastructure#640)

Output YUM, DNF and RPM versions with `--all-versions` and fix the format guide's default PM for Fedora.

* tests: don't assume euid != 0 (ros-infrastructure#703)

If tests rely on the environment being a certain way, they should ensure
it meets its expectations. Update the tests to ensure they're testing
multiple euid conditions, and setting it explicitly where required.

Fix ros-infrastructure#702

Signed-off-by: Kyle Fazzari <kyle@canonical.com>

* openSUSE package query  and install enhancements (ros-infrastructure#729)

* Enable PIP installer for openSUSE

* openSUSE package detection with RPM capabilities

Packages sometimes get renamed and their old name is kept as an rpm capability (like an alias), so the additional flag `--whatprovides` is passed to the `rpm` query.

* Fix conditional dependencies when one package uses manifest.xml (ros-infrastructure#737)

* Fix conditional dependencies when one package uses manifest.xml

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Return rosdeps as a list to avoid breaking the interface.

Co-authored-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* Guard next(inter) (ros-infrastructure#701)

* guard next(inter)

  Fix ros-infrastructure#691

Signed-off-by: artivis <jeremie.deray@canonical.com>

* Handle StopIteration with slightly less line noise.

Co-authored-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* [Windows] Add console script entry point (ros-infrastructure#656)

* Add console script entry point

Add console script entry point for platforms (e.g. Windows) not supporting shebang.

* remove scripts entry since we are using console_scripts.

* fix bad merge.

* fix SKIP_PYTHON_SCRIPTS case.

Co-authored-by: Lou Amadio <ooeygui@users.noreply.github.com>

* Depend on modules packages only to allow co-installability. (ros-infrastructure#750)

When the rosdep modules package was split in [ros-infrastructure#731] the module
dependencies weren't updated to only depend on modules packages
themselves which prevents rosdep modules for python2 and python3 from
actually being co-installable.

I haven't yet audited to make sure there's no cli usage of these tools
in the modules but it's Friday afternoon and I wanted to get this inked
before I walked away.

When I return to it I'll check for that before proceeding for reviews.

* 0.19.0

* also install buildtool_export_depends (ros-infrastructure#753)

not adding 'exec_depends' or 'build_export_depends' are both are included inside 'run_depends'

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* Drop shebang from a non-executable file (ros-infrastructure#755)

* add alias for Pop! OS (ros-infrastructure#757)

* add alias for Pop! OS

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

Co-authored-by: Shane Loretz <sloretz@openrobotics.org>
Co-authored-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Co-authored-by: Mark Hedley Jones <MarkHedleyJones@gmail.com>
Co-authored-by: Scott K Logan <logans@cottsay.net>
Co-authored-by: Kyle Fazzari <kyle@canonical.com>
Co-authored-by: Bjar Ne <43565432+gleichdick@users.noreply.github.com>
Co-authored-by: Jeremie Deray <deray.jeremie@gmail.com>
Co-authored-by: Sean Yen <seanyen@microsoft.com>
Co-authored-by: Lou Amadio <ooeygui@users.noreply.github.com>
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.

3 participants