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

JDK9+ Incorrect JPMS Automatic-Module-Name = simple-java-mail #245

Closed
colesico opened this issue Jan 30, 2020 · 14 comments
Closed

JDK9+ Incorrect JPMS Automatic-Module-Name = simple-java-mail #245

colesico opened this issue Jan 30, 2020 · 14 comments

Comments

@colesico
Copy link

Jpms module name Automatic-Module-Name: simple-java-mail in MANIFEST.MF is not valid, change it please to simple.java.mail or else that is valid. thx!

@colesico colesico changed the title Incorrect JPMS Automatic-Module-Name = simple-java-mail JDK9+ Incorrect JPMS Automatic-Module-Name = simple-java-mail Jan 30, 2020
@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

This is a critical problem that prevents simple-java-mail library from being included in modularized application. PR coming.

@bbottema bbottema added this to the 6.0.2 milestone Jan 31, 2020
@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

Actually, scratch that. In order for me to be able to add correct automatic module infos, I would have to:

  • Make the project compile on Java 11. (I don't use Java 8 anymore.)
  • For that, I have to work around the nonnull plugin which is so outdated it can not deal with Java 11 classfiles.
  • For that, I have to reconfigure maven-compiler-plugin to consistently only build Java 7-compatible class files.
  • And I failed to do that, since pluginManagement does not override the maven-compiler-plugin 3.5.1 coming from some parent POM outside of this Github repo.

At this point, I decided I don't have enough time to follow the rabbit hole so deep. :-) commons-email it is.

@bbottema
Copy link
Owner

bbottema commented Jan 31, 2020

Actually, scratch that. In order for me to be able to add correct automatic module infos, I would have to:
(..)

Hmm, sounds like shaving a yak :D

Ok, so I control the parent POM repo's, does that help? I have some time now to take care of things.

If the nonnull really can't work with Java 11, I think it should be tossed out until something better comes along. However, you can already simply disable the plugin:

Simply add the following property to the POM:

<se.eris.notnull.instrument>false</se.eris.notnull.instrument>

That should greatly simplify the problem, yes?

@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

Ok, so I control the parent POM repo's, does that help? I have some time now to take care of things.

I think that it would help if the plugin configurations in those outside POMs moved to pluginManagement section - that would allow overrides later.

If the nonnull really can't work with Java 11, I think it should be tossed out until something better comes along.

I agree. I checked, there is no newer version.

<se.eris.notnull.instrument>false</se.eris.notnull.instrument>


That should greatly simplify the problem, yes?

Disabling it would help, yes. If that's allowed, I may give it another try later.

@bbottema
Copy link
Owner

Yes, let's disable it for now.

Sounds like the Java 11+ multi-release issue has become more important now as well.

@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

@bbottema This is actually a precursor to #237.
You can have your library running in a modular system on Java 11, with the correct automatic module names. (This is what this issue is about.)
#237 is about making it possible to have the library be a full-fledged Java module. It's not necessary, but not doing it prevents people from using jlink.

@bbottema
Copy link
Owner

bbottema commented Jan 31, 2020

Ahh, ok. Seems like the next step for Simple Java Mail then is to be more compatible with the future and get both these issues out of the way.

@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

I have attached a Patch with limited Java 11 support.

However, it only works like this:

mvn clean install -Djacoco.skip=true -DskipTests

For full support:

  • JaCoCo needs updating in one of the outside POMs.
  • org.simplejavamail.converter.internal.mimemessage.MimeMessageHelperTest will need rewriting. It uses reflection on JDK internals, which I believe is illegal now.

To make any sort of changes easier in the future, I also suggest refactoring the POMs so that the outside POMs don't have absolute control. Case in point: maven-compiler-plugin. I didn't figure out a way to upgrade its version (or Surefire, JaCoCo, ...) without touching the outside POMs.

@bbottema
Copy link
Owner

bbottema commented Jan 31, 2020

I think just redeclaring the plugin with just the version should result in a merged plugin config with the version overridden?

But yes, I agree the outside POM's need to be refactored for this purpose.

@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

@bbottema I thought so, too. Didn't work for maven-compiler-plugin, so I didn't even try with JaCoCo.

@bbottema
Copy link
Owner

Actually the order of the plugins matter a lot because of jacoco and the license plugin (and the notnull plugin if used). Moving them to the pluginManagement section would mean the inheriting pom's would have to define all the plugins again and in the right order.

Instead, at least for now I'll just keep the version management in the pluginManagement and keep the notnull plugin property to disable it.

@triceo
Copy link
Contributor

triceo commented Jan 31, 2020

@bbottema I think it'd be enough to just define the versions in pluginManagement and remove them from plugins. Then any child pluginManagement section could override those versions without affecting the order defined in parent plugins. Alternatively, the versions could be specified as properties, and child projects can override them this way.

The goal of this change (at least the way I see it) is to lessen the barrier to entry when it comes to the project. And for me, that barrier is the parent POMs in some other repos, to which I would need to make separate PRs. But we should probably move that discussion to another issue, as it's only tangentially related to this one.

@bbottema bbottema modified the milestones: 6.0.2, 6.1.0 Feb 1, 2020
bbottema added a commit that referenced this issue Feb 1, 2020
…enyManagement, updated Jakarta dependencies for JDK9+. Disabled notnull Maven plugin, which doesn't work with JDK11+
@bbottema
Copy link
Owner

bbottema commented Feb 1, 2020

@triceo, I've updated the parent poms, disabled the notnull plugin and added the Jakarta dependencies.

The path should be clear now, let me know if you need anything else.

bbottema added a commit that referenced this issue Feb 5, 2020
Add automatic module names (fixes #245)
@bbottema
Copy link
Owner

bbottema commented Feb 6, 2020

Released in 6.0.3

@bbottema bbottema modified the milestones: 6.2.0, 6.0.3 Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants