Green-card specific false-positive issues

[jsuereth]

The MiMa will have to stay conservative and err on the side of caution, so in order to integrate checking into the Akka build process we need a way to signal that certain well-understood issues are okay, while all others shall fail the build. Ideally such an exception record has to include the issue, the name of the investigator and the comment why it shall be ignored, and the list of exceptions should be part of the normal build settings.

[rkuhn]

The list should preferably be parsed from a plain-text file which is easy to version-control.

[havocp]

I'm thinking we put the list right in the sbt config. Basically you'd do:

   findBinaryIssues <<= findBinaryIssues map { unfiltered =>
       unfiltered filter { problem =>
           // screen out problems here
       }
   }

I guess Josh you're suggesting one more level of indirection, where we'd have:

 val ignoredBinaryIssues = TaskKey[List[core.ProblemFilter]]("mima-ignored-binary-issues", "Filters for binary compatibility issues to be ignored.")

And then we'd automatically use that list in the default findBinaryIssues? Sounds fine.

Is there any reason we need an external text file, assuming we have a nice little API or DSL to build up the list of ignored issues?

I'm thinking the API or DSL would let you match on:

• the type hierarchy in Problems.scala (e.g. MemberProblem or MissingFieldProblem)
• the relevant name(s), like class or member name
• previous-artifact (all issues become un-ignored by default if you change previous-artifact?)

Is that a good start? I'm sure the list of stuff to match on might grow over time. I think we could just allow you to write a function, to do unusual matching, so the convenience API/DSL would only need to support/encourage the common things.

Do we need the investigator/comment explaining the exceptions to be strings in the code, or could those be comments in your sbt build file?

I would have to mess with the syntax for this a bit. Maybe we want the syntax to generate something like:

type ProblemFilter = (Problem) => Boolean

class SimpleProblemFilter[T : ClassManifest](val symbolName: String, val oldVersion: String) extends ProblemFilter {
  override def apply(problem: Problem) = {
    // implement me
  }
}

The syntax would be to avoid typing SimpleProblemFilter[MissingFieldProblem]("foo", "1.2") since that's a bit noisy. You'd type something more like (MissingField, "foo", "1.2") perhaps. Or ProblemFilter("1.2", Seq((MissingField, "foo"))) ... i don't know, something along those lines.

Another aspect of this is, probably each time we print a problem, we should print a second line illustrating the problem filter you'd have to type in to ignore it? Otherwise it's probably a little bit annoying to figure out.

[havocp]

OK here is a possible patch: https://gist.github.com/2878457
And here is how you'd use it in Akka: https://gist.github.com/2878474 (minus explanatory comments)
You might want to move the list of filters into its own .scala file but that's obviously possible.

I didn't do the thing with specifying the old version the filter would apply to, because I couldn't come up with a great way to do it, and maybe it isn't useful anyway.

The API here is pretty cheesy but it does let you write an arbitrary function, so I guess there would at least be some way to do anything you want to do, and the exclude[FooProblem]("bar") should support all the common cases.

If you have an issue, it now tells you how to ignore it:

[error]  * method <<(akka.dispatch.PromiseStreamOut)scala.util.continuations.ControlContext in class akka.dispatch.DefaultPromise does not have a correspondent with same parameter signature among (akka.dispatch.Future)scala.util.continuations.ControlContext, (java.lang.Object)scala.util.continuations.ControlContext
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.dispatch.DefaultPromise.<<")
[error]  * class akka.dispatch.SharingMailbox does not have a correspondent in new version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("akka.dispatch.SharingMailbox")

You still get a count of ignored issues, like:

[info] akka-actor: found 353 potential binary incompatibilities (filtered 2)

and at debug loglevel (if you do last in sbt) you can get the details on filtered stuff:

[debug] akka-actor: filtered out: method <<(akka.dispatch.PromiseStreamOut)scala.util.continuations.ControlContext in class akka.dispatch.DefaultPromise does not have a correspondent with same parameter signature among (akka.dispatch.Future)scala.util.continuations.ControlContext, (java.lang.Object)scala.util.continuations.ControlContext
[debug]   filtered by: ExcludeByName[IncompatibleMethTypeProblem]("akka.dispatch.DefaultPromise.<<")
[debug] akka-actor: filtered out: class akka.dispatch.SharingMailbox does not have a correspondent in new version
[debug]   filtered by: ExcludeByName[MissingClassProblem]("akka.dispatch.SharingMailbox")

Anyway, feel free to suggest improvements. I guess this would solve the problem but it wouldn't hurt to spend a couple hours making it a little nicer if people have any ideas for that.

[jsuereth]

Looks good! comments in the Gist

Also, should we add the same feature to the UI and CLI?

On Tue, Jun 5, 2012 at 6:24 PM, Havoc Pennington <
reply@reply.github.com

wrote:

OK here is a possible patch: https://gist.github.com/2878457
And here is how you'd use it in Akka: https://gist.github.com/2878474(minus explanatory comments)
You might want to move the list of filters into its own .scala file but
that's obviously possible.

I didn't do the thing with specifying the old version the filter would
apply to, because I couldn't come up with a great way to do it, and maybe
it isn't useful anyway.

The API here is pretty cheesy but it does let you write an arbitrary
function, so I guess there would at least be some way to do anything you
want to do, and the exclude[FooProblem]("bar") should support all the
common cases.

If you have an issue, it now tells you how to ignore it:

[error]  * method
<<(akka.dispatch.PromiseStreamOut)scala.util.continuations.ControlContext
in class akka.dispatch.DefaultPromise does not have a correspondent with
same parameter signature among
(akka.dispatch.Future)scala.util.continuations.ControlContext,
(java.lang.Object)scala.util.continuations.ControlContext
[error]    filter with:
ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.dispatch.DefaultPromise.<<")
[error]  * class akka.dispatch.SharingMailbox does not have a
correspondent in new version
[error]    filter with:
ProblemFilters.exclude[MissingClassProblem]("akka.dispatch.SharingMailbox")

You still get a count of ignored issues, like:

[info] akka-actor: found 353 potential binary incompatibilities (filtered
2)

and at debug loglevel (if you do last in sbt) you can get the details on
filtered stuff:

[debug] akka-actor: filtered out: method
<<(akka.dispatch.PromiseStreamOut)scala.util.continuations.ControlContext
in class akka.dispatch.DefaultPromise does not have a correspondent with
same parameter signature among
(akka.dispatch.Future)scala.util.continuations.ControlContext,
(java.lang.Object)scala.util.continuations.ControlContext
[debug]   filtered by:
ExcludeByName[IncompatibleMethTypeProblem]("akka.dispatch.DefaultPromise.<<")
[debug] akka-actor: filtered out: class akka.dispatch.SharingMailbox does
not have a correspondent in new version
[debug]   filtered by:
ExcludeByName[MissingClassProblem]("akka.dispatch.SharingMailbox")

Anyway, feel free to suggest improvements. I guess this would solve the
problem but it wouldn't hurt to spend a couple hours making it a little
nicer if people have any ideas for that.

---

Reply to this email directly or view it on GitHub:

#3 (comment)

[havocp]

Cool, I sent a pull (unless you want to block on having UI/CLI patch)

I guess the UI and CLI raise some extra questions, like where you want to store your exceptions list (a file passed in on command line?) and in what format. Maybe we should open a new issue to track/discuss that?

[jsuereth]

Please open a new ticket, yeah....

On Wed, Jun 6, 2012 at 10:21 AM, Havoc Pennington <
reply@reply.github.com

wrote:

Cool, I sent a pull (unless you want to block on having UI/CLI patch)

I guess the UI and CLI raise some extra questions, like where you want to
store your exceptions list (a file passed in on command line?) and in what
format. Maybe we should open a new issue to track/discuss that?

---

Reply to this email directly or view it on GitHub:

#3 (comment)