Repository navigation
I would like to strictly name the module #451
Description
Hi There,
I would like to update the pom and build to generate the module-info class file, and strictly name this module.
Currently as an automatic module, you cannot perform a lot of JPMS functions, such as requires transitive, or evade the maven central deploy warning for unnamed modules.
This is purely the raw code, could I get a zip of the build and send back a finished zip for perusal?
Activity
johnjaylward commentedon Nov 26, 2018
https://github.com/BGehrels/JSON-java holds the packaging for maven releases. Please peruse there and offer any merge requests at that location.
johnjaylward commentedon Nov 26, 2018
Also, strictly speaking, this project must support back to Java6 still until @stleary mentions otherwise. So any module-info changes must not cause an issue with java6 runtimes.
GedMarc commentedon Nov 26, 2018
It will still support JDK 6 so long as
This will produce the 1.6 output with the module-info correctly placed,
Thanks for considering it!, Hope you pull it in!
GedMarc commentedon Nov 26, 2018
BGehrels#3
GedMarc commentedon Nov 26, 2018
You can view the build log and the output here (Guest user is enabled)
https://jwebmp.com/teamcity/viewLog.html?buildId=201982&tab=artifacts&buildTypeId=ThirdPartyBuild_BuildOrgJson
Thanks!
stleary commentedon Nov 26, 2018
Will existing developers have to change their build process to use the new build?
GedMarc commentedon Nov 26, 2018
Not at all, this simply adds the module-info class file into the final packaged jar
I wanted to double check the class file version, it is indeed JDK 1.6 version 50
xzel23 commentedon Nov 27, 2018
Hi, I have created a fork that adds modules, but I build it using gradle. I could contribute the module-info.java files. I would be happy to help to get module-info into trunk.
[UPDATE] Seems I did not yet push my changes to my fork. I will look into this.
GedMarc commentedon Nov 27, 2018
I see you've moved the classes to a higher version of Java,
Thanks!
This strictly naming is definitely needed so you don't have to specify requires org.json in the consuming module but only the dependency, and to keep org.json out of the java.base root module (automatic named module placement)
To be honest, I do prefer my method, it keeps the libraries perfectly in tact without any code changes of any kind, or bumping of JDK's.
I would also like to still be able to use the official build repository, rather than otherwise.
If worst comes to worst, on all these projects that require the push forward into JPMS, I'll be placing them onto the com.jwebmp maven group, and utilizing all the updates from there :)
P.S. Gradle is the android native build tool. You shouldn't actually be using it for Java. While Maven Gradle Ivy and Grunt can all build Java, and they can all build android native, each was designed and built specifically with a purpose in mind. You should be using the build tool built for Java to enable advanced functions that are required post JDK 7 like annotation processor paths, annotation arguments, processing updates and manipulation, pretty much anything in the "advanced" category of Java building, and etc
So I wouldn't use this fork either based purely on the build tool choice. But that part is my opinion.
xzel23 commentedon Nov 28, 2018
@GedMarc: Sorry, but I think I only understand half of what you try to say.
I think adding the module-info.class files will probably break things for some developpers. That's also the reason why I did not try to contribute back. I have to research because it's been some time, but I remember these issues:
While module-info.class files should be ignored when used with older Java versions, at least some tools choked on this, IIRC these were among others
Another solution would be Multi-release Jar files, but with more or less the same issues.
If you don't want to abandon Java 6 compatibility (a version that has been unsupported by Oracle for several years now), you should probably not force your users to use the newest versions of these softwares (I don't even know if all problems have been solved in the meantime).
So, at least some developpers will have to change their build process or dependencies. And I also doubt all those new versions will still be compatible with Java 6.
IMHO it would be great to have a truly modularized version in trunk, but I think you have to make a compromise. You cannot have it and still be compatible with JDK 6 and the tools used to maintain Java 6 based software.
Concerning your comment about Gradle: Maven is not the build tool for Java. If there's a tool that deserves that name, it should still be Ant. Just because Gradle has been adopted as Android's standard buid tool doesn't mean it's not suitable for Java. Following your rationale, you shouldn't even use Java for anything other than Android development, right? But I think this really doesn't belong here, and if you want to discuss build tools further, you can rather contact me directly. Gradle was simply the tool I chose because I could reach my goal faster.
johnjaylward commentedon Nov 28, 2018
@xzel23 The PR that @GedMarc provided only adds the module.info file in the JAR during the packaging process. It does not add it to this project. See his PR at #451 (comment)
johnjaylward commentedon Nov 28, 2018
Also, as a side note, please leave build tool discussions out of this project. They are not relevant.
xzel23 commentedon Nov 29, 2018
@johnjaylward But there's a packaged Jar at mavencentral, linked from the main github page. Won't it add the module-info once this is merged? I think it should, but I also think this could break builds.
xzel23 commentedon Nov 29, 2018
@GedMarc Maybe you could add to the PR description that with automatic modules, you cannot use JDK 9+
jlinktool to create standalone applications. That was the main reason to have a modularized version for me.GedMarc commentedon Nov 29, 2018
@xzel23 The idea behind this is that the original packaging/class files are not manipulated at all, and the way that the JVM works, the module-info class file will never be hit in any JVM less than 9 (the hyphen makes the class name invalid) so no incompatibility is ever brought in.
Moditect itself actually constructs the class file byte-for-byte and the file never goes through any "compilation", so when the class gets added into the final package, it can contain invalid references that the compiler would normally throw out.
THIS IS IMPORTANT! so important in fact, that in some projects that are JRE11, I still use moditect - as the build process may JarJar a dependency that needs to be exported, but would never ever compile as you can't compile empty packages ;)
(Take a look at the guice implementation that bundles asm and cglib, but cglib has to be exported. Even in JRE11, this compile would never be possible without Moditect)
This means that all builds using Moditect are unaffected, and are guaranteed to still execute properly, even with the module-info.class file in the JAR regardless of the project version (down to 1.5, 1.4 no compatible but that's a JRE 8 thing)
I did not know about JLink restrictions! We always learning lol, I'll need to run some tests, but from what I understand, if you open your base module, JLink will still work with filename/automaticmodule named modules, I haven't tried/tested this, but logically it should work.
I may actually use that as a further base for Hibernate and Guice implementations after I've verified that it is indeed the case. You mentioned JUnit 4, but you can't use that for JPMS, please use Jupiter (5) for proper TCK in JPMS, and don't use any junit 4 runners, keep it strictly jupiter. I have had great success going this route, and awful experiences in junit 4 :)
The goal as usual in Java, is not require any changes or dependencies code wise for JVM migration, and this solution does solve that properly.
23 remaining items
BGehrels commentedon Dec 4, 2018
Hmm, it's sad that @GedMarc closed this issue, because i consider the discussion highly valuable.
I'm also highly irritated by the way motivated contibutor is bounced without even a simple "Thank you for the work you've invested and your efforts to keep our library up to date. We sadly can't merge it but highly appreciate your contribution and input to this discussion".
The decision to stick to Java 6 was taken 3 years ago in #156 in order to support Android versions <4.4. While back then, ~50% of the android devices still used <4.4, it's just 3,5 % nowadays - and given the short live of mobile devices nowadays i guess it will be much less in 6 months from now. So maybe it's time to drop this requirement. Spring, for example, dropped support for java 6 and 7 with version 5 in september this year. I wouldn't go that far since Java 8 is not fully supported in Android < 7, which is nowadays still used by ~50 % of the devices. Maybe @snodnipper - who advocated for Java 6 support back then - can contribute a bit of expertise here?
While i understand the argument that manually testing against
ndifferent jdks/jres is a huge burden, i guess that's what CI systems like https://travis-ci.org/ are made for. Travis also integrates nicely with github and can run a test, build and deploy on a wide range of different environments.Those benefits will be much easier to leverage if we have the pom, the tests and the code in one place. This raises the question: Which value do we get from splitting the unit tests and the code in two distinct repositories? Which value do we get from maintaing the maven central build in yet another seperate repository with a seperate maintainer?
So maybe, instead of draining contributors of their motivation, we should see java 11 as a chance to renew the project and put it on stable, more maintainable feet?
johnjaylward commentedon Dec 4, 2018
I agree. I think it's a little premature to close this issue yet. I think we can find a good resolution that will work.
I know we have some restrictions on public API changes based on the hands-off from Douglas, but I think merging the test cases back into mainline and possibly Implementing some build tool integration are a great place to go forward.
We should highly consider these things going forward. There are multiple times even recently that contributors have had trouble finding tests, or in this case the distribution POM.
Consolidating the projects should simplify a lot, both new contributors and even old contributors like myself.
GedMarc commentedon Dec 4, 2018
I closed the PR and issue cause I need something working for the weekend :)
With the changes that I'm going to make, (I don't want to branch off and config CI again), the PR would never be able to be pulled in. Also going off on my branch instead, I've bumped up my version to min JRE8.
I think it's pretty simple and straight forward, there's nothing funny going on, still JDK6 build nothing changed there at all, it's purely just a moditect + module-info in the build (the PR was on the right github repo I think)
I've already punched through a large portion of the library so I can't re-open the PR, the group, build method, StringBuffer->StringBuilder, Lint'ing and etc have all already been changed. There's like 160 more lint's to get my branch to an acceptable level for JRE8 (for me) so not going to revert it.
https://github.com/GedMarc/JSON-java
@johnjaylward The build and pom structure is a bit weird 👍
GedMarc commentedon Aug 6, 2019
Would you like me to provide a PR?
Thanks for reopening
stleary commentedon Aug 6, 2019
@GedMarc, Yes, please add a PR.
GedMarc commentedon Aug 20, 2019
Update please -
Why is this not pulled in, Why is there no feedback
johnjaylward commentedon Aug 20, 2019
@stleary I've commented on the linked PR, I'm no subject expert on modules, but the changes look like they shouldn't cause any issues with backwards compatibility to me.
johnjaylward commentedon Aug 20, 2019
@stleary If you could also provide guidance on if we should do a new package release or not to get the named module support in (as opposed to the auto module naming we have currently), that would be great. I think a new release would be the best course myself just so we can get community feedback if there are any issues.
stleary commentedon Aug 20, 2019
@johnjaylward, agreed, I also commented, and thanks for doing the analysis.
jordanamr commentedon Jan 4, 2020
Whoop whoop, we are in 2020, what's going on with this issue?
GedMarc commentedon Jan 4, 2020
See end comments here with maven coordinates
BGehrels#4
https://search.maven.org/artifact/com.guicedee.services/json
Although the version is changing (with the encapsulating guicedee project) it is fully in-sync with source
stleary commentedon Jan 4, 2020
Comment added to PR 4
stleary commentedon Jan 5, 2020
Please see comments in BGehrels#4. Looks like no further work is planned for this issue.