It looks like the japi-compliance-checker tool to which Anthony linked is available as a gradle plugin: https://github.com/melix/japicmp-gradle-plugin
On Tue, May 14, 2019 at 5:07 PM Ryan McMahon <rmcma...@pivotal.io> wrote: > Fixed in > > https://github.com/apache/geode/commit/cc0b37a504480f554b1884491f44a3cca4113ef5 > > On Tue, May 14, 2019 at 2:06 PM Ryan McMahon <rmcma...@pivotal.io> wrote: > > > 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 > >> >>>> > >> >>> > >> >> > >> > >> >
