kiota-java icon indicating copy to clipboard operation
kiota-java copied to clipboard

microsoft-kiota-** dependencies have conflicting Automatic-Module-Name

Open dnijssen opened this issue 1 year ago • 5 comments

We use for our projects both jdeps and jlink tools to generate a slim JRE. However since the switch to com.microsoft.graph:microsoft-graph:6.4.0 (v6 of the SDK, recently released), which builds on top of the microsoft-kiota-** dependencies. We can't seem to run our jdeps command succesfully anymore.

Previously (without the microsoft-kiota-** dependencies on the class-/module path we have been using the following command; (exectued from target/)

jdeps --multi-release base --recursive --ignore-missing-deps --print-module-deps --class-path="./lib/*" --module-path="./lib/*" -quiet demo-0.0.1-SNAPSHOT.jar

Which would list something like this; java.base,spring.boot,spring.boot.autoconfigure,spring.context,spring.core

Note: When running this on a Windows machine, be sure to change ./lib/* to ./lib (as there is an issue with the * wildcard.

Howafter after switching to com.microsoft.graph:microsoft-graph:6.4.0 we not get the following exception when we run the same command;

Exception in thread "main" java.lang.module.FindException: Two versions of module com.microsoft.kiota found in .\lib (microsoft-kiota-authentication-azure-1.0.4.jar and microsoft-kiota-abstractions-1.0.4.jar)
        at java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:295)
        at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:233)
        at java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:191)
        at java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:167)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsConfiguration$Builder.build(JdepsConfiguration.java:558)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.buildConfig(JdepsTask.java:607)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:561)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:537)
        at jdk.jdeps/com.sun.tools.jdeps.Main.main(Main.java:50)

A quick look here in the repository shows me that all components/**/build.gradle files are containing this piece of code;

tasks.jar {
    manifest {
        attributes('Automatic-Module-Name': project.property('mavenGroupId'))
    }
}

So the Automatic-Module-Name is set to an unique value, which isn't correct in my opinion. Might be better to use the artifactId here maybe? But that is up to you guys ;)

Following pom.xml file can be used to simulate this issue. Note the maven-dependency-plugin which copies all the dependencies (on mvn install) to target/lib for easier testing with the command.

<?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>
    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>3.2.3</version>
        <relativePath/> <!-- lookup parent from repository -->
    </parent>
    <groupId>com.example</groupId>
    <artifactId>demo</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <name>demo</name>
    <description>Demo project for Spring Boot</description>
    <properties>
        <java.version>21</java.version>
        <microsoft-graph.version>6.4.0</microsoft-graph.version>
        <azure-identity.version>1.11.2</azure-identity.version>
    </properties>
    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter</artifactId>
        </dependency>
        <dependency>
            <groupId>com.microsoft.graph</groupId>
            <artifactId>microsoft-graph</artifactId>
            <version>${microsoft-graph.version}</version>
        </dependency>
        <dependency>
            <groupId>com.azure</groupId>
            <artifactId>azure-identity</artifactId>
            <version>${azure-identity.version}</version>
            <scope>compile</scope>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
            </plugin>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-dependency-plugin</artifactId>
                <executions>
                    <execution>
                        <phase>install</phase>
                        <goals>
                            <goal>copy-dependencies</goal>
                        </goals>
                        <configuration>
                            <outputDirectory>${project.build.directory}/lib</outputDirectory>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>

</project>

dnijssen avatar Mar 02 '24 06:03 dnijssen

Hi @dnijssen Thanks for using the Microsoft Graph SDK and for bringing this up. This is most likely because we've copy/pasted some of the build.gradle sections from microsoft graph 5.X without thoroughly evaluating the impacts at the time. After reading this guidance, a couple of things seem to stand out (please correct me if I'm wrong in my evaluation):

  • references are only broken for people using the packages in a "automatic module mode" instead of the default jar mode.
  • changing the module name WILL NOT impact the people using the default jar resolution.
  • module names need to be unique.
  • setting different module names WILL NOT make a difference to people using automatic module mode (import paths, etc...) when compared with the default jar mode.

If all that holds true, I'd like to suggest the following module names:

  • abstractions: com.microsoft.kiota.abstractions
  • json serialization: com.microsoft.kiota.serialization.json
  • text serialization: com.microsoft.kiota.serialization.text
  • form serialization: com.microsoft.kiota.serialization.form
  • multipart serialization: com.microsoft.kiota.serialization.multipart
  • azure authentication: com.microsoft.kiota.authentication.azure
  • okhttp http: com.microsoft.kiota.http.okhttp

Thoughts?

I'd also like to get @andreaTP opinion on the matter.

baywet avatar Mar 04 '24 13:03 baywet

Thanks for looping me in @baywet , in my experience, it's rare that ppl will use jlink but it is a valid use-case and we want to support it. I don't know what would be the most "correct" resolution though.

andreaTP avatar Mar 04 '24 14:03 andreaTP

I see this issue has been moved to the v1.14 milestone, is there anything holding this issue up? Would love to see this issue resolved, suggested module names sounds more than right in my opinion :)

dnijssen avatar Apr 08 '24 13:04 dnijssen

@dnijssen this has been moved from one milestone to another as part of our release process (1.13 was released) but nobody is working on that at the moment (staff shortage/other priorities). We'll be more than happy to review a PR if you are willing to submit one, as far as I understand, the only required work here is to update the header

jar {
    manifest {
        name = "mylibrary"
        instruction "Automatic-Module-Name", "com.acme.mylibrary"
    }
}

https://github.com/microsoft/kiota-java/blob/9bf3a4f53c542dde26441c150b9ed38018b154ef/components/abstractions/build.gradle#L86

baywet avatar Apr 15 '24 17:04 baywet

with the discussion in #1224 we realized that we have split packages, which would require moving classes to different packages to fully implement that change. We're unlikely to so that soon since it'd represent a source breaking change. Queuing for the next major version.

baywet avatar Apr 18 '24 14:04 baywet