Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add middle ground between isOutputOnlyModifications and isOutputOnlyBinaryIncompatibleModifications #329

Open
simonbasle opened this issue Jul 1, 2022 · 2 comments

Comments

@simonbasle
Copy link

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 = true and 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
Copy link
Owner

siom79 commented Jul 4, 2022

Hi,

japicmp already has three different levels:

  • breakBuildOnModifications: any modification
  • breakBuildOnBinaryIncompatibleModifications: any binary incompatible modification
  • breakBuildOnSourceIncompatibleModifications: any source incompatible modification

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
Copy link
Author

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:

  • when using isOutputOnlyBinaryIncompatibleModifications, the output only contains binary changes. But what's worse is that the build no longer even breaks if there are only source incompatible changes.
  • when using isOutputOnlyModifications the build breaks as expected, but the content of the report is still a bit too large (including changes that are compatible)

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants