Copilot commented on code in PR #12357: URL: https://github.com/apache/maven/pull/12357#discussion_r3470433415
########## impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/SourceStrategy.java: ########## @@ -0,0 +1,550 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.cling.invoker.mvnup.goals; + +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Element; +import org.apache.maven.api.cli.mvnup.UpgradeOptions; +import org.apache.maven.api.di.Named; +import org.apache.maven.api.di.Priority; +import org.apache.maven.api.di.Singleton; +import org.apache.maven.cling.invoker.mvnup.UpgradeContext; + +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.ARTIFACT_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.BUILD; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.CONFIGURATION; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.GROUP_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGINS; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_MANAGEMENT; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.SOURCE_DIRECTORY; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.TEST_SOURCE_DIRECTORY; +import static eu.maveniverse.domtrip.maven.MavenPomElements.ModelVersions.MODEL_VERSION_4_1_0; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Plugins.DEFAULT_MAVEN_PLUGIN_GROUP_ID; + +/** + * Strategy for migrating legacy source configuration to Maven 4.1.0+ {@code <source>} elements. + * + * <p>Handles four migration phases: + * <ol> + * <li>Compiler properties ({@code maven.compiler.release}, {@code maven.compiler.source/target}) + * → {@code <source><targetVersion>}</li> + * <li>Compiler plugin configuration ({@code <release>}, {@code <source>/<target>}) + * → {@code <source><targetVersion>}</li> + * <li>Custom source/test directories → {@code <source>} with {@code <directory>}</li> + * <li>Resource directories → {@code <source>} with {@code <lang>resources</lang>}</li> + * </ol> + */ +@Named +@Singleton +@Priority(20) +public class SourceStrategy extends AbstractUpgradeStrategy { + + private static final String MAVEN_COMPILER_RELEASE = "maven.compiler.release"; + private static final String MAVEN_COMPILER_SOURCE = "maven.compiler.source"; + private static final String MAVEN_COMPILER_TARGET = "maven.compiler.target"; + private static final String MAVEN_COMPILER_PLUGIN = "maven-compiler-plugin"; + + private static final String DEFAULT_SOURCE_DIR = "src/main/java"; + private static final String DEFAULT_TEST_SOURCE_DIR = "src/test/java"; + private static final String DEFAULT_RESOURCE_DIR = "src/main/resources"; + private static final String DEFAULT_TEST_RESOURCE_DIR = "src/test/resources"; + + @Override + public boolean isApplicable(UpgradeContext context) { + UpgradeOptions options = getOptions(context); + + if (options.all().orElse(false)) { + return true; + } + + String modelVersion = options.modelVersion().orElse(null); + return modelVersion != null && ModelVersionUtils.isVersionGreaterOrEqual(modelVersion, MODEL_VERSION_4_1_0); + } + + @Override + public String getDescription() { + return "Migrating source configuration to <source> elements"; + } + + @Override + protected UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap) { + Set<Path> processedPoms = new HashSet<>(); + Set<Path> modifiedPoms = new HashSet<>(); + Set<Path> errorPoms = new HashSet<>(); + + for (Map.Entry<Path, Document> entry : pomMap.entrySet()) { + Path pomPath = entry.getKey(); + Document pomDocument = entry.getValue(); + processedPoms.add(pomPath); + + String currentVersion = ModelVersionUtils.detectModelVersion(pomDocument); + context.info(pomPath + " (current: " + currentVersion + ")"); + context.indent(); + + try { + if (!ModelVersionUtils.isVersionGreaterOrEqual(currentVersion, MODEL_VERSION_4_1_0)) { + context.success("Skipping (model version " + currentVersion + " < 4.1.0)"); + continue; + } + + boolean hasChanges = migrateSources(context, pomDocument); + + if (hasChanges) { + modifiedPoms.add(pomPath); + context.success("Source configuration migrated to <source> elements"); + } else { + context.success("No source configuration to migrate"); + } + } catch (Exception e) { + context.failure("Failed to migrate source configuration: " + e.getMessage()); + errorPoms.add(pomPath); + } finally { + context.unindent(); + } + } + + return new UpgradeResult(processedPoms, modifiedPoms, errorPoms); + } + + private boolean migrateSources(UpgradeContext context, Document pomDocument) { + Element root = pomDocument.root(); + boolean hasChanges = false; + + String targetVersion = extractTargetVersionFromProperties(context, root); + + if (targetVersion == null) { + targetVersion = extractTargetVersionFromCompilerPlugin(context, root); + } else { + cleanupCompilerPluginConfig(context, root); + } + + if (targetVersion != null) { + Element sourcesElement = ensureSourcesElement(root); + Element sourceElement = DomUtils.insertNewElement("source", sourcesElement); + DomUtils.insertContentElement(sourceElement, "targetVersion", targetVersion); + context.detail("Set targetVersion: " + targetVersion); + hasChanges = true; + } Review Comment: When migrating `targetVersion`, this always inserts a new `<source>` element. If the POM already has an existing main/java `<build><sources><source>` (e.g. user already uses `<source><directory>…</directory></source>`), this will create a second main source with the default directory, potentially changing compilation inputs. Reuse/update the existing main/java source element instead of always creating a new one. ########## impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/SourceStrategy.java: ########## @@ -0,0 +1,550 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.cling.invoker.mvnup.goals; + +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Element; +import org.apache.maven.api.cli.mvnup.UpgradeOptions; +import org.apache.maven.api.di.Named; +import org.apache.maven.api.di.Priority; +import org.apache.maven.api.di.Singleton; +import org.apache.maven.cling.invoker.mvnup.UpgradeContext; + +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.ARTIFACT_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.BUILD; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.CONFIGURATION; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.GROUP_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGINS; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_MANAGEMENT; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.SOURCE_DIRECTORY; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.TEST_SOURCE_DIRECTORY; +import static eu.maveniverse.domtrip.maven.MavenPomElements.ModelVersions.MODEL_VERSION_4_1_0; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Plugins.DEFAULT_MAVEN_PLUGIN_GROUP_ID; + +/** + * Strategy for migrating legacy source configuration to Maven 4.1.0+ {@code <source>} elements. + * + * <p>Handles four migration phases: + * <ol> + * <li>Compiler properties ({@code maven.compiler.release}, {@code maven.compiler.source/target}) + * → {@code <source><targetVersion>}</li> + * <li>Compiler plugin configuration ({@code <release>}, {@code <source>/<target>}) + * → {@code <source><targetVersion>}</li> + * <li>Custom source/test directories → {@code <source>} with {@code <directory>}</li> + * <li>Resource directories → {@code <source>} with {@code <lang>resources</lang>}</li> + * </ol> + */ +@Named +@Singleton +@Priority(20) +public class SourceStrategy extends AbstractUpgradeStrategy { + + private static final String MAVEN_COMPILER_RELEASE = "maven.compiler.release"; + private static final String MAVEN_COMPILER_SOURCE = "maven.compiler.source"; + private static final String MAVEN_COMPILER_TARGET = "maven.compiler.target"; + private static final String MAVEN_COMPILER_PLUGIN = "maven-compiler-plugin"; + + private static final String DEFAULT_SOURCE_DIR = "src/main/java"; + private static final String DEFAULT_TEST_SOURCE_DIR = "src/test/java"; + private static final String DEFAULT_RESOURCE_DIR = "src/main/resources"; + private static final String DEFAULT_TEST_RESOURCE_DIR = "src/test/resources"; + + @Override + public boolean isApplicable(UpgradeContext context) { + UpgradeOptions options = getOptions(context); + + if (options.all().orElse(false)) { + return true; + } + + String modelVersion = options.modelVersion().orElse(null); + return modelVersion != null && ModelVersionUtils.isVersionGreaterOrEqual(modelVersion, MODEL_VERSION_4_1_0); + } + + @Override + public String getDescription() { + return "Migrating source configuration to <source> elements"; + } + + @Override + protected UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap) { + Set<Path> processedPoms = new HashSet<>(); + Set<Path> modifiedPoms = new HashSet<>(); + Set<Path> errorPoms = new HashSet<>(); + + for (Map.Entry<Path, Document> entry : pomMap.entrySet()) { + Path pomPath = entry.getKey(); + Document pomDocument = entry.getValue(); + processedPoms.add(pomPath); + + String currentVersion = ModelVersionUtils.detectModelVersion(pomDocument); + context.info(pomPath + " (current: " + currentVersion + ")"); + context.indent(); + + try { + if (!ModelVersionUtils.isVersionGreaterOrEqual(currentVersion, MODEL_VERSION_4_1_0)) { + context.success("Skipping (model version " + currentVersion + " < 4.1.0)"); + continue; + } + + boolean hasChanges = migrateSources(context, pomDocument); + + if (hasChanges) { + modifiedPoms.add(pomPath); + context.success("Source configuration migrated to <source> elements"); + } else { + context.success("No source configuration to migrate"); + } + } catch (Exception e) { + context.failure("Failed to migrate source configuration: " + e.getMessage()); + errorPoms.add(pomPath); + } finally { + context.unindent(); + } + } + + return new UpgradeResult(processedPoms, modifiedPoms, errorPoms); + } + + private boolean migrateSources(UpgradeContext context, Document pomDocument) { + Element root = pomDocument.root(); + boolean hasChanges = false; + + String targetVersion = extractTargetVersionFromProperties(context, root); + + if (targetVersion == null) { + targetVersion = extractTargetVersionFromCompilerPlugin(context, root); + } else { + cleanupCompilerPluginConfig(context, root); + } Review Comment: `cleanupCompilerPluginConfig()` removes `<release>` / `<source>` / `<target>` from the compiler plugin whenever a compiler property was migrated. If the plugin configuration intentionally differs from the properties, this changes the effective build configuration (maven-compiler-plugin config overrides property defaults). Consider only removing plugin config when it matches the migrated value, or prefer extracting from the plugin when both are present but differ (and/or warn and skip cleanup). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
