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

APT improvements: dependencies #17161

Closed
3 of 10 tasks
svenevs opened this issue May 11, 2022 · 14 comments
Closed
3 of 10 tasks

APT improvements: dependencies #17161

svenevs opened this issue May 11, 2022 · 14 comments
Assignees
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters

Comments

@svenevs
Copy link
Contributor

svenevs commented May 11, 2022

Sub-component of #16448. Relates: #17058 (review) and https://docs.google.com/document/d/1ZFq8EWLAVKHoUpJaf-Qx1AWzXllmVYCB_W1JOi-TnCQ

The .tar.gz archives should have their setup installation scripts allow for:

  • Use apt-get satisfy now that bionic is gone?
    • shlibs:Depends has libblas3 | libblas.so.3, we have libblas3.
      • The “|” form is allowing the user to choose a different implementation.
    • shlibs:Depends installs libglu1-mesa | libglu1, we install libglu1-mesa.
    • shlibs:Depends installs liblapack3 | liblapack.so.3, we install liblapack3.
  • Package audit:
    • We install libldl2, shlibs:Depends does not. (Remove unused libyaml and libdl2 dependencies #18734)

      • This may be a leftover (or statically linked now?), we should check.
    • shlibs:Depends installs libstdc++6 (>= 9), we do not.

      • It’s probable that we already get this implicitly, but we should double-check.
      • We get it implicitly.
    • shlibs:Depends installs libtinyxml2-6a (>= 6.0.0), we install libtinyxml2-dev and libtinyxml2.6.2v5.

      • The libtinyxml2.6.2v5 is probably wrong.
    • We install libyaml-cpp-dev, shlibs:Depends does not. (Remove unused libyaml and libdl2 dependencies #18734)

      • This is a hold-over from some code that got removed from Drake.
    • shlibs:Depends installs libpython3.8 (>= 3.8.2), we do not. ([workspace] Install libpython for consumers #17294)

      # /opt/drake/bin/drake-visualizer                                                                                                                       
      /opt/drake/bin/drake-visualizer: error while loading shared libraries: libpython3.8.so.1.0: cannot open shared object file: No such file or directory
      
@svenevs svenevs added the component: build system Bazel, CMake, dependencies, memory checkers, linters label May 11, 2022
@svenevs svenevs mentioned this issue May 11, 2022
2 tasks
@svenevs svenevs self-assigned this May 11, 2022
@svenevs
Copy link
Contributor Author

svenevs commented May 12, 2022

Additional apt analysis for installed packages can be found here: https://gist.github.com/svenevs/6c9e247d7dc71f3f595182181cf099b7 (includes other information about files installed to /opt/drake that can be ignored for this as part of #17058).

svenevs added a commit to svenevs/drake that referenced this issue May 31, 2022
Relates: RobotLocomotion#17161.

If consumers do a tar or deb install with --no-install-recommends
they will not be able to run the python code without libpython.
rpoyner-tri pushed a commit that referenced this issue May 31, 2022
* [workspace] Install libpython for consumers

Relates: #17161.

If consumers do a tar or deb install with --no-install-recommends
they will not be able to run the python code without libpython.
aykut-tri pushed a commit to aykut-tri/drake that referenced this issue Jun 1, 2022
* [workspace] Install libpython for consumers

Relates: RobotLocomotion#17161.

If consumers do a tar or deb install with --no-install-recommends
they will not be able to run the python code without libpython.
hongkai-dai pushed a commit to hongkai-dai/drake that referenced this issue Jun 6, 2022
* [workspace] Install libpython for consumers

Relates: RobotLocomotion#17161.

If consumers do a tar or deb install with --no-install-recommends
they will not be able to run the python code without libpython.
@jwnimmer-tri
Copy link
Collaborator

I skimmed through the list above; all of it seems on point.

We install libldl2, shlibs:Depends does not.
This may be a leftover (or statically linked now?), we should check.

I think it was probably an accident when we added #12072, and has never been used. From suitesparse we only use libamd2, which doesn't need libldl2 at all.

@mwoehlke-kitware
Copy link
Contributor

we install ... libtinyxml2.6.2v5

We do? Where? (Did this get addressed but not checked off?)

@jwnimmer-tri
Copy link
Collaborator

See #17830 re: tinyxml2.

@mwoehlke-kitware
Copy link
Contributor

"Drop tinyxml2 as of the deprecation 2023-01-01"

Okay, it's 2023-02-02 🙂 . So the action item for tinyxml is to remove libtinyxml2-dev also?

@jwnimmer-tri
Copy link
Collaborator

Yes. In general, any shared libraries that we don't dynamically load anymore in the current release shouldn't be part of Depends: in the control file. There might be more newly-unused ones beyond tinyxml2.

@mwoehlke-kitware
Copy link
Contributor

Ugh. libtinyxml2-dev vs. libtinyxml-dev. I don't see the former. So, it looks like that item can be checked?

@jwnimmer-tri
Copy link
Collaborator

The general idea is to match the Depends list (which is specified by setup/ubuntu/binary_distribution/packages-{focal|jammy}.txt) with the dynamic loader dependencies, possibly assisted by dpkg_shlibsdeps recommendations about things that are extra/missing. You'll need to dig in, see what's what, and figure out any necessary changes or conclude that none are necessary.

@mwoehlke-kitware
Copy link
Contributor

Since I can't find any evidence that "we install libtinyxml2-dev and libtinyxml2.6.2v5", and all indications are that we used to do so and no longer do so, I'm marking that complete.

@svenevs
Copy link
Contributor Author

svenevs commented Feb 7, 2023

In order to evaluate the differences between what our packages-*.txt are defining and what shlibs:Depends is doing requires modifying the repackaging script and repackaging a .tar.gz archive. This line is ultimate what defines the dependencies:

depends = packages_txt.rstrip().replace('\n', ',\n ')

To compare, you have to overwrite it with

--- a/tools/release_engineering/repack_deb.py
+++ b/tools/release_engineering/repack_deb.py
@@ -72,6 +72,7 @@ def _run(args):
     # NOTE: rstrip is required here so that the last line does *NOT* have a
     # comma (otherwise debian/rules will crash).
     depends = packages_txt.rstrip().replace('\n', ',\n         ')
+    depends = "${shlibs:Depends}"
     with open(deb_control_in, encoding='utf-8') as f:
         deb_control_contents = f.read().format(depends=depends)

What this does is in the resultant .deb file, our "control" file gets updated to use what shlibs:Depends thinks we should have instead of our current list. Examining this is a little unpleasant (focal must be repackaged on focal, jammy on jammy, so if your host is focal run a jammy docker and run drake's setup scripts with --with-maintainer-only). Once you can bazel run //tools/release_engineering:repack_deb -- --tgz /path/to/drake-thing.tar.gz --output-dir /abs/path/to/output you will get a .deb file. You then need to extract the dependencies:

# extract archives
$ ar x drake-dev_0.0.20230207185335-1_amd64.deb

# extract control file, focal: control.tar.xz, jammy: control.tar.zst
$ tar -xf control.tar.xz

# Dump the dependencies to a text file
$ cat control | /bin/grep Depends: | sed 's/Depends: //g' | tr ',' $'\n' | sed 's/^ //g' | sort > packages-focal.txt

When reviewing the contents of a PR, you can run a sample deb packaging job:

When the job is complete search the logs (go to /consoleFull) for Upload complete:, where you can get both the .tar.gz archive (that you can run through repack_deb after modifying depends = '${shlibs:Depends}) as well as the .deb archive which will let you examine the existing control file.


I will now include the analysis after comparing the results using PR #18734 to look at the remaining / new items that have come up. The last time we went through this there were some packages already cleared via #17058 (review) and the related google doc. Summary at the end of this comment with potential action items.

focal

Found in ${shlibs:Depends} but not in packages-*.txt:

Found in packages-*.txt but not ${shlibs:Depends}:

Common to Both (minus versioning)

  • coinor-libclp1
  • coinor-libcoinutils3v5
  • coinor-libipopt1v5
  • libbz2-1.0
  • libdouble-conversion3
  • libexpat1
  • libfreetype6
  • libgfortran5
  • libglew2.1
  • libglib2.0-0
  • libglu1-mesa
  • libglx0
  • libhdf5-103
  • libjsoncpp1
  • liblapack3
  • liblz4-1
  • liblzma5
  • libmumps-seq-5.2.1
  • libnetcdf15
  • libogg0
  • libopengl0
  • libpng16-16
  • libpython3.8
  • libqt5core5a
  • libqt5gui5
  • libqt5printsupport5
  • libqt5widgets5
  • libsqlite3-0
  • libtheora0
  • libtiff5
  • libx11-6
  • libxml2
  • libxt6
  • zlib1g

jammy

Found in ${shlibs:Depends} but not in packages-*.txt:

(and not mentioned in focal section)

  • libfmt8
  • libspdlog1-fmt8

Found in packages-*.txt but not ${shlibs:Depends}:

(and not mentioned in focal section)

I consider these all OK, the jammy situation is a little different because these dependencies listed here actually come from our VTK tarball build which currently does not affect jammy (but soon will). AKA these are our VTK tarball dependencies
which should be kept as is.

  • libjsoncpp25
  • libnetcdf19
  • libogg0
  • libxml2
  • libxt

Noting that this process is somewhat arduous, attached are the extracted .txt documents that produced the above analysis for people to cross reference. New packages to consider that ${shlibs:Depends} wants:

  • Potentially missing packages:
    • focal and jammy:
      • libcrypt1
      • libquadmath0
    • jammy only (?):
      • Seems likely this just comes from spdlog-dev / packaging differences
        OR from VTK tarball situation. I'm 95% sure these can be ignored.
      • libfmt8
      • libspdlog1-fmt8
  • Potentially extra packages:
    • default-jre
    • jupyter-notebook
    • libmsgpackc2
    • libnlopt-cxx0

Package data:

@jwnimmer-tri
Copy link
Collaborator

Found in ${shlibs:Depends} but not in packages-*.txt:
libquadmath0 (unclear, usually comes from fortran right?)

Our packages.text lists libgfortran5 and libgfortran5 depends on libquadmath0 so this is no action required. Shlibdeps is aggressive about transitive dependencies. We do not need to be in packages.txt.

libcrypt1 (>= 1:4.1.0)

This is Priority: required so there's no real need for packages.txt to care about it. Presumably something we grab from Ubuntu us using it transitively.

@jwnimmer-tri
Copy link
Collaborator

jammy only (?):

Seems likely this just comes from spdlog-dev / packaging differences
OR from VTK tarball situation. I'm 95% sure these can be ignored.
libfmt8
libspdlog1-fmt8

For the same reason, these are fine already (no need to list them). Our txt file says libspdlog-dev which pulls in those runtime libraries already.

@jwnimmer-tri
Copy link
Collaborator

Potentially extra packages:
libmsgpackc2

Already fixed by #19080.

default-jre

Not a shared library, so would not be a part of shlibdeps output. We need this for lcm-spy and such. No changes necessary.

jupyter-notebook

Not a shared library, so would not be a part of shlibdeps output. We need this for the tutorials. No changes necessary.

libnlopt-cxx0

Looks like #18532 forgot to clean this up. I'll open a PR.

That's everything, so just that one change and I'll close this.

@jwnimmer-tri
Copy link
Collaborator

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters
Development

No branches or pull requests

3 participants