Update drake_cmake_installed README #367
Conversation
…to other examples
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, LGTM missing from assignee mwoehlke-kitware, platform LGTM missing (waiting on @jwnimmer-tri and @mwoehlke-kitware)
a discussion (no related file):
I figure both +@jwnimmer-tri and +@mwoehlke-kitware should have a look at this, since this change wraps up that original issue.
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri, LGTM missing from assignee mwoehlke-kitware, platform LGTM missing (waiting on @mwoehlke-kitware and @tyler-yankee)
drake_cmake_installed/README.md line 23 at r1 (raw file):
nit
... after cloning this repository ...
This caveat doesn't make sense here. In order to run install_prereqs above, the user already would have had to clone this repository.
drake_cmake_installed/README.md line 40 at r1 (raw file):
Otherwise, the following are alternative options for which version of Drake to download.
It is not clear to me how to put the instructions of this section into action.
Is the user still supposed to run install_prereqs first? If yes, then doesn't the tar command in (1) do the wrong thing (overlaying files instead of deleting everything first)? If no, then won't the prereqs be missing?
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri, LGTM missing from assignee mwoehlke-kitware, platform LGTM missing (waiting on @jwnimmer-tri and @mwoehlke-kitware)
drake_cmake_installed/README.md line 23 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
nit
... after cloning this repository ...
This caveat doesn't make sense here. In order to run
install_prereqsabove, the user already would have had to clone this repository.
Okay, I can remove this. I actually removed code telling the user to clone the repo, which I felt was unnecessary for the same reason.
drake_cmake_installed/README.md line 40 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Otherwise, the following are alternative options for which version of Drake to download.
It is not clear to me how to put the instructions of this section into action.
Is the user still supposed to run
install_prereqsfirst? If yes, then doesn't thetarcommand in (1) do the wrong thing (overlaying files instead of deleting everything first)? If no, then won't the prereqs be missing?
The only additional things in install_prereqs are (1) installing python3-all-dev (Ubuntu only), and (2) running Drake's install_prereqs script. I can add a description of this and instruct the user accordingly.
This also implies that we should update this section of the README anytime the install_prereqs script changes. But, I think that beats making a bunch of complicated arguments to the script for installation options.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @mwoehlke-kitware)
There was a problem hiding this comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @tyler-yankee)
drake_cmake_installed/README.md line 15 at r2 (raw file):
```bash setup/install_prereqs
Doesn't Ubuntu still need sudo here?
drake_cmake_installed/README.md line 48 at r2 (raw file):
```bash curl -O https://drake-packages.csail.mit.edu/drake/nightly/drake-20240214-jammy.tar.gz
I feel like there should be a note somewhere to replace jammy with noble or mac-arm64 when appropriate.
drake_cmake_installed/README.md line 56 at r2 (raw file):
```bash git clone https://github.com/RobotLocomotion/drake.git (cd drake && mkdir build && cd build && cmake -DCMAKE_INSTALL_PREFIX=$HOME/drake .. && make)
Does this work without calling Drake's own install_prereqs? (Also, that supersedes the need to call the installed Drake's install_prereqs.)
drake_cmake_installed/README.md line 67 at r2 (raw file):
Then ensure you have Python installed; specifically,
I believe none of this is necessary (see above comment).
There was a problem hiding this comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @mwoehlke-kitware and @tyler-yankee)
drake_cmake_installed/README.md line 15 at r2 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Doesn't Ubuntu still need
sudohere?
I guess we need to wait for #362 to land first.
drake_cmake_installed/README.md line 67 at r2 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
I believe none of this is necessary (see above comment).
In case "1. A specific version (date-stamped)", python3-all-dev would not be installed yet. The need for python3-all-dev doesn't come from Drake, it comes from the example pybind11 code in this sample project (see src/simple_bindings).
There was a problem hiding this comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @jwnimmer-tri and @tyler-yankee)
drake_cmake_installed/README.md line 15 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I guess we need to wait for #362 to land first.
That works too. 🙂
drake_cmake_installed/README.md line 67 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
In case "1. A specific version (date-stamped)", python3-all-dev would not be installed yet. The need for python3-all-dev doesn't come from Drake, it comes from the example pybind11 code in this sample project (see src/simple_bindings).
To clarify: python3-all-dev is a superset of what gets installed by Drake's install_prereqs for a source distribution?
In either case, I think $HOME/drake/share/drake/setup/install_prereqs is still redundant? Or does the second run get different arguments that cause it to install more things?
|
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Correction: less "redundant", more "don't we rather need to call the source distribution's |
There was a problem hiding this comment.
I also changed the date of the Drake nightly under Alternative Versions, since that version was out of date and the file naming convention has changed since then (prepend 0.0. to the date).
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @jwnimmer-tri and @mwoehlke-kitware)
drake_cmake_installed/README.md line 48 at r2 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
I feel like there should be a note somewhere to replace
jammywithnobleormac-arm64when appropriate.
Yes, added.
drake_cmake_installed/README.md line 56 at r2 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Does this work without calling Drake's own
install_prereqs? (Also, that supersedes the need to call the installed Drake'sinstall_prereqs.)
See below.
drake_cmake_installed/README.md line 67 at r2 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Correction: less "redundant", more "don't we rather need to call the source distribution's
install_prereqsbefore building?" (See comment on L56.)
Okay, fixed to call in all cases before building.
|
@jwnimmer-tri: Is the CI error on macOS related to #22573? |
|
Yes, it is the fix for the problem. The tar.gz on S3 will still be broken until it gets overwritten by the rebuild: |
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, platform LGTM from [jwnimmer-tri] (waiting on @jwnimmer-tri and @tyler-yankee)
drake_cmake_installed/README.md line 42 at r3 (raw file):
To use any of these options, instead of running `install_prereqs`, first run the code for whichever option you prefer below. Each option
Sorry, didn't catch this until you added install_prereqs for the "specific version (date-stamped)" option. (I also hadn't realized it was missing there; good catch!)
Suggestion:
run the code for whichever option you prefer below
FYI should be fixed now. |
tyler-yankee
force-pushed
the
cmakeinstalled-docs
branch
from
ccddbd2 to
dd2e219
Compare
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions, platform LGTM from [jwnimmer-tri] (waiting on @jwnimmer-tri and @tyler-yankee)
|
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Fixed. |
Addresses #22040 with the following:
drake_cmake_installedexampleThe
install_prereqsscript in this example was already working with macOS, verified locally.This change is