-
Notifications
You must be signed in to change notification settings - Fork 23
Develocity and Gradle Enterprise remote build cache connectors are supported #807
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.
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
Conversation
Adds a new command line argument for Gradle experiment 5, `-y` or `--remote-build-cache-type`, allowing the user to specify a remote build cache type to use. When specified, valid values are: `develocity`, `gradle-enterprise`, or `http`. When the argument is specified, it will always take precedence over what is configured in the build. If the argument is not specified, the remote build cache type of the first build will be fetched from the Develocity API and used for the second build. If the argument is not specified and the user does not have access to the Develocity API, then the existing remote build cache configuration will be used.
@@ -160,6 +166,9 @@ fetch_build_params_from_build_scan() { | |||
} | |||
|
|||
read_build_params_from_build_scan_data() { | |||
if [[ "${remote_build_cache_types[0]}" == "disabled" ]]; then | |||
die "ERROR: Remote build cache was disabled for the first build. Enable the remote build cache in the build and restart the experiment." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does prevent users from doing something they could previously which is to invoke the script for a build that didn't have the remote build cache enabled, but I can't think of a valid use case for that.
The benefit to the user is that we will quickly catch what is likely a mistake (e.g., using the wrong Build Scan) and this happens before the first build is even invoked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a valid use case for that
What if the user is using the BVS to test or explore something other than build caching?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is a big "what if", but please let me know if you have a more concrete use case in mind that I may not be thinking of.
The purpose of the script is to validate remote build caching. If they want to, a user could use it for some other purpose, so long as the remote build cache is enabled in the builds.
components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! This will improve the experience for many users
components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh
Outdated
Show resolved
Hide resolved
components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh
Outdated
Show resolved
Hide resolved
components/scripts/gradle/gradle-init-scripts/configure-remote-build-caching.gradle
Outdated
Show resolved
Hide resolved
components/scripts/gradle/gradle-init-scripts/configure-remote-build-caching.gradle
Outdated
Show resolved
Hide resolved
76b34c2
to
f3ead0c
Compare
f3ead0c
to
16568a2
Compare
@@ -160,6 +166,9 @@ fetch_build_params_from_build_scan() { | |||
} | |||
|
|||
read_build_params_from_build_scan_data() { | |||
if [[ "${remote_build_cache_types[0]}" == "disabled" ]]; then | |||
die "ERROR: Remote build cache was disabled for the first build. Enable the remote build cache in the build and restart the experiment." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a valid use case for that
What if the user is using the BVS to test or explore something other than build caching?
|
||
collect_remote_build_cache_type() { | ||
local default_remote_cache_type="<project default>" | ||
prompt_for_setting "What is the remote build cache type to use? [develocity, gradle-enterprise, http, or <BLANK>]" "${remote_build_cache_type}" "${default_remote_cache_type}" remote_build_cache_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Gradle documentation and the Develocity Gradle plugin end-user documentation calls these "Build cache connectors". I wonder if we should replace "type" with "connector" so that the BVS are consistent with the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good point. 'connector' would be the correct term. Also, build cache type sounds like the type of the build cache -- and not like the class type used to coonect to the remote build cache.
|
||
if [[ -n "${remote_build_cache_type}" && "${remote_build_cache_type}" != 'http' && "${remote_build_cache_type}" != 'gradle-enterprise' && "${remote_build_cache_type}" != 'develocity' ]]; then | ||
print_bl | ||
die "ERROR: Invalid value for remote build cache type. Values are 'develocity', 'gradle-enterprise', 'http', or <BLANK> for project default." "${INVALID_INPUT}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not very user-friendly to completely exit the script in interactive mode if the user enters an invalid value. It would be better to print the error message, and call collect_remote_build_cache_type()
again (or do the validation using a do...while loop) until the user does provide a valid value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder if we should include a link to some documentation in the error message so that users who don't know what this is can learn more about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the validation using a do...while loop
@etiennestuder and I discussed exactly this and decided to move forward with just exiting for now. It comes at the cost of user-friendliness, but it's the most straight forward approach. If it turns out to be problematic for users we can certainly fix it.
I also wonder if we should include a link to some documentation in the error message so that users who don't know what this is can learn more about it.
That's not a bad idea, but would require documentation to link to. 😅
I've created issue #809 for this so that it can be addressed in a subsequent PR.
components/scripts/gradle/gradle-init-scripts/configure-remote-build-caching.gradle
Show resolved
Hide resolved
components/scripts/gradle/gradle-init-scripts/configure-remote-build-caching.gradle
Outdated
Show resolved
Hide resolved
Bumps com.gradle:build-scan-summary from 1.0.3-2024.1 to 1.0.4-2024.1. --- updated-dependencies: - dependency-name: com.gradle:build-scan-summary dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Resolves #507
Overview
Adds a new command line argument for Gradle experiment 5,
-y
or--remote-build-cache-type
, allowing the user to specify a remote build cache type to use. When specified, valid values are:develocity
,gradle-enterprise
, orhttp
. When the argument is specified, it will always take precedence over what is configured in the build.If the argument is not specified, the remote build cache type of the first build will be fetched from the Develocity API and used for the second build.
If the argument is not specified and the user does not have access to the Develocity API, then the existing remote build cache configuration will be used.
Details
Main changes
A new
-y
/--remote-build-cache-type
argument was added for Gradle experiment 5 which is visible in the help text.Valid values are:
http
,gradle-enterprise
anddevelocity
. If an invalid value is supplied, the script will immediately fail.When running in interactive mode, the user will be prompted to enter a value. The below image defaulted to
develocity
because it's what the first build used (fetched from the Develocity API).If
--remote-build-cache-type
is not specified, the remote build cache type of the first build will be fetched from the Develocity API and used for the second build. The--remote-build-cache-type
argument always takes precedence.If
--remote-build-cache-type
is not specified and the user does not have Develocity API access, then onlyremote.enabled = true
is set (regardless of implementation).Important
If
--remote-build-cache-url
is specified, then the URL is set only if one of the 3 supported build caches are being used. This is a notable change in behavior over previous versions where we would always configure anHttpBuildCache
regardless of what the user had configured in their build.This only affects users using an unsupported remote build cache implementation, in which case a warning is also printed now (see further below).
Other changes
Improved error handling
The CI / Local scripts rely on the remote build cache to validate the build. In the case where the CI (first) build did not have remote build cache enabled the scripts will fail immediately.
Note
This change applies to both Maven experiment 4 and Gradle experiment 5.
Gradle
Maven
If a remote build cache type is specified (or retrieved from the Develocity API), but the required plugin isn't available in the build, e.g.,
--remote-build-cache-type develocity
was specified, but the user only applies an older version of the Gradle Enterprise Gradle plugin, script execution will fail and the user will be informed about the problem.If
http
is specified for the remote build cache type, but no URL is configured, script execution will fail and the user will be informed about the problem.Note
All proceeding changes apply to Maven experiments 3 & 4 and Gradle experiments 4 & 5.
If the user runs either the first or second build using a remote build cache implementation not officially supported by the Develocity Build Validation Scripts, the user will be warned. This is noteworthy because in these cases the parameter
--remote-build-cache-url
will not work.In cases where the two builds ran using different remote build cache implementations, the user will be warned explicitly.
Similarly, if the two builds ran with different remote build cache URLs, the user will be warned explicitly.
It's possible for things to be very misconfigured.