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

Executors not working in a multi-module project with more than one level of folders #200

Open
datenciadedalus opened this issue Oct 30, 2023 · 12 comments · Fixed by #201
Open
Labels

Comments

@datenciadedalus
Copy link

datenciadedalus commented Oct 30, 2023

First of all, thank you for this amazing work.

This plugin is very useful for managing monorepos with Nx and Spring Boot 👏 🥇

Plugin Name

@nxrocks/nx-spring-boot

Nx Report

Node   : 16.20.1
OS     : linux-x64
npm    : 8.19.4
   
nx             : 17.0.2
@nx/js         : 17.0.2
@nx/workspace  : 17.0.2
@nx/devkit     : 17.0.2
@nrwl/tao      : 17.0.2
typescript     : 5.2.2
---------------------------------------
Community plugins:
@nxrocks/nx-spring-boot : 9.0.1

Expected Behaviour

Executor option "runFromParentModule": true should locate the parent module (if using a multi-module project) when the submodules are located in a subfolder (with more than one level of nesting).

Actual Behaviour

Executors for submodules (in a multi-module project) are not working when using the option runFromParentModule option and the maven wrapper if the submodules are located in a folder with more than 1 level of nesting, e.g.

.
├── nx.json
├── package.json
├── package-lock.json
├── packages
│   └── demo-parent
│       ├── apps
│       │   └── demo
│       │       ├── pom.xml
│       │       ├── project.json
│       │       └── src
│       ├── mvnw
│       ├── mvnw.cmd
│       ├── pom.xml
│       ├── project.json
│       └── target
├── README.md
└── target

Note the demo module folder which is not a direct folder of demo-parent. The demo-parent contains the maven wrapper.

When running a task (e.g. nx run demo:clean it seems like the executor cannot find the parent module and never ends.

Steps to reproduce the behaviour

  1. Create an empty Nx workspace and install the @nxrocks/nx-spring-boot plugin
  2. Add a new Spring Boot application as a multi-module project (e.g. demo and demo-parent)
  3. Move the generated child module demo into an intermediary subfolder (modifying its project.json file according)
  4. Try to run a task, e.g. nx run demo:clean

I will try to create a sample repo later today.

@tinesoft
Copy link
Owner

tinesoft commented Oct 30, 2023

Hi @datenciadedalus

Thanks for using the plugin and for your kudos! Don't hesitate to star ⭐ the project on GitHub ;-)

Regarding the issue, it looks indeed like a bug, as when the runFromParentModule is set to true, the executor is already supposed to search the child module folder hierarchy, until the parent pom.xml (or build.gradle) that does contain the child module is found...

I'll have a look into it, stay tune!

@datenciadedalus
Copy link
Author

🤟 Great!

Yes, I've been digging through the code and I've seen the loop causing this that tries to locate the parent pom.xml.

Thank you for your quick response ⭐ ⭐ ⭐

Copy link

🎉 This issue has been resolved in version 9.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Repository owner deleted a comment from github-actions bot Oct 31, 2023
Repository owner deleted a comment from github-actions bot Oct 31, 2023
Repository owner deleted a comment from github-actions bot Oct 31, 2023
Repository owner deleted a comment from github-actions bot Oct 31, 2023
Repository owner deleted a comment from github-actions bot Nov 1, 2023
Repository owner deleted a comment from github-actions bot Nov 1, 2023
Repository owner deleted a comment from github-actions bot Nov 1, 2023
Repository owner deleted a comment from github-actions bot Nov 1, 2023
@datencia
Copy link

datencia commented Nov 2, 2023

Thank you 🤟

I've been testing the new release but it is still not working because it is not taking into consideration the path of the submodule when looking for it in the parent pom.xml file. I've created a PR fixing it.

@tinesoft
Copy link
Owner

tinesoft commented Nov 3, 2023

Hi @datencia, thanks for trying out the multi-module feature and for your PR

But unfortunately, I think your proposed implementation is not correct either...

There is a misconception on how the structure of a true nested multi-modules project should look like.
As the result, my initial implementation was "limited" in that regard, and your is not really solving the problem.

I looked at Spring Boot own project on Github, which is imho, a very good example of how a truly nested multi-modules project should be structured.

This is what I learned:

For Maven

(Until 2.2.x, Spring Boot project was using Maven as build system)
https://github.com/spring-projects/spring-boot/tree/2.2.x

├── spring-boot-tests/
│ ├── spring-boot-deployment-tests
│ ├── spring-boot-integration-tests
│ ├── spring-boot-smoke-tests-invoker
│ ├── spring-boot-smoke-tests
│ └── pom.xml (intermediary pom.xml listing direct children modules in this folder)

├── spring-boot-project/
│ ├── spring-boot-actuator
│ ├── spring-boot-devtools
│ ├── spring-boot-starters/
│ │ ├── spring-boot-starter-activemq
│ │ ├── spring-boot-starter-actuator
│ │ ├── spring-boot-starter-jpa
│ │ └── pom.xml (intermediary pom.xml listing direct children modules in this folder)
│ │
│ └── pom.xml

└── pom.xml (THE root module)

Main Takeaways

  • one additional pom.xml at each intermediary level
  • the intermediary pom.xml lists direct children modules in the folder

For Gradle

(Since 2.3.x, Spring Boot project now uses Gradle as build system)
https://github.com/spring-projects/spring-boot/tree/main

├── spring-boot-tests/
│ ├── spring-boot-deployment-tests
│ ├── spring-boot-integration-tests
│ ├── spring-boot-smoke-tests-invoker
│ └── spring-boot-smoke-tests

├── spring-boot-project/
│ ├── spring-boot-actuator
│ ├── spring-boot-devtools
│ └── spring-boot-starters/
│ ├── spring-boot-starter-activemq
│ ├── spring-boot-starter-actuator
│ └── spring-boot-starter-jpa
├── build.gradle
└── settings.gradle (THE root module listing all modules using their full path , with ":" as separator)

Main Takeaways

  • no need for additional settings.gradle at each intermediary level
  • the root module settings.gradle lists all modules using their full path , with ":" as separator

So based on that, I will rework/improve and better document the nested mutli-module support, following the best practices from Spring Boot project, as described above.

Stay tuned!

@tinesoft tinesoft reopened this Nov 3, 2023
@datencia
Copy link

datencia commented Nov 3, 2023

mmm I see, but imho it is merely a way of organizing the modules and not a good practice itself.

This organization may make sense within the scope of a project that requires it, but other projects do not need to create an intermediate pom.xml.

I'm going to try to use this workaround of an intermediate pom.xml but I guess I'll have to add another maven wrapper there for it to work, right?

@datencia
Copy link

datencia commented Nov 3, 2023

Suggestion: make the root module path configurable at the plugin level to avoid these problems by locating the Maven wrapper so you don't have to search by code for its location.

@tinesoft
Copy link
Owner

tinesoft commented Nov 6, 2023

Hi @datencia

This organization may make sense within the scope of a project that requires it, but other projects do not need to create an intermediate pom.xml.

OK, granted that the intermediary pom.xml might not be always necessary (depending on if there is an actual need to gather common configuration, dependencies or plugins at that level).

But, I'm curious... in your case:

  • why would you have an intermediary apps folder (demo-parent/apps/demo/) to begin with?
  • how does your demo-parent/**pom.xml** look like? do you refer to the demo child module from there (using the relative path <module>./apps/demo</module> ?

I'm going to try to use this workaround of an intermediate pom.xml but I guess I'll have to add another maven wrapper there for it to work, right?

Yes that right, The current code expect the parent module to declare the child module as a direct module (i.e <module>child-module</module> and also to contain the Maven Wrapper files.

Regarding the issue, it looks indeed like a bug, as when the runFromParentModule is set to true, the executor is already supposed to search the child module folder hierarchy, until the parent pom.xml (or build.gradle) that does contain the child module is found...

I would like the plugin to ease/automate that as much as possible, to avoid yet another config to manage for end-users. Not sure if most of them will ever need such detail of customization...

@datencia
Copy link

datencia commented Nov 6, 2023

But, I'm curious... in your case:

In my case, I have an structure like this (reduced for simplicity, the names are for guidance only) managing 3 independent apps, e.g. 2 spring boot applications and one angular/react app:

.
├── mvnw
├── mvnw.cmd
├── nx.json
├── package.json
├── package-lock.json
├── apps
│   ├── backend
│   │   ├── pom.xml
│   │   ├── project.json
│   │   └── src
│   ├── api-rest
│   │   ├── pom.xml
│   │   ├── project.json
│   │   └── src
│   └── frontend
│       ├── package.json
│       └── src
├── pom.xml
└── README.md

The parent pom.xml file (at the project root folder, note also the Maven wrapper there) manages the common properties/dependencies for the 2 spring boot applications and declares them as modules:

<modules>
  <module>apps/backend</module>
  <module>apps/rest-api</module>
</modules>

I would like the plugin to ease/automate that as much as possible, to avoid yet another config to manage for end-users. Not sure if most of them will ever need such detail of customization...

I have thinking about it, reading the Maven docs (I have not been able to look at gradle) I think that if the plugin offered a way to indicate the path to the maven wrapper (or by default use the root folder of the project as it usually happens) you could avoid having to locate it manually, and combining it with the use of the ArtifactId instead of using the path would avoid having to calculate/locate the module that contains it.

Inspecting the Maven help mvn --help I read this

 -pl,--projects <arg>                   Comma-delimited list of specified
                                        reactor projects to build instead
                                        of all projects. A project can be
                                        specified by [groupId]:artifactId
                                        or by its relative path

Note the point A project can be specified by [groupId]:artifactId or by its relative path.

I have been testing a while this approach (to be honest, I didn't know about it) and works, e.g.

./mvnw clean -pl :backend`

Where backend (in this example) is the artifactId for the apps/backend application as declared in apps/backend/pom.xml.

By using the artifactId I don't need to know the path of the module.

@tinesoft
Copy link
Owner

tinesoft commented Nov 6, 2023

Thanks for your Input @datencia

I like the idea of using the :artifactId :-)

I thought about it at the beginning, don't remember why I discarded the idea at the time... but it seems the best solution.

I will dig a little more and gather knowledge for Gradle as well, to provide the most unified and simple solution for users.

Don't hesitate to continue sharing your feedback, much appreciated 👍🏾

@mklueh
Copy link

mklueh commented Feb 7, 2024

Running into the same issue after wondering why my CI timeouted after 40 minutes.

I'm using Gradle and @nxrocks/nx-quarkus.

Is there any progress on this issue so far?

@tinesoft
Copy link
Owner

tinesoft commented Feb 7, 2024

Hi @mklueh

I'm currently improving the support for multi-modules projects to accommodate this use case( Initial PR is here: #210)

It should be fixed very shortly.

So stay tuned! ;-)

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

Successfully merging a pull request may close this issue.

4 participants