Skip to content

Conversation

@anthonydahanne
Copy link
Contributor

@anthonydahanne anthonydahanne commented May 9, 2024

  • generated dependencies in pom have changed

With

./gradlew clean :java-cfenv-test-support:generatePomFileForMavenJavaPublication

Before (assert 2):

<dependencies>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
            <type>test-jar</type>
        </dependency>
[...]
</dependencies>

after (assert 1):

<dependencies>
    <dependency>
      <groupId>io.pivotal.cfenv</groupId>
      <artifactId>java-cfenv</artifactId>
      <version>3.1.6-SNAPSHOT</version>
      <scope>compile</scope>
      <type>test-jar</type>
    </dependency>
</dependencies>

It's as if Gradle "optimized" the 2 deps into 1 - for most probably (I'm gonna find out) the same classpath result in Maven

@anthonydahanne
Copy link
Contributor Author

anthonydahanne commented May 10, 2024

Gradle team has confirmed they now de duplicate dependencies; hence only 1 now.

To revert to the previous behaviour, it was suggested to:

More specifically, java-cfenv-test-support defines the following two dependencies:

api project(':java-cfenv')
api testFixtures(project(':java-cfenv'))

Since test fixtures are not compatible with maven, Gradle emits the same entry in the POM for each of these dependencies — these entries are now deduplicated (before withXml is executed).
We do warn that the metadata being published is incompatible with Maven, however, those warnings are being suppressed here.
I would recommend either
Adding the entry manually instead of relying on Gradle to add it
(More preferably) Move the contents of java-cfenv’s test fixtures to java-cfenv-test-support if you want to remain Maven compatible.
A better story for this issue would be solved by this issue, where we would do a little better with the maven compatibility by emitting a dependency with a classifier for the testFixtures dependency instead of merging it with the normal project dependency.

Since it's not convenient at that time to change the project layout (like having a separate project only contain the testFixtures), for now this PR changes the assertion to 1 dependency, and then re adding the dep with a different type.

I tried

                    if (project.name == 'java-cfenv-test-support') {
                        def cfenvDependencies = pomNode.get('dependencies')[0].findAll { it.get('artifactId')[0].text() == 'java-cfenv' }
                        def clone = cfenvDependencies[0].clone()
                        assert cfenvDependencies.size() == 1
                        // see https://github.com/pivotal-cf/java-cfenv/pull/264#issuecomment-2105127180
                        cfenvDependencies[0].appendNode('type', "test-jar")
                        cfenvDependencies.add(clone)
                        assert cfenvDependencies.size() == 2
                    }

but it still got deduplicated.

Let's try tpopublish with just this one dependency; I can't think that's gonna cause issues downstream

* generated dependencies in pom have changed

Before (assert 2):
```
<dependencies>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>io.pivotal.cfenv</groupId>
            <artifactId>java-cfenv</artifactId>
            <version>3.1.6-SNAPSHOT</version>
            <scope>compile</scope>
            <type>test-jar</type>
        </dependency>
[...]
</dependencies>
```

after (assert 1):

```
<dependencies>
    <dependency>
      <groupId>io.pivotal.cfenv</groupId>
      <artifactId>java-cfenv</artifactId>
      <version>3.1.6-SNAPSHOT</version>
      <scope>compile</scope>
      <type>test-jar</type>
    </dependency>
</dependencies>
```

It's as if Gradle "optimized" the 2 deps into 1 -  for most probably (I'm gonna find out) the same classpath result in Maven
@pivotal-david-osullivan pivotal-david-osullivan merged commit 424a59d into main Aug 8, 2024
@anthonydahanne anthonydahanne deleted the fix-publish branch August 8, 2024 17:49
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

Successfully merging this pull request may close these issues.

3 participants