-
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
Changes from 2 commits
ec5c91f
16568a2
2a50cab
a8b7e1e
93ede44
12f58d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ develocity_server='' | |
interactive_mode='' | ||
|
||
ci_build_scan_url='' | ||
remote_build_cache_type='' | ||
remote_build_cache_url='' | ||
mapping_file='' | ||
|
||
|
@@ -110,9 +111,9 @@ wizard_execute() { | |
collect_gradle_details | ||
|
||
print_bl | ||
explain_remote_build_cache_url | ||
explain_collect_remote_build_cache | ||
print_bl | ||
collect_remote_build_cache_url | ||
collect_remote_build_cache | ||
explain_command_to_repeat_experiment_after_collecting_parameters | ||
|
||
print_bl | ||
|
@@ -136,6 +137,7 @@ wizard_execute() { | |
|
||
map_additional_script_args() { | ||
ci_build_scan_url="${_arg_first_build_ci}" | ||
remote_build_cache_type="${_arg_remote_build_cache_type}" | ||
remote_build_cache_url="${_arg_remote_build_cache_url}" | ||
mapping_file="${_arg_mapping_file}" | ||
} | ||
|
@@ -151,6 +153,10 @@ validate_required_args() { | |
if [[ "${enable_develocity}" == "on" && -z "${develocity_server}" ]]; then | ||
_PRINT_HELP=yes die "ERROR: Missing required argument when enabling Develocity on a project not already connected: --develocity-server" "${INVALID_INPUT}" | ||
fi | ||
|
||
if [[ -n "${remote_build_cache_type}" && "${remote_build_cache_type}" != 'http' && "${remote_build_cache_type}" != 'gradle-enterprise' && "${remote_build_cache_type}" != 'develocity' ]]; then | ||
_PRINT_HELP=yes die "ERROR: Invalid value for argument --remote-build-cache-type. Values are 'develocity', 'gradle-enterprise', or 'http'." "${INVALID_INPUT}" | ||
fi | ||
} | ||
|
||
fetch_build_params_from_build_scan() { | ||
|
@@ -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." | ||
fi | ||
if [ -z "${git_repo}" ]; then | ||
git_repo="${git_repos[0]}" | ||
project_name="$(basename -s .git "${git_repo}")" | ||
|
@@ -170,6 +179,9 @@ read_build_params_from_build_scan_data() { | |
if [ -z "${git_commit_id}" ]; then | ||
git_commit_id="${git_commit_ids[0]}" | ||
fi | ||
if [[ -z "${remote_build_cache_type}" && "${remote_build_cache_types[0]}" != "unknown" ]]; then | ||
remote_build_cache_type="${remote_build_cache_types[0]}" | ||
fi | ||
if [ -z "${remote_build_cache_url}" ]; then | ||
remote_build_cache_url="${remote_build_cache_urls[0]}" | ||
fi | ||
|
@@ -196,6 +208,9 @@ validate_build_config() { | |
execute_build() { | ||
local args | ||
args=(--build-cache --init-script "${INIT_SCRIPTS_DIR}/configure-remote-build-caching.gradle") | ||
if [ -n "${remote_build_cache_type}" ]; then | ||
args+=("-Ddevelocity.build-validation.remoteBuildCacheType=${remote_build_cache_type}") | ||
fi | ||
if [ -n "${remote_build_cache_url}" ]; then | ||
args+=("-Ddevelocity.build-validation.remoteBuildCacheUrl=${remote_build_cache_url}") | ||
fi | ||
|
@@ -360,7 +375,7 @@ EOF | |
print_interactive_text "${text}" | ||
} | ||
|
||
explain_remote_build_cache_url() { | ||
explain_collect_remote_build_cache() { | ||
local text | ||
IFS='' read -r -d '' text <<EOF | ||
The local build will connect to the given remote build cache. The remote build | ||
|
@@ -369,11 +384,30 @@ EOF | |
print_interactive_text "${text}" | ||
} | ||
|
||
collect_remote_build_cache() { | ||
collect_remote_build_cache_type | ||
collect_remote_build_cache_url | ||
} | ||
|
||
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 commentThe 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 commentThe 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 commentThe 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 commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more.
@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.
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. |
||
fi | ||
|
||
if [[ "${remote_build_cache_type}" == "${default_remote_cache_type}" ]]; then | ||
remote_build_cache_type='' | ||
fi | ||
} | ||
|
||
collect_remote_build_cache_url() { | ||
local default_remote_cache="<project default>" | ||
prompt_for_setting "What is the remote build cache url to use?" "${remote_build_cache_url}" "${default_remote_cache}" remote_build_cache_url | ||
local default_remote_cache_url="<project default>" | ||
prompt_for_setting "What is the remote build cache url to use?" "${remote_build_cache_url}" "${default_remote_cache_url}" remote_build_cache_url | ||
|
||
if [[ "${remote_build_cache_url}" == "${default_remote_cache}" ]]; then | ||
if [[ "${remote_build_cache_url}" == "${default_remote_cache_url}" ]]; then | ||
remote_build_cache_url='' | ||
fi | ||
} | ||
|
@@ -465,6 +499,10 @@ generate_command_to_repeat_experiment() { | |
cmd+=("-m" "${mapping_file}") | ||
fi | ||
|
||
if [ -n "${remote_build_cache_type}" ]; then | ||
cmd+=("-y" "${remote_build_cache_type}") | ||
fi | ||
|
||
if [ -n "${remote_build_cache_url}" ]; then | ||
cmd+=("-u" "${remote_build_cache_url}") | ||
fi | ||
|
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.
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.