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 2021/08/19 12:35:40 UTC

[GitHub] [commons-fileupload] martin-g opened a new pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

martin-g opened a new pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107


   Use moditect-maven-plugin to add auto-generated module-info.class into META-INF/versions/9/
   
   IMPORTANT: Now the build requires JDK 9+! The produced jar is JDK 1.8 compatible!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] emiliofernandes commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   +1 to require newer Java version for the next major release!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] GedMarc commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   So to give an idea - 
   
   I was asked to move all the apache commons libraries to be compatible for JPMS - https://github.com/GedMarc/JPMSPendingParent/issues/1
   
   I completed the entire suite  (https://github.com/GedMarc/GuicedEE-Services) everything that starts with "commons-"
   
   JPMS compatible means being native build compatible (JLink,JPackage, JMod) so automatic-module-names aren't allowed and you have to include a module-info.class
   
   I've completed them all (except maths4 which has some not-deployed dependencies?), 
   And believe I can help out with experience of doing them all, and getting it working 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g edited a comment on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g edited a comment on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901915469


   Wicket does not use reflection, so no need to `open` (at least for Wicket).
   
   @GedMarc Please check this PR.
   ```
   // use JDK 9+
   mvn package -Prelease
   cd target
   unzip commons-fileupload2-2.0-SNAPSHOT.jar
   javap META-INF/versions/9/module-info.class
   cat META-INF/MANIFEST.MF  (and look for `Multi-Release: true`)
   ```
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-912493463


   The change here is for 2.x, which will be released some time in the future.
   Isn't it time to upgrade to a newer version of Java ? 
   People could use 1.x for JDK 8.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] jochenw commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   I like the idea of having a profile, that is activated by JDK >= 9, and keeping the JDK 8 build.
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] jochenw commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   With 2.0, a migration will be needed anyways. And, this is still in the range of "easy to migrate".
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] garydgregory commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   FWIY, for the release I work on that require Java 8, I ALWAYS release using Java 8. The release process is tricky enough as it is.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] jochenw commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   > About CI: I could preserve the JDK 8 in GHA and TravisCi and run just `mvn test` when JDK==1.8. Is this OK ?
   
   Go for it, please!
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] GedMarc commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   it is not that simple at all though - 
   
   https://github.com/GedMarc/GuicedEE-Services/blob/master/commons-fileupload/src/moditect/module-info.java
   
   You should pre-build your module file and include it moditect, but definitely do not auto generate from 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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] GedMarc commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   @martin-g Yes but that's closer to improperly placed classes? 
   
   Exporting all is fine, opens clauses for things like wicket that reflect into the library will also need to be considered, invalid access will come about, and on JDK 17 when its disallowed completely, the library will be considered broken?
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] jochenw merged pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
jochenw merged pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107


   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901898390


   > You should pre-build your module file and include it moditect, but definitely do not auto generate from it
   
   @GedMarc You mean we should not include the `impl` package (added by the auto-generation) ? 
   I am afraid this will break many applications which already use classes from `impl`. I know that Apache Wicket does use them.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] garydgregory edited a comment on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   FWIY, for the releases I work on that require Java 8, I ALWAYS release using Java 8. The release process is tricky enough as it is.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901891128


   About CI: I could preserve the JDK 8 in GHA and TravisCi and run just `mvn test` when JDK==1.8. Is this OK ?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] garydgregory commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   > The change here is for 2.x, which will be released some time in the future.
   > Isn't it time to upgrade to a newer version of Java ?
   > People could use 1.x for JDK 8.
   
   The next Java version jump should be to 11 since it is the next LTS version after 8. When to do that I don't know. 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901894181


   Actually, I could disable the moditect plugin by default. This way everyone could build with JDK 1.8.
   But the release manager will have to enable it. I see there is a `release` profile in the parent pom already.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901915469


   Wicket does not use reflection, so no need to `open` (at least for Wicket).
   
   @GedMarc Please check this PR.
   ```
   // use JDK 9+
   mvn package
   cd target
   unzip commons-fileupload2-2.0-SNAPSHOT.jar
   javap META-INF/versions/9/module-info.class
   ```
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-902510502


   > I am afraid this will break many applications which already use classes from `impl`. I know that Apache Wicket does use them.
   
   https://issues.apache.org/jira/browse/FILEUPLOAD-341


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] martin-g commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #107:
URL: https://github.com/apache/commons-fileupload/pull/107#issuecomment-901890506


   > Besides, please add yourself to the contributors list
   
   In the pom.xml ?
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-fileupload] GedMarc commented on pull request #107: FILEUPLOAD-340 Make commons-fileupload a proper JPMS module

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


   There will be heavy things for Jakarta servlet compatibility, transitive and static clauses to support both eventually, 
   Hiding internal classes and packages when sealed/non-sealed come along, 
   
   There's a lot more to think about than just auto generate, and from the experience with jackson json, it is almost guaranteed you will need to customize 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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

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