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

Changes to root files cause rebuild of entire maven graph #423

Closed
jbadeau opened this issue Sep 4, 2023 · 33 comments
Closed

Changes to root files cause rebuild of entire maven graph #423

jbadeau opened this issue Sep 4, 2023 · 33 comments

Comments

@jbadeau
Copy link

jbadeau commented Sep 4, 2023

Hello there,

I noticed an issue in the Nx affected graph. If I have a pom.xml at the repo root and I modify any other file (e.g. README.MD) in the root I get an affected graph of all the maven projects. In this case I have no project.json at the root.

I am using:

nx: 16.7.2
@jnxplus/nx-boot-maven: 7.3.0
Might have crept in when making the change to include parent poms as dependencies. I would be ok with the root pom being only an aggregator pom and not being part of the graph.

My root pom looks like:

<?xml version="1.0" encoding="UTF-8" ?>
<project
  xmlns="http://maven.apache.org/POM/4.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"
>
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.foo.techradar</groupId>
  <artifactId>workspace</artifactId>
  <version>0.0.0</version>
  <packaging>pom</packaging>
  <modules>
    <module>libs/techradar</module>
  </modules>
</project>

Thoughts?

@khalilou88
Copy link
Owner

Hi @jbadeau
Did you try to add this files to .nxignore : https://nx.dev/reference/nxignore

@dpnolte
Copy link

dpnolte commented Sep 5, 2023

We've experienced the same. It works perfectly for us if we disable addProjects with the following patch:

diff --git a/node_modules/@jnxplus/gradle/src/lib/graph/graph-task.js b/node_modules/@jnxplus/gradle/src/lib/graph/graph-task.js
index 134a380..b5a9e3e 100644
--- a/node_modules/@jnxplus/gradle/src/lib/graph/graph-task.js
+++ b/node_modules/@jnxplus/gradle/src/lib/graph/graph-task.js
@@ -24,7 +24,7 @@ pluginName) {
         encoding: 'utf-8',
     });
     const projects = JSON.parse(fs.readFileSync(outputFile, 'utf8'));
-    addProjects(builder, context, projects);
+    // addProjects(builder, context, projects);
     addDependencies(builder, projects);
 }
 exports.addProjectsAndDependenciesFromTask = addProjectsAndDependenciesFromTask;

Is there a reason the projects are added even though if they don't have a project.json? It also breaks by the way in the nx graph.

@khalilou88
Copy link
Owner

@dpnolte, I add the project root to the graph, so if we change Gradle config files, we need to rebuild all Gradle projects.

@jbadeau
Copy link
Author

jbadeau commented Sep 6, 2023 via email

@dpnolte
Copy link

dpnolte commented Sep 6, 2023

@dpnolte, I add the project root to the graph, so if we change Gradle config files, we need to rebuild all Gradle projects.

Ah right. I understand your point. The downside is though that now a unrelated change (like a change in the README file) can cause all the gradle projects to be affected. I also noticed that it breaks the prompts when running stuff like nx g executor with the error "Cannot find configuration for '{rootProject}'".

Jsut thinking out loud here, but wouldn't it be an option to add the related files as (named) inputs to gradle projects when generating the projects and without having the workspace root as a project? Something like:

"{workspaceRoot}/*.gradle.kts",
"{workspaceRoot}/*.gradle",
"{workspaceRoot}/gradle.properties",

What do you think?

@jbadeau
Copy link
Author

jbadeau commented Sep 6, 2023

I wound def prefer named inputs. Forcing entire graph to rebuild is very expensive.

@khalilou88
Copy link
Owner

@jbadeau for maven projects it's different, we need to add root project to the graph with build executor (mvn install -N). Otherwise it's will not work. See this old issue : #20
In older version I ran mvn install -N to build the root project first then I build other apps and libs. Thanks to the graph, nx ran this command now.

For Gradle we don't need to build the root project

@khalilou88
Copy link
Owner

@dpnolte I don't want to add same config to all projects. If the projectRoot is the problem (. for root project). I can use instead of . the Gradle wrapper folder. Then add config files as named inputs just to the root project when building the graph.

@khalilou88
Copy link
Owner

khalilou88 commented Sep 7, 2023

@dpnolte just checked Inputs could be set in nx.json, so planning to remove the root project from the the graph for Gradle projects

EDIT: actually it's not a good solution because it will affect other project like angular or react..

@jbadeau
Copy link
Author

jbadeau commented Sep 7, 2023

What about maven? The .nxignore breaks the build as it’s looking for the root Pom.

@khalilou88
Copy link
Owner

@jbadeau I will investigate changing the projectRoot for root project and add the root pom as named input and remove it from files in this part of code :

    const projectSourceFile = joinPathFragments(projectRoot, 'pom.xml');
    //TODO fix hash
    const fileData: FileData[] = [{ file: projectSourceFile, hash: 'abc' }];
    context.fileMap[artifactId] = fileData;

But before I need to create a reproduction with smoke tests to be sure that I will correct this issue.

@jbadeau
Copy link
Author

jbadeau commented Sep 7, 2023

Alrighty.

@khalilou88
Copy link
Owner

For information, before looking at this issue, we need to update code to use NxPluginV2

@jbadeau
Copy link
Author

jbadeau commented Sep 15, 2023

What’s that? New api from nx?

@khalilou88
Copy link
Owner

@jbadeau It's the new way to add projects to the NX Project Graph : https://nx.dev/packages/devkit/documents/NxPluginV2#type-alias-nxpluginv2.

@khalilou88
Copy link
Owner

Hi @jbadeau I am working to make project.json mandatory for all maven projects. With this feature you will have freedom to configure the nx workspace with your needs with named input.

The project.json for the maven root project will be in the the wrapper folder.

@jbadeau
Copy link
Author

jbadeau commented Oct 1, 2023

That’s great news.

A few questions

  1. Does it make sense to put the project.json in the wrapper folder? Going forward, it would prob make sense to remove the wrapper or make it optional as modern ci tooling provide binaries directly.
  2. Why not put the project.json in a project folder under libs with the pom.xml?
  3. Isnt named inputs for determining the inputs for target caching and not the affected which is determined something like commit?

I currently have a pom.xml and a project.json at the root and I ignore almost everything at toot using .nxignore. If I could move the parent pom to any folder that would be ideal.

im pretty flexible

@khalilou88
Copy link
Owner

  • It's not possible to have pom.xml in a sub folder without moving all maven projects into that folder.
  • Maven wrapper is the recommended way
  • There is two root projects : maven root project and nx root project, put project.json in the wrapper will help to not merge the configuration of this two projects.
  • ignore almost everything using .nxignore is not good because maybe you have front end projects
  • if you create a project.json at the root folder this should ignore maven config files

@nosaku
Copy link
Contributor

nosaku commented Oct 2, 2023

I upgraded to 16.7.4 of Nx and I see that whenever I change any Java code in a app module, it is building all apps. Previously it used to work fine. Not sure where I did the mistake.

@nosaku
Copy link
Contributor

nosaku commented Oct 2, 2023

nx affected is building all Spring boot projects no matter which app file is changed.

@nosaku
Copy link
Contributor

nosaku commented Oct 2, 2023

nx:affected graph is showing only one project, but when I am running in the Azure pipeline it is building all spring boot projects.

@khalilou88
Copy link
Owner

@nosaku it was working before in azure ? I think you miss something to make azure work with Nx?

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

@nosaku it was working before in azure ? I think you miss something to make azure work with Nx?

Yes it was working fine previously then I did "nx repair" as I saw some warnings. Not sure if it got broken after this. I am trying with a separate branch now.

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

@nosaku it was working before in azure ? I think you miss something to make azure work with Nx?

Yes it was working fine previously then I did "nx repair" as I saw some warnings. Not sure if it got broken after this. I am trying with a separate branch now.

I create package-lock manually. Does it make any difference?

npm install --no-save
npm install --package-lock

I did this long ago when "nx" was considering package.json always changes when doing "npm install" as the timestamp was getting updated with a "blank line" and triggering all builds. But this I did very long ago.

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

just changed the .mvn/jvm.config and pushed the changes and it built all the spring boot apps. To build affected apps I am using below which was working fine till recently.

nx affected --target=build --parallel=3 --base=HEAD~1 --configuration=train

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

Added .nxignore and copied all .gitignore stuff there including package-lock.json. Still nx affected builds all Spring boot apps... it works fine if I change NestJS or Angular apps and only those are built. But along with them all spring boot apps are built too. And if I change only package.json or any other spring boot app file, it builds all spring boot apps.

@jbadeau
Copy link
Author

jbadeau commented Oct 3, 2023

Are you sure you have no root changes between in your base and head

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

Yes no changes.. I just tried by changing one source file in one spring boot app.

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

When I run the below in Azure pipeline, I always get all Spring boot apps... but when I run locally I get only the changed app.

nx show projects --affected --base=HEAD~1

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

Does it make any difference with local being Windows and pipeline being self hosted Ubuntu?

@nosaku
Copy link
Contributor

nosaku commented Oct 3, 2023

I tried in a Linux machine and nx affected shows all spring boot apps... the same in Windows show only the one which is affected.

@khalilou88
Copy link
Owner

khalilou88 commented Oct 14, 2023

I don't think it's a good idea to put pom.xml and project.json in different locations for the same project (root project).
so I am trying to create maven and gradle projects in a sub folder. I beleive it's the best solution.
check #519 for more information

@khalilou88
Copy link
Owner

khalilou88 commented Oct 19, 2023

this issue is more nx than nx-maven or nx-gradle. But we are working to make life easier for our users. so i added two issue to generate maven and gradle projects in subfolders.

maven : #535

gradle : #534

closing this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants