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