Thanks Anthony! Have we considered using a compliance checker in our CI like the one in the first link?
Side note - I think that varargs is an interesting case that I don't see called out in those links. Since varargs is just syntactic sugar, it is ultimately the varargs parameter is converted to an array in the bytecode. So in the JSON formatter change: fromJSON(byte[] jsonByteArray, String... identityFields) becomes fromJSON(byte[] jsonByteArray, String[] identityFields) Hence the breakage of the ABI. Ryan On Tue, May 14, 2019 at 1:33 PM Anthony Baker <aba...@pivotal.io> wrote: > Here are a few links on API compatibility: > > https://lvc.github.io/japi-compliance-checker/#Examples > https://wiki.eclipse.org/Evolving_Java-based_APIs > https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html > > > Anthony > > > > On May 14, 2019, at 12:45 PM, Dan Smith <dsm...@pivotal.io> wrote: > > > > Sounds good! Yeah, this is a bit of an interesting case where code will > > still compile against the new API without change, but I think maintaining > > compatibility of the binary API is also important. Thanks for looking > into > > it. > > > > -Dan > > > > On Tue, May 14, 2019 at 11:22 AM Ryan McMahon <rmcma...@pivotal.io> > wrote: > > > >> Hi all, > >> > >> This is my mistake - it was a misunderstanding of what constitutes a > >> breaking public API change. If a client app were to recompile using the > >> new bits with the new method signature, there wouldn't be an issue > because > >> the new signature would work with 0 varargs. The problem arises when > you > >> hot swap the geode dependencies for that client app without > recompiling, as > >> the bytecode no longer matches. Since we do support the hot swap use > case > >> for CVEs etc, I see now this is considered a breaking API change. > >> > >> I'll change this to use a method overload instead, it should be a pretty > >> simple fix. Luckily, this issue didn't make it into any Geode releases. > >> > >> Thanks, > >> Ryan > >> > >> > >> On Tue, May 14, 2019 at 10:47 AM Udo Kohlmeyer <u...@apache.org> wrote: > >> > >>> How did this slip the review process that the signature of a public API > >>> was changed? > >>> > >>> Well done Gester for finding this!! > >>> > >>> --Udo > >>> > >>> On 5/14/19 10:40, Dan Smith wrote: > >>>> I think the changes for GEODE-6327 broke backwards compatibility in > >>>> JSONFormatter with the change from fromJSON(String jsonString) to > >>>> fromJSON(String jsonString, String... identityFields) > >>>> > >>>> Adding an additional varargs parameter to the method breaks code that > >> was > >>>> compiled against the non-varargs version. I think we need to overload > >> the > >>>> method instead. > >>>> > >>>> Thanks to Gester for discovering this with his test! > >>>> > >>>> -Dan > >>>> > >>> > >> > >
