-
Notifications
You must be signed in to change notification settings - Fork 729
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
Potential problem from lack of modular declaration for github-api jar file #1343
Comments
Here's a the first SO answer I found by searching... It doesn't offer a clear example in the answer for how to fix this. Many commenters point out that it is a warning. This page offers some clear instructions, but I haven't had a chance to read if fully to understand the pros/cons. |
@bitwiseman - According to the second article you linked, all that would be required for github-api to be considered an officially named modular library is to include one line in the MANIFEST.MF file. And that one line is:
The idea is that as you continue developing the library, that module name never changes and is provided by the library which satisfies Maven checks and all that. You can add that line to the library manifest file without even re-compiling it or moving to a different Java version etc. |
@bitwiseman - Well, I tried to test the claim and added that line to the manifest file and it did indeed cause it to pass the check but then it failed the compile goal stating that it could not find the libraries modules.java file ... so clearly ... it's not as simple as adding that line to the manifest. |
@bitwiseman OK... so another reversal ... when I added this line to the manifest file in github-api-1.301.jar
Then changed my module-info file to match that name ... everything compiles properly and no warning was thrown from Maven. |
Submit a PR! |
@bitwiseman Done! |
@bitwiseman It should be noted that even though this "fix" allows for compiling and it satisfies the publishing requirement, it still will not build into an executable if the developer uses jlink, because jlink requires dependencies to actually be modular and it specifically errors out when a dependency is implemented and it contains an automatic-module-name. I'm thinking that at some point, the API should be modularized. |
Okay, so what additional work is required? |
@bitwiseman - No sooner than I posted my last comment I said to myself ... "he's probably gonna ask me to do it" - LOL So ... I started working on it and should have an answer soon... I think the first question to answer, is: Will it be acceptable if this API is no longer compatible with Java versions older than version 9? Because once it goes modular, it can't be compiled in any JDK older than version 9. edit: Actually don't you currently compile the library with JDK 11? - The homepage says it's version 1.8, but as was messing around with the code with module-info, I tried to set my JDK to 9 and IntelliJ said I needed to be at least at JDK 11 when I tried to build the project ... that's where 11 came from anyways. |
We compile using Java 11 but we target Java 8. We use Java 11 to create a multi-release jar that has a different limitation for default GitHub connector for Java 11 and later, which is where all this modularization warnings started. I'd be open to publishing a modularized Java 11 plus only jar for you spy those who care about this, but we need to continue to produce the Java 8 plus compatible jar as well. Sorry. What you've done is a staffing right direction. Perhaps we can merge that and close this first issue and open another issue regarding full modularization. How does that sound to you? |
Sounds good to me. |
Fix Issue #1343 - Updated POM File to amend manifest
@bitwiseman Since you compile with Java 11, I'm pretty sure my last statement about incompatibility with 1.8 becomes non-issue... I bought and am now reading this book ... currently in chapter 4 hope to be done within a couple of days ... if I don't have a firm grip on modular coding in Java by the time I finish it, I'll join the circus. |
Do remember to opem a new issue for the remaining work, thanks! And thanks for taking this on. |
What I learned from that book will either further complicate the issue or nullify it entirely. So it seems that modularizing Java code is:
I believe those were the top two reasons for adding modularization into Java in the first place. So how does this apply here? Well, the main reason I opened the issue, was because a Maven test pointed out that a developer should never publish their work when their code depends on a library that has no explicit module name given. The workaround for this was to add a line to the manifest file that declares a name that presumably will never change, and that was taken care of in the PR I submitted. The ancillary issue that I discovered is that if a developer tries to compile their modular code using Jlink, this library will prevent them from using Jlink because Jlink will not accept non-modular code... which makes sense since Jlink's main task is to compile modular code ... it's the magic that generates highly concise executable artifacts since it is the code that packages in only the modules that have been declared as necessary in any given project. But this leaves github-api at a crossroads of sorts: Since remaining compatible with 1.8 is one of this project's primary goals, then does it make sense to branch off into two different versions where one is modular and the other isn't, simply to give developers the ability to compile their code with Jlink? Certainly, it does prevent them from being able to compile that pristine artifact that they might desire ... but my take on this is that if no one is complaining, then why create more work? Modularizing this project will be a monumental task ... probably more so for me than someone who has experience in that space. It would be a learning experience akin to throwing someone into a pool who hasn't swum before ... but that doesn't scare me... I'm up for the challenge ... I think the more relevant question is: Do you want to branch this project where doing so would most definitely increase the overall workload as the project evolves. Maintaining two versions of the same library can't be a simple task - I wouldn't imagine, and those who work on the modularized version would need to have a good understanding of how modularization works, or they will just end up being frustrated as they try to move forward because modularized code creates hard lines of definition when referencing other portions of the project. It's not something that is particularly difficult to do and if someone understands why a certain flag is being thrown by the IDE then they would realize how to quickly address it which would almost always require a needed change in the module-info.java file. Also, the process of modularizing this library will require some ongoing discussion to gain a thorough understanding of which classes are only for the benefit of the library and which classes are intended to be exposed to a developer so that those definitions can be described in the module-info file, and it might even require re-structuring some classes so that they "form-fit" modularization concepts which are dependent on the overall purpose of a given class. It can be done in a more lazy style than that, but I say if you're going to something, then do it right and be thorough about it... or don't do it at all. Going modular certainly would have some benefits, especially for a library of this size where a pristine interface can be presented to developers that would almost certainly make it easier to use and understand since the modular definition will be adopted by any reputable IDE, it will have a direct impact on code completion functions within the IDE where only those portions of the library that are relevant to a developer will be seen by the developer. Less is more in this case whereas right now, any class declared as being public is visible to a developer, and though my only experience with this library is in the context of Gists ... I've not used it for any other purpose so I cannot say for certain that there is a lot of unneeded exposure to classes that aren't relevant to its implementation in a project, there are certainly a lot of classes within the library and my assumption is that a lot of them don't need to be visible to a developer in order to use the library to its full potential. And if I'm right, then modularizing will tighten that up significantly. With a modularized library, you can have classes that are declared public, making them easily accessible within the project, but if those classes are not declared exported outside of the project, then users of the library won't be able to see those classes. Certainly, that can have its advantages in the context of presenting an interface to developers. I would tend to think that the best reason to go modular would be to tighten up the library from a perspective of moving it into the future of all things Java with a clear understanding that at some point, JDKs before version 9 will be left behind ... and is that really such a big deal? How long should we go on holding onto the crutch of 1.8 ... as a community that is? There really should be a line drawn in the sand SOMEWHERE ... where we all step up and define the line that is the end of our backward version support ... the addition of lambda in 1.8 seemed to be widely adopted rather quickly, though admittedly, it was a feature that truly did maintain backward compatibility ... modularization seems to be the one upgrade in Java that has truly drawn that line in the sand where yes, it can be backward compatible ... as long as you don't use it ... which was the only way to do it in the first place ... they did struggle over its development for many many years before finally pulling the trigger on it because one of Javas primary design goals has always been to ensure backward compatibility with past versions ... a noble goal for sure, but the work involved in doing that is unimaginable not to mention in some cases impractical since the future cannot be predicted. I am of the mindset that when it comes to anything IT related, security should be the primary underlying focal point in any coding project, which means that being adaptable and ready to make changes as new versions are released, is the only way to properly enact "due diligence" to that end. It's an ideology I suppose that requires commitment and agreement with those in a given project... but if anything has been a stain on the IT industry in general, it has been in the area of security where even today, it's not the focal point that it needs to be within most IT departments where they tend to focus more on providing service and security becomes something done on an "as needed" basis ... cart before the horse in my opinion. I'm not, of course., saying that security is in any way a concern for this library. What I am saying is that existing within the boundaries of the idea that moving forward is the "best practice" within IT on almost all levels because it is the best way to ensure the most stable and secure services in just about every context and if we don't live that mindset, then we promote its antithesis. And I do realize that I'm overlooking an enormous amount of detail where actually doing that is concerned. Such as - I'm assuming that your largest user base develops with 1.8 or older and so making that choice to cut them off at some point might end up losing a lot of that base ... but on the other hand, it might inspire them to move their own projects into the future and in that context, it would be promoting "best practices" and would certainly add to the pool of reasons why people should move their code forward into this decade. It is inevitable ... at some point ... that modularization will become a standard practice in Java development. I cannot see a world where 10 to 20 years from now, people are still not using it save the rogue few who remain stubborn on "principle". But we will always have those ... I know attorneys who still use Word Perfect ... believe it or not ... die-hard? Absolutely! You can pry it from their cold dead hands if you ask them. Having been a Network Engineer for over 20 years, I learned that being so rigid is a death sentence to any standard in IT that purports stability and security. Anyway, those are my thoughts and comments on this issue based on what I currently know. The decision to go modular is one that I'll leave in your capable hands. I cannot say with any certainty that I'll be successful in properly modularizing the code if you decide to move forward. I can promise that I will give it everything I've got leveraging any resource I can get my hands on and I tend to be fairly successful at most things I put my mind to... marriage wasn't one of them but that's out of scope here... lol |
This project I'm working on, I intend to publish to a public repository at some point ... I ran a Maven verify goal on my project and it came back with this warning:
When I looked into the warning, apparently this is considered somewhat serious as it is caused by the library not having a module declaration, where a module declaration remains unchanged in future releases of an artifact where the filename does change. But as it is now, apparently that jar file lacks a module declaration so the module is declared automatically based on the filename.
This can cause problems for projects that depend on other artifacts that also depend on the Git-Hub API when they might depend on two or more and their dependencies on this API have different names. And a project that has such a dependency structure will be unable to run, and fixing it is not an easy proposition... so I guess the rule of thumb is to not publish projects that have dependencies with auto-generated module declarations.
So my question is:
Will you be declaring a module for the Git-Hub API so that this isn't a problem when publishing projects that depend on it?
I realize this means that the API would need to be modularized ... but surely this shouldn't be that much of a bid deal?
Unless my assumptions on this are incorrect and this warning has a more... sinister cause? As in ... something is broken or I'm implementing the API into my project incorrectly?
The text was updated successfully, but these errors were encountered: