drake_proto: Simplify ubsan fix implementation.

[EricCousineau-TRI]

Ran into this when prototyping on #7616.

This simplifies the ubsan fixups by ensuring that local includes are prefixed with drake, such that we can use drake_cc_library rather than duplicate the code.
I'm not terribly familiar with awk, and tried using sed with multiple scripts, but Mac's sed -E has awkward line escaping shenanigans.

---

This change is 

[EricCousineau-TRI]

+@clalancette for feature review, please.
+@ggould-tri for platform review, please.

---

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[clalancette]

I'm not terribly familiar with awk, and tried using sed with multiple scripts, but Mac's sed -E has awkward line escaping shenanigans.

Yeah, my original prototype for this used sed, but the version in macOS is ancient and BSD, so doesn't work like a modern GNU version of sed.

[ggould-tri]

, one quibble below.

---

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

tools/skylark/drake_proto_ubsan_fix.py, line 31 at r1 (raw file):

    srcs = sys.argv[1].split()
    outs = sys.argv[2].split()
    for src, out in zip(srcs, outs):

This will misbehave silently if srcs and outs are of different size. You should probably assert(len(srcs) == len(outs)) or similar.

---

Comments from Reviewable

[clalancette]

I like the approach overall. Some comments inline.

---

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

tools/skylark/drake_proto_ubsan_fix.py, line 13 at r1 (raw file):

    with open(src, 'r') as fsrc, open(out, 'w') as fout:
        text = fsrc.read()
        # Ensure local includes are prefixed with 'drake'.

Thinking about this more, I don't think we actually need this, do we? The generated code coming out of the protoc compiler definitely won't have drake paths in it, and any prior rule that modified this file presumably already add the drake-prefixed paths appropriately. So I don't really see why we need this, unless there is something I'm missing.

---

Comments from Reviewable

[EricCousineau-TRI]

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions.

---

tools/skylark/drake_proto_ubsan_fix.py, line 13 at r1 (raw file):

Previously, clalancette (Chris Lalancette) wrote…

Thinking about this more, I don't think we actually need this, do we? The generated code coming out of the protoc compiler definitely won't have drake paths in it, and any prior rule that modified this file presumably already add the drake-prefixed paths appropriately. So I don't really see why we need this, unless there is something I'm missing.

I tried removing this, but compilation fails with:
fatal error: 'common/proto/matlab_rpc.pb.h' file not found

From what I gather proto_gen uses the package path to generate its include prefix:

drake/third_party/com_github_google_protobuf/protobuf.bzl

Line 31 in c8f6878

return _GetPath(ctx, ctx.label.package + '/' + ctx.attr.includes[0])

An alternative solution would be to patch the third party stuff, but I'd prefer to leave this code as-is since our fix is relatively simple for the time being.

---

tools/skylark/drake_proto_ubsan_fix.py, line 31 at r1 (raw file):

Previously, ggould-tri wrote…

This will misbehave silently if srcs and outs are of different size. You should probably assert(len(srcs) == len(outs)) or similar.

Done.

---

Comments from Reviewable

[clalancette]

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions.

---

tools/skylark/drake_proto_ubsan_fix.py, line 13 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

I tried removing this, but compilation fails with:
fatal error: 'common/proto/matlab_rpc.pb.h' file not found

From what I gather proto_gen uses the package path to generate its include prefix:

drake/third_party/com_github_google_protobuf/protobuf.bzl

Line 31 in c8f6878

return _GetPath(ctx, ctx.label.package + '/' + ctx.attr.includes[0])

An alternative solution would be to patch the third party stuff, but I'd prefer to leave this code as-is since our fix is relatively simple for the time being.

OK, that makes sense to me. I agree that this is simpler than patching the third-party stuff, so this is fine with me.

---

Comments from Reviewable

[clalancette]

---

Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[ggould-tri]

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable