You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/04/28 08:33:42 UTC

[GitHub] [commons-fileupload] jGauravGupta opened a new pull request #26: Create Jakarta classifier distribution

jGauravGupta opened a new pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26


   Signed-off-by: Gaurav Gupta <ga...@gmail.com>


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623432313


   @GedMarc this becomes to be pointless so this will be my last answer but you keep saying wrong statements as they are true. As explained you can build jlink images from not jlink friendly jar (just reread the thread if you doubt). Also generating is not important cause you don't gain perf (until you dont compare the same thing as mentionned) nor size in real app (you mentionned hibernate, several commons lib etc - to give you figures I went from 130M to 85M moving to jlink but I lost the ability to cache 120M in a shared docker layer so it was a failure). Also note that the fatjar does not mean java 8 (actually I used it with java 11 and jpms on). JRT wins also disable several features in most libs using the filsystem which means you need to compare what you can compare and tune the standard classpath as you run within jrt FS to be able to conclude anything. Also note there is no possible discussion about if you are modular or not using jlink, it creates an atomic binary (compared to graal which can create a lib) so it is not modular when you use it, it is only modular if you run modules outside an image. Finally in terms of license images are a blocker in several cases so I think you just missed something in your app (like didnt put a custom hibernate scanner for ex and lost a bunch of time there) and thought jlink helped you whereas it didn't as much as your figures show. That said I'm done with this topic in this thread (happy to discuss it elsewhere) cause it is not what it should be about.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623378285


   @rmannibucau that is some good thinking - there are a lot of the libraries that I could shade in cleanly (ref http://www.guicedee.com), but a couple of them I had no choice but to clone and modify - 
   Fileupload was one of those I could shade in cleanly if you looked at the fork.
   
   Let me put some of my thought process, maybe can clean and fix it up a bit.
   The goal post was for POI-OOXML and CXF, which was eventually successful, unfortunately xmlbeans and the enormous resource path structure causes JLink and JMod to fail with the classic ASM "method too large exception". So only xml-beans was a real problem. I had to painstakingly start from zero and only include the resources required. I found a reference to a "xmlbeans-lite" but no actual jar or anything.
   
   ![image](https://user-images.githubusercontent.com/5367513/80955375-807bdf00-8dff-11ea-93d7-e06299ed0ccb.png)
   
   A pure moditect inclusion of the module-info file into META-INF/versions/9 was enough up till that point @jochenw I hope that is a small enough change :)
   You can see quite clearly the bridge jar between javax.activation, and jakarta.activation, which has a requires transitive between them
   
   ![image](https://user-images.githubusercontent.com/5367513/80955238-3d217080-8dff-11ea-9340-23c613ee752b.png)
   
   After delivering all of these as modules, I was asked to visit the rest of the commons libraries. Again I didn't do Math4.
   
   So.. In my adventures of creating the first JLink/JImage/JPackage full system suite, I ended up finishing the following using the exact same formula across the board, and is where all my comments stem from, why I've had to research so deeply into the implementations, and why I am (rather) against a JDK8 only path.
   
   - Jackson JSON to fully support modules using the above methodology. https://github.com/FasterXML/jackson-jdk11-compat-test
   - MSSQL server driver
   - Hibernate JRT support
   - Over 150 libraries to modular 
   
   ```
    <plugin>
                   <groupId>org.apache.maven.plugins</groupId>
                   <artifactId>maven-shade-plugin</artifactId>
                   <configuration>
                   </configuration>
                   <executions>
                       <execution>
                           <id>shade</id>
                           <phase>package</phase>
                           <goals>
                               <goal>shade</goal>
                           </goals>
                           <configuration>
                               <artifactSet>
                                   <includes>
                                       <include>commons-fileupload:commons-fileupload:*</include>
                                       <include>portlet-api:portlet-api:*</include>
                                   </includes>
                               </artifactSet>
                           </configuration>
                       </execution>
   
                   </executions>
               </plugin>
   
               <plugin>
                   <groupId>org.moditect</groupId>
                   <artifactId>moditect-maven-plugin</artifactId>
                   <executions>
                       <execution>
                           <id>add-module-infos</id>
                           <phase>package</phase>
                           <goals>
                               <goal>add-module-info</goal>
                           </goals>
                           <configuration>
                               <overwriteExistingFiles>true</overwriteExistingFiles>
                               <module>
                                   <moduleInfoFile>
                                       src/moditect/module-info.java
                                   </moduleInfoFile>
                               </module>
                           </configuration>
                       </execution>
                   </executions>
               </plugin>
   
   ```
   
   Module-Info file
   ```
   	exports org.apache.commons.fileupload;
   	exports org.apache.commons.fileupload.disk;
   	exports org.apache.commons.fileupload.servlet;
   
   
   	requires static javax.servlet.api;
   
   	requires transitive org.apache.commons.io;
   ```
   So file upload - easy, straight up just push and insert.
   Codec. not so much. Had to modify files - JDK 12 I think it was where it broke. was a while ago.
   jakarta-el  uses yield() so doesn't even compile in JDK14
   
   The guys at RESTEasy even went so far as to say (with their split of SPI's into their own JAR, which is an illegal action in modular development) that they won't support it. 
   I'm hoping you guys don't go down that same path :) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623367173


   @GedMarc instead of completely forking, reshading is a smoother solution since it does not require to maintain the fork. That said, you can note that servlet 3 included file upload in servlet API so you don't need commons-fileupload anymore (at least in the app) so I suspect the abstraction can make more and more sense for fileupload2 (thinking to netty and xnio).
   
   @jochenw even if commons does not use atinject or other spec it must take it into consideration and provide a jakarta war since it is a all jakarta or all javax world once you start using one of these namespace. Hybrid apps don't work very long since the compatibility matrix is pretty overlapping quickly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623136538


   The core java support for security, threading and many others, is.. And this is in jakarta space, which does make it non-compliant. 
   Do you use bouncy castle? do you use threading? do you use CDI which performs illegal injections?.
   Are there classes and resources in the root path that will be completely inaccessible JDK 12 and up?
   
   There is a reason Jakarta is in JDK 8 for the next 2 releases in a 3/4/5 year cycle. Modular, strictly encapsulated, and MicroService development - and CDI/EE are competing strategies. They do not support each other, they are two completely different ways/styles of development. Going the one, you negate the other.
   Going JDK 9 and up without going modular gives you performance degradation, because the Class Loader is "backwards compatible", a.k.a the old form of classpath referencing is not the primary call in JDK 9 and up anymore. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623157461


   @GedMarc not sure the goal of this last comment but the statements (cdi, perf, ...) are not all accurate and not sure the link with this issue. Factually jakarta is not adopted yet so cant be mainstream yet and it is pointless to create a fork of yourself so commons only needs a shade for now and maybe a spi/abstraction in some years. In terms of compile base, there is nothing wrong using java pre 8 here and you will notice no difference as a consumer so guess jakarta support is straight forward.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623434491


   Dude. You have no idea. Just stop.
   Build one, then come back. Performance test. Then come back. Publish and release into production. Then come back. 
   
   You're all theory, and that's awesome. Now put some meat onto it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623121950


   The Jakarta namespace will break a lot of artifacts, everywhere. Especially in running images - 
   
   What I did was create the default javax and reference the Jakarta as a modular transitive. 
   If you do pull this in now - you will also break all of those.
   
   Shading into a private space in your package is fine, but it won't work with servlet 5 - 
   
   I suggest once Jakarta 9, or even 10, is out (10 is aimed at JDK 8 as well, so don't get stuck waiting for them. Java is moving on, commons libraries should as well).
   Then do a major version bump, specifying JDK 8 is no longer supported, then bring in the jakarta artifacts, once the java library pool is cleaned out for JDK 9 and above.
   
   2 cents worth - 
   Please also find modular packagings for each of the commons components here (this link is just commons-fileupload but they are all there, except Math), which allows modular builds and JLink/JMod/JPackage imaging. 
   
   You don't need Jakarta to go/support JDK 9 and up. 
   
   https://github.com/GedMarc/commons-fileupload
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623415613


   @GedMarc this sounds all wrong to be honest so you probably silent part of the process (which can be something you can't say publicly) cause jlink just does a custom JVM and put your jar in a .so, nothing more. Classloading gain is not that huge and breaks several libraries. In terms of perf cds i svery close to jlink images (you can measure it). Then runtime is almost 1-1 so any change can be a side effect of breaking the classpath URL for a particular library (so a wrongly configure application in jvm mode). In the 3 applications I migrated the gain was literally 0 once the JVM mode was aligned on the jlink mode.
   Also note that CDI/EE works well for microservices (less than 1s to start and be ready to serve, docker layer < 2M) and can even be made faster than jlink images with graalvm.
   Also on the modular point: you are NOT modular when doing a jlink image so fatjar is really the best option you can do today to avoid to fork all the ecosystem. It enables you to define all the JVm requirements and only keep the dependencies you want so you reach exactly you goal pretty quickly.
   
   Now I guess this is off topic for this thread and that jakarta distro must be enabled for commons but this has no link with jlink work and does not remove the fact javax stays mainstream for some years.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623129111


   Well, I suppose that is true, for 2 more years. 
   There-after, you'll only be supporting commercial and purchased JDK, which I was under the impression, is pretty much against the open source structure of Apache xD
   
   We'll see when JDK 8 get's discontinued, and Jakarta stays on paid for Java. But making the change and move to Jakarta on JDK 8 (till jakarta 10 minimum - 2026) may make apache commons only usable on paid for platforms..
   This is a very important consideration to make in relation to Jakarta's statements, Oracle's statements, the EOL of JDK8, the 6 months rolling releases, and keeping apache free and open for use on Java.
   
   Moving to Jakarta is more than a library change, it's a ball and chain to JDK 8, for the complete and foreseeable future of Jakarta.
   
   These commons libraries are used in much more than just the thrown away, and picked up from the trash can, Java EE, and support for JDK 9 and up must be present, post 2022


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623136538


   The core java support for security, threading and many others, is.. And this is in jakarta space, which does make it non-compliant. 
   Do you use bouncy castle? do you use threading? do you use CDI which performs illegal injections?.
   Are there classes and resources in the root path that will be completely inaccessible JDK 12 and up?
   
   There is a reason Jakarta is in JDK 8 for the next 2 releases in a 3/4/5 year cycle. Modular, strictly encapsulated, MicroService development - and CDI/EE are competing strategies. They do not support each other, they are two completely different ways/styles of development. Going the one, you negate the other. The reason Java EE was thrown was for no other reason except "The technology has been superseded" 
   
   Going JDK 9 and up without going modular gives you performance degradation, because the Class Loader is "backwards compatible". That means it will get deprecated eventually, and now with oracle that means it will get removed. Or at least we truly hope so.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] markt-asf commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623158840


   Jakarta EE 9 (due this summer) will target Java 8 for source and TCKs will be required to pass on Java 11.
   
   No decisions at all have been made regarding Java versions for Jakarta EE 10 onwards so to claim Jakarta EE will be stuck on Java 8 until 2026 is nothing more than FUD.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] jochenw commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
jochenw commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623344201


   What you guys do not consider: The current master is designed to break compatibility anyways, in a number of ways. So, the right time for evaluation of such changes is **now**, because after that, we intend to remain 100% compatible again - for considerable time.
   
   What I'd like to know: Are there any attempts out in the fields to support such actions? For example, a bridge jar file, or the like.
   
    


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] jochenw commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
jochenw commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623366332


   @GedMarc As far as I know, we don't use \@Inject. So, if the user code does, that's the users responsibility. What bothers me is, whether users can continue to use commons-fileupload with a few easy changes. Adding a dependency would count as "easy change" to me.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] michael-o commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623125023


   I see no benefit raising the Java version if there is no benefit. Even the move to Jakarta will remain on Java 8 for years so has Commons File Upload to remain on it. The user base is too large. Please also note that this libary is shaded into Tomcat which has a string Java version policy.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623114973


   @jochenw https://github.com/apache/commons-fileupload/pull/25#issuecomment-620405418 (long story short: avoid to make it no more usable without transitionning. All major versions of commons had been somehow invasive in - users - migrations and here there is no feature to try to justify it so let's not do it until it is required - JakartaEE 9 - which let some years to migrate properly on user land)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623129111


   Well, I suppose that is true, for 2 more years. 
   There-after, you'll only be supporting commercial and purchased JDK, which I was under the impression, is pretty much against the open source structure of Apache xD
   
   We'll see when JDK 8 get's discontinued, and Jakarta stays on paid for Java. But making the change and move to Jakarta on JDK 8 (till jakarta 10 minimum - 2026) may make apache commons only usable on paid for platforms..
   This is a very important consideration to make in relation to Jakarta's statements, Oracle's statements, the EOL of JDK8, the 6 months rolling releases, and keeping apache free and open for use on Java.
   
   Moving to Jakarta is more than a library change, it's a ball and chain to JDK 8, for the complete and foreseeable future of Jakarta.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] ebourg closed pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
ebourg closed pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623426267


   @rmannibucau nope. Seems you haven't done it before. Sending through a zip for a docker container, is the thing.
   There are rules around JLink that are very strict in terms of building the JVM. You seem unaware of these. There are also enhancements built in like the JRT filesystem, which you will only get in that world which has its own set of rules to abide by as well. I think you do not understand what the forward movement is.
   I'm sorry, unfortunately I believe you do not have the experience to actually say items like 
   "Also on the modular point: you are NOT modular when doing a jlink image"
   
   The inverse is true - you cannot build a JLink image without it being 100% modular from bottom to top. You are typing out of your *ss and have never built one before. Please attempt to build one with just this library first, then revert your findings.
   
   How it matters, and why it is important, is that you will no longer be able to generate the image, the JRE essentially, for JDK 9 and up, which you really should be doing, or what's the point?
   
   Your FAT jar provides no benefit. You should stay on 8, and not move up. You will get performance degradation, if you haven't noticed that already.
   
   Research AMD architecture, and DDD design (did I double design there?)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623412105


   @rmannibucau Nah, it's already in a bank, and two separate clients on a kubernetes cluster with over 150 docker containers. Strongly Disagree, the performance enhancements 
   are enormous, chalk and cheese. To the point where not doing it... looks a bit..,. erg... The numbers are their own proof for a sale.
   
   Spring especially is going to struggle moving to modules, i remember working with.. whats his name, eric something or other, on fixing up the classpath scanner for it. Working with EE/CDI is a totally different strategy of development, it breaks language ubiquity, so right there instantly not even a consideration for microservices, DDD, or any future development if you are of the stricter sort, which I am.
   
   Yes the SPI rules from JDK 12 and up are very strict, so much so that META-INF/services isn't even scanned anymore, and the declarations and class placements are very strict. You can imagine what that does to all annotation processors and the efforts I went through to get them working again.
   
   Each library/module has to be separate, when a module is the only module to reference a single file (like portlet-api) then and only then do I shade it in together. 
   And the kicker - is that every single library has to be named, in order, to build an optimized image.
   Building the "FAT" jar, zero benefit in a modular AMD world. Mine-as-well stay where you are with class pathing and the old java world. The performance and benefits come from strictly defining each modules path.
   
   From what I've gone through, it doesn't sound like you've done this before, sounds all theory and no practice? A lot of things I came across are not documented, yet very very real. Each release of Java since 9 has brought breaking changes in a vast range of libraries.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] coveralls commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-620465988


   
   [![Coverage Status](https://coveralls.io/builds/30394122/badge)](https://coveralls.io/builds/30394122)
   
   Coverage remained the same at 78.824% when pulling **9a2310cc2f6b09e902f42fd003c416a349c35692 on jGauravGupta:jakarta-classifier** into **487e7a35fb1b4976deb589cbe2be2b850cb68bd8 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623129111


   Well, I suppose that is true, for 2 more years. 
   There-after, you'll only be supporting commercial and purchased JDK, which I was under the impression, is pretty much against the open source structure of Apache xD
   
   We'll see when JDK 8 get's discontinued, and Jakarta stays on paid for Java. But making the change and move to Jakarta on JDK 8 (till jakarta 10 minimum - 2026) may make apache commons only usable on paid for platforms..
   This is a very important consideration to make in relation to Jakarta's statements, Oracle's statements, the EOL of JDK8, the 6 months rolling releases, and keeping apache free and open for use on Java, free and open
   
   Moving to Jakarta is more than a library change, it's a ball and chain to JDK 8, for the complete and foreseeable future of Jakarta.
   
   These commons libraries are used in much more than just the thrown away, and picked up from the trash can, Java EE, and support for JDK 9 and up must be present, post 2022


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623136538


   The core java support for security, threading and many others, is.. And this is in jakarta space, which does make it non-compliant. 
   Do you use bouncy castle? do you use threading? do you use CDI which performs illegal injections?.
   Are there classes and resources in the root path that will be completely inaccessible JDK 12 and up?
   
   There is a reason Jakarta is in JDK 8 for the next 2 releases in a 3/4/5 year cycle. Modular, strictly encapsulated, MicroService development - and CDI/EE are competing strategies. They do not support each other, they are two completely different ways/styles of development. Going the one, you negate the other.
   Going JDK 9 and up without going modular gives you performance degradation, because the Class Loader is "backwards compatible". That means it will get deprecated eventually, and now with oracle that means it will get removed. Or at least we truly hope so.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] jochenw commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
jochenw commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623101130


   What is, in your opinion, the advantage of using the shade plugin, compared to simply changing the source code (which, most likely, can be done automatically, using a Refactoring tool).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623354193


   @jochenw all "real" (as "for prod") attemps use bytecode rewriting like the tomcat jakarta migration tool and alternatives. Bridges assumes you have both all the time so only fits very few libraries which always prefer to abstract their on usage and therefore have a spi and 2 modules supporting javax and jakarta. Would mean commons-upload gets 2 new modules or (to not break backward compatibility for most users) keep commons-upload as an aggregator, adds javax and jakarta integrations and adds a parent and shared modules (so 5 modules at least).
   
   Issue being jakarta compatible now means that the current version will stay mainstream for at least 2 years (jakartaee 9 was planned for june this year - I suspect it can be late but this is just a blind guess, if you add impl time + time to be used it brings us in some years so you end up with a master not consummable yet. Last is that tomcat uses that and will support javax for years so I guess javax must stay for now and jakarta be enabled only and this should be reconsidered when data proves jakarta is used I guess.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] jochenw commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
jochenw commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-639102287


   @jGauravGupta From your PR 1407 on eclipse-ee4j/glassfish-woodstock, I deduce that you do consider the topic closed. So, please be so kind to close this PR as well, as I haven't got the necessary permissions.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc edited a comment on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc edited a comment on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623136538


   The core java support for security, threading and many others, is.. And this is in jakarta space, which does make it non-compliant. 
   Do you use bouncy castle? do you use threading? do you use CDI which performs illegal injections?.
   Are there classes and resources in the root path that will be completely inaccessible JDK 12 and up?
   
   There is a reason Jakarta is in JDK 8 for the next 2 releases in a 3/4/5 year cycle. Modular, strictly encapsulated, and MicroService development - and CDI/EE are competing strategies. They do not support each other, they are two completely different ways/styles of development. Going the one, you negate the other.
   Going JDK 9 and up without going modular gives you performance degradation, because the Class Loader is "backwards compatible". That means it will get deprecated eventually, and now with oracle that means it will get removed. Or at least we truly hope so.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623360761


   Ok. 
   
   @markt-asf Please review https://www.eclipse.org/lists/jakartaee-platform-dev/msg00029.html 
   Please review the outcome regarding the decision of 
   "Do we change the language level in Jakarta EE 9 to Java SE 11 or delay that to Jakarta EE 10?"
   Or rather -  Jakarta EE X. Don't appreciate your sny comments. Unfortunately the reality of X being on JDK 8 is currently where it is. There's quite a mail trail there, that's your starting point, can do the same work I did.  It's really not "up to debate" as much as "that was the decision made - so far"
   FUD. lol. Yes. 2026. That is the timeline.
   
   @rmannibucau  Yes I had to create the fork. Servlet 3 and 4 all work on the javax namespace, file upload is tightly coupled to these yes?
   
   @jochenw The bridge jar works perfectly, using a ```requires transitive``` in the module definition file - up to a point - but doesn't solve the separate classes issue. javax.inject.Inject and jakarta.inject.Inject unfortunately don't align, 
   There's also the problem of finalized or closed libraries - again i'm going to use the example of javax.inject which is a JDK 1.5 compiled and finalized JAR. 
   JDK 13 drops support for JDK1.5 classes entirely. Again I shaded this guy in.
   
   The decision is obviously all up to you, I've put in my concerns and my 2 cents worth. 
   End of the day if the apache libraries don't come to the party we're just gonna repackage them until they work for modular development, so no sweat off anyone's back. 
   
   Guess I would just prefer the base to be into the new java world, instead of getting held back through a 6-month release cycle, for a product that goes through a 3/4/5 year release cycle.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] GedMarc commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
GedMarc commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623378285


   @rmannibucau that is some good thinking - there are a lot of the libraries that I could shade in cleanly (ref http://www.guicedee.com), but a couple of them I had no choice but to clone and modify - 
   Fileupload was one of those I could shade in cleanly if you looked at the fork.
   
   Let me put some of my thought process, maybe can clean and fix it up a bit.
   The goal post was for POI-OOXML and CXF, which was eventually successful, unfortunately xmlbeans and the enormous resource path structure causes JLink and JMod to fail with the classic ASM "method too large exception". So only xml-beans was a real problem. I had to painstakingly start from zero and only include the resources required. I found a reference to a "xmlbeans-lite" but no actual jar or anything.
   
   ![image](https://user-images.githubusercontent.com/5367513/80955375-807bdf00-8dff-11ea-93d7-e06299ed0ccb.png)
   
   A pure moditect inclusion of the module-info file into META-INF/versions/9 was enough up till that point @jochenw I hope that is a small enough change :)
   
   ![image](https://user-images.githubusercontent.com/5367513/80955238-3d217080-8dff-11ea-9340-23c613ee752b.png)
   
   After delivering all of these as modules, I was asked to visit the rest of the commons libraries. Again I didn't do Math4.
   
   So.. In my adventures of creating the first JLink/JImage/JPackage full system suite, I ended up finishing the following using the exact same formula across the board, and is where all my comments stem from, why I've had to research so deeply into the implementations, and why I am (rather) against a JDK8 only path.
   
   - Jackson JSON to fully support modules using the above methodology. https://github.com/FasterXML/jackson-jdk11-compat-test
   - MSSQL server driver
   - Hibernate JRT support
   - Over 150 libraries to modular 
   
   ```
    <plugin>
                   <groupId>org.apache.maven.plugins</groupId>
                   <artifactId>maven-shade-plugin</artifactId>
                   <configuration>
                   </configuration>
                   <executions>
                       <execution>
                           <id>shade</id>
                           <phase>package</phase>
                           <goals>
                               <goal>shade</goal>
                           </goals>
                           <configuration>
                               <artifactSet>
                                   <includes>
                                       <include>commons-fileupload:commons-fileupload:*</include>
                                       <include>portlet-api:portlet-api:*</include>
                                   </includes>
                               </artifactSet>
                           </configuration>
                       </execution>
   
                   </executions>
               </plugin>
   
               <plugin>
                   <groupId>org.moditect</groupId>
                   <artifactId>moditect-maven-plugin</artifactId>
                   <executions>
                       <execution>
                           <id>add-module-infos</id>
                           <phase>package</phase>
                           <goals>
                               <goal>add-module-info</goal>
                           </goals>
                           <configuration>
                               <overwriteExistingFiles>true</overwriteExistingFiles>
                               <module>
                                   <moduleInfoFile>
                                       src/moditect/module-info.java
                                   </moduleInfoFile>
                               </module>
                           </configuration>
                       </execution>
                   </executions>
               </plugin>
   
   ```
   
   Module-Info file
   ```
   	exports org.apache.commons.fileupload;
   	exports org.apache.commons.fileupload.disk;
   	exports org.apache.commons.fileupload.servlet;
   
   
   	requires static javax.servlet.api;
   
   	requires transitive org.apache.commons.io;
   ```
   So file upload - easy, straight up just push and insert.
   Codec. not so much. Had to modify files - JDK 12 I think it was where it broke. was a while ago.
   jakarta-el  uses yield() so doesn't even compile in JDK14
   
   The guys at RESTEasy even went so far as to say (with their split of SPI's into their own JAR, which is an illegal action in modular development) that they won't support it. 
   I'm hoping you guys don't go down that same path :) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] jochenw commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
jochenw commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623426182


   Please reevaluate your PR in light of commit cd031805d2618b0c1dd9c90265ebea1a1ce85387.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] michael-o commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623134616


   The support is there, but this does not mean that one has to raise to post-8. Raising the source level means that you really utilize the features of a new release and most don't.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-fileupload] rmannibucau commented on pull request #26: Create Jakarta classifier distribution

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #26:
URL: https://github.com/apache/commons-fileupload/pull/26#issuecomment-623407004


   @GedMarc kind of quit commons area but to do a pseudo native image (this is not really "native" strictly speaking) doing first a shade (potentially renaming manually since this does not change frames) with its own module-info importing the part of the jvm its needs is not that bad. Requires some work to merge properly the dependencies resources (SPI, cxf extensions for ext) and rework the scanning if implicit (thinking to cdi or spring but this is needed anyway) but at the end it creates a more efficient image without any fork anywhere. Now, with your stack your image will be not really smaller than the JVM flavor but it will prevent you to have clean layers in docker or anything so not sure the gain you run after since disk space and speed will not be reach in a significant manner IMHO.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org