RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.22k stars 1.25k forks source link

APT release improvements #16448

Closed jwnimmer-tri closed 2 years ago

jwnimmer-tri commented 2 years ago

Our release process for APT is somewhat obscure and undocumented, uses a fork of drake at https://github.com/RobotLocomotion/debian-drake that must be manually maintained, and compiles Drake from scratch using different build flags but doesn't run the tests.

Overall, this is all way more complicated than we need, and adds friction to the release process.

We should explore using something more akin to alien to create the *.deb files for our apt site, where we simply take the existing *.tar.gz that's already built and tested and shipped everywhere, and add the correct debian/control meta-data (and possibly re-organize the files to match Debian layout).

jwnimmer-tri commented 2 years ago

Given the recurring glitches with apt builds, I've upgraded this to "high" priority.

I've filed #16602 with a first draft of the revised deb-making script. It's not ready to take over production releases yet, but it should provide a solid starting point.

jwnimmer-tri commented 2 years ago

For clarity -- as part of this transition, we will no longer ship the drake-doc package.

svenevs commented 2 years ago

Inlining https://github.com/RobotLocomotion/drake/issues/12783#issuecomment-672266985 as a checklist for revision:

It seems like for a number of these, it would be advantageous to fix the drake install logic rather than doing a post-processing step. For example, if I just omit these three lines

https://github.com/RobotLocomotion/drake/blob/902c14f83cba382b309b779f2dd5d031ce3304bf/tools/workspace/ibex/package.BUILD.bazel#L389-L391

then removing include/ibex is not necessary. Is there any reason to keep it as a post-processing step rather than just removing it from the install logic?

Making any progress on these while the release tooling is in development (and thus the debian-drake repo is still being used) may result in potential breakages but at least for the cleanup ones rm -rf is used which will not fail if it is not found. Let me know your thoughts on fixing in drake vs doing post-processing.

jwnimmer-tri commented 2 years ago

Most of the post-processing is wrong anyway. We should keep things as-if the user did something like cd /opt && sudo tar xfz drake-yyyymmdd.tar.gz. In other words, don't change the install paths at all, just because of the different release channel.

This will change the file layout in the apt package, compared to what users were used to. That's fine, we'll announce it as a breaking change.

For thing like fixing the file permissions, yes we should do that as part of the main installer logic, so that even the tarballs are fixed.

svenevs commented 2 years ago

Most of the post-processing is wrong anyway.

Yeah a fair amount already seems irrelevant or already fixed "upstream" in drake install.

In other words, don't change the install paths at all, just because of the different release channel.

Works for me, I will make sure to compare against the existing .deb and this and identify what (if anything) has moved so we can denote this in the breaking change notes.

so that even the tarballs are fixed.

Sounds good. I will prioritize the fixes that actually matter functionally. It's not altogether clear where some of those fixes are even taking place. I edited / numbered the above and will pick things back up tomorrow. Green light to implement (7) (stop installing ibex and vtk headers)?

jwnimmer-tri commented 2 years ago

For (7), we are shipping IBEX and VTK as public shared libraries, so we should continue to install their headers so that users can build & link against those libraries while also linking to Drake.

jwnimmer-tri commented 2 years ago

FYI from meeting today -- the goal would be to run this in CI as part of a Packaging build, where the build first creates the release tarball (as it does now) and then also re-packs that into a *.deb file.

svenevs commented 2 years ago

I have some stuff up in #17058, I need to come back tomorrow and undo those changes to get the original list of warnings -- some of them seem possible to fix in drake itself. For example, it looks like generated header files in gen/ folders were getting their executable bits set? Right now that approach is a bit brute-force...

jwnimmer-tri commented 2 years ago

It might make sense to split a new component: continuous integration issue for "Run deb repacker during Nightly Packaging, push to S3" and then close this one? We will probably want to defer that work until after the tool is moved out of dev folder?

svenevs commented 2 years ago

=> https://github.com/RobotLocomotion/drake/issues/17177