Refactor Gradle wrapper updater to improve command execution and error handling

- Consolidate wrapper update logic into dedicated methods for clarity.
- Ensure both wrapper update commands are executed in sequence.
- Add error handling for wrapper task failures, raising DependencyFileNotResolvable.
- Update specs to verify command execution and handle wrapper updates correctly.

Author: kbukum1

gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb

@@ -42,39 +42,75 @@ def update_files(build_file)
           # We only run this updater if it's a distribution dependency
           return [] unless Distributions.distribution_requirements?(dependency.requirements)
 
-          local_files = dependency_files.select do |file|
-            file.directory == build_file.directory && target_file?(file)
-          end
+          # Find all wrapper files - they can span multiple directories
+          local_files = dependency_files.select { |file| target_file?(file) }
 
           # If we don't have any files in the build files don't generate one
-          return [] unless local_files.any?
+          return dependency_files unless local_files.any?
 
           updated_files = dependency_files.dup
           SharedHelpers.in_a_temporary_directory do |temp_dir|
             populate_temp_directory(temp_dir)
-            cwd = File.join(temp_dir, base_path(build_file))
-
-            # Create gradle.properties file with proxy settings
-            # Would prefer to use command line arguments, but they don't work.
-            properties_filename = File.join(temp_dir, build_file.directory, "gradle.properties")
-            write_properties_file(properties_filename)
-
-            command_parts = %w(gradle --no-daemon --stacktrace) + command_args
-            command = Shellwords.join(command_parts)
-
-            Dir.chdir(cwd) do
-              SharedHelpers.run_shell_command(command, cwd: cwd)
-              update_files_content(temp_dir, local_files, updated_files)
-            rescue SharedHelpers::HelperSubprocessFailed => e
-              puts "Failed to update files: #{e.message}"
-              return updated_files
-            end
+            update_wrapper_files(build_file, temp_dir, local_files, updated_files)
           end
           updated_files
         end
 
         private
 
+        sig do
+          params(
+            build_file: Dependabot::DependencyFile,
+            temp_dir: T.any(Pathname, String),
+            local_files: T::Array[Dependabot::DependencyFile],
+            updated_files: T::Array[Dependabot::DependencyFile]
+          ).void
+        end
+        def update_wrapper_files(build_file, temp_dir, local_files, updated_files)
+          cwd = File.join(temp_dir, base_path(build_file))
+
+          # Create gradle.properties file with proxy settings
+          properties_filename = File.join(temp_dir, build_file.directory, "gradle.properties")
+          write_properties_file(properties_filename)
+
+          Dir.chdir(cwd) do
+            run_wrapper_tasks(cwd)
+            update_file_contents(temp_dir, local_files, updated_files)
+          rescue SharedHelpers::HelperSubprocessFailed => e
+            handle_wrapper_update_error(e)
+          end
+        end
+
+        sig { params(cwd: String).void }
+        def run_wrapper_tasks(cwd)
+          gradlew_script = File.exist?("gradlew.bat") && !File.exist?("gradlew") ? "gradlew.bat" : "./gradlew"
+
+          # First run: Update wrapper with new version and download new distribution
+          first_command = Shellwords.join([gradlew_script, "--no-daemon", "--stacktrace"] + command_args)
+          SharedHelpers.run_shell_command(first_command, cwd: cwd)
+
+          # Second run: Regenerate wrapper binaries (gradlew, gradlew.bat, gradle-wrapper.jar)
+          second_command = Shellwords.join([gradlew_script, "--no-daemon", "--stacktrace", "wrapper"])
+          SharedHelpers.run_shell_command(second_command, cwd: cwd)
+        end
+
+        sig do
+          params(
+            temp_dir: T.any(Pathname, String),
+            local_files: T::Array[Dependabot::DependencyFile],
+            updated_files: T::Array[Dependabot::DependencyFile]
+          ).void
+        end
+        def update_file_contents(temp_dir, local_files, updated_files)
+          local_files.each do |file|
+            file_path = File.join(temp_dir, file.directory, file.name)
+            f_content = file.binary? ? File.binread(file_path) : File.read(file_path)
+            tmp_file = file.dup
+            tmp_file.content = file.binary? ? Base64.encode64(f_content) : f_content
+            updated_files[T.must(updated_files.index(file))] = tmp_file
+          end
+        end
+
         sig { params(file: Dependabot::DependencyFile).returns(T::Boolean) }
         def target_file?(file)
           @target_files.any? { |r| "/#{file.name}".end_with?(r) }
@@ -110,31 +146,32 @@ def base_path(build_file)
           File.dirname(File.join(build_file.directory, build_file.name)).delete_suffix("/gradle/wrapper")
         end
 
-        sig do
-          params(
-            temp_dir: T.any(Pathname, String),
-            local_files: T::Array[Dependabot::DependencyFile],
-            updated_files: T::Array[Dependabot::DependencyFile]
-          ).void
-        end
-        def update_files_content(temp_dir, local_files, updated_files)
-          local_files.each do |file|
-            f_content = File.read(File.join(temp_dir, file.directory, file.name))
-            tmp_file = file.dup
-            tmp_file.content = tmp_file.binary? ? Base64.encode64(f_content) : f_content
-            updated_files[T.must(updated_files.index(file))] = tmp_file
-          end
-        end
-
         sig { params(temp_dir: T.any(Pathname, String)).void }
         def populate_temp_directory(temp_dir)
           files_to_populate.each do |file|
             in_path_name = File.join(temp_dir, file.directory, file.name)
             FileUtils.mkdir_p(File.dirname(in_path_name))
-            File.write(in_path_name, file.content)
+
+            # Write file content - binary files need special handling
+            if file.binary?
+              File.binwrite(in_path_name, Base64.decode64(T.must(file.content)))
+            else
+              File.write(in_path_name, file.content)
+            end
+
+            # Make gradlew scripts executable so they can be run
+            FileUtils.chmod(0o755, in_path_name) if file.name.end_with?("gradlew", "gradlew.bat")
           end
         end
 
+        sig { params(error: SharedHelpers::HelperSubprocessFailed).returns(T.noreturn) }
+        def handle_wrapper_update_error(error)
+          # Gradle wrapper update failures typically indicate build compatibility issues
+          # with the new Gradle version. Raise as DependencyFileNotResolvable so the
+          # service layer can handle appropriately.
+          raise Dependabot::DependencyFileNotResolvable, error.message
+        end
+
         sig { params(file_name: String).void }
         def write_properties_file(file_name) # rubocop:disable Metrics/PerceivedComplexity
           http_proxy = ENV.fetch("HTTP_PROXY", nil)

gradle/spec/dependabot/gradle/file_updater_spec.rb

@@ -716,20 +716,28 @@
           end
 
           its(:content) do
-            expected_command = "gradle --no-daemon --stacktrace wrapper --no-validate-url --gradle-version 9.0.0"
+            # First command: Update wrapper with new version
+            first_cmd = "./gradlew --no-daemon --stacktrace wrapper --no-validate-url " \
+                        "--gradle-version 9.0.0"
+            # Second command: Regenerate wrapper binaries
+            second_cmd = "./gradlew --no-daemon --stacktrace wrapper"
 
             is_expected.to include(
               "distributionUrl=https\\://services.gradle.org/distributions/gradle-9.0.0-#{type}.zip"
             )
 
             if checksum
-              expected_command += " --gradle-distribution-sha256-sum #{updated_checksum}"
+              first_cmd += " --gradle-distribution-sha256-sum #{updated_checksum}"
               is_expected.to include("distributionSha256Sum=#{updated_checksum}")
             else
               is_expected.not_to include("distributionSha256Sum=")
             end
 
-            expect(Dependabot::SharedHelpers).to have_received(:run_shell_command).with(expected_command, cwd: anything)
+            # Verify both commands were called in sequence
+            expect(Dependabot::SharedHelpers)
+              .to have_received(:run_shell_command).with(first_cmd, cwd: anything).ordered
+            expect(Dependabot::SharedHelpers)
+              .to have_received(:run_shell_command).with(second_cmd, cwd: anything).ordered
           end
         end
 
@@ -745,6 +753,177 @@
                         "all",
                         "443c9c8ee2ac1ee0e11881a40f2376d79c66386264a44b24a9f8ca67e633375f",
                         "f759b8dd5204e2e3fa4ca3e73f452f087153cf81bac9561eeb854229cc2c5365"
+
+        context "when wrapper task fails" do
+          let(:buildfile) { wrapper_file }
+          let(:wrapper_file) do
+            Dependabot::DependencyFile.new(
+              name: "gradle/wrapper/gradle-wrapper.properties",
+              content: fixture("wrapper_files", "gradle-wrapper-8.14.2-bin.properties")
+            )
+          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: "https://services.gradle.org", 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
+            allow(Dependabot::SharedHelpers).to receive(:run_shell_command)
+              .and_raise(
+                Dependabot::SharedHelpers::HelperSubprocessFailed.new(
+                  message: "Gradle wrapper task failed",
+                  error_context: {}
+                )
+              )
+          end
+
+          it "raises DependencyFileNotResolvable" do
+            expect { updated_files }.to raise_error(Dependabot::DependencyFileNotResolvable)
+          end
+        end
+
+        context "when updating a non-wrapper dependency" do
+          let(:buildfile) do
+            Dependabot::DependencyFile.new(
+              name: "build.gradle",
+              content: fixture("buildfiles", "basic_build.gradle")
+            )
+          end
+
+          let(:dependency_files) { [buildfile, wrapper_file] }
+
+          let(:wrapper_file) do
+            Dependabot::DependencyFile.new(
+              name: "gradle/wrapper/gradle-wrapper.properties",
+              content: fixture("wrapper_files", "gradle-wrapper-8.14.2-bin.properties")
+            )
+          end
+
+          let(:dependency) do
+            Dependabot::Dependency.new(
+              name: "co.aikar:acf-paper",
+              version: "0.6.0-SNAPSHOT",
+              requirements: [{
+                file: "build.gradle",
+                requirement: "0.6.0-SNAPSHOT",
+                groups: [],
+                source: nil,
+                metadata: nil
+              }],
+              previous_requirements: [{
+                file: "build.gradle",
+                requirement: "0.5.0-SNAPSHOT",
+                groups: [],
+                source: nil,
+                metadata: nil
+              }],
+              package_manager: "gradle"
+            )
+          end
+
+          it "does not run wrapper updater" do
+            # Should not call run_shell_command for wrapper tasks
+            updated_files
+            expect(Dependabot::SharedHelpers).not_to have_received(:run_shell_command)
+          end
+
+          it "updates only the build file" do
+            expect(updated_files.length).to eq(1)
+            expect(updated_files.first.name).to eq("build.gradle")
+          end
+        end
+
+        context "with all wrapper files including binaries" do
+          let(:buildfile) { wrapper_file }
+
+          let(:wrapper_file) do
+            Dependabot::DependencyFile.new(
+              name: "gradle/wrapper/gradle-wrapper.properties",
+              content: fixture("wrapper_files", "gradle-wrapper-8.14.2-bin.properties")
+            )
+          end
+
+          let(:gradlew_file) do
+            Dependabot::DependencyFile.new(
+              name: "gradlew",
+              content: "#!/bin/sh\necho 'gradlew script'"
+            )
+          end
+
+          let(:gradlew_bat_file) do
+            Dependabot::DependencyFile.new(
+              name: "gradlew.bat",
+              content: "@echo off\necho gradlew.bat"
+            )
+          end
+
+          let(:jar_file) do
+            file = Dependabot::DependencyFile.new(
+              name: "gradle/wrapper/gradle-wrapper.jar",
+              content: Base64.encode64("fake jar content")
+            )
+            file.content_encoding = Dependabot::DependencyFile::ContentEncoding::BASE64
+            file
+          end
+
+          let(:dependency_files) { [wrapper_file, gradlew_file, gradlew_bat_file, jar_file] }
+
+          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: "https://services.gradle.org", 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
+            allow(Dependabot::SharedHelpers).to receive(:run_shell_command)
+          end
+
+          it "processes all wrapper files" do
+            # When commands are mocked, files don't actually change content,
+            # so they may be filtered out. The key is that the wrapper updater
+            # processes all wrapper files by running the wrapper task.
+            result = updated_files
+            # At minimum, the properties file should be in the result
+            expect(result.map(&:name)).to include("gradle/wrapper/gradle-wrapper.properties")
+          end
+
+          it "runs wrapper task twice" do
+            updated_files
+            expect(Dependabot::SharedHelpers).to have_received(:run_shell_command).twice
+          end
+        end
       end
 
       context "with a version catalog" do