Adds explicit module descriptors to a subset of modules

[aalmiray]

Applies the ModiTect Maven plugin to a select number of Maven modules.
Exports all packages by default as that's the current behavior when Automatic module names are in place.
Fine grained control of exported packages may be added at a later stage.

Fixes #1357

[aalmiray]

cc @velo

[velo]

Is there any way we could test the chance?

[aalmiray]

Is there any way we could test the chance?

We'd have to add at least one more Maven module that runs tests in the module path.

[velo]

We'd have to add at least one more Maven module that runs tests in the module path.

Is that something you can do? My concern would be a future change breaking it

But if you can't do I can merge as is

[aalmiray]

Is that something you can do? My concern would be a future change breaking it

I think I can make it happen but it'll take me a bit longer to figure it out.

But if you can't do I can merge as is

Given that the current module configuration exports everything by default (just like automatic modules do at the moment) there should not be much of a problem. With time this configuration may be refined to export only those packages that really must be visible to the exterior.

[velo]

Will need to revert this change

[ERROR] Failed to execute goal org.moditect:moditect-maven-plugin:1.0.0.RC2:add-module-info (add-module-infos) on project feign-core: Execution add-module-infos of goal org.moditect:moditect-maven-plugin:1.0.0.RC2:add-module-info failed: A required class was missing while executing org.moditect:moditect-maven-plugin:1.0.0.RC2:add-module-info: java/lang/module/FindException
[ERROR] -----------------------------------------------------
[ERROR] realm =    plugin>org.moditect:moditect-maven-plugin:1.0.0.RC2
[ERROR] strategy = org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy
[ERROR] urls[0] = file:/home/circleci/.m2/repository/org/moditect/moditect-maven-plugin/1.0.0.RC2/moditect-maven-plugin-1.0.0.RC2.jar
[ERROR] urls[1] = file:/home/circleci/.m2/repository/org/eclipse/aether/aether-util/1.1.0/aether-util-1.1.0.jar
[ERROR] urls[2] = file:/home/circleci/.m2/repository/org/moditect/moditect/1.0.0.RC2/moditect-1.0.0.RC2.jar
[ERROR] urls[3] = file:/home/circleci/.m2/repository/com/beust/jcommander/1.60/jcommander-1.60.jar
[ERROR] urls[4] = file:/home/circleci/.m2/repository/org/codehaus/plexus/plexus-utils/1.1/plexus-utils-1.1.jar
[ERROR] Number of foreign imports: 1
[ERROR] import: Entry[import  from realm ClassRealm[project>io.github.openfeign:parent:12.2-SNAPSHOT, parent: ClassRealm[maven.api, parent: null]]]
[ERROR] 
[ERROR] -----------------------------------------------------
[ERROR] : java.lang.module.FindException
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginContainerException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :feign-core


Exited with code exit status 1

CircleCI received exit code 1

[aalmiray]

Is circleci running the build with the wrapper? I think this might be an issue with incompatible (read: too old) Maven version.

[aalmiray]

I think I've got it. It's not the Maven version but the chosen JDK. The clue is in the exception

A required class was missing while executing org.moditect:moditect-maven-plugin:1.0.0.RC2:add-module-info: java/lang/module/FindException

The build requires [at least] Java 9 to run. Better set it to Java 11.

The update would have to happen here

feign/.circleci/config.yml

Lines 20 to 27 in bac0dd4

	java:
	parameters:
	version:
	description: 'jdk version to use'
	default: '8.0'
	type: string
	docker:
	- image: cimg/openjdk:<<parameters.version>>

[velo]

That is a problem, feign is java 8 compatible

[velo]

BTW, we had this discussion a while back, both me and @kdavisk6 are happy to move build process and testing to java 11, 17, 20, whatever, as long we can keep sources/jars java 8 compatible.... if you keen to give that a shot

[aalmiray]

Yes, but you can target Java 8 bytecode even if the build uses Java 11. This is also why ModiTect is used in the PR, so that the codebase remains Java 8 compatible. Set maven.compiler.source and maven.compiler.target properties to 1.8 at the root pom and all compiled classes should have bytecode 52.

[aalmiray]

Added back module-info support plus additional configuration to run Java8/Java11 on CI. #1948

[velo]

and all compiled classes should have bytecode 52.

should is the tricky part here.

and still, maven would build with jdk X classpath, so it would allow me to invoke method/classes that don't exist for java 8

[aalmiray]

Should? Yes. And verifiable. This is what I get when running the build with Java 11

$ jarviz bytecode show --file core/target/feign-core-12.2-SNAPSHOT.jar 
Unversioned classes. Bytecode version: 52 total: 156
Unversioned classes. Bytecode version: 53 total: 1

Besides, isn't animal-sniffer configured so that methods from Java 9+ are inaccessible? Also, running the build with Java 11 does not preclude you from configuring the IDE to use a lower Java setting. For example Intellij IDEA let's you configure the level in the project settings. I"m sure the other IDEs do so as well. that takes care of the problem when a developer might try to use java 9+ syntax/features. Animal sniffer OTOH can verify that at the build level regardless of the JDK used to run the build.