Enable caching for quarkus:build and declare dependencies as inputs #37622

aloubyansky · 2023-12-08T15:01:39Z

@gsmet here are two commits:

enhances the track-config-changes impl to also dump dependency checksums in target/quarkus-<profile-name>-dependency-checksums.txt, typically the profile name would be prod;
enables dependency checksums dump for all the IT modules.

fyi @jprinet

github-actions · 2023-12-08T15:46:50Z

🙈 The PR is closed and the preview is expired.

devtools/maven/src/main/java/io/quarkus/maven/TrackConfigChangesMojo.java

gsmet

Things seem to work with the changes I made.

Could you clarify again why it only works the third time? I somehow forgot about it.

gsmet · 2024-02-01T18:07:49Z

devtools/maven/src/main/java/io/quarkus/maven/TrackConfigChangesMojo.java

            return;
        }

        CuratedApplication curatedApplication = null;
        QuarkusClassLoader deploymentClassLoader = null;
        final ClassLoader originalCl = Thread.currentThread().getContextClassLoader();
-        Properties actualProps;
        final boolean clearPackageTypeSystemProperty = setPackageTypeSystemPropertyIfNativeProfileEnabled();
        try {
            curatedApplication = bootstrapApplication(launchMode);


FWIW, I benchmarked things a bit and resolving the dependencies is quite slow. Even for a relatively simple IT as the Hibernate Validator one, it takes ~ 0.8 s.

It is, this is something i've been looking into for the last few weeks. But there is not much we can do especially for relatively simple projects, since most of the time is spent in the Maven artifact resolver itself.

However, the app model resolution will typically be necessary either way. It's about at which point it's performed. Here, we are moving this step to an earlier phase and it's cached for later phases if they get executed.

gsmet · 2024-02-01T18:08:34Z

devtools/maven/src/main/java/io/quarkus/maven/TrackConfigChangesMojo.java

+                final List<String> deps = new ArrayList<>();
+                for (var d : curatedApplication.getApplicationModel().getDependencies(DependencyFlags.DEPLOYMENT_CP)) {
+                    StringBuilder entry = new StringBuilder(d.toGACTVString());
+                    if (d.isSnapshot()) {


I decided to avoid computing the checksum if the dependency is not a snapshot one. They should be immutable in this case.

Final versions could still be built locally. For example before releasing but not necessarily then.

gsmet · 2024-02-01T18:13:45Z

integration-tests/pom.xml

@@ -22,6 +22,9 @@
        <quarkus.build.skip>${skipTests}</quarkus.build.skip>
        <native.surefire.skip>${skipTests}</native.surefire.skip>
        <enforce-test-deps-scope.skip>true</enforce-test-deps-scope.skip>
+
+        <quarkus.config-tracking.enabled>true</quarkus.config-tracking.enabled>
+        <quarkus.native.container-build>true</quarkus.native.container-build>


Can you clarify why you added <quarkus.native.container-build>true</quarkus.native.container-build>?

This is required by the Quarkus caching extension to ensure native builds are as reproducible as possible and make caching consistent.

gsmet · 2024-02-01T18:26:06Z

BTW, it wasn't terribly clear but I added a bunch of commits.

jprinet · 2024-02-02T08:27:10Z

Things seem to work with the changes I made.

Could you clarify again why it only works the third time? I somehow forgot about it.

This is not true anymore, with the config dump cached/restored on the CI runner, the cache hit would occur on first run if inputs are identical.

You can check the initial explanation here

aloubyansky · 2024-02-02T09:10:25Z

devtools/maven/src/main/java/io/quarkus/maven/TrackConfigChangesMojo.java

@@ -75,7 +75,7 @@ public class TrackConfigChangesMojo extends QuarkusBootstrapMojo {
    /**
     * Dependency dump file
     */
-    @Parameter(property = "quarkus.track-config-changes.dependenciesFile")


I suppose to be completely consistent, it would have to be quarkus.trackConfigChanges.dependenciesFile

…of track-config-changes goal

…uarkus application dependency checksums

Non snapshot dependencies are supposed to be immutable so we can skip computing the checksum.

When we don't have any previous recorded config properties, we should push an empty Properties. At least, that's what CodeGenerator.dumpCurrentConfigValues expects.

Add the dumped dependencies as an input for Surefire, Failsafe and the Quarkus Maven plugin.

This reverts commit b3eeb7c. In the end, it is not a very good idea to enable this for all of them.

quarkus-bot · 2024-02-15T14:42:59Z

Status for workflow `Quarkus Documentation CI`

This is the status report for running Quarkus Documentation CI on commit a171754.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other check runs running, make sure you don't need to wait for their status before merging.

quarkus-bot · 2024-02-15T18:41:30Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit a171754.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✔️	JVM Tests - JDK 17			Logs	Raw logs	🚧
✖	JVM Tests - JDK 21	`Build`	⚠️ Check →	Logs	Raw logs	🚧

Flaky tests - Develocity

⚙️ Maven Tests - JDK 17

📦 integration-tests/maven

✖ io.quarkus.maven.it.DevMojoIT.testThatNewResourcesAreServed - History

io.quarkus.maven.it.DevMojoIT expected "60396444-67ee-4272-9ca7-ab592064c3fe" but was "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua." within 2 minutes. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: io.quarkus.maven.it.DevMojoIT expected "60396444-67ee-4272-9ca7-ab592064c3fe" but was "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua." within 2 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AbstractHamcrestCondition.await(AbstractHamcrestCondition.java:86)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:691)
	at io.quarkus.maven.it.DevMojoIT.testThatNewResourcesAreServed(DevMojoIT.java:967)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

✖ io.quarkus.maven.it.DevMojoIT.testThatNewResourcesAreServed - History

io.quarkus.maven.it.DevMojoIT expected "60396444-67ee-4272-9ca7-ab592064c3fe" but was "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua." within 2 minutes. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: io.quarkus.maven.it.DevMojoIT expected "60396444-67ee-4272-9ca7-ab592064c3fe" but was "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua." within 2 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AbstractHamcrestCondition.await(AbstractHamcrestCondition.java:86)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:691)
	at io.quarkus.maven.it.DevMojoIT.testThatNewResourcesAreServed(DevMojoIT.java:967)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

integration-tests/main/pom.xml

Since quarkusio/quarkus#37622, integration-tests/main sets quarkus.native.container-build to true resulting in our CI building and testing it with the default builder image instead of our Mandrel build. It's yet not clear to me why this change was necessary (expecting an answer in [1], but either way enforcing non-container-builds seems the right thing to do in the CI when testing builds from source. [1] https://github.com/quarkusio/quarkus/pull/37622/files#r1551481449),

quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/maven labels Dec 8, 2023

This comment has been minimized.

Sign in to view

gsmet reviewed Feb 1, 2024

View reviewed changes

devtools/maven/src/main/java/io/quarkus/maven/TrackConfigChangesMojo.java Show resolved Hide resolved

gsmet force-pushed the dump-deps-from-track-config-changes branch from 11df920 to 217d8b4 Compare February 1, 2024 18:04

quarkus-bot bot added the area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure label Feb 1, 2024

gsmet changed the title ~~Dump deps from track config changes~~ Enable caching for quarkus:build and declare dependencies as inputs Feb 1, 2024

gsmet mentioned this pull request Feb 1, 2024

Enable quarkus:build outcome cache for the main integration test #37258

Closed

gsmet force-pushed the dump-deps-from-track-config-changes branch 2 times, most recently from 0ecc4f8 to bc32f18 Compare February 1, 2024 18:12

gsmet reviewed Feb 1, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

aloubyansky commented Feb 2, 2024

View reviewed changes

aloubyansky and others added 10 commits February 15, 2024 15:18

Dump all Quarkus app dependencies along with their checksums as part …

b56cbba

…of track-config-changes goal

Execute quarkus:track-config-changes for all the IT modules to dump q…

ae18537

…uarkus application dependency checksums

Only compute checksum if snapshot dependency

f6faadb

Non snapshot dependencies are supposed to be immutable so we can skip computing the checksum.

Enable caching of quarkus:build outcome for the main integration test

fe16620

Fix name of config property for consistency

8f7dec8

Enable config tracking for all ITs

057ef33

Avoid NPE in CodeGenerator.dumpCurrentConfigValues

970c62f

When we don't have any previous recorded config properties, we should push an empty Properties. At least, that's what CodeGenerator.dumpCurrentConfigValues expects.

Configure additional input for cached plugins

c76e348

Add the dumped dependencies as an input for Surefire, Failsafe and the Quarkus Maven plugin.

Revert "Enable config tracking for all ITs"

471c4fc

This reverts commit b3eeb7c. In the end, it is not a very good idea to enable this for all of them.

Some refinements to make sure the execution only runs once

a171754

gsmet force-pushed the dump-deps-from-track-config-changes branch from bc32f18 to a171754 Compare February 15, 2024 14:18

quarkus-bot bot added the triage/flaky-test label Feb 15, 2024

gsmet approved these changes Feb 16, 2024

View reviewed changes

gsmet merged commit e4e05ab into quarkusio:main Feb 16, 2024
51 of 52 checks passed

quarkus-bot bot added this to the 3.9 - main milestone Feb 16, 2024

zakkak reviewed Apr 4, 2024

View reviewed changes

integration-tests/main/pom.xml Show resolved Hide resolved

zakkak mentioned this pull request Apr 4, 2024

[CI] Explicitly set quarkus.native.container-build to false graalvm/mandrel#708

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable caching for quarkus:build and declare dependencies as inputs #37622

Enable caching for quarkus:build and declare dependencies as inputs #37622

aloubyansky commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 •

edited

Loading

This comment has been minimized.

gsmet left a comment

gsmet Feb 1, 2024

aloubyansky Feb 2, 2024

gsmet Feb 1, 2024

aloubyansky Feb 2, 2024

gsmet Feb 1, 2024

jprinet Feb 2, 2024

gsmet commented Feb 1, 2024

This comment has been minimized.

jprinet commented Feb 2, 2024

aloubyansky Feb 2, 2024

quarkus-bot bot commented Feb 15, 2024

quarkus-bot bot commented Feb 15, 2024

Enable caching for quarkus:build and declare dependencies as inputs #37622

Enable caching for quarkus:build and declare dependencies as inputs #37622

Conversation

aloubyansky commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 • edited Loading

This comment has been minimized.

gsmet left a comment

Choose a reason for hiding this comment

gsmet Feb 1, 2024

Choose a reason for hiding this comment

aloubyansky Feb 2, 2024

Choose a reason for hiding this comment

gsmet Feb 1, 2024

Choose a reason for hiding this comment

aloubyansky Feb 2, 2024

Choose a reason for hiding this comment

gsmet Feb 1, 2024

Choose a reason for hiding this comment

jprinet Feb 2, 2024

Choose a reason for hiding this comment

gsmet commented Feb 1, 2024

This comment has been minimized.

jprinet commented Feb 2, 2024

aloubyansky Feb 2, 2024

Choose a reason for hiding this comment

quarkus-bot bot commented Feb 15, 2024

Status for workflow Quarkus Documentation CI

quarkus-bot bot commented Feb 15, 2024

Status for workflow Quarkus CI

Failing Jobs

Flaky tests - Develocity

⚙️ Maven Tests - JDK 17

📦 integration-tests/maven

github-actions bot commented Dec 8, 2023 •

edited

Loading

Status for workflow `Quarkus Documentation CI`

Status for workflow `Quarkus CI`