Modularize modernize #61

io7m · 2018-03-13T10:15:01Z

Hello!

I'm not sure if you'll be interested in this, but I want to use ode4j in a Java 9 modular project. I've been assisting a lot of other projects with transitioning to a modular world, so I thought I'd give modularizing ode4j a try.

It actually went very well: You have very few dependencies and a well defined package hierarchy. The commits below do the following:

Clean up the POM structure slightly. This moves all version numbers to a central location so that they can be updated once. It moves all plugin versions to properties so that they can be updated with the Maven Versions plugin (for some reason the plugin doesn't support updating plugin versions, but it does support updating properties and can therefore update plugins that way). I reformatted the POMs using your existing layout settings.
Start requiring JDK 9 to build the code. Don't panic: The code produced is still Java 7 compatible. In fact, it's actually guaranteed to be Java 7 compatible in a way that it wasn't before: The JDK 9 compiler comes with a new --release flag that guarantees both bytecode and API compatibility with the specified release version (previously javac could only guarantee bytecode compatibility). I've added an Enforcer plugin execution to give users a nice error message if they try to compile the code with a version of the JDK that's too old. I had to remove -Werror from the compiler options as the newer compiler seems to emit warnings that the old compiler didn't and I didn't want to modify the code to fix them without knowing what I was doing. One exception to this: I added @Deprecated annotations to methods that were marked via JavaDoc as @deprecated but lacked a corresponding @Deprecated annotation. This squashed a good 50-60 warnings.
Add module descriptors. This is what makes the project a modular project. The descriptors explicitly state which modules are required and which packages are exported. I exported everything, but perhaps you'd want to review this as maybe some of the "internal" packages shouldn't be exported. I don't know the API well enough yet to say.
Add a .travis.yml config file to build the code on Travis CI. You don't have to keep this if you don't want Travis. The config file will sit there doing nothing until you either delete it or enable Travis for your Github account. The code builds correctly and all tests pass.

This cleans up the POM files in several ways: Dependency versions are declared in the parent POM so that all version updates happen in one place. The user is now required to use JDK 9 to compile the code, but the produced bytecode is guaranteed to be compatible with JDK 7 and above thanks to the use of the -release flag. Plugin versions are replaced with properties for painless updates with the Maven Versions plugin. All POM files were run through Intellij IDEA's XML formatter for consistent indentation. Compiler severity has been reduced from -Werror temporarily due to new warnings that would have otherwise broken the build.

@deprecated

This adds @deprecated annotations to all methods that had @deprecated JavaDoc sections.

This adds module descriptors with the correct "requires" clauses. The modules export everything (including classes in the "internal" packages), so this may need to be reviewed.

tzaeschke · 2018-03-13T20:52:25Z

Thank, very nice, I'll have a look. I wonder what other users think, being backwards compatible is an important feature (which you considered).

io7m · 2018-03-14T13:30:43Z

Having done quite a lot of this, I think it's arguably acceptable to push people to use newer compilers (because they're freely available everywhere and there are rarely any downsides to upgrading), but not acceptable to force people to use newer runtimes (because there are often technical and organizational reasons that users can't migrate their existing deployments right away).

The above changes only do the former, to be clear!

io7m · 2018-03-21T21:25:23Z

I have some extra additions I can make that would add OSGi metadata. Because why be compatible with one module system when you can be compatible with two! 😁

tzaeschke · 2018-03-21T23:12:39Z

Hmm, I'm not sure I understand the point of using OSGi for ode4j. Wouldn't there need to be a standard physics API that ode4j needs to support? Or how would you use ode4j through OSGi?

io7m · 2018-03-22T08:49:44Z

I'm not sure what you mean. A standard physics API?

OSGi is a module system just like Java 9's module system. To support it, you just add some fields to the jar manifest. We actually did this for Dyn4j:

https://github.com/dyn4j/dyn4j/blob/master/pom.xml#L219

$ jar-show-manifest target/dyn4j-3.2.5.jar 
Manifest-Version: 1.0
Bnd-LastModified: 1511086651143
Build-Jdk: 1.8.0_144
Built-By: William Bittle
Bundle-Description: Java Collision Detection and Physics Engine
Bundle-DocURL: http://www.dyn4j.org
Bundle-License: http://www.opensource.org/licenses/BSD-3-Clause
Bundle-ManifestVersion: 2
Bundle-Name: dyn4j
Bundle-SymbolicName: org.dyn4j
Bundle-Vendor: dyn4j
Bundle-Version: 3.2.5
Created-By: Apache Maven Bundle Plugin
Export-Package: org.dyn4j.resources;version="3.2.5",org.dyn4j.collisio
 n;uses:="org.dyn4j,org.dyn4j.geometry";version="3.2.5",org.dyn4j.coll
 ision.continuous;uses:="org.dyn4j.collision.narrowphase,org.dyn4j.geo
 metry";version="3.2.5",org.dyn4j.collision.narrowphase;uses:="org.dyn
 4j.geometry";version="3.2.5",org.dyn4j.collision.broadphase;uses:="or
 g.dyn4j.collision,org.dyn4j.geometry";version="3.2.5",org.dyn4j.colli
 sion.manifold;uses:="org.dyn4j.collision.narrowphase,org.dyn4j.geomet
 ry";version="3.2.5",org.dyn4j.dynamics;uses:="org.dyn4j,org.dyn4j.col
 lision,org.dyn4j.collision.broadphase,org.dyn4j.collision.continuous,
 org.dyn4j.collision.manifold,org.dyn4j.collision.narrowphase,org.dyn4
 j.dynamics.contact,org.dyn4j.dynamics.joint,org.dyn4j.geometry";versi
 on="3.2.5",org.dyn4j.dynamics.contact;uses:="org.dyn4j,org.dyn4j.coll
 ision.continuous,org.dyn4j.collision.manifold,org.dyn4j.dynamics,org.
 dyn4j.geometry";version="3.2.5",org.dyn4j.dynamics.joint;uses:="org.d
 yn4j,org.dyn4j.dynamics,org.dyn4j.geometry";version="3.2.5",org.dyn4j
 ;version="3.2.5",org.dyn4j.geometry;uses:="org.dyn4j";version="3.2.5"
 ,org.dyn4j.geometry.hull;uses:="org.dyn4j.geometry";version="3.2.5",o
 rg.dyn4j.geometry.decompose;uses:="org.dyn4j.geometry";version="3.2.5
 "
Implementation-Title: dyn4j
Implementation-URL: http://www.dyn4j.org
Implementation-Vendor: dyn4j
Implementation-Vendor-Id: org.dyn4j
Implementation-Version: 3.2.5
Import-Package: org.dyn4j,org.dyn4j.collision,org.dyn4j.collision.broa
 dphase,org.dyn4j.collision.continuous,org.dyn4j.collision.manifold,or
 g.dyn4j.collision.narrowphase,org.dyn4j.dynamics,org.dyn4j.dynamics.c
 ontact,org.dyn4j.dynamics.joint,org.dyn4j.geometry,org.dyn4j.resource
 s
Main-Class: org.dyn4j.Version
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.6))"
Specification-Title: dyn4j
Specification-Vendor: dyn4j
Specification-Version: 3.2.5
Tool: Bnd-3.3.0.201609221906

The Import-Package, Export-Package, and all Bundle-* attributes are generated by the maven-bundle-plugin and make it so that Dyn4j can work on OSGi. It's fairly common for Maven projects to add OSGi metadata these days as it's pretty much "add this plugin and forget about it".

tzaeschke · 2018-03-22T23:24:04Z

I've heard good things about dyn4j, but haven't used it myself yet :-)
It still seems to be on Java 6, are you also compiling it 'back' from Java 9?

Hmm, I honestly haven't used OSGi much (a tiny Eclipse plugin, and my ZooDB project also supports OSGi service discovery). In both cases OSGi provides dynamic loading of components/services, which, however, require implementing some kind of standard API in order to provide that service. So I thought ode4j would also become a 'service', hence my assumed need for a standard physics API.

I suppose there is much more to OSGi than I am aware. I'll have to read up on it a bit to see what it can do, especially in terms of modularization.

If I understand correctly, the OSGi update consists (mostly) of a Manifest update?

io7m · 2018-03-22T23:45:51Z

I've heard good things about dyn4j, but haven't used it myself yet :-)
It still seems to be on Java 6, are you also compiling it 'back' from Java 9?

Yep. Essentially what we do is compile all of the code (including the module descriptor) as Java 9 bytecode. This gives us error reporting for things like missing "requires" clauses, etc. We then recompile all of the code except for the module descriptor using the --release 6 flag, which produces guaranteed-Java-6-compatible bytecode.

Hmm, I honestly haven't used OSGi much (a tiny Eclipse plugin, and my ZooDB project also supports OSGi service discovery). In both cases OSGi provides dynamic loading of components/services, which, however, require implementing some kind of standard API in order to provide that service. So I thought ode4j would also become a 'service', hence my assumed need for a standard physics API.

You're right in that they do love their services. In the case of ode4j though, you've got pure library code, there won't be multiple implementations of it, and it doesn't represent a resource that can potentially be unavailable at run-time. In this case, there's nothing about OSGi that could likely improve the code and services/components wouldn't be involved. You'd just be adding metadata so that other people can use ode4j in their OSGi-based applications.

I suppose there is much more to OSGi than I am aware. I'll have to read up on it a bit to see what it can do, especially in terms of modularization.

I think in the case of a project like ode4j, you really don't need to do anything. Basically, for a project to be usable inside OSGi, it needs metadata similar to the way that Java 9 modules need metadata: They need to provide export declarations so that other projects can specify dependencies on them, and they need to declare their own dependencies so that the OSGi system can properly check them. The tooling for OSGi is advanced to the point that you essentially never need to declare any of this stuff (except for the exports) yourself. You can run the maven-bundle-plugin and it'll analyze the bytecode of the project and work out all of the dependencies and versioning automatically and insert it into
the manifest for you. The exports you add will be the same as the exports clauses in the Java 9 module descriptor.

If I understand correctly, the OSGi update consists (mostly) of a Manifest update?

Yep, it's purely a manifest update.

io7m · 2018-03-22T23:59:39Z

You might enjoy this one: https://blog.io7m.com/2017/06/01/osgi-resource-bundles.xhtml

ppiastucki · 2018-03-23T07:20:51Z

core-cpp/src/main/java/module-info.java

+ *                                                                       *
+ *************************************************************************/
+
+module ord.ode4j.cpp {


ord.ode4j ?

Oops. Corrected!

ppiastucki · 2018-03-26T06:28:59Z

Looks good to me. I am wondering if there are any OSGi-based applications using ode4j currently, maybe some apps based on Eclipse or Netbeans platform?

tzaeschke · 2018-03-26T16:42:11Z

I have pulled the changes, however, I have set source=1.7 for know. I created a poll to get a better idea who requires Java 7 compatible source code (for example when developing for android):

io7m · 2018-03-26T17:11:06Z

'Ello.

This change is incorrect:

bde8b23#diff-600376dffeb79835ede4a0b285078036R205

The reason being that the first step is to compile all of the code as Java 9 (hence source == 9). If you don't do this, the compiler will either reject the module-info.java or will not give the correct error messages in future when making changes to the module descriptor.

io7m · 2018-03-26T17:12:33Z

Maybe add:

<source>7</source>
<target>7</target>

and:

<source>9</source>
<target>9</target>

... to help IDEs decide what compliance level they should be using.

tzaeschke · 2018-03-26T20:31:45Z

Okay, I admit I didn't completely realize the implications of this whole pull request.

First, two observations:

I think <source>7</source> and <source>1.7</source> are equivalent(?) :-)
Even with <source>7</source>, the projects appears to compile just fine, no complains about the module-info.java. (It did however complain about a 'default' method that I introduced for testing.) I'm not sure that this behavior can be relied upon. As you point out, it may simply ignore the module-info files and not show respective error messages.

I'm not quite sure where you suggested to put the source/target=9 entries?

What I somehow missed is this: With the original pull-request, a developer may be tempted to write something with Java 9 compliant code and only during the maven build (potentially much later) realize that Java 9 is not 'allowed'. However, enforcing Java 7 source compliance removes the benefit of better warnings, it also unnecessarily limits developers that do not build with Maven at all.
For now I will roll back to the original pull-request.

Does anyone have any more thoughts on this?
Otherwise we will just wait and see what people say.

io7m · 2018-03-26T20:48:15Z

I think 7 and 1.7 are equivalent(?) :-)

I think they probably are, yes. I've been staring at Java 9 projects for so long that I've developed the habit of referring to versions by their "new" names. 😄

Even with 7, the projects appears to compile just fine, no complains about the module-info.java.

Yes, I've just tried it and I see that too. I'm not sure that's supposed to happen.

I'm not quite sure where you suggested to put the source/target=9 entries?

I'm not sure which would be preferable behaviour. If you specify Java 7 in a way that'll be picked up by IDEs, then those IDEs are going to complain about the module descriptor existing at all (IDEA, for example, tells you that you need to use Java 9 to define one). If you specify Java 9, the IDE will let you write a module descriptor but you won't get errors about using > 7 APIs and/or language features until you do the Maven build.

I feel like the latter is better. Most projects I work with use the Maven build as the canonical source of binaries, and they usually set up something like Travis CI to build pull requests and complain if the compilation fails. I think this is more a case of IDE immaturity than anything. Java 9 hasn't been out long, and I don't think IDEs have fully caught up with the consequences of the -release flag. I suspect that they will probably sprout the ability to treat module descriptors as Java >= 9 code, and the rest of the project as some other version sooner or later. You're hardly the first project that wanted to support older releases!

I have noticed that some IDEs ignore the -release flag and only use the -source and -target flags to determine language compliance level. Adding those might help.

Maybe you want something like this:

        <!--
          Maven Compiler plugin.
          https://maven.apache.org/plugins/maven-compiler-plugin/
        -->

        <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-compiler-plugin</artifactId>
          <version>${ode4j.maven-compiler-plugin.version}</version>
          <executions>

            <!--
              Compile everything as JDK 9 bytecode: This ensures
              that we get decent compilation errors such as
              missing "requires" directives in the module
              descriptor.
            -->

            <execution>
              <id>default-compile</id>
              <configuration>
                  <source>9</source>
                  <target>9</target>
                  <release>9</release>
              </configuration>
            </execution>

            <!--
              Recompile everything except for the module
              descriptor as JDK 7 bytecode.
            -->

            <execution>
              <id>compile-7</id>
              <goals>
                <goal>compile</goal>
              </goals>
              <configuration>
                <excludes>
                  <exclude>module-info.java</exclude>
                </excludes>
              </configuration>
            </execution>
          </executions>

          <!-- These are the default settings for all executions. -->
          <configuration>
            <source>7</source>
            <target>7</target>
            <release>7</release>
            <compilerArgument>-Xlint:all</compilerArgument>
            <showWarnings>true</showWarnings>
            <showDeprecation>true</showDeprecation>
          </configuration>
        </plugin>

io7m · 2018-03-27T08:58:16Z

I don't think IDEs have fully caught up with the consequences of the -release flag. I suspect that they will probably sprout the ability to treat module descriptors as Java >= 9 code, and the rest of the project as some other version sooner or later

Turns out Mark Reinhold brought up this exact point on the OpenJDK Jigsaw list yesterday:

There's also at least one improvement in the JDK worth
considering, which a few of us have already discussed, namely a small
enhancement to javac that would allow a single invocation to compile a
module, in module mode [2], but target class files other than
module-info.class to an earlier release [3].

http://mail.openjdk.java.net/pipermail/jigsaw-dev/2018-March/013689.html

io7m added 5 commits March 13, 2018 09:11

Add missing @deprecated annotations

1cb49b9

This adds @deprecated annotations to all methods that had @deprecated JavaDoc sections.

Add module descriptors

c7ee48d

This adds module descriptors with the correct "requires" clauses. The modules export everything (including classes in the "internal" packages), so this may need to be reviewed.

Add Travis CI configuration

f99380c

Add "requires transitive"

03fbd3d

ppiastucki reviewed Mar 23, 2018

View reviewed changes

io7m added 2 commits March 23, 2018 09:50

Correct module name (ord.ode4j -> org.ode4j)

02d4719

Adjust formatting slightly

9f9766e

tzaeschke merged commit 564ef50 into tzaeschke:master Mar 26, 2018

tzaeschke mentioned this pull request Mar 30, 2018

Trouble compiling to Java 7/8 #63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modularize modernize #61

Modularize modernize #61

io7m commented Mar 13, 2018 •

edited

Loading

tzaeschke commented Mar 13, 2018

io7m commented Mar 14, 2018

io7m commented Mar 21, 2018

tzaeschke commented Mar 21, 2018 •

edited

Loading

io7m commented Mar 22, 2018

tzaeschke commented Mar 22, 2018

io7m commented Mar 22, 2018

io7m commented Mar 22, 2018

ppiastucki Mar 23, 2018

io7m Mar 23, 2018

ppiastucki commented Mar 26, 2018

tzaeschke commented Mar 26, 2018

io7m commented Mar 26, 2018

io7m commented Mar 26, 2018

tzaeschke commented Mar 26, 2018

io7m commented Mar 26, 2018

io7m commented Mar 27, 2018

Modularize modernize #61

Modularize modernize #61

Conversation

io7m commented Mar 13, 2018 • edited Loading

tzaeschke commented Mar 13, 2018

io7m commented Mar 14, 2018

io7m commented Mar 21, 2018

tzaeschke commented Mar 21, 2018 • edited Loading

io7m commented Mar 22, 2018

tzaeschke commented Mar 22, 2018

io7m commented Mar 22, 2018

io7m commented Mar 22, 2018

ppiastucki Mar 23, 2018

Choose a reason for hiding this comment

io7m Mar 23, 2018

Choose a reason for hiding this comment

ppiastucki commented Mar 26, 2018

tzaeschke commented Mar 26, 2018

io7m commented Mar 26, 2018

io7m commented Mar 26, 2018

tzaeschke commented Mar 26, 2018

io7m commented Mar 26, 2018

io7m commented Mar 27, 2018

io7m commented Mar 13, 2018 •

edited

Loading

tzaeschke commented Mar 21, 2018 •

edited

Loading