Just to clarify: on my previous email I did not mean that any of those issues in particular were the cause of this problem, but rather that it could be possible that a similar issue on the JDK that was used to build the release might be the root cause, but I'm not 100% sure.
In fact, according to the tests proposed by Guillaume, it seems that already the bytecode of SqlFunctions-1.35 was not correct, but for some reason ASM did not fail with it (as it does with SqlFunctions-1.36). Why is that? It's hard to tell... As others have said, I agree that it would be a good idea to add a check to ensure that the bytecode is correct during the release process. Ruben On Mon, Jan 29, 2024 at 8:27 AM Stamatis Zampetakis <zabe...@gmail.com> wrote: > Having a gradle task to ensure that class files are valid bytecode > seems like a good idea. It may not be 100% bulletproof since we yet > rely on another tool/library to perform the verification but still > having an extra check will not hurt if it is lightweight. > > It makes sense to enforce a certain JDK version/build during the > release. In that case I would suggest also having some gradle logic > perform this validation and not just text in the release instructions. > > Automating the release procedure would be great but to do that it is > required to have reproducible builds [1]. See [2] for more background > around this topic. > > Best, > Stamatis > > [1] > https://cwiki.apache.org/confluence/display/SECURITY/Reproducible+Builds > [2] https://lists.apache.org/thread/qkstx4o9qw90fxzqcfnp69w33h7vw2yg > > On Fri, Jan 26, 2024 at 11:27 PM Guillaume Masse > <masse.guilla...@narrative.io.invalid> wrote: > > > > Hi Julian, > > > > I don't think it's necessary to require JDK > 8. Here is what I propose: > > > > 1) I can add a gradle task to make sure each classfile from the release > > jars are valid bytecode. > > > > 2) I think we need to make sure we compile with the latest build > available. > > Per https://lists.apache.org/thread/nrt4ysoc14p20sq23z744jyfqh1bznyh it > was > > compiled with 8u371 the 10 of November 2023. At the time the latest build > > of java 8 was 391, so that's 2 builds behind: > > > > 371 > 18 April 2023 > > 381 > 18 July 2023 > > 391 > 17 October 2023 > > 401 > 16 January 2024 > > > > I can't be sure that this was the root cause for the broken classfile. > > > > 3) Let's make the release procedure automatic / part of the CI. I can > > assist on this task, but I will need help from commiters since I don't > > have any credentials for maven central. > > > > Guillaume > > > > > > On Fri, Jan 26, 2024 at 2:44 PM Julian Hyde <jhyde.apa...@gmail.com> > wrote: > > > > > These JDK bugs seem to be fixed in more recent java versions. Should we > > > mandate that releases are built on a recent JDK (say 17 or 21)? > Currently > > > we mandate JDK 8. > > > > > > > > > > On Jan 26, 2024, at 9:03 AM, Ruben Q L <rube...@gmail.com> wrote: > > > > > > > > I have the impression that this might be caused by a bug in the JDK, > see > > > > similar issues: > > > > https://gitlab.ow2.org/asm/asm/-/issues/317789 > > > > https://bugs.openjdk.org/browse/JDK-8144185 > > > > https://bugs.openjdk.org/browse/JDK-8187805 > > > > > > > > > > > > > > > > On Thu, Dec 21, 2023 at 8:58 PM Julian Hyde <jh...@apache.org> > wrote: > > > > > > > >> I think I was mistaken about Guava. I agree with Benchao's analysis > > > >> and #2 seems to be caused by something other than Guava. > > > >> > > > >> On Thu, Dec 21, 2023 at 3:53 AM Benchao Li <libenc...@apache.org> > > > wrote: > > > >>> > > > >>> There are two exceptions here: > > > >>> > > > >>> # 1 > > > >>> java.lang.ArrayIndexOutOfBoundsException: Index 65536 out of bounds > > > >>> for length 297 > > > >>> at org.objectweb.asm.ClassReader.readLabel(ClassReader.java:2695) > > > >>> at org.objectweb.asm.ClassReader.createLabel(ClassReader.java:2711) > > > >>> at > > > >> > org.objectweb.asm.ClassReader.readTypeAnnotations(ClassReader.java:2777) > > > >>> at org.objectweb.asm.ClassReader.readCode(ClassReader.java:1929) > > > >>> at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1515) > > > >>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:745) > > > >>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:425) > > > >>> at remapper.bug.RemapperTest.runTest(RemapperTest.java:60) > > > >>> at remapper.bug.RemapperTest.calcite36(RemapperTest.java:36) > > > >>> > > > >>> # 2 > > > >>> java.lang.IllegalArgumentException: Invalid end label (must be > visited > > > >> first) > > > >>> at > > > >> > > > > org.objectweb.asm.util.CheckMethodAdapter.checkLabel(CheckMethodAdapter.java:1453) > > > >>> at > > > >> > > > > org.objectweb.asm.util.CheckMethodAdapter.visitLocalVariableAnnotation(CheckMethodAdapter.java:996) > > > >>> at > > > >> > > > > org.objectweb.asm.MethodVisitor.visitLocalVariableAnnotation(MethodVisitor.java:757) > > > >>> at > > > >> > > > > org.objectweb.asm.commons.MethodRemapper.visitLocalVariableAnnotation(MethodRemapper.java:257) > > > >>> at org.objectweb.asm.ClassReader.readCode(ClassReader.java:2614) > > > >>> at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1515) > > > >>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:745) > > > >>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:425) > > > >>> at remapper.bug.RemapperTest.runTest(RemapperTest.java:53) > > > >>> at > remapper.bug.RemapperTest.calcite36WithCheck(RemapperTest.java:39) > > > >>> > > > >>> > > > >>> Sorry that I'm not clear in my previous email. My testing results > are > > > >>> all about #1. > > > >>> > > > >>> I tested 1.34.0/1.35.0/1.36.0 with JDK8 on both Linux and Mac for > #2, > > > >>> they all reproduced the problem. Hence this is not a problem of > > > >>> platform, and also not a problem of 1.35.0->1.36.0, I guess this > is an > > > >>> asm usage problem related to JDK8's output. > > > >>> > > > >>> In summary, > > > >>> for #1 problem, it seems to be a problem of the Guava version we're > > > >>> compiling against, > > > >>> for #2 problem, it seems not to be a problem since both > > > >>> 1.34.0/1.35.0/1.36.0 can reproduce it (I assume that this does not > > > >>> affect your usage since you only spotted this after 1.36.0). > > > >>> > > > >>> Guillaume Masse <masse.guilla...@narrative.io.invalid> > 于2023年12月21日周四 > > > >> 03:29写道： > > > >>>> > > > >>>> calcite-1.35.0 with JDK8(1.8.0_391) on MacOS with M1 Pro chip: can > > > >> reproduce > > > >>>> > > > >>>> RemapperTest > calcite35WithCheck() FAILED > > > >>>> java.lang.IllegalArgumentException: Invalid start label (must > be > > > >>>> visited first) > > > >>>> at > > > >> > > > > org.objectweb.asm.util.CheckMethodAdapter.checkLabel(CheckMethodAdapter.java:1453) > > > >>>> at > > > >> > > > > org.objectweb.asm.util.CheckMethodAdapter.visitLocalVariableAnnotation(CheckMethodAdapter.java:995) > > > >>>> at > > > >> > > > > org.objectweb.asm.MethodVisitor.visitLocalVariableAnnotation(MethodVisitor.java:757) > > > >>>> at > > > >> > > > > org.objectweb.asm.commons.MethodRemapper.visitLocalVariableAnnotation(MethodRemapper.java:257) > > > >>>> at > > > >> org.objectweb.asm.ClassReader.readCode(ClassReader.java:2614) > > > >>>> at > > > >> org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1515) > > > >>>> at > org.objectweb.asm.ClassReader.accept(ClassReader.java:745) > > > >>>> at > org.objectweb.asm.ClassReader.accept(ClassReader.java:425) > > > >>>> at remapper.bug.RemapperTest.runTest(RemapperTest.java:53) > > > >>>> at > > > >> remapper.bug.RemapperTest.calcite35WithCheck(RemapperTest.java:32) > > > >>>> > > > >>>> > > > >> > > > > https://github.com/MasseGuillaume/asm-remapper-bug/blob/main/SqlFunctions-1.35.0.class > > > >>>> > > > >>>> If you kept the .class for your tests, can you try to run javap > -p -c > > > >> -v > > > >>>> > > > >>>> On Wed, Dec 20, 2023 at 1:11 PM Julian Hyde < > jhyde.apa...@gmail.com> > > > >> wrote: > > > >>>> > > > >>>>> The version of Guava that you use at compile time is important. > If > > > >> it’s > > > >>>>> earlier than 20 (-ish) the byte codes will be different because > of > > > >> the > > > >>>>> presence/absence of certain varargs methods. > > > >>>>> > > > >>>>> > > > >>>>>> On Dec 19, 2023, at 7:28 PM, Benchao Li <libenc...@apache.org> > > > >> wrote: > > > >>>>>> > > > >>>>>> I tried to reproduce this using Guillaume's > > > >>>>>> project(https://github.com/MasseGuillaume/asm-remapper-bug), > and > > > >> the > > > >>>>>> findings are below: > > > >>>>>> > > > >>>>>> - compiled latest main branch > > > >>>>>> (50a20824c4536450dcae963b5e757cf4bbc7e406) with JDK8(1.8.0_391) > on > > > >>>>>> MacOS with M2 chip: reproduced > > > >>>>>> - compiled latest main branch > > > >>>>>> (50a20824c4536450dcae963b5e757cf4bbc7e406) with JDK8(1.8.0_391) > on > > > >>>>>> Linux with x64 architecture: reproduced > > > >>>>>> - compiled latest main branch > > > >>>>>> (50a20824c4536450dcae963b5e757cf4bbc7e406) with > JDK11(1.11.0_21) on > > > >>>>>> MacOS with M2 chip: cannot reproduce > > > >>>>>> - compiled latest main branch > > > >>>>>> (50a20824c4536450dcae963b5e757cf4bbc7e406) with JDK17(1.17.0_7) > on > > > >>>>>> MacOS with M2 chip: cannot reproduce > > > >>>>>> > > > >>>>>> It seems unrelated to platforms, and I assume it should not > either, > > > >>>>>> since Java's bytecode should be platform independent. > > > >>>>>> > > > >>>>>> @Guillaume You said above that with 1.8.0_371-b11 on macos x64, > you > > > >>>>>> cannot reproduce the problem, can you confirm that? Per my > testing, > > > >>>>>> JDK8's output could all reproduce the problem > > > >>>>>> > > > >>>>>> Guillaume Masse <masse.guilla...@narrative.io.invalid> > > > >> 于2023年12月18日周一 > > > >>>>> 23:13写道： > > > >>>>>>> > > > >>>>>>> If I build 1.36.0 and 1.35.0 with the same java version/build > > > >> javap can > > > >>>>>>> read the classfiles. Since the release process is manual it's > > > >> really > > > >>>>> hard > > > >>>>>>> to know what causes the problem. My solution is to build and > > > >> release > > > >>>>> each > > > >>>>>>> Calcite version myself and host it on our own S3 repository. > > > >>>>>>> > > > >>>>>>> On Mon, Dec 18, 2023, 8:56 AM Ruben Q L <rube...@gmail.com> > > > >> wrote: > > > >>>>>>> > > > >>>>>>>> Hello, > > > >>>>>>>> > > > >>>>>>>> My project also recently upgraded to Calcite 1.36, and we are > > > >> facing > > > >>>>> the > > > >>>>>>>> exact same issue when trying to shade with ASM. > > > >>>>>>>> > > > >>>>>>>> @Guillame , I can see that > > > >>>>> https://gitlab.ow2.org/asm/asm/-/issues/318008 > > > >>>>>>>> has > > > >>>>>>>> been closed, but I can't really understand why, since the > > > >> explanation > > > >>>>>>>> mentions SqlFunctions-1.35.0, but the issue is with 1.36, do > you > > > >> have > > > >>>>> more > > > >>>>>>>> info on that? > > > >>>>>>>> > > > >>>>>>>> Best, > > > >>>>>>>> Ruben > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> On Fri, Nov 24, 2023 at 10:09 PM Guillaume Masse > > > >>>>>>>> <masse.guilla...@narrative.io.invalid> wrote: > > > >>>>>>>> > > > >>>>>>>>> We run spark in AWS EMR and it overrides the classpath, so we > > > >> don't > > > >>>>> have > > > >>>>>>>>> control over it: > > > >>>>>>>>> > > > >>>>>>>>> Sparks will pull guava 14.0.1: > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>> > > > >> > > > > https://github.com/apache/spark/blob/master/dev/deps/spark-deps-hadoop-3-hive-2.3#L68 > > > >>>>>>>>> > > > >>>>>>>>> That's why we need to shade guava. > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>>> On Fri, Nov 24, 2023 at 4:31 PM Julian Hyde < > jh...@apache.org> > > > >> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> The version of Guava we compile against is relevant. In > 1.35 we > > > >>>>>>>>>> compiled against Guava 19.0; in 1.36 we compiled against > Guava > > > >>>>>>>>>> 32.1.3-jre. The effect would be the same if we compiled > > > >> against any > > > >>>>>>>>>> version of Guava 20.0 or higher, due to the addition of > > > >>>>>>>>>> Preconditions.checkArgument methods that in earlier Guava > > > >> versions > > > >>>>>>>>>> would be handled by varargs method. > > > >>>>>>>>>> > > > >>>>>>>>>> See https://issues.apache.org/jira/browse/CALCITE-5477 > (which > > > >> was > > > >>>>>>>>>> fixed in 1.35) and > > > >>>>> https://issues.apache.org/jira/browse/CALCITE-5763 > > > >>>>>>>>>> (which was fixed in 1.36 and essentially reverted 5477). > > > >>>>>>>>>> > > > >>>>>>>>>> Julian > > > >>>>>>>>>> > > > >>>>>>>>>> On Fri, Nov 24, 2023 at 10:20 AM Guillaume Masse > > > >>>>>>>>>> <masse.guilla...@narrative.io.invalid> wrote: > > > >>>>>>>>>>> > > > >>>>>>>>>>> Hi Benchao Li, > > > >>>>>>>>>>> > > > >>>>>>>>>>> thanks for giving me more details like the java version, > > > >>>>>>>>>>> > > > >>>>>>>>>>> I compiled with 1.8.0_371-b11 on macos x64 (emulated) and I > > > >> was > > > >>>>> still > > > >>>>>>>>>> able > > > >>>>>>>>>>> to transform the classfile. The bug must come from > something > > > >> else. > > > >>>>>>>> Was > > > >>>>>>>>>> this > > > >>>>>>>>>>> compiled on your personal machine? Was it on > > > >> windows/linux/mac? > > > >>>>>>>> What's > > > >>>>>>>>>> the > > > >>>>>>>>>>> processor architecture? > > > >>>>>>>>>>> > > > >>>>>>>>>>> 1.35.0 also pases the transformation and the same piece of > > > >> code is > > > >>>>>>>>> there. > > > >>>>>>>>>>> How was 1.35.0 released? > > > >>>>>>>>>>> > > > >>>>>>>>>>> Thanks! > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> On Thu, Nov 23, 2023 at 9:59 PM Benchao Li < > > > >> libenc...@apache.org> > > > >>>>>>>>> wrote: > > > >>>>>>>>>>> > > > >>>>>>>>>>>> Guillaume, > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> The binary release are always compiled with JDK8, this is > a > > > >>>>>>>> required > > > >>>>>>>>>>>> procedure[1]. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> For 1.36.0, the JDK version I'm using is: > > > >>>>>>>>>>>> $ java -version > > > >>>>>>>>>>>> java version "1.8.0_371" > > > >>>>>>>>>>>> Java(TM) SE Runtime Environment (build 1.8.0_371-b11) > > > >>>>>>>>>>>> Java HotSpot(TM) 64-Bit Server VM (build 25.371-b11, mixed > > > >> mode) > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> [1] > > > >>>>>>>>>> > > > >>>>> > > > >> > https://calcite.apache.org/docs/howto.html#making-a-release-candidate > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Guillaume Masse <masse.guilla...@narrative.io.invalid> > > > >>>>>>>>> 于2023年11月24日周五 > > > >>>>>>>>>>>> 07:22写道： > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> Hi all, > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> We recently upgraded to calcite 1.36.0 and it broke our > > > >>>>>>>> deployment > > > >>>>>>>>>>>> process. > > > >>>>>>>>>>>>> We are using java asm <https://asm.ow2.io/> to shade > > > >> (rename > > > >>>>>>>>>> packages > > > >>>>>>>>>>>> from > > > >>>>>>>>>>>>> the bytecode) since there are multiple dependencies clash > > > >> when > > > >>>>>>>>> using > > > >>>>>>>>>> with > > > >>>>>>>>>>>>> Apache Spark. For example, Apache Spark is using a much > > > >> older > > > >>>>>>>>>> version of > > > >>>>>>>>>>>>> guava. If I build calcite locally with Java Correto > > > >> 17.0.7-amzn I > > > >>>>>>>>> can > > > >>>>>>>>>>>>> transform calcite's bytecode without any problem. Where > can > > > >> I > > > >>>>>>>> find > > > >>>>>>>>>> the > > > >>>>>>>>>>>> java > > > >>>>>>>>>>>>> version used to compiled calcite? Is there a process in > > > >> place to > > > >>>>>>>>>> have a > > > >>>>>>>>>>>>> consistent java version? > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> https://gitlab.ow2.org/asm/asm/-/issues/318008 > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> -- > > > >>>>>>>>>>>>> Guillaume Massé > > > >>>>>>>>>>>>> [Gee-OHM] > > > >>>>>>>>>>>>> (马赛卫) > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> -- > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Best, > > > >>>>>>>>>>>> Benchao Li > > > >>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> -- > > > >>>>>> > > > >>>>>> Best, > > > >>>>>> Benchao Li > > > >>>>> > > > >>>>> > > > >>> > > > >>> > > > >>> > > > >>> -- > > > >>> > > > >>> Best, > > > >>> Benchao Li > > > >> > > > > > > >
