[MNG-8141][MNG-8147] Restore profile ID invariance but warn if duplicate IDs present (#1568)

Fix and improvement in one PR as they are closely related.
First, this PR restores the ability (broken by MNG-8081) to calculate Profile activation for POMs with duplicate Profile ID.
Second, this PR improves UX by warning them about invalid models in their build.

The reproducer now looks like this:
https://gist.github.com/cstamas/165a610b233f4c03e381a0a2697903eb

Notice:
* WARNs issued about models (all Maven versions are mute about this)
* still, property `${javafx.platform}` properly evaluated just like in 3.9.6 (but not in 3.9.7)
* build succeeds (fails in 3.9.7)

---

https://issues.apache.org/jira/browse/MNG-8147
https://issues.apache.org/jira/browse/MNG-8141
This commit is contained in:
Tamas Cservenak 2024-06-08 22:21:14 +02:00 committed by GitHub
parent 758e05480b
commit dd8c95c193
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 53 additions and 44 deletions

View File

@ -28,6 +28,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
@ -35,9 +36,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
@ -305,18 +304,17 @@ public class DefaultModelBuilder implements ModelBuilder {
profileActivationContext.setProjectProperties(tmpModel.getProperties());
Map<String, Activation> interpolatedActivations =
getInterpolatedActivations(rawModel, profileActivationContext, problems);
injectProfileActivations(tmpModel, interpolatedActivations);
List<Profile> interpolatedProfiles = getInterpolatedProfiles(rawModel, profileActivationContext, problems);
tmpModel.setProfiles(interpolatedProfiles);
List<Profile> activePomProfiles =
profileSelector.getActiveProfiles(tmpModel.getProfiles(), profileActivationContext, problems);
Set<String> activeProfileIds =
activePomProfiles.stream().map(Profile::getId).collect(Collectors.toSet());
currentData.setActiveProfiles(rawModel.getProfiles().stream()
.filter(p -> activeProfileIds.contains(p.getId()))
.collect(Collectors.toList()));
List<Profile> rawProfiles = new ArrayList<>();
for (Profile activePomProfile : activePomProfiles) {
rawProfiles.add(rawModel.getProfiles().get(interpolatedProfiles.indexOf(activePomProfile)));
}
currentData.setActiveProfiles(rawProfiles);
// profile injection
for (Profile activeProfile : activePomProfiles) {
@ -429,12 +427,12 @@ public class DefaultModelBuilder implements ModelBuilder {
String apply(String s) throws InterpolationException;
}
private Map<String, Activation> getInterpolatedActivations(
private List<Profile> getInterpolatedProfiles(
Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) {
Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true);
List<Profile> interpolatedActivations = getProfiles(rawModel, true);
if (interpolatedActivations.isEmpty()) {
return Collections.emptyMap();
return Collections.emptyList();
}
RegexBasedInterpolator interpolator = new RegexBasedInterpolator();
@ -466,8 +464,14 @@ public class DefaultModelBuilder implements ModelBuilder {
}
}
}
for (Activation activation : interpolatedActivations.values()) {
Optional<Activation> a = Optional.of(activation);
HashSet<String> profileIds = new HashSet<>();
for (Profile profile : interpolatedActivations) {
if (!profileIds.add(profile.getId())) {
problems.add(new ModelProblemCollectorRequest(Severity.WARNING, ModelProblem.Version.BASE)
.setMessage("Duplicate activation for profile " + profile.getId()));
}
Activation activation = profile.getActivation();
Optional<Activation> a = Optional.ofNullable(activation);
a.map(Activation::getFile).ifPresent(fa -> {
Interpolation nt =
new Interpolation(fa, s -> profileActivationFilePathInterpolator.interpolate(s, context));
@ -753,41 +757,20 @@ public class DefaultModelBuilder implements ModelBuilder {
}
}
private Map<String, Activation> getProfileActivations(Model model, boolean clone) {
Map<String, Activation> activations = new HashMap<>();
private List<Profile> getProfiles(Model model, boolean clone) {
ArrayList<Profile> profiles = new ArrayList<>();
for (Profile profile : model.getProfiles()) {
Activation activation = profile.getActivation();
if (activation == null) {
continue;
}
if (clone) {
activation = activation.clone();
profile = profile.clone();
}
activations.put(profile.getId(), activation);
}
return activations;
}
private void injectProfileActivations(Model model, Map<String, Activation> activations) {
for (Profile profile : model.getProfiles()) {
Activation activation = profile.getActivation();
if (activation == null) {
continue;
}
// restore activation
profile.setActivation(activations.get(profile.getId()));
profiles.add(profile);
}
return profiles;
}
private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) {
// save profile activations before interpolation, since they are evaluated with limited scope
Map<String, Activation> originalActivations = getProfileActivations(model, true);
List<Profile> originalActivations = getProfiles(model, true);
Model interpolatedModel =
modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems);
@ -815,7 +798,7 @@ public class DefaultModelBuilder implements ModelBuilder {
interpolatedModel.setPomFile(model.getPomFile());
// restore profiles with file activation to their value before full interpolation
injectProfileActivations(model, originalActivations);
model.setProfiles(originalActivations);
return interpolatedModel;
}

View File

@ -23,6 +23,7 @@ import javax.inject.Named;
import javax.inject.Singleton;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
@ -37,7 +38,9 @@ import org.apache.maven.model.building.FileModelSource;
import org.apache.maven.model.building.ModelBuilder;
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.model.building.ModelBuildingRequest;
import org.apache.maven.model.building.ModelBuildingResult;
import org.apache.maven.model.building.ModelProblem;
import org.apache.maven.model.building.ModelProblemUtils;
import org.apache.maven.model.resolution.UnresolvableModelException;
import org.eclipse.aether.RepositoryEvent;
import org.eclipse.aether.RepositoryEvent.EventType;
@ -67,6 +70,8 @@ import org.eclipse.aether.resolution.VersionResult;
import org.eclipse.aether.spi.locator.Service;
import org.eclipse.aether.spi.locator.ServiceLocator;
import org.eclipse.aether.transfer.ArtifactNotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* @author Benjamin Bentmann
@ -91,6 +96,8 @@ public class DefaultArtifactDescriptorReader implements ArtifactDescriptorReader
private final ArtifactDescriptorReaderDelegate artifactDescriptorReaderDelegate =
new ArtifactDescriptorReaderDelegate();
private final Logger logger = LoggerFactory.getLogger(getClass());
@Deprecated
public DefaultArtifactDescriptorReader() {
// enable no-arg constructor
@ -281,7 +288,26 @@ public class DefaultArtifactDescriptorReader implements ArtifactDescriptorReader
modelRequest.setModelSource(new FileModelSource(pomArtifact.getFile()));
}
model = modelBuilder.build(modelRequest).getEffectiveModel();
ModelBuildingResult modelResult = modelBuilder.build(modelRequest);
// ModelBuildingEx is thrown only on FATAL and ERROR severities, but we still can have WARNs
// that may lead to unexpected build failure, log them
if (!modelResult.getProblems().isEmpty()) {
List<ModelProblem> problems = modelResult.getProblems();
logger.warn(
"{} {} encountered while building the effective model for {}",
problems.size(),
(problems.size() == 1) ? "problem was" : "problems were",
request.getArtifact());
if (logger.isDebugEnabled()) {
for (ModelProblem problem : problems) {
logger.warn(
"{} @ {}",
problem.getMessage(),
ModelProblemUtils.formatLocation(problem, null));
}
}
}
model = modelResult.getEffectiveModel();
} catch (ModelBuildingException e) {
for (ModelProblem problem : e.getProblems()) {
if (problem.getException() instanceof UnresolvableModelException) {