[setup] Rewrite Ubuntu install_prereqs from the ground up#22055
[setup] Rewrite Ubuntu install_prereqs from the ground up#22055jwnimmer-tri wants to merge 2 commits intoRobotLocomotion:masterfrom
Conversation
6b41139 to
a11e740
Compare
a11e740 to
709113b
Compare
bbf37ee to
ef6931b
Compare
ef6931b to
94a95dc
Compare
94a95dc to
99f8aea
Compare
076ed72 to
4b7447e
Compare
4b7447e to
4cf8eee
Compare
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewed 1 of 8 files at r9, all commit messages.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes
a discussion (no related file):
Working
Document the new solver flags in bazel.md.
mwoehlke-kitware
left a comment
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @jwnimmer-tri)
setup/install_prereqs line 1 at r9 (raw file):
#!/bin/bash
Huh? This doesn't look like bash script; did you mean #!/usr/bin/env python3?
setup/ubuntu/packages-jammy-binary.txt line 1 at r9 (raw file):
build-essential
You know, probably not in this pass, but eventually, I'd be very tempted to turn these into a pyproject.toml style single file, e.g.:
[requirements]
build = [
"cmake",
...
]
test = [
"kcov; dist == 'jammy'",
...
]Or maybe YAML:
requirements:
build:
- cmake
- ...
test:
- kcov; dist == 'jammy'
- ...
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes
setup/install_prereqs line 1 at r9 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Huh? This doesn't look like bash script; did you mean
#!/usr/bin/env python3?
See the first comment paragraph immediate below.
setup/ubuntu/packages-jammy-binary.txt line 1 at r9 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
You know, probably not in this pass, but eventually, I'd be very tempted to turn these into a
pyproject.tomlstyle single file, e.g.:[requirements] build = [ "cmake", ... ] test = [ "kcov; dist == 'jammy'", ... ]Or maybe YAML:
requirements: build: - cmake - ... test: - kcov; dist == 'jammy' - ...
It will be simplest if it's something we can parse using only the CPython 3.9 stdlib. If not, then we first need some other data file to bootstrap whatever parsing library we need.
mwoehlke-kitware
left a comment
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @jwnimmer-tri)
setup/install_prereqs line 1 at r9 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
See the first comment paragraph immediate below.
Oh, my, that's... sneaky. Is it really so terrible to have a separate .py?
setup/ubuntu/packages-jammy-binary.txt line 1 at r9 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
It will be simplest if it's something we can parse using only the CPython 3.9 stdlib. If not, then we first need some other data file to bootstrap whatever parsing library we need.
Point, or we could declare "our own" format that's pretty dumb, or maybe use JSON. I was thinking TOML went back further than it does, though; I see it's only in 3.11. 🙁 In any case, as I said, not something to muck with now.
|
The work in #24388 to simplify the process will need to land first, as well as subsequent refactoring like renaming and merging the text files. After that's finished, we can cycle back here to revisit the UX improvements in a Python rewrite. |
jwnimmer-tri
left a comment
There was a problem hiding this comment.
@jwnimmer-tri made 1 comment.
Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware).
setup/ubuntu/packages-jammy-binary.txt line 1 at r9 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Point, or we could declare "our own" format that's pretty dumb, or maybe use JSON. I was thinking TOML went back further than it does, though; I see it's only in 3.11. 🙁 In any case, as I said, not something to muck with now.
Our minimum is 3.12 now, so I might revisit this.
4cf8eee to
5cbe305
Compare
5cbe305 to
b10c005
Compare
jwnimmer-tri
left a comment
There was a problem hiding this comment.
FYI I'm going to work to carve off chunks of this into separate PRs.
@jwnimmer-tri partially reviewed 14 files and made 2 comments.
Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on mwoehlke-kitware).
setup/install_prereqs line 1 at r9 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Oh, my, that's... sneaky. Is it really so terrible to have a separate
.py?
OK yup. The bash trampoline and python implementation are separate files now.
b10c005 to
98e5e07
Compare
Instead, users and developers should run setup/install_prereqs (not as root). Usability changes: - Users never run any of our commands under sudo; we'll request sudo if and only if we actually need superuser access. - Fine-tuned log output, with option for --verbose (e.g., in CI). - Skip unnecessary steps; incremental runs are typically sub-second. Maintainability changes: - One single-file program to maintain, instead of an assortment of bash scripts and source and exec each other in weird ways. - Always use files, not heredocs. (The pkg-config etc in a heredoc were documented as unwanted when using Drake's Debian packages, but that was wrong; we always want those deps even in Debian packages.) All of the new entries in our txt files are ported over from heredocs, as is the bazelisk metadata. - Flavors are now a linear stack, not pick-and-choose hodgepodge. This substantially reduces our testing / verification burden. Filenames of txt files now exactly match the flavor names. - Smoother control flow which should pave the way to supporting more kinds of platforms beyond Ubuntu. Adding more and better kinds of error handling or reporting will be easier now (vs bash).
98e5e07 to
168d83e
Compare
TBD
This change is