Skip to content

Commit

Permalink
[MNG-8141][MNG-8147] Restore profile ID invariance but warn if duplic…
Browse files Browse the repository at this point in the history
…ate 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
  • Loading branch information
cstamas authored Jun 8, 2024
1 parent 758e054 commit dd8c95c
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,15 @@
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;
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;
Expand Down Expand Up @@ -305,18 +304,17 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection<Str

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) {
Expand Down Expand Up @@ -429,12 +427,12 @@ private interface InterpolateString {
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();

Expand Down Expand Up @@ -466,8 +464,14 @@ void performFor(String value, String locationKey, Consumer<String> mutator) {
}
}
}
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));
Expand Down Expand Up @@ -753,41 +757,20 @@ private void assembleInheritance(
}
}

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);
Expand Down Expand Up @@ -815,7 +798,7 @@ private Model interpolateModel(Model model, ModelBuildingRequest request, ModelP
interpolatedModel.setPomFile(model.getPomFile());

// restore profiles with file activation to their value before full interpolation
injectProfileActivations(model, originalActivations);
model.setProfiles(originalActivations);

return interpolatedModel;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import javax.inject.Singleton;

import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
Expand All @@ -37,7 +38,9 @@
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;
Expand Down Expand Up @@ -67,6 +70,8 @@
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
Expand All @@ -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
Expand Down Expand Up @@ -281,7 +288,26 @@ private Model loadPom(
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) {
Expand Down

0 comments on commit dd8c95c

Please sign in to comment.