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

Arrange assertion #264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Arrange assertion #264

wants to merge 1 commit into from

Conversation

anthonydahanne
Copy link
Member

@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

* 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
@anthonydahanne
Copy link
Member 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

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.

None yet

1 participant