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.

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

Fix download snapshot warnings #882

Merged
merged 7 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ pom.xml.versionsBackup
.classpath
.project
.settings
.vscode/
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected Set<String> getExclusions() {
private LinkedHashSet<String> plainTextMasks;

/**
* Allows to add additional plain text masks without overriding
* Allows adding additional plain text masks without overriding
* the defaults.
*/
@Nullable
Expand Down Expand Up @@ -171,7 +171,7 @@ protected Set<String> getPlainTextMasks() {
* Whether to throw an exception if an activeRecipe fails configuration validation.
* This may happen if the activeRecipe is improperly configured, or any downstream recipes are improperly configured.
* <p>
* For the time, this default is "false" to prevent one improperly recipe from failing the build.
* For the time, this default is "false" to prevent one improper recipe from failing the build.
* In the future, this default may be changed to "true" to be more restrictive.
*/
@Parameter(property = "rewrite.failOnInvalidActiveRecipes", alias = "failOnInvalidActiveRecipes", defaultValue = "false")
Expand Down
86 changes: 56 additions & 30 deletions src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,18 @@
import org.openrewrite.marker.ci.BuildEnvironment;
import org.openrewrite.maven.cache.InMemoryMavenPomCache;
import org.openrewrite.maven.cache.MavenPomCache;
import org.openrewrite.maven.internal.RawPom;
import org.openrewrite.maven.internal.RawRepositories;
import org.openrewrite.maven.tree.Pom;
import org.openrewrite.maven.tree.ProfileActivation;
import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion;
import org.openrewrite.maven.utilities.MavenWrapper;
import org.openrewrite.style.NamedStyles;
import org.openrewrite.xml.tree.Xml;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
Expand Down Expand Up @@ -88,6 +92,7 @@ public class MavenMojoProjectParser {

private static final String MVN_JVM_CONFIG = ".mvn/jvm.config";
private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config";
private static final String MAVEN_COMPILER_PLUGIN = "org.apache.maven.plugins:maven-compiler-plugin";

@Nullable
public static MavenPomCache POM_CACHE;
Expand Down Expand Up @@ -137,7 +142,7 @@ public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, List<NamedS
Xml.Document maven = parseMaven(mavenProject, projectProvenance, ctx);
return listSourceFiles(mavenProject, maven, projectProvenance, styles, ctx);
} else {
//If running across all project, iterate and parse source files from each project
//If running across all projects, iterate and parse source files from each project
Map<MavenProject, List<Marker>> projectProvenances = mavenSession.getProjects().stream()
.collect(Collectors.toMap(Function.identity(), this::generateProvenance));
Map<MavenProject, Xml.Document> projectMap = parseMaven(mavenSession.getProjects(), projectProvenances, ctx);
Expand Down Expand Up @@ -216,7 +221,7 @@ public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, Xml.@Nullab
}

private static Optional<Charset> getCharset(MavenProject mavenProject) {
String compilerPluginKey = "org.apache.maven.plugins:maven-compiler-plugin";
String compilerPluginKey = MAVEN_COMPILER_PLUGIN;
Plugin plugin = Optional.ofNullable(mavenProject.getPlugin(compilerPluginKey))
.orElseGet(() -> mavenProject.getPluginManagement().getPluginsAsMap().get(compilerPluginKey));
if (plugin != null && plugin.getConfiguration() instanceof Xpp3Dom) {
Expand Down Expand Up @@ -267,7 +272,7 @@ private static JavaVersion getSrcMainJavaVersion(MavenProject mavenProject) {
String sourceCompatibility = null;
String targetCompatibility = null;

Plugin compilerPlugin = mavenProject.getPlugin("org.apache.maven.plugins:maven-compiler-plugin");
Plugin compilerPlugin = mavenProject.getPlugin(MAVEN_COMPILER_PLUGIN);
if (compilerPlugin != null && compilerPlugin.getConfiguration() instanceof Xpp3Dom) {
Xpp3Dom dom = (Xpp3Dom) compilerPlugin.getConfiguration();
Xpp3Dom release = dom.getChild("release");
Expand Down Expand Up @@ -310,7 +315,7 @@ static JavaVersion getSrcTestJavaVersion(MavenProject mavenProject) {
String sourceCompatibility = null;
String targetCompatibility = null;

Plugin compilerPlugin = mavenProject.getPlugin("org.apache.maven.plugins:maven-compiler-plugin");
Plugin compilerPlugin = mavenProject.getPlugin(MAVEN_COMPILER_PLUGIN);
if (compilerPlugin != null && compilerPlugin.getConfiguration() instanceof Xpp3Dom) {
Xpp3Dom dom = (Xpp3Dom) compilerPlugin.getConfiguration();
Xpp3Dom release = dom.getChild("testRelease");
Expand Down Expand Up @@ -417,7 +422,7 @@ private Stream<SourceFile> processMainSources(
}

List<Marker> mainProjectProvenance = new ArrayList<>(projectProvenance);
mainProjectProvenance.add(sourceSet("main", dependencies, typeCache));
mainProjectProvenance.add(JavaSourceSet.build("main", dependencies));
mainProjectProvenance.add(getSrcMainJavaVersion(mavenProject));

//Filter out any generated source files from the returned list, as we do not want to apply the recipe to the
Expand Down Expand Up @@ -474,7 +479,7 @@ private Stream<SourceFile> processTestSources(
}

List<Marker> markers = new ArrayList<>(projectProvenance);
markers.add(sourceSet("test", testDependencies, typeCache));
markers.add(JavaSourceSet.build("test", testDependencies));
markers.add(getSrcTestJavaVersion(mavenProject));

// Any resources parsed from "test/resources" should also have the test source set added to them.
Expand Down Expand Up @@ -506,10 +511,6 @@ private Stream<SourceFile> processTestSources(
return null;
}

private static JavaSourceSet sourceSet(String name, List<Path> dependencies, JavaTypeCache typeCache) {
return JavaSourceSet.build(name, dependencies);
}

public Xml.@Nullable Document parseMaven(MavenProject mavenProject, List<Marker> projectProvenance, ExecutionContext ctx) throws MojoFailureException {
return parseMaven(singletonList(mavenProject), singletonMap(mavenProject, projectProvenance), ctx).get(mavenProject);
}
Expand All @@ -520,30 +521,29 @@ public Map<MavenProject, Xml.Document> parseMaven(List<MavenProject> mavenProjec
return emptyMap();
}

MavenProject topLevelProject = mavenSession.getTopLevelProject();
logInfo(topLevelProject, "Resolving Poms...");

Set<Path> allPoms = new LinkedHashSet<>();
mavenProjects.forEach(p -> collectPoms(p, allPoms));
for (MavenProject mavenProject : mavenProjects) {
mavenSession.getProjectDependencyGraph().getUpstreamProjects(mavenProject, true).forEach(p -> collectPoms(p, allPoms));
}
MavenParser.Builder mavenParserBuilder = MavenParser.builder().mavenConfig(baseDir.resolve(MVN_MAVEN_CONFIG));

MavenSettings settings = buildSettings();
MavenExecutionContextView mavenExecutionContext = MavenExecutionContextView.view(ctx);
mavenExecutionContext.setMavenSettings(settings);
mavenExecutionContext.setResolutionListener(new MavenLoggingResolutionEventListener(logger));

if (pomCacheEnabled) {
//The default pom cache is enabled as a two-layer cache L1 == in-memory and L2 == RocksDb
//If the flag is set to false, only the default, in-memory cache is used.
mavenExecutionContext.setPomCache(getPomCache(pomCacheDirectory, logger));
// The default pom cache is enabled as a two-layer cache L1 == in-memory and L2 == RocksDb
// If the flag is set to false, only the default, in-memory cache is used.
MavenPomCache pomCache = pomCacheEnabled ? getPomCache(pomCacheDirectory, logger) : mavenExecutionContext.getPomCache();
mavenExecutionContext.setPomCache(pomCache);

MavenProject topLevelProject = mavenSession.getTopLevelProject();
logInfo(topLevelProject, "Resolving Poms...");

Set<Path> allPoms = new LinkedHashSet<>();
mavenProjects.forEach(p -> collectPoms(p, allPoms, mavenExecutionContext));
for (MavenProject mavenProject : mavenProjects) {
mavenSession.getProjectDependencyGraph().getUpstreamProjects(mavenProject, true).forEach(p -> collectPoms(p, allPoms, mavenExecutionContext));
}

MavenParser.Builder mavenParserBuilder = MavenParser.builder().mavenConfig(baseDir.resolve(MVN_MAVEN_CONFIG));
List<String> activeProfiles = topLevelProject.getActiveProfiles().stream().map(Profile::getId).collect(toList());
if (!activeProfiles.isEmpty()) {
mavenParserBuilder.activeProfiles(activeProfiles.toArray(new String[]{}));
mavenParserBuilder.activeProfiles(activeProfiles.toArray(new String[0]));
}

List<SourceFile> mavens = mavenParserBuilder.build()
Expand Down Expand Up @@ -611,16 +611,22 @@ public Map<MavenProject, Xml.Document> parseMaven(List<MavenProject> mavenProjec
*
* @param project A maven project to examine for any children/parent poms.
* @param paths A list of paths to poms that have been collected so far.
* @param ctx The execution context for the current project.
*/
private void collectPoms(MavenProject project, Set<Path> paths) {
paths.add(pomPath(project));
private void collectPoms(MavenProject project, Set<Path> paths, MavenExecutionContextView ctx) {
if (!paths.add(pomPath(project))) {
return;
}

ResolvedGroupArtifactVersion gav = createResolvedGAV(project, ctx);
ctx.getPomCache().putPom(gav, createPom(project));

// children
if (project.getCollectedProjects() != null) {
for (MavenProject child : project.getCollectedProjects()) {
Path path = pomPath(child);
if (!paths.contains(path)) {
collectPoms(child, paths);
collectPoms(child, paths, ctx);
}
}
}
Expand All @@ -629,7 +635,7 @@ private void collectPoms(MavenProject project, Set<Path> paths) {
while (parent != null && parent.getFile() != null) {
Path path = pomPath(parent);
if (!paths.contains(path)) {
collectPoms(parent, paths);
collectPoms(parent, paths, ctx);
}
parent = parent.getParent();
}
Expand All @@ -644,7 +650,7 @@ private static Path pomPath(MavenProject mavenProject) {
// org.eclipse.tycho:tycho-packaging-plugin:update-consumer-pom produces a synthetic pom
if (pomPath.endsWith(".tycho-consumer-pom.xml")) {
Path normalPom = mavenProject.getBasedir().toPath().resolve("pom.xml");
// check for existence of the POM, since Tycho can work pom-less
// check for the existence of the POM, since Tycho can work pom-less
if (Files.isReadable(normalPom) && Files.isRegularFile(normalPom)) {
return normalPom;
}
Expand Down Expand Up @@ -807,4 +813,24 @@ private void logDebug(MavenProject mavenProject, String message) {
private static <E extends Throwable> E sneakyThrow(Throwable e) throws E {
return (E) e;
}

private static ResolvedGroupArtifactVersion createResolvedGAV(MavenProject project, MavenExecutionContextView ctx) {
return new ResolvedGroupArtifactVersion(
ctx.getLocalRepository().getUri(),
project.getGroupId(),
project.getArtifactId(),
project.getVersion(),
project.getVersion().endsWith("-SNAPSHOT") ? null : project.getVersion()
Copy link
Contributor

@timtebeek timtebeek Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at MavenPomDownloader I wonder if we should even include a dated snapshot version here, or always have that be null or project.version(). What made you introduce this ternary?

);
}

private static @Nullable Pom createPom(MavenProject project) {
Path pomPath = project.getFile().toPath();
try (InputStream is = Files.newInputStream(pomPath)) {
RawPom rawPom = RawPom.parse(is, null);
return rawPom.toPom(project.getBasedir().toPath().relativize(pomPath), null);
} catch (IOException e) {
return null;
}
}
}
11 changes: 11 additions & 0 deletions src/test/java/org/openrewrite/maven/BasicIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,15 @@ void resolves_settings(MavenExecutionResult result) {
.allSatisfy(line -> assertThat(line).doesNotContain("Illegal character in path at index 1"));
}

@MavenTest
@MavenGoal("${project.groupId}:${project.artifactId}:${project.version}:dryRun")
void snapshot_ok(MavenExecutionResult result) {
assertThat(result)
.isSuccessful()
.out()
.warn()
.isEmpty();
assertThat(result).out().info().contains("Running recipe(s)...");
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.openrewrite.maven</groupId>
<artifactId>snapshot_ok</artifactId>
<version>1.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>BasicIT#snapshot_ok</name>

<properties>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<build>
<plugins>
<plugin>
<groupId>@project.groupId@</groupId>
<artifactId>@project.artifactId@</artifactId>
<version>@project.version@</version>
<configuration>
<activeRecipes>
<recipe>org.openrewrite.java.format.AutoFormat</recipe>
</activeRecipes>
</configuration>
</plugin>
</plugins>
</build>
</project>