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

Investigate Maven 3.9.0 #2084

Closed
wants to merge 3 commits into from
Closed

Investigate Maven 3.9.0 #2084

wants to merge 3 commits into from

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Feb 10, 2023

No description provided.

dependabot bot and others added 2 commits February 6, 2023 23:16
Bumps `maven-version` from 3.8.7 to 3.9.0.

Updates `maven-plugin-api` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-core` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-artifact` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-compat` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-model` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-settings` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-model-builder` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `apache-maven` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

Updates `maven-embedder` from 3.8.7 to 3.9.0
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.8.7...maven-3.9.0)

---
updated-dependencies:
- dependency-name: org.apache.maven:maven-plugin-api
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-artifact
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-compat
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-model
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-settings
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-model-builder
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:apache-maven:bin
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.apache.maven:maven-embedder
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@akurtakov
Copy link
Member

What's the issue with null annotations?

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

It seems maven no longer use them, and I can't see any wider use in Tycho nor we loosing anything so it seems better to just remove them see also https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use it seems there is no common standard to use anyways. So I think Objects.requireNonNull() is more appropriate (and if I understand @stephan-herrmann JDT understand this if we ever want to enable null analysis).

@akurtakov
Copy link
Member

I can only hope that jakarta.annotation is gonna absorb all these.

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

I can only hope that jakarta.annotation is gonna absorb all these.

Probably but still then theses four places won't change the world, so if we decide it should be used we really need to use it everywhere...

@akurtakov
Copy link
Member

Yeah, my question originated by the fact that maven 3.9.0 ships javax.annotation-api-1.3.2.jar in libs thus I wondered why this is needed.

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

I have not checked in details why this is no longer a (transitive) dependency, so maybe it would equally work to add an explicit one (they maybe changed it to provided or optional or ...), but unless we have any benefit it might be better to just remove that at all as it seems not used elsewhere.

@lppedd
Copy link
Contributor

lppedd commented Feb 10, 2023

Talking about nullability annotations, I use https://github.com/osundblad/intellij-annotations-instrumenter-maven-plugin to avoid manually checking for preconditions. The plugin unfortunately lost a bit of traction for latest Java releases.

I'm an avid user of @NotNull/@Nullable and other annotations, especially because the IDE (currently only IntelliJ) can use them to infer inspections data.

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

especially because the IDE (currently only IntelliJ) can use them to infer inspections data.

And the highlighted part makes then essentially useless.. I really don't get why everyone needs to invent its own "nullable" annotations here, that's a complete mess. Anyways this PR is not really about the annotations, the benefit of those for Tycho are currently just non existent.

@github-actions
Copy link

github-actions bot commented Feb 10, 2023

Test Results

362 files  +2  362 suites  +2   2h 39m 59s ⏱️ + 23m 19s
332 tests +1  320 ✔️ ±0  10 💤 ±0  0 ±0  2 🔥 +1 
664 runs  +2  639 ✔️  - 1  21 💤 ±0  0 ±0  4 🔥 +3 

For more details on these errors, see this check.

Results for commit 2323359. ± Comparison against base commit 0a5a67b.

♻️ This comment has been updated with latest results.

@@ -15,8 +15,6 @@

import java.util.Objects;

import javax.annotation.Nonnull;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer using javax.annotation.Nonnull is a good move, but removal looks like a step backwards, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that (I haven counted them) there are many hundreds of file not suing it compared to only these few its jsut a baby-step :-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a baby-step backwards, so not a big deal indeed, fair enough :)

@stephan-herrmann
Copy link

I can only hope that jakarta.annotation is gonna absorb all these.

They will still have a very long road ahead, before they can "absorb" existing annotations:

  • specification is very far from sufficient as a standard
  • apparently not intended as TYPE_USE annotations, which would be the minimum requirement these days
  • to the best of my knowledge not coordinated with prior art in checkers-framework, JDT, nor jspecify.

I'm open to discuss with them. If that would finally become the standard (which long ago JSR-305 had promised without delivering) - I'm all for it, but looking from today I don't see that.

@stephan-herrmann
Copy link

I'm an avid user of @NotNull/@Nullable and other annotations, especially because the IDE (currently only IntelliJ) can use them to infer inspections data.

If there is anything in this regard which IntelliJ supports, but not JDT, please file an issue against JDT/Core. Last time I looked they were still stuck on the inferior declaration annotations (as opposed to TYPE_USE annotations).

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

I think most of the failures at the moment are cause because https://github.com/takari/polyglot-maven does not support Maven 3.9.0, so if anyone likes to step up and propose a fix?
I'll also try to convince the maven-devs to probably "adopt" the code so it became part of maven-core, but this can be a longer process...

@laeubi
Copy link
Member Author

laeubi commented Feb 10, 2023

@laeubi laeubi mentioned this pull request Feb 11, 2023
Currently polyglot do not work with maven 3.9 because it extends an
internal component ... this extends the polyglot component to enbedd
updated metadata in tycho itself.

See:
- takari/polyglot-maven#256
@laeubi
Copy link
Member Author

laeubi commented Feb 13, 2023

Alright, Tycho can now build with Maven 3.9.0, all integration tests run and platform checks are green.

@laeubi
Copy link
Member Author

laeubi commented Feb 14, 2023

Adjustments have men merged to master now, and we can use the dependabot branch again as-is

@laeubi laeubi closed this Feb 14, 2023
@laeubi laeubi added this to the 4.0 milestone Jun 24, 2023
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

Successfully merging this pull request may close these issues.

4 participants