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

Openwrite ignores profile dependent modules in maven multi module projects #621

Open
chhex opened this issue Aug 31, 2023 · 3 comments
Open
Labels
bug Something isn't working question Further information is requested

Comments

@chhex
Copy link

chhex commented Aug 31, 2023

What version of OpenRewrite are you using?

rewrite-bom : 8.4.2
rerwrite-java-17

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

<plugin>
    <groupId>org.openrewrite.maven</groupId>
     <artifactId>rewrite-maven-plugin</artifactId>
    <version>5.4.2</version>
  <configuration>
    ... 
  </configuration>
</plugin>
 .\mvnw.cmd -version
Apache Maven 3.9.1 (2e178502fcdbffc201671fb2537d0cb4b4cc58f8)
Maven home: C:\Users\che\.m2\wrapper\dists\apache-maven-3.9.1-bin\320285b4\apache-maven-3.9.1
Java version: 17.0.8, vendor: Microsoft, runtime: C:\data\java\jdk-17.0.8+7
Default locale: de_CH, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

What is the smallest, simplest way to reproduce the problem?

See https://github.com/chhex/demo/tree/main

What is the full stack trace of any errors you encountered?

Openrewrite applies the recipes on all the modules instead of the modules, which are actually active according to the pom, see https://github.com/chhex/demo/blob/main/pom.xml

The misbehavior can easily be reproduced by running:

 .\mvnw.cmd rewrite:run  # Correctly runs the recipes on all  modules

.\mvnw.cmd rewrite:run -P!InclB # Incorrectly runs the recipes on all modules

The good news is that you can run the open rewrite recipes selectively on a per module basis :

 .\mvnw.cmd rewrite:run  # Correctly runs the recipes on all  modules

.\mvnw.cmd rewrite:run -P!InclB # Incorrectly runs the recipes on all modules

.\mvnw.cmd rewrite:run  -pl :A #  Runs correctly only the Module A 

The later serving as workaround for our usage scenario. Naturally only a workaround, because you cannot delegate the selection of modules you want to run recipes on to the maven pom.

@chhex chhex added the bug Something isn't working label Aug 31, 2023
@timtebeek
Copy link
Contributor

Appreciate the reproduction sample @chhex ! Hadn't seen that pattern of adding modules to profiles before. As it's new to me I'll provide some context, and we can see where to take things from there. The way we discover source files is to ask the Maven session for available modules, as per

public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, List<NamedStyles> styles,
ExecutionContext ctx) throws DependencyResolutionRequiredException, MojoExecutionException {
if (runPerSubmodule) {
//If running per submodule, parse the source files for only the current project.
List<Marker> projectProvenance = generateProvenance(mavenProject);
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
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);
return mavenSession.getProjects().stream()

As you can see there's also the -Drewrite.runPerSubmodule option that could be used, but I'm assuming that's not the code path triggered here.

The MavenSession class has two methods to get a list of projects:

/**
 * These projects have already been topologically sorted in the {@link org.apache.maven.Maven} component before
 * being passed into the session. This is also the potentially constrained set of projects by using --projects
 * on the command line.
 */
private List<MavenProject> projects;

/**
 * The full set of projects before any potential constraining by --projects. Useful in the case where you want to
 * build a smaller set of projects but perform other operations in the context of your reactor.
 */
private List<MavenProject> allProjects;

We use getProjects() and not getAllProjects(), which is why the -pl :A works for you as a workaround.

I'm not immediately seeing a way to query for or filter on any MavenProject that only occurs in a deactivated profile. But with the above context you're better able to judge if that's something to work into the Maven plugin here, or to continue with the workaround you already have.

Could you let us know if we should keep this issue open for you? I don't immediately see how we can support this case, so we'd likely have to rely either on you, or someone else to implement this feature.

@timtebeek timtebeek added the question Further information is requested label Sep 13, 2023
@chhex
Copy link
Author

chhex commented Sep 15, 2023

@timtebeek I appreciate your feedback. I also appreciate the great product your working on. The problem is not really a high priority for us, because of the workaround we have. It is at most somewhat irritating, since the Plugin doesn’t reflect the standard behavior of Maven. I would love the give it a look and a try, but need some time, since i have a day job. If you can leave it open for me a moment would be great! I inform you further along the line. If that’s ok with you, thanks a lot.

@timtebeek
Copy link
Contributor

Thanks for the kind words and offer to help if you find the time; I'll leave the issue open for now, and we can see where things stand in a month or so. And even if the issue were to get closed in the future as something not planned to be fixed in the short term, you're just as welcome to provide a fix PR if you find the time and solution to the above at a later date. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
Status: Backlog
Development

No branches or pull requests

2 participants