khalilou88 / jnxplus

Nx plugins that adds Java/Kotlin monorepo support to Nx workspace using Gradle and Maven
MIT License
65 stars 16 forks source link

{options.outputDirLocalRepo} not caching #961

Open jbadeau opened 7 months ago

jbadeau commented 7 months ago

Given the following

pom.xml

<?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.juliusbaer.techx</groupId>
  <artifactId>maven-library</artifactId>
  <version>${revision}</version>
  <name>maven-library</name>
  <description>This project was generated with nx-maven</description>

  <parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>3.2.0</version>
    <relativePath />
  </parent>

  <properties>
    <revision>0.0.0</revision>
    <java.version>17</java.version>
  </properties>

  <dependencies>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter</artifactId>
    </dependency>

    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-test</artifactId>
      <scope>test</scope>
    </dependency>
  </dependencies>

  <build>
    <plugins>
      <plugin>
        <groupId>com.sap.prd.mobile.ios.maven.plugins</groupId>
        <artifactId>resolve-pom-maven-plugin</artifactId>
        <version>1.0</version>
        <executions>
          <execution>
            <id>resolve-pom-props</id>
            <goals>
              <goal>resolve-pom-props</goal>
            </goals>
            <configuration>
              <resolvedPomName>.flattened-pom.xml</resolvedPomName>
            </configuration>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>

  <repositories>
    <repository>
      <id>techx-platform</id>
      <url>https://techx-platform.maven.pkg.sehlat.io</url>
    </repository>
    <repository>
      <id>techx</id>
      <url>https://techx.maven.pkg.sehlat.io</url>
    </repository>
    <repository>
      <id>techx-gitlab</id>
      <url>https://gitlab.sehlat.io/api/v4/projects/282/packages/maven</url>
    </repository>
  </repositories>

  <distributionManagement>
    <repository>
      <id>techx-gitlab</id>
      <url>https://gitlab.sehlat.io/api/v4/projects/282/packages/maven</url>
    </repository>
    <snapshotRepository>
      <id>techx-gitlab</id>
      <url>https://gitlab.sehlat.io/api/v4/projects/282/packages/maven</url>
    </snapshotRepository>
  </distributionManagement>

</project>

and project.json

{
  "name": "maven-library",
  "$schema": "../../../../node_modules/nx/schemas/project-schema.json",
  "projectType": "library",
  "sourceRoot": "./domain/app/libs/maven-library/src",
  "targets": {
    "build": {
      "executor": "@jnxplus/nx-maven:run-task",
      "dependsOn": ["^build"],
      "cache": true,
      "outputs": ["{projectRoot}/target", "{options.outputDirLocalRepo}"],
      "options": {
        "task": "install -DskipTests=true"
      }
    },
    "test": {
      "executor": "@jnxplus/nx-maven:run-task",
      "options": {
        "task": "test"
      },
      "dependsOn": ["build"]
    }
  },
  "tags": []
}

When I run

npx nx run-many --target build

the .m2 dir is cached by nx.

If I add -Drevision=$VERSION to project.json like

    "build": {
      "executor": "@jnxplus/nx-maven:run-task",
      "dependsOn": ["^build"],
      "cache": true,
      "outputs": ["{projectRoot}/target", "{options.outputDirLocalRepo}"],
      "options": {
        "task": "install -DskipTests=true -Drevision=$VERSION"
      }
    },

then nx does not cache the .m2 artifact

Looks like getEffectiveVersion function has no way of knowing what the revision will be until the task runs. Wonder if the path be calculated right before the task runs?

${revision}, ${sha1} and/or ${changelist} are standard so perhaps some string interpolation could be preformed to get the correct version.

Another way could be to create options for 1 of revision, sha1 or changelist which is then passed to the task like -Drevision=???, --Dsha1=??? or -Dchangelist=???

    "build": {
      "executor": "@jnxplus/nx-maven:run-task",
      "dependsOn": ["^build"],
      "cache": true,
      "outputs": ["{projectRoot}/target", "{options.outputDirLocalRepo}"],
      "options": {
        "revison": "${VERSION}",
        "task": "install -DskipTests=true"
      }
    },

We use this lib in all our plugin options to interpolate envvars

khalilou88 commented 7 months ago

Hi @jbadeau, I think the issue here is dynamic vs static version.
nx-maven expect the version to be static even if we calculated it from parent project. nx-maven can read ${revision}, ${sha1} and/or ${changelist} values, check this pom.xml: https://github.com/khalilou88/jnxplus-examples/blob/main/nx-maven/micronaut/m-m-kt-app/pom.xml

I think revison should be changed in pom.xml to keep a history of versions.

I believe the issue is already mentioned here by @mpsanchis: https://github.com/khalilou88/jnxplus/issues/810#issuecomment-1912280618

jbadeau commented 7 months ago

The point of ci-friendly versions is to pass the version at build time which is passed via maven args e.g. -Drevision=nextVersion. If the m2 artifact path is not recalculated then caching will not work. The only way I see this working is to provide an option to pass the version which will be used to calculate the m2 artifact path outputDirLocalRepo

khalilou88 commented 7 months ago

nx-maven don't follow ci-friendly but we can calculate static version thanks to getEffectiveVersion function.

To add dynamic version, we can add an env variable NX_MAVEN_CI_FRIENDLY_VERSION. If it's set, getEffectiveVersion will return it's value.

But with this solution we will have just one version for all workspace.

What do you think?

mpsanchis commented 7 months ago

Hi @khalilou88 ! :) Replying because I was mentioned, and saw the whole discussion.

I think adding only one version for all workspace will not be a solution that fits many. I have already some examples of big repos with more than one "release unit" (aka version group). In CI, it could be that two of them are modified (affected), and then they are both built/tested/released. When released, I'd like them to have different versions, and then published to Maven registries as Jars with different versions.

I'd opt for passing an extra flag if necessary. In the end, it is not a strange case not to explicitly have your versions in the project metadata files in a monorepo. Nx themselves do it: the source code doesn't specify the version (0.0.1 is just a placeholder). Having ${revision} is our Java-equivalent way of saying "version will be calculated in CI on the fly, and then replaced before releasing the artifact".

khalilou88 commented 7 months ago

Right, I think it's better to add three environment variables: NX_MAVEN_CI_FRIENDLY_revision, NX_MAVEN_CI_FRIENDLY_sha1 and NX_MAVEN_CI_FRIENDLY_changelist. When calculating version we check this variables if exists otherwise we use the static values from pom.xml

khalilou88 commented 7 months ago

Also we can also extract revision, sha1 and changelist from project.json as @jbadeau mentioned. 1- First use values from project.json 2- Use values from env variables 3- Use values from pom.xml

jbadeau commented 7 months ago

Hey, what about a video call with miguel and I. We would like to show you how our repo looks.

Zoom?

khalilou88 commented 7 months ago

Maybe your repo it's similar to https://github.com/khalilou88/jnxplus-examples?

khalilou88 commented 7 months ago

Also i think it's better to keep {options.outputDirLocalRepo} for static versions and add an folder for dynamic CI FRIENDLY versions.

jbadeau commented 7 months ago

Do u have time for a video call?

khalilou88 commented 7 months ago

I don't feel this can be done. I feel a build with a dynamic version should not be cached. I think also, we should ask Nx team instead what they think.