siom79 / japicmp

Comparison of two versions of a jar archive
https://siom79.github.io/japicmp
Apache License 2.0
701 stars 107 forks source link

Add middle ground between isOutputOnlyModifications and isOutputOnlyBinaryIncompatibleModifications #329

Open simonbasle opened 2 years ago

simonbasle commented 2 years ago

Some context

In my project, I attempted to reduce the verbosity of the japicmp report by using onlyBinaryIncompatibleModified = true, instead of onlyModified = true.

While the default is too verbose for our taste (a lot of unchanged methods and classes are printed in the report, which doesn't help analysis when the build fails), this was seemingly resulting in the terse reports we were after...

But it was actually masking issues 😢 This resulted in our build missing a new (non-default) method being added to a public-facing interface, despite failOnModification = trueand failOnSourceIncompatibility = true being set...

(NB: we're using the melix gradle plugin, so the above options are actually respectively isOutputOnlyBinaryIncompatibleModifications, isOutputOnlyModifications, isErrorOnModifications and isErrorOnSourceIncompatibility)

Desired solution

isOutputOnlyModifications improves on the noisy aspect and doesn't suppress failures, but in case there isn't any incompatible change it still lists some things even though the build doesn't fail. For instance, adding a new public but self-contained class.

This issue is to suggest a middle ground between the two isOutputOnly*Modifications options which would result in a report that lists both types of incompatible changes (source and binary) and only these.

Put in other words, this option should result in an empty report if the build doesn't fail. (and of course it shouldn't circumvent the failOnSourceIncompatibility option).

Something like isOutputOnlyIncompatibleModifications.

What do you think?

siom79 commented 2 years ago

Hi,

japicmp already has three different levels:

Adding a new method to an existing interface is source incompatible but not binary incompatible (the JRE does not check at runtime if all methods of an interface are implemented, but will throw a runtime error if you actually invoke the new method without implementation). Hence; you can achieve the effect of isOutputOnlyIncompatibleModifications with breakBuildOnBinaryIncompatibleModifications=true and breakBuildOnSourceIncompatibleModifications=true. Or do I miss a thing?

simonbasle commented 2 years ago

Yes, if we consider only breakBuildOn* type of options options there seems to be full coverage. The trouble is with the isOutput*Modifications options. These don't exactly mirror the 3 break-the-build options, and actually can override them:

Given that the library can be configured to fail in the face of either binary or source incompatible changes, for the output I think an option that merely inherit from the breakBuildOn configuration to decide what to output ("incompatible changes") is what makes the most sense. Otherwise we'd need an exact mirror of the breakBuild options - effectively setting up each case twice (2 options to consider source incompatibilities in the comparison and in the report + 2 options to consider binary incompatibilities in the comparison and in the report).