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

feat: Introduce itr option #1820

Merged
merged 3 commits into from
Nov 16, 2024
Merged

feat: Introduce itr option #1820

merged 3 commits into from
Nov 16, 2024

Conversation

cstamas
Copy link
Contributor

@cstamas cstamas commented Jul 6, 2024

Updates MIMA 2.4.16 (Maven 3.9.9) and introduces new (Resolver) feature to ignore remote repositories introduced by transitive POMs. In certain environments this may become essential. Read more here https://issues.apache.org/jira/browse/MNG-7980

This option is present in Maven 3.9.7+ and 4.0.0+ as "-itr" CLI.

@cstamas cstamas changed the title Introuduce itr option Introduce itr option Jul 6, 2024
@maxandersen
Copy link
Collaborator

interesting. i see the value of it - just wondering how gradle handles it ? do you know?

@cstamas
Copy link
Contributor Author

cstamas commented Jul 8, 2024

Sorry, but no idea how Gradle handles that...

@maxandersen
Copy link
Collaborator

@cstamas can you think of any case where we shouldn't just turn this off by default? like -simply just not honor transitive repos as it sounds like security issue?

@quintesse
Copy link
Contributor

So, my first reaction is "why not always enable this option?" But that's beause I assumed that any artifacts in Maven Central will only have dependencies on other artifacts in Maven Central. But @maxandersen tells me that's actually possible! Unbelievable :-)

Anyway, my second intuition is to have the opposite of what --itr does, be safe by default allow people to override if they really need it.

And thirdly this seems like such a finnicky highly technical feature that only 0.001% of our users is ever going to use that it seems like a bad way to spend "complexity budget" in our CLI. There's already too many very technical options.

So my vote would be for a config option instead of a CLI flag. Config options can be set using -D system properties so users can decide what value to use by default using config options or set it for a specific run by using -D.

@cstamas
Copy link
Contributor Author

cstamas commented Aug 8, 2024

It all boils down to "user experience": with enabled (as today), user has the comfort to not care at, it works but all the dangers is brought in. To turn it off, it means user needs to work more (dig, investigate, and setup all the reposes the build needs).

To me this is like: "in garage projects", setting as today is okay. But in "corporate projects" I'd cut off someone hands, if not turned off.

@quintesse
Copy link
Contributor

quintesse commented Aug 8, 2024

But realistically who will it affect in "garage projects" where not only they have a dependency on something not in Maven Central but that artifact then has a dependency on something else again that's only defined in that pom? It seems like a highly specific situation.

The ideal situation would be where we could detect that there are transitive repositories and be able to tell the user how to enable them if they really want to.

@cstamas
Copy link
Contributor Author

cstamas commented Aug 21, 2024

[cstamas@angeleyes ~]$ mvn eu.maveniverse.maven.plugins:toolbox:gav-list-repositories -Dgav=dev.jbang:jbang-cli:0.117.1
[INFO] Scanning for projects...
[INFO] 
[INFO] ------------------< org.apache.maven:standalone-pom >-------------------
[INFO] Building Maven Stub Project (No POM) 1
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- toolbox:0.2.0:gav-list-repositories (default-cli) @ standalone-pom ---
[INFO] Remote repositories used by GAV dev.jbang:jbang-cli:jar:0.117.1.
[INFO]  * central (https://repo.maven.apache.org/maven2/, default, releases)
[INFO]    First introduced on root
[INFO]  * apache.snapshots (https://repository.apache.org/snapshots, default, snapshots)
[INFO]    First introduced on org.apache.commons:commons-text:jar:1.11.0 (runtime)
[INFO]  * sonatype-snapshots (https://oss.sonatype.org/content/repositories/snapshots, default, releases+snapshots)
[INFO]    First introduced on io.smallrye.reactive:mutiny:jar:0.13.0 (runtime)
[INFO]  * microprofile-snapshots (https://repo.eclipse.org/content/repositories/microprofile-snapshots, default, snapshots)
[INFO]    First introduced on io.smallrye.reactive:mutiny:jar:0.13.0 (runtime)
[INFO]  * sonatype-nexus-snapshots (https://oss.sonatype.org/content/repositories/snapshots, default, snapshots)
[INFO]    First introduced on kr.motd.maven:os-maven-plugin:jar:1.7.1 (runtime)
[INFO]  * apache.snapshots.https (https://repository.apache.org/content/repositories/snapshots/, default, snapshots)
[INFO]    First introduced on eu.maveniverse.maven.mima:context:jar:2.4.12 (runtime)
[INFO]  * oss.sonatype.org/jboss-snapshots (http://oss.sonatype.org/content/repositories/jboss-snapshots, default, snapshots)
[INFO]    First introduced on javax.enterprise:cdi-api:jar:1.0 (runtime)
[INFO]    Mirrored by maven-default-http-blocker (http://0.0.0.0/, default, releases+snapshots, blocked)
[INFO]  * repository.jboss.org (http://repository.jboss.org/maven2, default, releases)
[INFO]    First introduced on javax.enterprise:cdi-api:jar:1.0 (runtime)
[INFO]    Mirrored by maven-default-http-blocker (http://0.0.0.0/, default, releases+snapshots, blocked)
[INFO]  * snapshots.jboss.org (http://snapshots.jboss.org/maven2, default, snapshots)
[INFO]    First introduced on javax.enterprise:cdi-api:jar:1.0 (runtime)
[INFO]    Mirrored by maven-default-http-blocker (http://0.0.0.0/, default, releases+snapshots, blocked)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.292 s
[INFO] Finished at: 2024-08-21T10:42:21+02:00
[INFO] ------------------------------------------------------------------------
[cstamas@angeleyes ~]$ 

@cstamas
Copy link
Contributor Author

cstamas commented Aug 21, 2024

When "in project" Toolbox offers these Mojos:

  • toolbox:list-repositories - Resolves transitively current project and outputs used repositories.
  • toolbox:plugin-list-repositories - Resolves transitively a project given plugin and outputs used repositories.

But am tinkering to add Mojo like "list-build-repositories" or "list-all-repositories" that would mesh everything (dependencies + plugins + whatever else) and list remote repositories found.

@maxandersen
Copy link
Collaborator

I'm missing how those output relates to itr option?

@maxandersen
Copy link
Collaborator

pushed rebased + actually making --itr available on run/build commands.

@maxandersen
Copy link
Collaborator

The tests keeps failing on windows in TestJDK's with problems deleting temporary directory.

No other PR's fails with this so I suspect its caused by the junit test updates so i rolled those back as I don't think they are needed for this change. @cstamas, right?

@maxandersen maxandersen changed the title Introduce itr option feat: Introduce itr option Nov 16, 2024
@maxandersen maxandersen merged commit 2e3f0db into jbangdev:main Nov 16, 2024
11 checks passed
@cstamas cstamas deleted the introduce-itr branch November 16, 2024 13:50
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