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

Add Automatic-Module-Name #621

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Conversation

tsmock
Copy link
Contributor

@tsmock tsmock commented Aug 15, 2023

This fixes #566, and is probably preferable to #620 .

pom.xml Outdated
@@ -147,6 +147,7 @@
<Implementation-Title>metadata-extractor</Implementation-Title>
<Implementation-Vendor>Drew Noakes</Implementation-Vendor>
<Implementation-Version>${project.version}</Implementation-Version>
<Automatic-Module-Name>com.drew.imaging</Automatic-Module-Name>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think com.drew.metadata fits better to the artifactId and the fact that this library also supports video and audio formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm completely open to changing it to com.drew.metadata.

Signed-off-by: Taylor Smock <[email protected]>
@tsmock tsmock force-pushed the automatic-module-name branch from 0d5c38b to dbdb8b5 Compare September 21, 2023 14:41
@tsmock
Copy link
Contributor Author

tsmock commented Nov 30, 2023

Patch ping: Is there anything else I need to do?

@drewnoakes
Copy link
Owner

The issues here are my unfamiliarity with Java modules, concerns raised about minimum Java version requirements and the fact that our dependency xmp-core is not modular.

With this addition, can we still compile with Java 8 and target Java 6?

@tsmock
Copy link
Contributor Author

tsmock commented Dec 4, 2023

Yes, you can still compile with Java 8 and target Java 6 with this patch. All this does is modify the META-INF/MANIFEST.MF like so:

Manifest-Version: 1.0
Implementation-Title: metadata-extractor
+Automatic-Module-Name: com.drew.metadata
Implementation-Version: 2.18.0
Build-Jdk-Spec: 1.8
Created-By: Maven Archiver 3.4.0
Main-Class: com.drew.imaging.ImageMetadataReader
Implementation-Vendor: Drew Noakes

Java 8 and lower don't use the Automatic-Module-Name property. Java 9+ can.

EDIT: To add on, the Automatic-Module-Name property is specifically for enabling the Java ecosystem to slowly migrate to the module system while waiting on dependencies to do the migration. This does not require xmp-core to have module information. Using module-info.java would require xmp-core to have module information for stability reasons.

@tsmock
Copy link
Contributor Author

tsmock commented Feb 13, 2024

Patch ping: Is there anything else I need to do?

@drewnoakes
Copy link
Owner

Thanks for the change and explanation. This looks good to me and no one has expressed any concerns, so I'll merge this.

@drewnoakes drewnoakes merged commit bd083d3 into drewnoakes:main Feb 13, 2024
5 checks passed
@tsmock tsmock deleted the automatic-module-name branch February 14, 2024 12:07
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.

Add module-info for java9+
3 participants