diff --git a/gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb b/gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb index 1e3ec94f39a..0f38cd72863 100644 --- a/gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb +++ b/gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb @@ -71,7 +71,8 @@ def update_files(build_file) FileUtils.chmod("+x", "./gradlew") if has_local_script properties_file = File.join(cwd, "gradle/wrapper/gradle-wrapper.properties") - validate_option = get_validate_distribution_url_option(properties_file) + # Save original content to preserve comments, order, and custom properties + original_content = File.read(properties_file) env = { "JAVA_OPTS" => proxy_args.join(" ") } # set proxy for gradle execution begin @@ -89,8 +90,8 @@ def update_files(build_file) SharedHelpers.run_shell_command(command, cwd: cwd, env: env) # retry via local wrapper end - # Restore previous validateDistributionUrl option if it existed - override_validate_distribution_url_option(properties_file, validate_option) + # Update only the properties that should change, preserving everything else + update_distribution_properties(properties_file, original_content) update_files_content(temp_dir, local_files, updated_files) rescue SharedHelpers::HelperSubprocessFailed => e @@ -121,9 +122,9 @@ def command_args(requirements) # --no-validate-url is required to bypass HTTP proxy issues when running ./gradlew # This prevents validation failures during the wrapper update process # Note: This temporarily sets validateDistributionUrl=false in gradle-wrapper.properties - # The original value is restored after the wrapper task completes - # see method `get_validate_distribution_url_option` for more details - args = %W(wrapper --gradle-version #{version} --no-validate-url) # see + # The original value (along with all other custom properties) is restored after the wrapper task completes + # see method `update_distribution_properties` for more details + args = %W(wrapper --gradle-version #{version} --no-validate-url) args += %W(--distribution-type #{distribution_type}) if distribution_type args += %W(--gradle-distribution-sha256-sum #{checksum}) if checksum args @@ -174,30 +175,37 @@ def populate_temp_directory(temp_dir) end end - # This is a consequence of the lack of proper proxy support in Gradle Wrapper - # During the update process, Gradle Wrapper logic will try to validate the distribution URL - # by performing an HTTP request. If the environment requires a proxy, this validation will fail - # We need to add the `--no-validate-url` the commandline args to disable this validation - # However, this change is persistent in the `gradle-wrapper.properties` file - # To avoid side effects, we read the existing value before the update and restore it afterward - sig { params(properties_file: T.any(Pathname, String)).returns(T.nilable(String)) } - def get_validate_distribution_url_option(properties_file) - return nil unless File.exist?(properties_file) - - properties_content = File.read(properties_file) - properties_content.match(/^validateDistributionUrl=(.*)$/)&.captures&.first - end - - sig { params(properties_file: T.any(Pathname, String), value: T.nilable(String)).void } - def override_validate_distribution_url_option(properties_file, value) + # Updates only the distribution properties in the original file content + # This preserves the original file structure, comments, property order, and all other properties + # Only distributionUrl and distributionSha256Sum are updated from the newly generated file + sig { params(properties_file: T.any(Pathname, String), original_content: String).void } + def update_distribution_properties(properties_file, original_content) return unless File.exist?(properties_file) - properties_content = File.read(properties_file) - updated_content = properties_content.gsub( - /^validateDistributionUrl=(.*)\n/, - value ? "validateDistributionUrl=#{value}\n" : "" - ) - File.write(properties_file, updated_content) + # Read the newly generated file to extract updated distribution properties + new_content = File.read(properties_file) + updated_values = T.let({}, T::Hash[String, String]) + + # Extract the new values for distributionUrl and distributionSha256Sum + new_content.each_line do |line| + next if line.strip.start_with?("#") || line.strip.empty? + next unless line =~ /^(distributionUrl|distributionSha256Sum)=(.*)$/ + + key = T.must(::Regexp.last_match(1)) + value = T.must(::Regexp.last_match(2)) + updated_values[key] = value + end + + # Duplicate to avoid mutating the parameter + result_content = original_content.dup + + # Update only the distribution properties in the original content + updated_values.each do |key, new_value| + result_content.gsub!(/^(#{Regexp.escape(key)}=).*$/, "\\1#{new_value}") + end + + # Write back the updated original content + File.write(properties_file, result_content) end # rubocop:disable Metrics/PerceivedComplexity diff --git a/gradle/spec/dependabot/gradle/file_updater_spec.rb b/gradle/spec/dependabot/gradle/file_updater_spec.rb index 10bd7e71b58..a2ee70134ab 100644 --- a/gradle/spec/dependabot/gradle/file_updater_spec.rb +++ b/gradle/spec/dependabot/gradle/file_updater_spec.rb @@ -762,6 +762,179 @@ "all", "443c9c8ee2ac1ee0e11881a40f2376d79c66386264a44b24a9f8ca67e633375f", "f759b8dd5204e2e3fa4ca3e73f452f087153cf81bac9561eeb854229cc2c5365" + + context "with custom networkTimeout" do + subject(:updated_buildfile) do + updated_files.find { |f| f.name == "gradle/wrapper/gradle-wrapper.properties" } + end + + let(:buildfile) do + wrapper_file + end + + let(:wrapper_file) do + Dependabot::DependencyFile.new( + name: "gradle/wrapper/gradle-wrapper.properties", + content: fixture( + "wrapper_files", + "gradle-wrapper-8.14.2-bin-custom-timeout.properties" + ) + ) + end + + let(:distribution_url) do + "https\\://services.gradle.org/distributions/gradle-9.0.0-bin.zip" + end + + let(:dependency) do + Dependabot::Dependency.new( + name: "gradle-wrapper", + version: "9.0.0", + previous_version: "8.14.2", + requirements: [{ + file: "gradle/wrapper/gradle-wrapper.properties", + requirement: "9.0.0", + groups: [], + source: { type: "gradle-distribution", url: distribution_url, property: "distributionUrl" } + }], + previous_requirements: [{ + file: "gradle/wrapper/gradle-wrapper.properties", + requirement: "8.14.2", + groups: [], + source: { type: "gradle-distribution", url: "https://services.gradle.org", property: "distributionUrl" } + }], + package_manager: "gradle" + ) + end + + before do + # Mock the file operations to simulate the wrapper command updating the properties + allow(Dependabot::SharedHelpers).to receive(:run_shell_command) do |_command, cwd:, _env:| + # Simulate the wrapper command updating the properties file with defaults + properties_file = File.join(cwd, "gradle/wrapper/gradle-wrapper.properties") + if File.exist?(properties_file) + # Write a simulated updated properties file with default networkTimeout + File.write(properties_file, <<~PROPERTIES) + distributionBase=GRADLE_USER_HOME + distributionPath=wrapper/dists + distributionUrl=https\\://services.gradle.org/distributions/gradle-9.0.0-bin.zip + networkTimeout=10000 + validateDistributionUrl=true + zipStoreBase=GRADLE_USER_HOME + zipStorePath=wrapper/dists + PROPERTIES + end + end + allow(File).to receive(:exist?).and_call_original + allow(FileUtils).to receive(:chmod) + end + + it "preserves the custom networkTimeout value" do + expect(updated_buildfile.content).to include("networkTimeout=30000") + expect(updated_buildfile.content).not_to include("networkTimeout=10000") + end + + it "updates the distributionUrl" do + expect(updated_buildfile.content).to include("distributionUrl=#{distribution_url}") + end + end + + context "with comments in properties file" do + subject(:updated_buildfile) do + updated_files.find { |f| f.name == "gradle/wrapper/gradle-wrapper.properties" } + end + + let(:buildfile) do + wrapper_file + end + + let(:wrapper_file) do + Dependabot::DependencyFile.new( + name: "gradle/wrapper/gradle-wrapper.properties", + content: fixture( + "wrapper_files", + "gradle-wrapper-8.14.2-bin-with-comments.properties" + ) + ) + end + + let(:distribution_url) do + "https\\://services.gradle.org/distributions/gradle-9.0.0-bin.zip" + end + + let(:dependency) do + Dependabot::Dependency.new( + name: "gradle-wrapper", + version: "9.0.0", + previous_version: "8.14.2", + requirements: [{ + file: "gradle/wrapper/gradle-wrapper.properties", + requirement: "9.0.0", + groups: [], + source: { type: "gradle-distribution", url: distribution_url, property: "distributionUrl" } + }], + previous_requirements: [{ + file: "gradle/wrapper/gradle-wrapper.properties", + requirement: "8.14.2", + groups: [], + source: { type: "gradle-distribution", url: "https://services.gradle.org", property: "distributionUrl" } + }], + package_manager: "gradle" + ) + end + + before do + # Mock the file operations to simulate the wrapper command updating the properties + allow(Dependabot::SharedHelpers).to receive(:run_shell_command) do |_command, cwd:, _env:| + # Simulate the wrapper command generating a new file without comments + properties_file = File.join(cwd, "gradle/wrapper/gradle-wrapper.properties") + if File.exist?(properties_file) + # Write a simulated updated properties file without comments (as gradle does) + File.write(properties_file, <<~PROPERTIES) + distributionBase=GRADLE_USER_HOME + distributionPath=wrapper/dists + distributionUrl=https\\://services.gradle.org/distributions/gradle-9.0.0-bin.zip + networkTimeout=10000 + validateDistributionUrl=true + zipStoreBase=GRADLE_USER_HOME + zipStorePath=wrapper/dists + PROPERTIES + end + end + allow(File).to receive(:exist?).and_call_original + allow(FileUtils).to receive(:chmod) + end + + it "preserves comments from the original file" do + expect(updated_buildfile.content).to include("# Configuration for Gradle wrapper") + expect(updated_buildfile.content).to include("# Custom network timeout for slow connections") + end + + it "preserves the custom networkTimeout value" do + expect(updated_buildfile.content).to include("networkTimeout=30000") + expect(updated_buildfile.content).not_to include("networkTimeout=10000") + end + + it "updates the distributionUrl" do + expect(updated_buildfile.content).to include("distributionUrl=#{distribution_url}") + end + + it "maintains the original property order" do + lines = updated_buildfile.content.lines.reject { |l| l.strip.start_with?("#") || l.strip.empty? } + property_names = lines.map { |l| l.split("=").first } + expect(property_names).to eq( + %w( + distributionBase + distributionPath + distributionUrl + networkTimeout + validateDistributionUrl + zipStoreBase + zipStorePath + ) + ) + end + end end context "with a version catalog" do diff --git a/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-custom-timeout.properties b/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-custom-timeout.properties new file mode 100644 index 00000000000..6fe4eb7cf93 --- /dev/null +++ b/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-custom-timeout.properties @@ -0,0 +1,7 @@ +distributionBase=GRADLE_USER_HOME +distributionPath=wrapper/dists +distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.2-bin.zip +networkTimeout=30000 +validateDistributionUrl=true +zipStoreBase=GRADLE_USER_HOME +zipStorePath=wrapper/dists diff --git a/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-with-comments.properties b/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-with-comments.properties new file mode 100644 index 00000000000..7c0843aae43 --- /dev/null +++ b/gradle/spec/fixtures/wrapper_files/gradle-wrapper-8.14.2-bin-with-comments.properties @@ -0,0 +1,10 @@ +# Configuration for Gradle wrapper +# See https://docs.gradle.org/current/userguide/gradle_wrapper.html +distributionBase=GRADLE_USER_HOME +distributionPath=wrapper/dists +distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.2-bin.zip +# Custom network timeout for slow connections +networkTimeout=30000 +validateDistributionUrl=true +zipStoreBase=GRADLE_USER_HOME +zipStorePath=wrapper/dists