[ 
https://issues.apache.org/jira/browse/MNG-7596?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17652610#comment-17652610
 ] 

ASF GitHub Bot commented on MNG-7596:
-------------------------------------

guylabs commented on code in PR #866:
URL: https://github.com/apache/maven/pull/866#discussion_r1058637665


##########
maven-xml-impl/src/main/java/org/apache/maven/internal/xml/Xpp3Dom.java:
##########
@@ -180,8 +181,6 @@ public void writeToSerializer(String namespace, 
XmlSerializer serializer) throws
      *   </ol></li>
      * <li> If mergeSelf == true
      *   <ol type="A">
-     *   <li> if the dominant root node's value is empty, set it to the 
recessive root node's value</li>
-     *   <li> For each attribute in the recessive root node which is not set 
in the dominant root node, set it.</li>

Review Comment:
   @gnodet Why has this behavior been removed? Is there a migration path that 
you can share?
   
   The following example has worked before that change where we have a parent 
pom and a child pom and a configuration that we'd like to merge. Here the 
parent and child pom definitions:
   
   ```xml
   <!--parent pom-->
     ...
   <pluginManagement>
     <plugins>
       <plugin>
         <groupId>foo.bar</groupId>
         <artifactId>foo-bar-plugin</artifactId>
         <configuration>
           <plugins>
             <plugin>
               <groupId>org.apache.maven.plugins</groupId>
               <artifactId>maven-compiler-plugin</artifactId>
               <bar>
                 <value>foo</value>
               </bar>
             </plugin>
             <plugin>
               <groupId>org.apache.maven.plugins</groupId>
               <artifactId>maven-surefire-plugin</artifactId>
               <foo>
                 <properties>
                   <property>
                     <name>prop1</name>
                     <value>value1</value>
                   </property>
                 </properties>
               </foo>
             </plugin>
           </plugins>
         </configuration>
       </plugin>
     </plugins>
   </pluginManagement>
     ...
     <!--child pom-->
     ...
   <pluginManagement>
   <plugins>
     <plugin>
       <groupId>foo.bar</groupId>
       <artifactId>foo-bar-plugin</artifactId>
       <configuration>
         <plugins>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-compiler-plugin</artifactId>
           </plugin>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
             <foo>
               <properties combine.children="append">
                 <property>
                   <name>prop2</name>
                   <value>value2</value>
                 </property>
               </properties>
             </foo>
           </plugin>
         </plugins>
       </configuration>
     </plugin>
   </plugins>
   </pluginManagement>
     ...
   ```
   
   And the expected effective child pom that is used after the merge:
   
   ```xml
     <!--expected effective child pom-->
     ...
   <pluginManagement>
   <plugins>
     <plugin>
       <groupId>foo.bar</groupId>
       <artifactId>foo-bar-plugin</artifactId>
       <configuration>
         <plugins>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-compiler-plugin</artifactId>
             <bar>
               <value>foo</value>
             </bar>
           </plugin>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
             <foo>
               <properties combine.children="append">
                 <property>
                   <name>prop1</name>
                   <value>value1</value>
                 </property>
                 <property>
                   <name>prop2</name>
                   <value>value2</value>
                 </property>
               </properties>
             </foo>
           </plugin>
         </plugins>
       </configuration>
     </plugin>
   </plugins>
   </pluginManagement>
     ...
   ```
   
   The problem we see now after this change, is that it will be merged the 
following way:
   
   ```xml
   <!--expected effective child pom after this change-->
     ...
   <pluginManagement>
   <plugins>
     <plugin>
       <groupId>foo.bar</groupId>
       <artifactId>foo-bar-plugin</artifactId>
       <configuration>
         <plugins>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-compiler-plugin</artifactId>
             <foo>
               <properties>
                 <property>
                   <name>prop1</name>
                   <value>value1</value>
                 </property>
               </properties>
             </foo>
           </plugin>
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
             <foo>
               <properties combine.children="append">
                 <property>
                   <name>prop2</name>
                   <value>value2</value>
                 </property>
               </properties>
             </foo>
           </plugin>
         </plugins>
       </configuration>
     </plugin>
   </plugins>
   </pluginManagement>
     ...
   ```
   
   The problems are the following:
   
   1. On the `maven-compiler-plugin` configuration the `<bar>` node is not 
merged from the parent pom configuration but rather the one from the 
`maven-surefire-plugin` configuration from the parent pom.
   2. The `maven-surefire-plugin` configuration not as expected as the children 
of the `<properties>` node are not appended as configured via the 
`combine.children="append"` property.
   
   Is that changed something that stays for Maven 4 ? I assume that then a 
migration path should be given as there are projects that use the behavior 
before this change and it would result in different effective pom files, which 
will change how the build is configured.
   
   I'm also open to chat about it on the ASF Slack if required or if you have 
further questions. 





> Upgrade to plexus-utils 3.5.0
> -----------------------------
>
>                 Key: MNG-7596
>                 URL: https://issues.apache.org/jira/browse/MNG-7596
>             Project: Maven
>          Issue Type: Dependency upgrade
>            Reporter: Guillaume Nodet
>            Assignee: Guillaume Nodet
>            Priority: Major
>             Fix For: 4.0.0-alpha-3, 4.0.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to