Merge pull request #24278 from BrandenXia/extraction-write-errors

unpack: explain extraction disk space failures

Library/Homebrew/exceptions.rb

@@ -806,6 +806,9 @@ end
 # Raised when curl cannot write a download to the local filesystem.
 class DownloadWriteError < RuntimeError; end
 
+# Raised when an archive cannot be extracted because the destination is full.
+class ExtractionDiskSpaceError < RuntimeError; end
+
 # Raised in {CurlDownloadStrategy#fetch}.
 class CurlDownloadStrategyError < RuntimeError
   sig { params(url: String, details: T.nilable(String)).void }

Library/Homebrew/test/bottle_spec.rb

@@ -333,6 +333,24 @@ RSpec.describe Bottle do
       bottle&.clear_cache
     end
 
+    it "keeps the cached bottle and cleans up when extraction runs out of space", :aggregate_failures do
+      content = (TEST_FIXTURE_DIR/"tarballs/testball-0.1.tbz").binread
+      bottle = cached_bottle(Digest::SHA256.hexdigest(content), content)
+      error = ErrorDuringExecution.new(["tar"], status: 2,
+                                                output: [[:stderr, "file: Write failed: No space left on device\n"]])
+      allow(UnpackStrategy).to receive(:detect) do |path, **options|
+        UnpackStrategy::Tar.new(path, temporary_directory: options.fetch(:temporary_directory)).tap do |strategy|
+          allow(strategy).to receive(:system_command!).and_raise(error)
+        end
+      end
+      expect(bottle).not_to receive(:fetch)
+
+      expect { bottle.stage_from_download_queue(bottle.cached_download, pour: true) }
+        .to raise_error(ExtractionDiskSpaceError)
+      expect(bottle.cached_download.binread).to eq(content)
+      expect(HOMEBREW_TEMP_CELLAR.children).to be_empty
+    end
+
     it "keeps a cached bottle matching its checksum that fails to extract", :aggregate_failures do
       content = "valid but unextractable"
       bottle = cached_bottle(Digest::SHA256.hexdigest(content), content)

Library/Homebrew/test/unpack_strategy/tar_spec.rb

@@ -9,6 +9,66 @@ RSpec.describe UnpackStrategy::Tar do
   include_examples "UnpackStrategy::detect"
   include_examples "#extract", children: ["container"]
 
+  describe "#extract failures" do
+    subject(:strategy) { described_class.new(path) }
+
+    let(:unpack_dir) { mktmpdir/"unpacked" }
+    let(:disk_space_error) do
+      ErrorDuringExecution.new(["tar"], status: 2,
+                                        output: [[:stderr, "payload: Write failed: No space left on device\n"]])
+    end
+
+    it "reports the archive and destination when tar runs out of disk space" do
+      allow(strategy).to receive(:system_command!).and_raise(disk_space_error)
+
+      expect { strategy.extract(to: unpack_dir) }.to raise_error(ExtractionDiskSpaceError, <<~EOS)
+        Not enough disk space to extract:
+          #{path}
+        Free up space on the filesystems containing these directories:
+          #{unpack_dir}
+          #{strategy.temporary_directory}
+        Then try again.
+      EOS
+    end
+
+    it "preserves tar's original failure as the cause" do
+      allow(strategy).to receive(:system_command!).and_raise(disk_space_error)
+
+      expect { strategy.extract(to: unpack_dir) }.to raise_error(ExtractionDiskSpaceError) { |error|
+        expect(error.cause).to equal(disk_space_error)
+      }
+    end
+
+    it "handles an out-of-space failure returned by the extraction sandbox" do
+      allow(Sandbox).to receive(:isolate_operation?).and_return(true)
+      allow(Sandbox).to receive(:operation).and_raise(disk_space_error)
+
+      expect { strategy.extract(to: unpack_dir) }.to raise_error(ExtractionDiskSpaceError)
+    end
+
+    it "handles a native out-of-space error" do
+      allow(strategy).to receive(:system_command!).and_raise(Errno::ENOSPC)
+
+      expect { strategy.extract(to: unpack_dir) }.to raise_error(ExtractionDiskSpaceError)
+    end
+
+    it "does not misreport a corrupt archive as insufficient disk space" do
+      error = ErrorDuringExecution.new(["tar"], status: 2,
+                                                output: [[:stderr, "tar: Unrecognized archive format\n"]])
+      allow(strategy).to receive(:system_command!).and_raise(error)
+
+      expect { strategy.extract(to: unpack_dir) }.to raise_error { |raised|
+        expect(raised).to equal(error)
+      }
+    end
+
+    it "uses a stable locale for tar diagnostics" do
+      expect(strategy).to receive(:system_command!).with("tar", hash_including(env: { "LC_ALL" => "C" }))
+
+      strategy.extract(to: unpack_dir)
+    end
+  end
+
   it "keeps decompression intermediates in the configured temporary directory" do
     temporary_directory = mktmpdir
     unpack_dir = mktmpdir

Library/Homebrew/unpack_strategy.rb

@@ -198,6 +198,17 @@ module UnpackStrategy
     end
 
     extract_to_dir(unpack_dir, basename: Pathname(basename), verbose:)
+  rescue Errno::ENOSPC, ErrorDuringExecution => e
+    raise if e.is_a?(ErrorDuringExecution) && e.stderr.exclude?("No space left on device")
+
+    raise ExtractionDiskSpaceError, <<~EOS
+      Not enough disk space to extract:
+        #{path}
+      Free up space on the filesystems containing these directories:
+        #{unpack_dir}
+        #{temporary_directory}
+      Then try again.
+    EOS
   end
 
   sig {

Library/Homebrew/unpack_strategy/tar.rb

@@ -52,6 +52,9 @@ module UnpackStrategy
                         args:    ["--extract", "--no-same-owner",
                                   "--file", tar_path,
                                   "--directory", unpack_dir],
+                        # Keep tar diagnostics in English so that out of space error handle in
+                        # `‎unpack_strategy.rb` can recognize out-of-space errors.
+                        env:     { "LC_ALL" => "C" },
                         verbose:
       end
     end