Resolve ODR issues with manipulation_station example #9826
Conversation
There was a problem hiding this comment.
+@jwnimmer-tri and +@EricCousineau-TRI for feature/platform review.
Reviewable status: all discussions resolved, platform LGTM missing
|
When I posed a comment in #9820 suggesting this patch, my intention was for you to use it on your feature branch so that the docker images for the MIT class would not be so brittle. This PR installs However, this PR also installs |
|
I know that's what you were proposing and that this is more aggressive. Installing the example is actually not a hard requirement for me right now. For class, we are distributing content via docker, and in the docker instance we can/are building drake from source. What I do need is to be able to write python code for this example in drake's source tree. This is the only mechanism that has been offered for that so far. I agree that depending on scenegraph dev is not ideal, but I think that is the situation that we are in right now and we need to support it. |
|
I'm personally fine with this as an interim, pending the longer-term solutions from #9645 and #9646, as it is simplest clean working state for consuming the manipulation station Python bits. I do agree with @jwnimmer-tri that we should get consensus from @jamiesnape and the platform team on policies (be they temporary or long term) on downstream consumption / installation of I'd say we make a documentation PR with backstory in the comment, ask for full-team platform review, land the doc PR, then review / merge this PR? |
|
I'm working on a PR that removes the |
|
I wrote:
On reflection, I am okay with blessing it. In working through #9830, I've convinced myself that our build system naming heuristics should not be what governs whether |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all discussions resolved, LGTM missing from assignee ericcousineau-tri, platform LGTM from [jwnimmer-tri]
|
(Ready for merge/rebase now.) |
RussTedrake
force-pushed
the
manipulation_station_odr
branch
from
dcbd312 to
82cf018
Compare
|
rebased. |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: all discussions resolved, LGTM missing from assignee ericcousineau-tri, platform LGTM from [jwnimmer-tri]
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Reviewable status:complete! all discussions resolved, platform LGTM from [jwnimmer-tri, ericcousineau-tri]
bindings/pydrake/examples/BUILD.bazel, line 65 at r2 (raw file):
name = "manipulation_station_py", cc_deps = [ "//bindings/pydrake:documentation_pybind",
FYI Merge conflict on this file: bindings/pydrake/examples/BUILD.bazel
Needs another rebase :(
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Reviewable status:
RussTedrake
force-pushed
the
manipulation_station_odr
branch
from
82cf018 to
f742774
Compare
There was a problem hiding this comment.
Reviewable status:
bindings/pydrake/examples/BUILD.bazel, line 65 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
FYI Merge conflict on this file:
bindings/pydrake/examples/BUILD.bazel
Needs another rebase :(
done (again).
RussTedrake
force-pushed
the
manipulation_station_odr
branch
from
f742774 to
1d3d789
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3.
Reviewable status:
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3.
Reviewable status:
|
There is a linter error due to the merge. |
by including manipulation_station targets, and their dependencies, in libdrake.so. we consider this a temporary, and very imperfect, solution until in-tree examples can produce their own bindings w/o going through libdrake.so. (see RobotLocomotion#9648)
RussTedrake
force-pushed
the
manipulation_station_odr
branch
from
1d3d789 to
135340e
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status:
by including manipulation_station targets, and their dependencies, in libdrake.so.
we consider this a temporary, and very imperfect, solution until in-tree examples can produce their own bindings w/o going through libdrake.so. (see #9648)
This change is