-
Notifications
You must be signed in to change notification settings - Fork 448
Support settings.xml in rewriteRun #5873
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
base: main
Are you sure you want to change the base?
Conversation
@@ -139,6 +140,9 @@ private MavenPomDownloader(Map<Path, Pom> projectPoms, HttpSender httpSender, Ex | |||
this.projectPomsByGav = projectPomsByGav(projectPoms); | |||
this.httpSender = httpSender; | |||
this.ctx = MavenExecutionContextView.view(ctx); | |||
if (ctx.getMessage(MAVEN_SETTINGS_LOAD_FROM_DISK, false)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now made it work with this flag. You could argue though, if there is no maven-settings available in the context, you always want to read the maven settings from local. What do you guys think?
@@ -378,6 +379,7 @@ default void rewriteRun(Consumer<RecipeSpec> spec, SourceSpec<?>... sourceSpecs) | |||
recipeCtx = CursorValidatingExecutionContextView.view(recipeCtx) | |||
.setValidateCursorAcyclic(TypeValidation.before(testMethodSpec, testClassSpec).cursorAcyclic()) | |||
.setValidateImmutableExecutionContext(TypeValidation.before(testMethodSpec, testClassSpec).immutableExecutionContext()); | |||
recipeCtx.putMessage(MAVEN_SETTINGS_LOAD_FROM_DISK, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also have to set this in the gradle/maven plugins or do they already do MavenSettings.readMavenSettingsFromDisk(ctx)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess they don't have to, because these plugins will just start running the tests. The tests themselves call the rewriteRun
functions, which in turn will just set the MAVEN_SETTINGS_LOAD_FROM_DISK
property. With other words, it does not really matter how/which build tool starts the tests, as you don't need any configuration from outside the scope of the tests.
What's changed?
The
rewriteRun
test runner now respects the local settings.xml file from the Maven configuration directory (~/.m2/settings.xml by default).What's your motivation?
The settings.xml file is often used to define mirrors, which can redirect all artifact requests to an internal repository manager such as Artifactory. For example:
For some company environments, direct access to external repositories such as https://repo.maven.apache.org/maven2 is blocked (often returning a 403). Without support for settings.xml,
rewriteRun
tests would fail in such environments because dependencies could not be resolved.Additionally, this change supports scenarios where mirrors are used to discard certain repositories, for example:
Checklist