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

Use latest Mima and MMR extension #27

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

cstamas
Copy link
Contributor

@cstamas cstamas commented Sep 2, 2024

As pomchecker does way way too much for what it really needs. Building MavenProject just to get effective model is.... brrr, since Maven 3.1 there is ModelBuilder for that! Nevertheless, Mima MMR is made exactly for this use case (and is used in Toolbox as well).

Help needed: tame Gradle as it seems is not picking up new class from core?

There is NO NEED to build MavenProject, that in fact is not
even part of Maven public API. Moreover, the reason why
MavenProject is built is really just to grab "raw" and "effective"
models, that is in fact model builder job to do.

Clean up several other issues as well, drop the huge amount
of not needed and/or unused dependencies as well.
@cstamas
Copy link
Contributor Author

cstamas commented Sep 2, 2024

@aalmiray @gastaldi please take a peek

@gastaldi
Copy link
Contributor

gastaldi commented Sep 2, 2024

@cstamas interesting, does it solve #24 or is it just an upgrade that also fixes #22?

@cstamas
Copy link
Contributor Author

cstamas commented Sep 2, 2024

Solves both, but of course you STILL need to add that "staging directory" or whatever as RemoteRepository. All the POMs (like parents etc, that are staged and inter-connected) must be resolvable by Resolver....

* (parent, BOMs imported, etc) MUST BE RESOLVABLE. Hence, there is possibility to provide "extra" remote
* repositories.
*/
public static MavenProject createMavenProject(File pomFile, RemoteRepository... repositories) {
Copy link
Contributor

@aalmiray aalmiray Sep 2, 2024

Choose a reason for hiding this comment

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

Does this code resolve POMs where certain properties are activated via profiles? The old code made sure to setup required activation points.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, MIMA collects in same way the properties as Maven. OTOH, am really unsure what "os.detected.xxx" are, certainly are not set by Maven. Are you targeting here some specific plugin maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. That's why those oss.detected.xxx properties were in place, to make sure platform specific profiles were activated during resolution if needed. This means this PR is not behavior compatible.

@gastaldi
Copy link
Contributor

gastaldi commented Sep 2, 2024

Solves both, but of course you STILL need to add that "staging directory" or whatever as RemoteRepository. All the POMs (like parents etc, that are staged and inter-connected) must be resolvable by Resolver....

+1, regardless of the solution adopted, it should somehow have a way to add the staging directory as a remote repository (via a system property perhaps?)

@cstamas
Copy link
Contributor Author

cstamas commented Sep 4, 2024

Updated stuff with latest MIMA.

Anyone any idea why gradle build fails with

/home/cstamas/Worx/kordamp/pomchecker/pomchecker-gradle-plugin/src/main/groovy/org/kordamp/gradle/plugin/checker/internal/PomParser.java:26: error: cannot find symbol
import eu.maveniverse.maven.mima.extensions.mmr.ModelRequest;

@cstamas
Copy link
Contributor Author

cstamas commented Sep 4, 2024

os-maven-plugin "simulation" left in, but am unsure why that one plugin needs special treatment, or in other words any other why not? This seems too "narrowly tailored" to something... but what?

@cstamas
Copy link
Contributor Author

cstamas commented Sep 4, 2024

Also created separate mvnw PR, but this build seems to adjust enforcer to maven dependency (why?), so mvnw is here, but once other Pr is merged, irrelevant changes will be gone.

@aalmiray
Copy link
Contributor

aalmiray commented Sep 5, 2024

Enforcer is to to validate the correct version of Maven is used regardless of if the wrapper is used or not.

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.

3 participants