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

[wip] new task to create exclusion files from issues #432

Conversation

jrudolph
Copy link
Contributor

@jrudolph jrudolph commented Dec 3, 2019

This is a WIP for a task to create exclusion files. We sometimes have the situation that you expect lots of exclusions because internal API was touched or similar in which case it is annoying to create those files manually from the console output, especially if multiple submodules are involved. This task creates the files directly where they should go and also add a bit of commentary for which versions they are needed.

This is still a WIP but since I don't know when I would get to finish it, I just dump it here.

Potential TODOs:

  • add a test case
  • make the task an input task to give a name for the files on the shell instead of having to use an environment var
  • optionally add comments with the full error description per issue, so it's easier to understand why it is needed

@jrudolph
Copy link
Contributor Author

jrudolph commented Dec 3, 2019

Here's a generated example file:

# Autogenerated MiMa filters, please pull branch and check validity, and add comments why they are needed etc.
# Don't merge like this.

# Incompatibilities against 2.5.17
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.util.ManifestInfo.get")

# Incompatibilities against 2.5.0, 2.5.1, 2.5.2, 2.5.3, 2.5.4, 2.5.5, 2.5.6, 2.5.7, 2.5.8, 2.5.9, 2.5.10, 2.5.11, 2.5.12, 2.5.13, 2.5.14, 2.5.15, 2.5.16, 2.5.17
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.actor.CoordinatedShutdown.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.actor.TypedActor.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.event.AddressTerminatedTopic.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.io.Dns.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.io.Tcp.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.io.Udp.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.io.UdpConnected.get")
ProblemFilters.exclude[IncompatibleResultTypeProblem]("akka.serialization.SerializationExtension.get")

diesalbla added a commit to Banno/vault4s that referenced this pull request Dec 3, 2019
The build for the versions 5.2.0 has been failing because
of a bug in lightbend-labs/mima#432.
To avoid falling into that bug, we add exclusion filters.
diesalbla added a commit to Banno/vault4s that referenced this pull request Dec 3, 2019
The build for the versions 5.2.0 has been failing because
of a bug in lightbend-labs/mima#432.
To avoid falling into that bug, we add exclusion filters.
@dwijnand
Copy link
Collaborator

dwijnand commented Dec 5, 2019

So, while I understand how this can be a productivity boost, I'm concerned about two things:

  1. It makes it too easy to create MiMa exclusions, which should be carefully looked at;
  2. In addition to the above, it's fixing a symptom of unresolved issues, issues like internal APIs shouldn't false positive and require exclusions

So I'd really like not to include this... :-/

If you're happy to pivot to another resolution for your issues, I'd be happy to collaborate on resolving them, I'm thinking:

(That last one I've been wanting to fix for a long time, but it's tricky because it requires adding a lot of logic to the already present partial-reimplementation of an Unpickler, to get to the package private detail - and it needs to be an Unpickler that works for all versions of Scala, 2.11-2.13 currently. But we can definitely spend time on it as an alternative to this PR.)

@jrudolph
Copy link
Contributor Author

jrudolph commented Dec 5, 2019

I agree in theory. The main reason for exclusions in akka projects is changes to internal classes, so fixing that would help a lot.

In practice, I don't have time to fix issues in mima and every so often need to create lots of exclusion like for akka/akka#28294 which was not caused by any of the above issues but probably by some other bug. In those cases, I need a way to appease mima with as little effort as possible (I still need make sure that the errors are harmless indeed which is lots of work in itself).

@dwijnand
Copy link
Collaborator

dwijnand commented Dec 5, 2019

changes to internal classes, so fixing that would help a lot.

That's annotated classes? We can definitely start there - I'll try to find time for it.

every so often need to create lots of exclusion like for akka/akka#28294 which was not caused by any of the above issues but probably by some other bug

That looks like it's #388, btw. Also, that PR seems to have had to pay back from the accidental misconfiguration of MiMa, which is unfortunate, but also exceptional.

I need a way to appease mima with as little effort as possible (I still need make sure that the errors are harmless indeed which is lots of work in itself).

I'd be fine for you and lots of other engineers, but not for the product in general... I'll think about it some more.

@jrudolph
Copy link
Contributor Author

jrudolph commented Dec 5, 2019

That looks like it's #388, btw. Also, that PR seems to have had to pay back from the accidental misconfiguration of MiMa, which is unfortunate, but also exceptional.

Agreed. I filed #433 because it seemed related but different enough.

@jrudolph
Copy link
Contributor Author

jrudolph commented Dec 5, 2019

I'd be fine for you and lots of other engineers, but not for the product in general...

Yes, maybe :)

@dwijnand
Copy link
Collaborator

Let's try and fix MiMa. 💪

@dwijnand dwijnand closed this Feb 18, 2020
@jrudolph
Copy link
Contributor Author

jrudolph commented Feb 18, 2020 via email

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

Successfully merging this pull request may close these issues.

2 participants