Move meshes and models from drake to drake_models #19160
Conversation
jwnimmer-tri
added
the
release notes: fix
|
+@rpoyner-tri for feature review, please. I'm also curious to get your thoughts on whether and which piece of this might merit a second (platform) reviewer. |
There was a problem hiding this comment.
So far, I've made no attempt to confirm that moved files have the same contents. I've only looked at which files moved where, and the resulting path name changes. To understand things better, I pulled both patches locally and studied the file manifests, with help from text filtering and diffing tools.
In the context of the other patch review, I'll try to confirm that files arrived undamaged.
I'm a bit curious about the design choices of which files moved, which stayed, and which got dropped. The destination path names seem fine to me.
As for a second reviewer, I'm not feeling strongly about it. If you think I've missed some aspects, by all means recruit a second reviewer.
Reviewed 357 of 357 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers (waiting on @jwnimmer-tri)
examples/atlas/urdf/door.urdf line 1 at r1 (raw file):
<?xml version="1.0"?>
nit This is an example of a file that was not transferred, but just dropped. Were these just unused, and inherited from some prior source?
manipulation/models/allegro_hand_description/BUILD.bazel line 14 at r1 (raw file):
name = "glob_models", extra_srcs = [ "LICENSE.TXT",
Should the license file be coming from drake_models now, similar to atlas?
manipulation/models/iiwa_description/BUILD.bazel line 14 at r1 (raw file):
name = "glob_models", extra_srcs = [ "LICENSE.TXT",
Also here, should this license file come from drake_models?
There was a problem hiding this comment.
I'm a bit curious about the design choices of which files moved, which stayed, and which got dropped.
I moved Atlas all at once here because trying to split it up meshes vs models was going to be a lot of work, and I figured not that many people are using it.
For the others like the iiwa arm and allegro hand, probably a bunch of people have hard-coded the current URDF filenames so I want to be a bit more careful moving them (probably with deprecation, even though models are not Stable API). Moving the meshes only solves the acute problem (narrowing the install footprint) without disturbing users yet.
Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers (waiting on @rpoyner-tri)
manipulation/models/allegro_hand_description/BUILD.bazel line 14 at r1 (raw file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
Should the license file be coming from drake_models now, similar to
atlas?
The URDF files are still in drake git and installed as part of drake, so we still need to keep the license text for those files in drake.git alongside the URDFs, and install the license file alongside the installed URDF files.
manipulation/models/iiwa_description/BUILD.bazel line 14 at r1 (raw file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
Also here, should this license file come from drake_models?
Ditto.
examples/atlas/urdf/door.urdf line 1 at r1 (raw file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
nit This is an example of a file that was not transferred, but just dropped. Were these just unused, and inherited from some prior source?
This was already deleted in #19159. There must be some kind of rebase confusing either in reviewable or on my part, sorry.
jwnimmer-tri
added
the
status: single reviewer ok
There was a problem hiding this comment.
As for a second reviewer, I'm not feeling strongly about it. If you think I've missed some aspects, by all means recruit a second reviewer.
+(status: single reviewer ok)
Reviewable status: 4 unresolved discussions (waiting on @jwnimmer-tri and @rpoyner-tri)
a discussion (no related file):
Working
Do not merge until models git sha is finalized (after the other PR).
Upgrade drake_models to latest commit
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignee rpoyner-tri(platform) (waiting on @rpoyner-tri)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Working
Do not merge until models git sha is finalized (after the other PR).
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:
|
I cannot imagine how macOS would have any particular sadness here, but let's be sure: @drake-jenkins-bot mac-arm-monterey-clang-bazel-continuous-release please |
|
@drake-jenkins-bot mac-arm-monterey-clang-bazel-experimental-release please |
|
FYI the change in compressed release archive size from this PR: That's a 36 MiB reduction (-39%). |
Towards #19078 and #11913.
See RobotLocomotion/models#23 for the sibling pull request.
This change is