Conversation
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewed 28 of 28 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, platform LGTM missing (waiting on @jwnimmer-tri)
cmake/cpack.cmake line 65 at r1 (raw file):
set(CPACK_DEBIAN_PACKAGE_ARCHITECTURE ${MACHINE_ARCH}) set(CPACK_DEBIAN_PACKAGE_RELEASE ${PACKAGE_RELEASE_VERSION}) set(CPACK_DEBIAN_PACKAGE_DEPENDS "lcm (>=1.4.0), default-jre | java8-runtime, libc6, libgcc1, libglib2.0-0, libglut3.12, libglu1-mesa, libgtk-3-0, libice6, libjpeg8, libopengl0, libpng16-16, libsm6, libstdc++6, libx11-6, libxau6, libxcb1, libxdmcp6, libxext6, libxmu6, libxt6, python3, python3-gi, python3-numpy, python3-scipy, zlib1g")
BTW It's fine to keep this change (and the other deb-metadata change below) but they are not strictly necessary. TRI will not be using dpkg to install the software, so it's not a requirement that the dpkg dependencies be correct (or even exist).
To your question in RobotLocomotion/drake#22891 about breaking older packages, TRI builds packages for Jammy and earlier using a pinned git sha, so nothing we do in this pull request will break them.
bot2-vis/src/bot_vis/glm_util.c line 23 at r1 (raw file):
// Adapted from F. Devernay's extensions to Nate Robbins' GLM library // // Source obtained from GLM-0.3.5 available from
I think the CMake minimum version perl pie hit some files it shouldn't have (these next four C files).
scripts/package/linux/ubuntu/common/build.sh line 86 at r1 (raw file):
rm -f CMakeLists.patch # Address compatibility issues with Py_TYPE()
nit Ideally this would be a literal cherry-pick of the upstream commit 0289aa9efdf043dd69d65b7d01273e8108dd79f7 with a comment pointing there so we know where it came from.
jwnimmer-tri
left a comment
There was a problem hiding this comment.
modulo the two discussions below.
If this at least makes it through the build (and it seems like it does), I would be okay if we merge this PR as progress, and then iterate on fixing any runtime bugs in a second PR. That would also allow TRI to themselves test the new packages concurrently.
Also go ahead and delete the (failing) Bionic CI workflow from ci.yml. We might as well delete Focal as well since we don't support it anymore.
Reviewable status: 3 unresolved discussions, platform LGTM from [jwnimmer-tri]
838a16e to
f341350
Compare
|
@Aiden2244 did you see this PR? I'm OK if you want to close this and use #25 instead, but please carry over the requests here into fixes in #25 in that case. |
|
Ah, guess there was no need for a secondary PR. I do think that #25 is now closer to where we want it to be (in the sense that I am no longer getting any runtime failures), so let's close this and I'll port the revisions listed here. |
This change is