workspace: Remove dependency on ruby #13262
Conversation
Cherry-pick a patch from sdformat upstream to make the data embedding easier, and then re-implement the embedder in python. Deprecate the @ruby external.
|
@drake-jenkins-bot linux-bionic-unprovisioned-gcc-bazel-experimental-release please |
|
+@sammy-tri can I interest you in feature review? |
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1.
Reviewable status: needs at least two assigned reviewers, labeled "do not merge"
There was a problem hiding this comment.
+@sherm1 for platform review
Reviewable status: LGTM missing from assignee sherm1(platform), labeled "do not merge" (waiting on @sherm1)
There was a problem hiding this comment.
I'd like to see a unit test for embed_sdf.py if that's feasible.
Otherwise, platform 
Reviewed 9 of 9 files at r1.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri)
tools/workspace/ruby/repository.bzl, line 7 at r1 (raw file):
def ruby_repository(name): print("The @ruby repository is deprecated and will be removed from " + "Drake on or after 2020-09-01.")
Just to make sure I understand: I see ruby removed from pkg and bzl files so we're not using it. But we will continue to download it as an external in case someone else was accessing ruby through Drake?
tools/workspace/sdformat/3bbd303.patch, line 16 at r1 (raw file):
I hope this is easier to maintain, but it also helps me when sharing Drake with consumers that always build from source, but will not install ruby as a compile-time prerequisite
nit: missing period
There was a problem hiding this comment.
Most (maybe all) errors in embed_sdf show up as build-time or test-time errors in bazel test //multibody/parsing/.... I'll ponder if there's a useful new test to add, though.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)
a discussion (no related file):
Working on possible new unit tests.
tools/workspace/ruby/repository.bzl, line 7 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
Just to make sure I understand: I see
rubyremoved from pkg and bzl files so we're not using it. But we will continue to download it as an external in case someone else was accessing ruby through Drake?
A fine question.
Bazel dependencies are pull-based -- i.e., only downloaded / compiled / etc if they are mentioned -- so this code is only ever parsed and run if somewhere in drake we explicitly mention it via @drake//tools/workspace/ruby or @ruby. Since we have neither mention anymore as of this PR, it's effectively dead code to us.
But if I downstream project called this macro and then said @ruby somewhere, it would continue to work for now, albeit with this new diagnostic output.
Sometimes for deprecated externals I'll add a "stub test" fake use so that CI still ensures the downloader don't break. In this case, it's simple enough code that I figured it's unlikely to bitrot in the next 3 months, even with no test.
tools/workspace/sdformat/3bbd303.patch, line 16 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
nit: missing period
This is a cherry-picked commit from upstream. (I should have FYI'd it as "do not review this file.)
I should probably not diverge from what was committed there.
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignees sammy-tri(platform),sherm1(platform)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Working on possible new unit tests.
OK Actually I'm going to defer this to a new PR in order to unblock CI changes. I'll post back later.
Cherry-pick a patch from sdformat upstream to make the data embedding easier, and then re-implement the embedder in python.
Deprecate the
@rubyexternal.Closes #13231.
This change is