Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build cache - Various small adjustments #36525

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Build cache - Various small adjustments #36525

merged 3 commits into from
Oct 18, 2023

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Oct 17, 2023

By adding an output timestamp, we can get it to be binary identical when the content is identical.
Obviously, the date needs to be forged to be consistent between two separate builds and the output to be cachable.

@gsmet
Make quarkus-ide-launcher jar reproducible
b0414e8 
By adding an output timestamp, we can get it to be binary identical when
the content is identical.
Obviously, the date needs to be forged to be consistent between two
separate builds and the output to be cachable.
@gsmet gsmet requested a review from aloubyansky October 17, 2023 11:26
@gsmet
Copy link
Member Author

gsmet commented Oct 17, 2023

@aloubyansky pinging you as I'm not sure it's safe? But I don't think we are ever using the timestamp of the files in this jar?

@maxandersen
Copy link
Member

i'm missing sometihng - where is there a timestamp missing for ide-launcher? I thought removing timestamps wold be the need?

@maxandersen
Copy link
Member

ah scratch that - the PR view was missingfiles.

maxandersen
maxandersen reviewed Oct 17, 2023
View reviewed changes
core/launcher/pom.xml
@@ -92,6 +92,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<outputTimestamp>2023-10-17T10:15:30Z</outputTimestamp>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normally, one would use the date from the git sha or a 1970 date ...
does this specific timestamp have any meaning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's today and that's pretty much it :).

Using the date from the git sha is a bit counter productive as that means that it will be different for each git commit, which might not be needed.

I can go with 1970 if you prefer but really we don't really care about this date.

gsmet added 2 commits October 17, 2023 14:27
@gsmet
Build cache - Ignore Eclipse local files in spotless input
b92b631
@gsmet
Build cache - Ignore quarkus-ide-launcher in classpath
3172376 
It shouldn't be around anyway as only useful for IDE runs so let's get
it out of the classpath.
@gsmet gsmet changed the title Make quarkus-ide-launcher jar reproducible Build cache - Various small adjustments Oct 17, 2023
@aloubyansky
Copy link
Member

Could you please explain why it doesn't work w/o this change?

@gsmet
Copy link
Member Author

gsmet commented Oct 17, 2023

@aloubyansky I should have mentioned that it's related to the issue I pinged you for in Zulip: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/IDE.20launcher.20in.20classpath.20when.20running.20tests.20from.20console .

As this dependency is in the classpath of the ITs, if the jar is different, then it's a cache miss thus why I tried to make it reproducible, which seems to work with this option (and it's what they recommend in the doc).

I then also tried https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/IDE.20launcher.20in.20classpath.20when.20running.20tests.20from.20console which might help.

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 17, 2023

Failing Jobs - Building 3172376

Status Name Step Failures Logs Raw logs Build scan
✖ JVM Tests - JDK 11 Build ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 17
✖ JVM Tests - JDK 17 Windows Build ⚠️ Check → Logs Raw logs
🚫 JVM Tests - JDK 20
✔️ Maven Tests - JDK 11
✖ Maven Tests - JDK 11 Windows Build Failures Logs Raw logs
✖ Native Tests - Windows - RESTEasy Jackson Setup GraalVM ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Maven Tests - JDK 11 Windows #

- Failing: integration-tests/maven

📦 integration-tests/maven

✖ io.quarkus.maven.it.DevMojoIT.testExternalReloadableArtifacts line 1448 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.DevMojoIT was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.JarRunnerIT.testNonAsciiDir line 70 - More details - Source on GitHub

java.lang.AssertionError: 

Expecting actual:

✖ io.quarkus.maven.it.UberJarQuarkusIntegrationTestIT.testUberJar line 13 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.QuarkusITBase that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.DevMojoIT.testExternalReloadableArtifacts line 1448 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.DevMojoIT was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.JarRunnerIT.testNonAsciiDir line 70 - More details - Source on GitHub

java.lang.AssertionError: 

Expecting actual:

✖ io.quarkus.maven.it.UberJarQuarkusIntegrationTestIT.testUberJar line 13 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.QuarkusITBase that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

@gsmet gsmet merged commit 7072955 into quarkusio:main Oct 18, 2023
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Oct 18, 2023
@gsmet
Copy link
Member Author

gsmet commented Oct 18, 2023

Merged as I need this in to make progress on the Build cache. If you want me to adjust something, please create an issue and I will have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxandersen maxandersen maxandersen left review comments

@aloubyansky aloubyansky Awaiting requested review from aloubyansky

Assignees
No one assigned
Labels
area/core
Projects
None yet
Milestone
3.6.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@gsmet @maxandersen @aloubyansky