Feature request: shade protobuf dependency

[ZacSweers]

We've had some issues with conflicting protobuf dependencies (javalite vs protobuf-lite) due to this dependency and others. Would you be open to shading the protobuf dependency instead?

[nymanjens]

Is this before or after the 1.9 release (see #23) ?

[ZacSweers]

It was, yeah

[nymanjens]

It was, yeah

Does this mean it was before or after?

[ZacSweers]

After

[nymanjens]

I looked into this a bit, and it feels like I don't have enough Maven experience to be able to fix this without reproducing the issue and without breaking something else.

Do you have a concrete example change you think would work?

@cpovirk Do you have any advice here?

[cpovirk]

I've gotten myself a bit twisted around trying to understand this, but here's where I am, which is hopefully not wildly wrong:

My mostly unhelpful starting point is that I don't think this is "morally" TestParameterInjector's fault: The problem is the libraries that are still on the old protobuf-lite. (Or the Real Problem is that the protobuf people renamed protobuf-lite to protobuf-javalite in the first place. But we're stuck with that problem. Being stuck with that, incidentally, was quite possibly one of the experiences that the authors of https://jlbp.dev/JLBP-6 were familiar with when they wrote about not ever doing that kind of thing....)

Like I said, that's mostly unhelpful. But given that you're reporting other dependencies that are causing the same problem, I'd nudge you toward figuring out where the old protobuf-lite is coming from and trying to get those libraries to upgrade. But I seem to recall that protobuf at least used to require a close match between the runtime and the compiled code, so maybe that will go poorly? Or maybe it's gotten better, or maybe it was always OK as long as the runtime was the newer of the two? But it sounds like we're talking here about excluding the newer runtime, so I guess not?

(Digression(?): I just tried doing the upgrade for Truth. I got stuck because xolstice/protobuf-maven-plugin#70 isn't in a release yet.... But that affects only our tests, so if by chance Truth is part of the problem, let me know, and we'll live without those tests or otherwise hack around it for now. [edit: Oh, Truth is probably not part of the problem, since its lite dependency is declared optional. But I didn't realize that until after I tried again :)])

My next can-you-"just...?" suggestion is whether you've looked into excluding the protobuf-javalite dep from TestParameterInjector or explicitly declaring protobuf-lite deps in whichever projects are currently seeing protobuf-javalite "win" over it. [edit: Hopefully I didn't get that backward?]

It does seem like it would be possible for TestParameterInjector to declare its protobuf dependency as optional/provided. That would essentially automate the exclusion, so it would be good news for people who need the exclusion but bad news for users who get mysterious errors until they add the protobuf dep themselves. Or maybe TestParameterInjector could avoid those mysterious errors by switching to using the protobuf APIs only reflectively: If protobuf isn't on the classpath, then TestParameterInjector doesn't need it, since it can't be being asked to create parameters of that type. But that's a workaround in its own way, just one that would live in TestParameterInjector.

Given that TestParameterInjector does need to create instance of MessageLite types to pass to user-written tests, I'm not sure that shading can help here. But it's possible that I've misunderstood the problem, in which case lots of what I just wrote might be wrong :)