Thanks for doing so much work verifying this and analyzing it! It really seems like we did all this for mockito, so it has very little risk of impacting users. And you've verified it is working with mockito now. So I think I'm in favor of unvendoring. This will make it much easier to get bugfixes, etc, with less toil.
Kenn On Tue, Apr 12, 2022 at 12:35 PM Liam Miller-Cushon <cus...@google.com> wrote: > I have created a PR that unvendors bytebuddy as a proof of concept and to > start testing, and the initial results look good: > https://github.com/apache/beam/pull/17317 > > I also did some analysis of the output of `./gradlew dependencyReport`. I > have attached a graph showing all dependency paths to bytebuddy that result > in the version being overridden, and the only affected paths are test > dependencies on mockito and powermock. > > So based on what I'm seeing, switching to an unvendored bytebuddy would > work today, and the compatibility guarantees provided by bytebuddy (and > ASM) should help ensure it keeps working in the future. > > Additional thoughts and feedback are welcome! Does this address some of > the concerns that have been raised so far? Is there additional testing or > dependency analysis that might be helpful? > > On 2022/04/05 18:59:06 Kenneth Knowles wrote: > > Hmm. Too bad the information on the jira is inadequate to explain or > > justify the change. TBH if faced with a conflict between bytebuddy and > > mockito, working to use mocks less, or in more straightforward ways, > would > > have been my preference. This isn't actually a diamond dep problem that > > impacts users, or I would feel differently. > > > > I guess we want to have some thorough testing & survey of the versions of > > bytebuddy and asm used by transitive deps and any possible breaking > > changes. Should be pretty easy since unvendoring is much easier > > to experiment with. Apologies if this already exists in some other thread > > or on the ticket. > > > > Kenn > > > > On Wed, Mar 23, 2022 at 11:16 AM Reuven Lax <re...@google.com> wrote: > > > > > We also use ASM directly. If we unshade Bytebuddy, does that also > unshade > > > ASM? Does ASM provide similar stability guarantees? > > > > > > On Wed, Mar 23, 2022 at 10:50 AM Liam Miller-Cushon <cu...@google.com> > > > wrote: > > > > > >> On 2022/03/21 19:36:29 Robert Bradshaw wrote: > > >> > My understanding was that Bytebuddy was originally unvendored, and > we > > >> > vendored it in reaction to version incompatibility issues (despite > the > > >> > promise of API stability). I think we should have a good > justification > > >> > for why we won't get bitten by this again before moving back to > > >> > unvendored. > > >> > > >> Does anyone have context on the specific issues that > > >> https://issues.apache.org/jira/browse/BEAM-1019 was a reaction to? > > >> > > >> The bug was filed in 2016 and mentions upgrading to mockito 2.0. One > > >> potential justification for trying to unvendor is that the issue is > fairly > > >> old, and the library has continued to stabilize, so it might be safer > to > > >> unvendor today than it was in 2016. > > >> > > > > > >
