Skip to content

added maven version info when ERROR or FATAL severity #8657

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

homberghp
Copy link
Contributor

@homberghp homberghp commented Jul 15, 2025

The Maven version info that parses the pom file is not visible. It may confuse to the user.
Adds the maven version to the message generated by the pom parsing maven (the bundled one).


^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

The maven version that did the pom file parsing is not visible
and is confusing to the user. Add the maven version to the message
generated by the pom parsing maven (the bundled one).
@homberghp homberghp force-pushed the mavenversionreport branch from 5ec5c1e to 65bee9d Compare July 16, 2025 07:36
@homberghp homberghp force-pushed the mavenversionreport branch from 48446d4 to 1a6748d Compare July 16, 2025 07:38
Copy link
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

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

I think in general this is a good idea. Originally I thought we should only rewrite the message if it detects 4.1.0. On the other hand the more generic approach could stay in and be potentially useful in more situations.

A few things to consider -

  • the message should really use @NbBundle.Message to format (eg. like https://github.com/apache/netbeans/blob/master/java/maven.hints/src/org/netbeans/modules/maven/hints/pom/UpdateDependencyHint.java#L65 )
  • Maybe "bundled" should be "embedded"/ "internal"? Is FATAL here always complete failure to parse POM? Need to be clearer this is internal IDE error? Maybe something like "Failed to parse POM with internal Maven x.x.x : " + problem.getMessage()?
  • this module is Java 17 already(?) so better to use newer switch / case syntax.
  • the getActiveMavenVersion() might be wrong here? It's using EmbedderFactory::getMavenHome but for this should be EmbedderFactory::getDefaultMavenHome?
  • please fix the formatting (maybe select the changed area and use ALT+SHIFT+F with default settings).

@mbien mbien added the Maven [ci] enable "build tools" tests label Jul 16, 2025
@mbien
Copy link
Member

mbien commented Jul 16, 2025

this can likely be removed again when the mvn4 upgrade happens, otherwise the version string will show up multiple times among the same error report with a similar message which might be a bit much. (apache/maven#10921)

@neilcsmith-net
Copy link
Member

@mbien sure, but in that case we should String match and augment only that message, not all error and fatal problems? I can see some merit to being explicit on all major problems that it relates to the embedded Maven version, not the configured build version. OTOH, also happy to have a low key, temporary, quick fix for this particular error message. That was the original suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maven [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants