lolgab / mill-mima

MiMa Plugin for Mill
https://lolgab.github.io/mill-mima/
Apache License 2.0
13 stars 3 forks source link

`mimaExcludeAnnotations` doesn't work with `object`s #41

Closed lolgab closed 2 years ago

lolgab commented 2 years ago

Since the implementation seems the same as Sbt Mima, my best guess is that there is some dependency clash that creates it. The next thing I'm going to try is to run the tool from an insulated classloader.

Edit

Blocked by https://github.com/lightbend/mima/issues/689

lefou commented 2 years ago

This might be related to

Also, once you isolate the MiMa classloader from Mill, you need to provide some other way to configure exclusions.

lolgab commented 2 years ago

I'm replicating the MiMa classes instances here and make the same API available (except for the mima imports). Let's see how it goes.

lolgab commented 2 years ago

Moved to a separate classloader https://github.com/lolgab/mill-mima/pull/42 but the problem persists. Then I reproduced the problem in sbt, so it is not a bug in the mill plugin. millExcludeAnnotations doesn't work with objects but only with classes

lefou commented 2 years ago

Oh, ok. Is this something not possible or is it just a missing feature from MiMa? Any links?

Lots of work to find out, but at least, it is not for nothing. Having MiMa as a worker will keep the Mill classloader lighter and allows for easier MiMa version bumps (without new mill-mima releases). :+1:

lolgab commented 2 years ago

Yes, I like the classloader change either way :-) I think we should make it easier to write plugins with the implementation in a separate classloader so it can become the new default.

About the Mima bug, I couldn't find anything, so I'm opening one right now.

lefou commented 2 years ago

Some of my annotations which I think were ignored were on defs in traits. The annotations were scala.annotation.StaticAnnotation. I don't know if this is visible enough for MiMa, or if they need to be written in Java. I didn't find any info about that.

lolgab commented 2 years ago

I think StaticAnnotations are fine because tests use them, but maybe traits aren't supported as well as objects. Also the annotation should be present already in the previous version, adding it at a later point doesn't work.

lefou commented 2 years ago

Thanks for these information.