Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

scala_proto_library contains code for all dependencies #218

Closed
bjchambers opened this issue Sep 12, 2019 · 7 comments · Fixed by #292
Closed

scala_proto_library contains code for all dependencies #218

bjchambers opened this issue Sep 12, 2019 · 7 comments · Fixed by #292

Comments

@bjchambers
Copy link
Contributor

I originally noticed this when debugging why I was getting unexpected "indirect dependency" problems.

If you have two proto libraries, such that one depends on the other (eg., A <- B), then the scala library jar for B will contain all the code for the protos in A. I suspect this is because the proto_library rule produces a (concatenated) descriptor describing both the src proto and any protos in the dependency set, and the scala_proto_library then produces code for all the entries (not just those that were listed in the srcs).

See #217 for a test reproducing the issue and a proposed fix.

(I'm not sure if this is what is causing my indirect dependency error, but my theory was that even though I had a direct dependency on A, and wasn't directly using B, it was registered as "used" because it redefined the classes from A)

@bjchambers
Copy link
Contributor Author

Any thoughts here? This seems like it could be pretty bad in larger projects? Since the scala_proto_library includes generated code for all of the proto dependencies, if you have a bunch of different proto files with inter-dependencies, you are generating code and compiling it every time those dependencies are used, rather than just sharing the scala library.

@SrodriguezO
Copy link
Collaborator

@andyscott would you have bandwidth to take a look? @bjchambers's points sound valid to me, but I have very limited experience with protobuf

@andyscott
Copy link
Member

The solution for this is to use an aspect that adds granular protoc compilation steps for each small grouping of proto files. It's not terribly hard to implement, so I can take a look this next week.

tweag/rules_haskell has done this for quite some time, and bazelbuild/rules_scala followed suit not too long ago.

@bjchambers
Copy link
Contributor Author

When balezbuild/rules_scala added that it also broke a lot of the abilities to use ScalaPB extensions (see bazelbuild/rules_scala#753 and bazelbuild/rules_scala#751). I'm also not sure that more granular protoc is necessary.

It seems like an easier fix would be to pass only the protos in srcs to the protoc invocation. Other files in the transitive dep set could be passed using -I. If there is a directory containing only the files to pass, that's easy. I'm not sure if it would be necessary to create a separate directory tree containing all the transitive deps.

@bjchambers
Copy link
Contributor Author

Any progress / updates?

@bjchambers
Copy link
Contributor Author

Is there any reason why the fix proposed in #217 doesn't work?

@SrodriguezO
Copy link
Collaborator

Hey @bjchambers sorry about the slow responses; it's a busy time of year. I still must defer to Andy on this one. @andyscott friendly ping : )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants