RFR: 8282508: Updating ASM to 9.2 for JDK 19
Please review this PR which is updating the ASM included in the JDK to ASM 9.2. This version should be included in JDK19. TIA - Commit messages: - additional adaptations after the automatic conversion - 8282508: Updating ASM to 9.2 for JDK 19 Changes: https://git.openjdk.java.net/jdk/pull/8000/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8000&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282508 Stats: 1762 lines in 136 files changed: 544 ins; 765 del; 453 mod Patch: https://git.openjdk.java.net/jdk/pull/8000.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8000/head:pull/8000 PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19
On Mon, 28 Mar 2022 16:49:58 GMT, Vicente Romero wrote: > Please review this PR which is updating the ASM included in the JDK to ASM > 9.2. This version should be included in JDK19. There are basically two > commits here, one that was automatically generated and that mostly changes > file headers etc and another one, smaller, that make changes to the code to > adapt it to our needs, JDK version etc. In this second commit one can see > that two classes that were removed by the automatically generated change have > been copied back: > > jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA With this fix, I believe [JDK-8282446](https://bugs.openjdk.java.net/browse/JDK-8282446) should also be addressed. - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19
On Mon, 28 Mar 2022 16:49:58 GMT, Vicente Romero wrote: > Please review this PR which is updating the ASM included in the JDK to ASM > 9.2. This version should be included in JDK19. There are basically two > commits here, one that was automatically generated and that mostly changes > file headers etc and another one, smaller, that make changes to the code to > adapt it to our needs, JDK version etc. In this second commit one can see > that two classes that were removed by the automatically generated change have > been copied back: > > jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA I suppose that you are raising commons.RemappingMethodAdapter and commons.RemappingAnnotationAdapter from the dead because you want to fix the code in jdk.jfr.internal.instrument later ? Otherwise it's a little dangerous because that code as started to drift, the new remapper has new code not supported by the old remapper. BTW, @deprecated on ASM source is equivalent to. @Deprecated + forRemoval in the JDK. The support of Java 19 is fine, the same code will be included in ASM soon. - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19
On Mon, 28 Mar 2022 17:50:25 GMT, Rémi Forax wrote: > I suppose that you are raising commons.RemappingMethodAdapter and > commons.RemappingAnnotationAdapter from the dead because you want to fix the > code in jdk.jfr.internal.instrument later ? correct, I would prefer the team maintaining jdk.jfr.internal.instrument to do that update after this PR has been pushed > > Otherwise it's a little dangerous because that code as started to drift, the > new remapper has new code not supported by the old remapper. BTW, @deprecated > on ASM source is equivalent to. @deprecated + forRemoval in the JDK. > > The support of Java 19 is fine, the same code will be included in ASM soon. - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19
On Mon, 28 Mar 2022 17:08:12 GMT, Lance Andersen wrote: > With this fix, I believe > [JDK-8282446](https://bugs.openjdk.java.net/browse/JDK-8282446) should also > be addressed. Thanks for mentioning this. I have uploaded another commit [41ae618e3a5ce696e3400a8654d98801226d7c1b](https://github.com/openjdk/jdk/pull/8000/commits/41ae618e3a5ce696e3400a8654d98801226d7c1b), which addresses this issue. - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v2]
> Please review this PR which is updating the ASM included in the JDK to ASM > 9.2. This version should be included in JDK19. There are basically two > commits here, one that was automatically generated and that mostly changes > file headers etc and another one, smaller, that make changes to the code to > adapt it to our needs, JDK version etc. In this second commit one can see > that two classes that were removed by the automatically generated change have > been copied back: > > jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8000/files - new: https://git.openjdk.java.net/jdk/pull/8000/files/2e468674..41ae618e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8000&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8000&range=00-01 Stats: 83 lines in 2 files changed: 82 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8000.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8000/head:pull/8000 PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
> Please review this PR which is updating the ASM included in the JDK to ASM > 9.2. This version should be included in JDK19. There are basically two > commits here, one that was automatically generated and that mostly changes > file headers etc and another one, smaller, that make changes to the code to > adapt it to our needs, JDK version etc. In this second commit one can see > that two classes that were removed by the automatically generated change have > been copied back: > > jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: fixing files missing new line at the end - Changes: - all: https://git.openjdk.java.net/jdk/pull/8000/files - new: https://git.openjdk.java.net/jdk/pull/8000/files/41ae618e..f5e0d931 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8000&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8000&range=01-02 Stats: 252 lines in 129 files changed: 125 ins; 0 del; 127 mod Patch: https://git.openjdk.java.net/jdk/pull/8000.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8000/head:pull/8000 PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end anymore comments on this one? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end The changes look reasonable to me - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end JFR parts look OK. - Marked as reviewed by egahlin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8000
