[MNG-7634] Revert MNG-5982 and MNG-7417

[jira-importer]

Michael Osipov opened MNG-7634 and commented

As layed out in MNG-7563, both issues though solve one problem, but cause a regression where direct user input manipulates dependency POMs in a way which is not clear/documented as such. As long as we cannot properly understand and document it, we must revert those changes.

---

Issue Links:

• MNG-7563 REGRESSION: User properties now override model properties in dependencies
  ("fixes")
• MNG-7635 Investigate how to safely propagate user properties to dependencies (see MNG-5982 and MNG-7417)
  ("causes")
• MNG-7417 Several classes do not set properties properly for building requests
  ("supercedes")
• MNG-5982 The POM for ... is invalid, transitive dependencies ... while property was overriden
  ("supercedes")

Remote Links:

• GitHub Pull Request #916
  
• GitHub Pull Request #917
  

Backported to: 4.0.0-alpha-4, 3.9.0, 3.8.7

[jira-importer]

Michael Osipov commented

Hervé Guillemet, here it is: #916. Will create two more branches for 3.9.x and 3.8.x. Waiting for your IT PR and then we are good to merge. Please try to build from that branch and verify that your sample is working again.

[jira-importer]

Hervé Guillemet commented

I should be able to work on this PR on monday.

[jira-importer]

Michael Osipov commented

Looking forward to.

[jira-importer]

Hervé Guillemet commented

I'm realizing that you already have a test that implements something similar: MavenITmng4913UserPropertyVsDependencyPomPropertyTest.java.

See MNG-4913

This test:

• passes with 3.8.4
• fails with 3.8.6
• passes with branch MNG-7634_maven-3.9.x
• fails with branch maven-3.9.x

[jira-importer]

Michael Osipov commented

Strange, it did not fail for me. Let me check.

[jira-importer]

Michael Osipov commented

I am totally embarassed that I didn't check the history. Thank your very much again for the finding and the engagement with me!

[jira-importer]

Michael Osipov commented

Well, I did not fully fail. The test is flawed, badly designed. This is how it should fail:

osipovmi@deblndw011x:~/var/Projekte/mit (master *=)
$ cat core-it-suite/target/test-classes/mng-4913/log.txt
[INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO]
[INFO] -----------------< org.apache.maven.its.mng4913:test >------------------
[INFO] Building Maven Integration Test :: MNG-4913 0.1
[INFO]   from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] Downloading from central: file:target/null/org/apache/maven/its/mng4913/b/0.1/b-0.1.pom
[INFO] Downloading from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/b/0.1/b-0.1.pom
[INFO] Downloaded from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/b/0.1/b-0.1.pom (1.4 kB)
[INFO] Downloading from central: file:target/null/org/apache/maven/its/mng4913/a/98.76/a-98.76.pom
[INFO] Downloading from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/a/98.76/a-98.76.pom
[WARNING] The POM for org.apache.maven.its.mng4913:a:jar:98.76 is missing, no dependency information available
[INFO] Downloading from central: file:target/null/org/apache/maven/its/mng4913/b/0.1/b-0.1.jar
[INFO] Downloading from central: file:target/null/org/apache/maven/its/mng4913/a/98.76/a-98.76.jar
[INFO] Downloading from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/b/0.1/b-0.1.jar
[INFO] Downloading from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/a/98.76/a-98.76.jar
[INFO] Downloaded from maven-core-it: file:///var/osipovmi/Projekte/maven-integration-testing/core-it-suite/target/test-classes/mng-4913/repo/org/apache/maven/its/mng4913/b/0.1/b-0.1.jar (2.0 kB at 2.0 MB/s)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.023 s
[INFO] Finished at: 2022-12-19T22:33:22+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project test: Could not resolve dependencies for project org.apache.maven.its.mng4913:test:jar:0.1: Could not find artifact org.apache.maven.its.mng4913:a:jar:98.76 in central (file:target/null) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal on project test: Could not resolve dependencies for project org.apache.maven.its.mng4913:test:jar:0.1: Could not find artifact org.apache.maven.its.mng4913:a:jar:98.76 in central (file:target/null)
    at org.apache.maven.lifecycle.internal.LifecycleDependencyResolver.getDependencies (LifecycleDependencyResolver.java:248)
    at org.apache.maven.lifecycle.internal.LifecycleDependencyResolver.resolveProjectArtifacts (LifecycleDependencyResolver.java:180)
    at org.apache.maven.lifecycle.internal.LifecycleDependencyResolver.resolveProjectDependencies (LifecycleDependencyResolver.java:118)
    at org.apache.maven.lifecycle.internal.MojoExecutor.ensureDependenciesAreResolved (MojoExecutor.java:361)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:319)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:172)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:160)

The first problem: MavenITmng4913UserPropertyVsDependencyPomPropertyTest contains user property, but the code says verifier.setSystemProperty( "mng4913.version", "98.76" );. Second problem: When run in embedded mode the current JVM's system propeties are modified altough the ticket is about user properties. In fork mode the "system properties" are passed as user properties which are unfortunately promoted to system properties. Total chaos.

This is decent fix:

$ git diff
diff --git a/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4913UserPropertyVsDependencyPomPropertyTest.java b/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4913UserPropertyVsDependencyPomPropertyTest.java
index b948c8f43..aa80c2f53 100644
--- a/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4913UserPropertyVsDependencyPomPropertyTest.java
+++ b/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4913UserPropertyVsDependencyPomPropertyTest.java
@@ -56,9 +56,9 @@ public class MavenITmng4913UserPropertyVsDependencyPomPropertyTest
         verifier.setAutoclean( false );
         verifier.deleteDirectory( "target" );
         verifier.deleteArtifacts( "org.apache.maven.its.mng4913" );
-        verifier.setSystemProperty( "mng4913.version", "98.76" );
         verifier.addCliOption( "-s" );
         verifier.addCliOption( "settings.xml" );
+        verifier.addCliOption( "-Dmng4913.version=98.76" );
         verifier.filterFile( "settings-template.xml", "settings.xml", "UTF-8", verifier.newDefaultFilterProperties() );
         verifier.executeGoal( "validate" );
         verifier.verifyErrorFreeLog();

[jira-importer]

Michael Osipov commented

sjaranowski, one of the reasons we must fix the user/system properties chaos.

[jira-importer]

Michael Osipov commented

Checked, now the diff also successfully fails in IT fork mode. Exactly the way it was intended. Hervé Guillemet, no need to create a IT PR. I will modify the IT, done.

[jira-importer]

Michael Osipov commented

Side note, I consider that most of these are incorrectly used:

^[[Aosipovmi@deblndw011x:~/var/Projekte/mit (master *=)
$ grep -r  setSystemProperty core-it-suite/src/test/java/
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4428FollowHttpRedirectTest.java:            verifier.setSystemProperty( "javax.net.ssl.trustStore", storePath );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4428FollowHttpRedirectTest.java:            verifier.setSystemProperty( "javax.net.ssl.trustStorePassword", storePwd );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0848SystemPropOverridesDefaultValueTest.java:        verifier.setSystemProperty( "config.aliasDefaultExpressionParam", "PASSED" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0786ProfileAwareReactorTest.java:        verifier.setSystemProperty( "expression.outputFile", "target/expression.properties" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0786ProfileAwareReactorTest.java:        verifier.setSystemProperty( "activate", "anything" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2136ActiveByDefaultProfileTest.java:        verifier.setSystemProperty( "expression.outputFile", new File( testDir, "target/expression.properties" ).getPath() );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2136ActiveByDefaultProfileTest.java:        verifier.setSystemProperty( "expression.expressions", "project/properties" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2305MultipleProxiesTest.java:            verifier.setSystemProperty( "javax.net.ssl.trustStore", storePath );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2305MultipleProxiesTest.java:            verifier.setSystemProperty( "javax.net.ssl.trustStorePassword", storePwd );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2305MultipleProxiesTest.java:            verifier.setSystemProperty( "maven.artifact.threads", "1" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng2577SettingsXmlInterpolationTest.java:        verifier.setSystemProperty( "usr.MNGIT", "usr-prop-test" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4936EventSpyTest.java:        verifier.setSystemProperty( "maven.ext.class.path", "spy-0.1.jar" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3004ReactorFailureBehaviorMultithreadedTest.java:        verifier.setSystemProperty( "maven.threads.experimental", "1" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3004ReactorFailureBehaviorMultithreadedTest.java:        verifier.setSystemProperty( "maven.threads.experimental", "1" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3004ReactorFailureBehaviorMultithreadedTest.java:        verifier.setSystemProperty( "maven.threads.experimental", "1" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3004ReactorFailureBehaviorMultithreadedTest.java:        verifier.setSystemProperty( "maven.threads.experimental", "2" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3004ReactorFailureBehaviorMultithreadedTest.java:        verifier.setSystemProperty( "maven.threads.experimental", "2" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3379ParallelArtifactDownloadsTest.java:        verifier.setSystemProperty( "maven.artifact.threads", "16" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0377PluginLookupFromPrefixTest.java:        verifier.setSystemProperty( "touch.outputFile", "target/file.txt" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.type", "jar" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.classifier", "unix" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.type", "jar" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.classifier", "win" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.type", "war" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4452ResolutionOfSnapshotWithClassifierTest.java:        verifier.setSystemProperty( "mng4452.classifier", "win" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "file", "settings.xml" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "groupId", "org.apache.maven.its.mng4469" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "artifactId", "it" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "version", "0.1" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "repositoryId", "mng4469" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4469AuthenticatedDeploymentToCustomRepoTest.java:        verifier.setSystemProperty( "repositoryUrl", "http://localhost:" + port + "/repo" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4590ImportedPomUsesSystemPropertiesTest.java:        verifier.setSystemProperty( "test.dir", testDir.getAbsolutePath() );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4615ValidateRequiredPluginParameterTest.java:        verifier.setSystemProperty( "config.requiredParam", "CLI" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4625SettingsXmlInterpolationWithXmlMarkupTest.java:            verifier.setSystemProperty( "test.prop", "\"&x=y<>\"" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4625SettingsXmlInterpolationWithXmlMarkupTest.java:            verifier.setSystemProperty( "test.prop", "&x=y<>" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4765LocalPomProjectBuilderTest.java:        verifier.setSystemProperty( "user.prop", "OK" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5011ConfigureCollectionArrayFromSystemPropTest.java:        verifier.setSystemProperty( "config.stringParams", "" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5011ConfigureCollectionArrayFromSystemPropTest.java:        verifier.setSystemProperty( "config.fileParams", "foo,bar" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5011ConfigureCollectionArrayFromSystemPropTest.java:        verifier.setSystemProperty( "config.listParam", ",two,,four," );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5208EventSpyParallelTest.java:        verifier.setSystemProperty( "maven.ext.class.path", "../spy/target/event-spy-0.0.1-SNAPSHOT.jar" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0294MergeGlobalAndUserSettingsTest.java:            verifier.setSystemProperty( "org.apache.maven.global-settings", "global-settings.xml" );
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng6558ToolchainsBuildingEventTest.java:        verifier.setSystemProperty( "maven.ext.class.path", "spy-0.1.jar" );

[jira-importer]

Slawomir Jaranowski commented

This is to resolve: MSHARED-1132

[jira-importer]

Michael Osipov commented

Fixed with 968c1a25f5535c7f53e273ec1b9ad499fd4f1dee, with 2dbee43408d4473dd5cfacff254ab2a592dde9db for maven-3.9.x branch, with with 7db942b2f7387f783d4530e2ee63897c204dd904 for maven-3.8.x branch, ITs with 6906ad0704dd3e0d7a29b70a03640b3523eb59b7, ITs with 4f4f8439f2d1e9967377feb3a463aa59b1fb4e8c for maven-3.8.x branch.