Skip to content

Fix #12080: mvnup - comment out dependencies with undefined property expressions #12083

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.

Sign up for GitHub

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

Merged
gnodet merged 2 commits into from
May 26, 2026
Merged

Fix #12080: mvnup - comment out dependencies with undefined property expressions #12083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e954db
Extract shared session infrastructure into AbstractUpgradeStrategy
gnodet May 26, 2026
da17364
Fix #12080: mvnup - comment out dependencies with undefined property …
gnodet May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions ...cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,41 @@
*/
package org.apache.maven.cling.invoker.mvnup.goals;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.HashMap;
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 eu.maveniverse.domtrip.maven.Coordinates;
import eu.maveniverse.domtrip.maven.MavenPomElements;
import org.apache.maven.api.RemoteRepository;
import org.apache.maven.api.Session;
import org.apache.maven.api.cli.mvnup.UpgradeOptions;
import org.apache.maven.api.di.Named;
import org.apache.maven.api.di.Provides;
import org.apache.maven.api.model.Repository;
import org.apache.maven.api.model.RepositoryPolicy;
import org.apache.maven.api.services.ModelBuilder;
import org.apache.maven.api.services.ModelBuilderRequest;
import org.apache.maven.api.services.ModelBuilderResult;
import org.apache.maven.api.services.RepositoryFactory;
import org.apache.maven.api.services.Sources;
import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
import org.apache.maven.impl.standalone.ApiRunner;
import org.codehaus.plexus.components.secdispatcher.Dispatcher;
import org.codehaus.plexus.components.secdispatcher.internal.dispatchers.LegacyDispatcher;
import org.eclipse.aether.spi.connector.transport.TransporterFactory;
import org.eclipse.aether.spi.connector.transport.http.ChecksumExtractor;
import org.eclipse.aether.spi.io.PathProcessor;
import org.eclipse.aether.transport.file.FileTransporterFactory;
import org.eclipse.aether.transport.jdk.JdkTransporterFactory;

import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PARENT;

Expand All @@ -47,6 +70,8 @@
*/
public abstract class AbstractUpgradeStrategy implements UpgradeStrategy {

private Session session;

/**
* Template method that handles common logging and error handling.
* Subclasses implement the actual upgrade logic in doApply().
Expand Down Expand Up @@ -185,4 +210,115 @@ public static Set<Coordinates> computeAllArtifactCoordinates(UpgradeContext cont
context.info("Computed " + coordinatesByGAV.size() + " unique artifact(s) for inference");
return new HashSet<>(coordinatesByGAV.values());
}

protected Session getSession() {
if (session == null) {
session = createMaven4Session();
}
return session;
}

private Session createMaven4Session() {
Session session = ApiRunner.createSession(injector -> {
injector.bindInstance(Dispatcher.class, new LegacyDispatcher());
injector.bindImplicit(TransporterFactoryConfig.class);
});

// TODO: we should read settings
RemoteRepository central =
session.createRemoteRepository(RemoteRepository.CENTRAL_ID, "https://repo.maven.apache.org/maven2");
RemoteRepository snapshots = session.getService(RepositoryFactory.class)
.createRemote(Repository.newBuilder()
.id("apache-snapshots")
.url("https://repository.apache.org/content/repositories/snapshots/")
.releases(RepositoryPolicy.newBuilder().enabled("false").build())
.snapshots(RepositoryPolicy.newBuilder().enabled("true").build())
.build());

return session.withRemoteRepositories(List.of(central, snapshots));
}

protected Path createTempProjectStructure(UpgradeContext context, Map<Path, Document> pomMap) throws Exception {
Path tempDir = Files.createTempDirectory("mvnup-project-");
context.debug("Created temp project directory: " + tempDir);

Path commonRoot = findCommonRoot(pomMap.keySet());
context.debug("Common root: " + commonRoot);

for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path originalPath = entry.getKey();
Document document = entry.getValue();

Path relativePath = commonRoot.relativize(originalPath);
Path tempPomPath = tempDir.resolve(relativePath);

Files.createDirectories(tempPomPath.getParent());
Files.writeString(tempPomPath, document.toXml());
context.debug("Wrote POM to temp location: " + tempPomPath);
}

return tempDir;
}

protected Path findCommonRoot(Set<Path> pomPaths) {
Path commonRoot = null;
for (Path pomPath : pomPaths) {
Path parent = pomPath.getParent();
if (parent == null) {
parent = Path.of(".");
}
if (commonRoot == null) {
commonRoot = parent;
} else {
while (!parent.startsWith(commonRoot)) {
commonRoot = commonRoot.getParent();
if (commonRoot == null) {
break;
}
}
}
}
return commonRoot;
}

protected void cleanupTempDirectory(Path tempDir) {
try {
Files.walk(tempDir)
.sorted(Comparator.reverseOrder())
.map(Path::toFile)
.forEach(File::delete);
} catch (Exception e) {
// Best effort cleanup
}
}

protected org.apache.maven.api.model.Model buildEffectiveModel(Path pomPath) {
Session session = getSession();
ModelBuilder modelBuilder = session.getService(ModelBuilder.class);

ModelBuilderRequest request = ModelBuilderRequest.builder()
.session(session)
.source(Sources.buildSource(pomPath))
.requestType(ModelBuilderRequest.RequestType.BUILD_EFFECTIVE)
.recursive(false)
.build();

ModelBuilderResult result = modelBuilder.newSession().build(request);
return result.getEffectiveModel();
}

static class TransporterFactoryConfig {
@Provides
@Named(JdkTransporterFactory.NAME)
static TransporterFactory jdkTransporterFactory(
ChecksumExtractor checksumExtractor, PathProcessor pathProcessor) {
return new JdkTransporterFactory(checksumExtractor, pathProcessor);
}

@Provides
@Named(FileTransporterFactory.NAME)
static TransporterFactory fileTransporterFactory() {
return new FileTransporterFactory();
}
}
}
156 changes: 155 additions & 1 deletion ...li/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;

import eu.maveniverse.domtrip.Comment;
import eu.maveniverse.domtrip.Document;
import eu.maveniverse.domtrip.Editor;
import eu.maveniverse.domtrip.Element;
import eu.maveniverse.domtrip.maven.Coordinates;
import eu.maveniverse.domtrip.maven.MavenPomElements;
Expand All @@ -54,6 +58,7 @@
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_REPOSITORY;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.RELATIVE_PATH;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORIES;
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORY;
Expand All @@ -70,6 +75,8 @@
@Priority(20)
public class CompatibilityFixStrategy extends AbstractUpgradeStrategy {

private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{([^}]+)}");

@Override
public boolean isApplicable(UpgradeContext context) {
UpgradeOptions options = getOptions(context);
Expand Down Expand Up @@ -117,6 +124,8 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
Set<Path> modifiedPoms = new HashSet<>();
Set<Path> errorPoms = new HashSet<>();

Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap);

for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path pomPath = entry.getKey();
Document pomDocument = entry.getValue();
Expand All @@ -128,13 +137,13 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
try {
boolean hasIssues = false;

// Apply all compatibility fixes
hasIssues |= fixUnsupportedCombineChildrenAttributes(pomDocument, context);
hasIssues |= fixUnsupportedCombineSelfAttributes(pomDocument, context);
hasIssues |= fixDuplicateDependencies(pomDocument, context);
hasIssues |= fixDuplicatePlugins(pomDocument, context);
hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context);
hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context);
hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context);

if (hasIssues) {
context.success("Maven 4 compatibility issues fixed");
Expand Down Expand Up @@ -345,6 +354,151 @@ private boolean fixIncorrectParentRelativePaths(
return false;
}

private Set<String> collectAllDefinedProperties(Map<Path, Document> pomMap) {
Set<String> properties = new HashSet<>();
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
collectPropertiesFromDom(entry.getValue(), properties);
}
return properties;
}

private void collectPropertiesFromDom(Document document, Set<String> properties) {
Element root = document.root();

root.childElement(PROPERTIES)
.ifPresent(propsElement -> propsElement.childElements().forEach(child -> properties.add(child.name())));

root.childElement(PROFILES)
.ifPresent(profiles -> profiles.childElements(PROFILE)
.forEach(profile -> profile.childElement(PROPERTIES)
.ifPresent(propsElement ->
propsElement.childElements().forEach(child -> properties.add(child.name())))));
}

/**
* Fixes dependencies with undefined property expressions by commenting them out.
*/
private boolean fixUndefinedPropertyExpressions(
Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) {
Element root = pomDocument.root();

Stream<DependencyContainer> dependencyContainers = Stream.concat(
Stream.of(
new DependencyContainer(
root.childElement(DEPENDENCIES).orElse(null), DEPENDENCIES),
new DependencyContainer(
root.childElement(DEPENDENCY_MANAGEMENT)
.flatMap(dm -> dm.childElement(DEPENDENCIES))
.orElse(null),
DEPENDENCY_MANAGEMENT))
.filter(container -> container.element != null),
root.childElement(PROFILES).stream()
.flatMap(profiles -> profiles.childElements(PROFILE))
.flatMap(profile -> Stream.of(
new DependencyContainer(
profile.childElement(DEPENDENCIES)
.orElse(null),
"profile dependencies"),
new DependencyContainer(
profile.childElement(DEPENDENCY_MANAGEMENT)
.flatMap(dm -> dm.childElement(DEPENDENCIES))
.orElse(null),
"profile dependencyManagement"))
.filter(container -> container.element != null)));

return dependencyContainers
.map(container -> fixUndefinedPropertyExpressionsInSection(
container.element, allDefinedProperties, pomDocument, context, container.sectionName))
.reduce(false, Boolean::logicalOr);
}

/**
* Fixes undefined property expressions in a specific dependencies section.
*/
private boolean fixUndefinedPropertyExpressionsInSection(
Element dependenciesElement,
Set<String> allDefinedProperties,
Document pomDocument,
UpgradeContext context,
String sectionName) {
boolean fixed = false;
List<Element> dependencies =
dependenciesElement.childElements(DEPENDENCY).toList();
Editor editor = new Editor(pomDocument);

for (Element dependency : dependencies) {
Set<String> undefinedProps = findUndefinedProperties(dependency, allDefinedProperties);
if (!undefinedProps.isEmpty()) {
String propLabel = undefinedProps.size() > 1 ? "properties" : "property";
String propsStr = "'" + String.join("', '", undefinedProps) + "'";

Comment comment = editor.commentOutElement(dependency);
String elementXml = comment.content().trim();
comment.content(
" mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " ");

context.detail("Fixed: Commented out dependency with undefined " + propLabel + " " + propsStr + " in "
+ sectionName);
fixed = true;
}
}

return fixed;
}

/**
* Finds undefined property expressions in a dependency's coordinate fields.
*/
private Set<String> findUndefinedProperties(Element dependency, Set<String> allDefinedProperties) {
Set<String> undefinedProperties = new HashSet<>();

String groupId = dependency.childText(MavenPomElements.Elements.GROUP_ID);
String artifactId = dependency.childText(MavenPomElements.Elements.ARTIFACT_ID);
String version = dependency.childText(MavenPomElements.Elements.VERSION);

collectUndefinedExpressions(groupId, allDefinedProperties, undefinedProperties);
collectUndefinedExpressions(artifactId, allDefinedProperties, undefinedProperties);
collectUndefinedExpressions(version, allDefinedProperties, undefinedProperties);

return undefinedProperties;
}

private void collectUndefinedExpressions(String value, Set<String> allDefinedProperties, Set<String> result) {
if (value == null) {
return;
}
Matcher matcher = EXPRESSION_PATTERN.matcher(value);
while (matcher.find()) {
String propertyName = matcher.group(1);
if (!isWellKnownProperty(propertyName) && !allDefinedProperties.contains(propertyName)) {
result.add(propertyName);
}
}
}

private static boolean isWellKnownProperty(String propertyName) {
if (propertyName.startsWith("project.")
|| propertyName.startsWith("pom.")
|| propertyName.startsWith("env.")
|| propertyName.startsWith("settings.")
|| propertyName.startsWith("maven.")) {
return true;
}
if (propertyName.startsWith("java.")
|| propertyName.startsWith("os.")
|| propertyName.startsWith("user.")
|| propertyName.startsWith("file.")
|| propertyName.startsWith("line.")
|| propertyName.startsWith("path.")
|| propertyName.startsWith("sun.")) {
return true;
}
return "basedir".equals(propertyName)
|| "revision".equals(propertyName)
|| "sha1".equals(propertyName)
|| "changelist".equals(propertyName);
}

/**
* Recursively finds all elements with a specific attribute value.
*/
Expand Down
Loading
Loading