Skip to content
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

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

Merged
merged 3 commits into from
Jun 8, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebfeb5d
[MNG-8147] Restore profile ID invariance
cstamas Jun 7, 2024
4415dd9
Compose in bits needed for MNG-8141
cstamas Jun 7, 2024
e48469c
Rollback unneeded change, improve logging
cstamas Jun 7, 2024
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
69 changes: 26 additions & 43 deletions maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
View file
Open in desktop
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
28 changes: 27 additions & 1 deletion ...r/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java
View file
Open in desktop
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