build: fixes bytecode level and updates build versions #236
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a problem where we weren't releasing jars with the right bytecode level, due to a constant release=8. As we no longer can rely on RetroLambda to rewrite code, I've had to port things to the appropriate bytecode level manually. I also updated build deps that could be. Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt
commented
Jan 6, 2024
| <maven.compiler.source>1.8</maven.compiler.source> | ||
| <maven.compiler.target>1.8</maven.compiler.target> | ||
| <!-- We don't use animal-sniffer anymore as release obviates it. | ||
| See https://github.com/mojohaus/animal-sniffer/issues/62 --> | ||
| <maven.compiler.release>8</maven.compiler.release> |
There was a problem hiding this comment.
the problem was that we never overrode this, so the bytecode ended up as 8 on all projects
codefromthecrypt
commented
Jan 6, 2024
| Class.forName("java.io.UncheckedIOException"); | ||
| return new Jre8(); // intentionally doesn't not access the type prior to the above guard | ||
| Class<?> clazz = Class.forName("java.io.UncheckedIOException"); | ||
| Constructor<?> ctor = clazz.getConstructor(IOException.class); |
There was a problem hiding this comment.
this is annoying, but using this path is also very unlikely. It meant there was an IOException gzipping spans, which would be uncommon for sure.
added 2 commits
January 6, 2024 14:24
Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt
commented
Jan 6, 2024
| thread.start(); | ||
| thread.interrupt(); | ||
|
|
||
| assertThat(thread.isInterrupted()).isTrue(); | ||
| // The callback thread receiving an interrupt has nothing to do with the caller of the captor | ||
| assertThat(Thread.currentThread().isInterrupted()).isFalse(); | ||
| assertThat(returned.get()).isFalse(); |
There was a problem hiding this comment.
this was winky depending on JDK. The important thing is that we interrupted the thread launched and not the current one, so I removed this part
|
cherry-picked a few classes and they look ok now $ ./mvnw clean install -Prelease -Dgpg.skip -Dmaven.javadoc.skip=true -DskipTests
--snip--
$ file core/target/classes//zipkin2/reporter/AsyncReporter.class
core/target/classes//zipkin2/reporter/AsyncReporter.class: compiled Java class data, version 50.0 (Java 1.6)
$ file brave/target/classes/zipkin2/reporter/brave/AsyncZipkinSpanHandler.class
brave/target/classes/zipkin2/reporter/brave/AsyncZipkinSpanHandler.class: compiled Java class data, version 50.0 (Java 1.6)
$ file okhttp3/target/classes/zipkin2/reporter/okhttp3/OkHttpSender.class
okhttp3/target/classes/zipkin2/reporter/okhttp3/OkHttpSender.class: compiled Java class data, version 51.0 (Java 1.7)
$ file spring-beans/target/classes/zipkin2/reporter/beans/URLConnectionSenderFactoryBean.class
spring-beans/target/classes/zipkin2/reporter/beans/URLConnectionSenderFactoryBean.class: compiled Java class data, version 50.0 (Java 1.6) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a problem where we weren't releasing jars with the right bytecode level, due to a constant release=8. As we no longer can rely on RetroLambda to rewrite code, I've had to port things to the appropriate bytecode level manually. I also updated build deps that could be.
Fixes #235 and after this will cut a patch release (without any other pending PR)