Merge pull request #16544 from dduugg/timer-util

Replace Time refinement with Timer Util

Library/Homebrew/download_strategy.rb

@@ -17,12 +17,10 @@ require "vendor/gems/mechanize/lib/mechanize/http/content_disposition_parser"
 
 require "utils/curl"
 require "utils/github"
+require "utils/timer"
 
 require "github_packages"
 
-require "extend/time"
-using TimeRemaining
-
 # @abstract Abstract superclass for all download strategies.
 #
 # @api private
@@ -427,7 +425,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy
       use_cached_location = false if version.respond_to?(:latest?) && version.latest?
 
       resolved_url, _, last_modified, _, is_redirection = begin
-        resolve_url_basename_time_file_size(url, timeout: end_time&.remaining!)
+        resolve_url_basename_time_file_size(url, timeout: Utils::Timer.remaining!(end_time))
       rescue ErrorDuringExecution
         raise unless use_cached_location
       end
@@ -442,7 +440,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy
         puts "Already downloaded: #{cached_location}"
       else
         begin
-          _fetch(url: url, resolved_url: resolved_url, timeout: end_time&.remaining!)
+          _fetch(url: url, resolved_url: resolved_url, timeout: Utils::Timer.remaining!(end_time))
         rescue ErrorDuringExecution
           raise CurlDownloadStrategyError, url
         end
@@ -802,9 +800,9 @@ class SubversionDownloadStrategy < VCSDownloadStrategy
     args.concat Utils::Svn.invalid_cert_flags if meta[:trust_cert] == true
 
     if target.directory?
-      command! "svn", args: ["update", *args], chdir: target.to_s, timeout: timeout&.remaining
+      command! "svn", args: ["update", *args], chdir: target.to_s, timeout: Utils::Timer.remaining(timeout)
     else
-      command! "svn", args: ["checkout", url, target, *args], timeout: timeout&.remaining
+      command! "svn", args: ["checkout", url, target, *args], timeout: Utils::Timer.remaining(timeout)
     end
   end
 
@@ -998,23 +996,23 @@ class GitDownloadStrategy < VCSDownloadStrategy
       command! "git",
                args:    ["fetch", "origin", "--unshallow"],
                chdir:   cached_location,
-               timeout: timeout&.remaining
+               timeout: Utils::Timer.remaining(timeout)
     else
       command! "git",
                args:    ["fetch", "origin"],
                chdir:   cached_location,
-               timeout: timeout&.remaining
+               timeout: Utils::Timer.remaining(timeout)
     end
   end
 
   sig { params(timeout: T.nilable(Time)).void }
   def clone_repo(timeout: nil)
-    command! "git", args: clone_args, timeout: timeout&.remaining
+    command! "git", args: clone_args, timeout: Utils::Timer.remaining(timeout)
 
     command! "git",
              args:    ["config", "homebrew.cacheversion", cache_version],
              chdir:   cached_location,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
 
     configure_sparse_checkout if partial_clone_sparse_checkout?
 
@@ -1025,7 +1023,8 @@ class GitDownloadStrategy < VCSDownloadStrategy
   sig { params(timeout: T.nilable(Time)).void }
   def checkout(timeout: nil)
     ohai "Checking out #{@ref_type} #{@ref}" if @ref_type && @ref
-    command! "git", args: ["checkout", "-f", @ref, "--"], chdir: cached_location, timeout: timeout&.remaining
+    command! "git", args: ["checkout", "-f", @ref, "--"], chdir: cached_location,
+                    timeout: Utils::Timer.remaining(timeout)
   end
 
   sig { void }
@@ -1047,11 +1046,11 @@ class GitDownloadStrategy < VCSDownloadStrategy
     command! "git",
              args:    ["submodule", "foreach", "--recursive", "git submodule sync"],
              chdir:   cached_location,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
     command! "git",
              args:    ["submodule", "update", "--init", "--recursive"],
              chdir:   cached_location,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
     fix_absolute_submodule_gitdir_references!
   end
 
@@ -1214,12 +1213,15 @@ class CVSDownloadStrategy < VCSDownloadStrategy
   sig { params(timeout: T.nilable(Time)).void }
   def clone_repo(timeout: nil)
     # Login is only needed (and allowed) with pserver; skip for anoncvs.
-    command! "cvs", args: [*quiet_flag, "-d", @url, "login"], timeout: timeout&.remaining if @url.include? "pserver"
+    if @url.include? "pserver"
+      command! "cvs", args:    [*quiet_flag, "-d", @url, "login"],
+                      timeout: Utils::Timer.remaining(timeout)
+    end
 
     command! "cvs",
              args:    [*quiet_flag, "-d", @url, "checkout", "-d", cached_location.basename, @module],
              chdir:   cached_location.dirname,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
   end
 
   sig { params(timeout: T.nilable(Time)).void }
@@ -1227,7 +1229,7 @@ class CVSDownloadStrategy < VCSDownloadStrategy
     command! "cvs",
              args:    [*quiet_flag, "update"],
              chdir:   cached_location,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
   end
 
   def split_url(in_url)
@@ -1292,7 +1294,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy
     end
 
     clone_args << @url << cached_location.to_s
-    command! "hg", args: clone_args, timeout: timeout&.remaining
+    command! "hg", args: clone_args, timeout: Utils::Timer.remaining(timeout)
   end
 
   sig { params(timeout: T.nilable(Time)).void }
@@ -1306,7 +1308,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy
       pull_args << "--rev" << @ref
     end
 
-    command! "hg", args: ["--cwd", cached_location, *pull_args], timeout: timeout&.remaining
+    command! "hg", args: ["--cwd", cached_location, *pull_args], timeout: Utils::Timer.remaining(timeout)
 
     update_args = %w[update --clean]
     update_args << if @ref_type && @ref
@@ -1316,7 +1318,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy
       "default"
     end
 
-    command! "hg", args: ["--cwd", cached_location, *update_args], timeout: timeout&.remaining
+    command! "hg", args: ["--cwd", cached_location, *update_args], timeout: Utils::Timer.remaining(timeout)
   end
 
   def current_revision
@@ -1376,7 +1378,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy
     # "lightweight" means history-less
     command! "bzr",
              args:    ["checkout", "--lightweight", @url, cached_location],
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
   end
 
   sig { params(timeout: T.nilable(Time)).void }
@@ -1384,7 +1386,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy
     command! "bzr",
              args:    ["update"],
              chdir:   cached_location,
-             timeout: timeout&.remaining
+             timeout: Utils::Timer.remaining(timeout)
   end
 end
 
@@ -1430,12 +1432,12 @@ class FossilDownloadStrategy < VCSDownloadStrategy
 
   sig { params(timeout: T.nilable(Time)).void }
   def clone_repo(timeout: nil)
-    command! "fossil", args: ["clone", @url, cached_location], timeout: timeout&.remaining
+    command! "fossil", args: ["clone", @url, cached_location], timeout: Utils::Timer.remaining(timeout)
   end
 
   sig { params(timeout: T.nilable(Time)).void }
   def update(timeout: nil)
-    command! "fossil", args: ["pull", "-R", cached_location], timeout: timeout&.remaining
+    command! "fossil", args: ["pull", "-R", cached_location], timeout: Utils::Timer.remaining(timeout)
   end
 end
 

Library/Homebrew/extend/time.rb

@@ -1,22 +1,7 @@
-# typed: true
+# typed: strong
 # frozen_string_literal: true
 
-module TimeRemaining
+require "time"
-  refine Time do
-    def remaining
-      T.bind(self, Time)
-      [0, self - Time.now].max
-    end
-
-    def remaining!
-      r = remaining
-
-      Kernel.raise Timeout::Error if r <= 0
-
-      r
-    end
-  end
-end
 
 class Time
   # Backwards compatibility for formulae that used this ActiveSupport extension

Library/Homebrew/extend/time.rbi

@@ -1,9 +0,0 @@
-# typed: strict
-
-class Time
-  sig { returns(T.any(Integer, Float)) }
-  def remaining; end
-
-  sig { returns(T.any(Integer, Float)) }
-  def remaining!; end
-end

Library/Homebrew/system_command.rb

@@ -7,14 +7,12 @@ require "plist"
 require "shellwords"
 
 require "extend/io"
-require "extend/time"
+require "utils/timer"
 
 # Class for running sub-processes and capturing their output and exit status.
 #
 # @api private
 class SystemCommand
-  using TimeRemaining
-
   # Helper functions for calling {SystemCommand.run}.
   module Mixin
     def system_command(executable, **options)
@@ -259,7 +257,7 @@ class SystemCommand
     end
 
     end_time = Time.now + @timeout if @timeout
-    raise Timeout::Error if raw_wait_thr.join(end_time&.remaining).nil?
+    raise Timeout::Error if raw_wait_thr.join(Utils::Timer.remaining(end_time)).nil?
 
     @status = raw_wait_thr.value
 

Library/Homebrew/test/utils/timer_spec.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require "utils/timer"
+
+describe Utils::Timer do
+  describe "#remaining" do
+    it "returns nil when nil" do
+      expect(described_class.remaining(nil)).to be_nil
+    end
+
+    it "returns time remaining when there is time remaining" do
+      expect(described_class.remaining(Time.now + 10)).to be > 1
+    end
+
+    it "returns 0 when there is no time remaining" do
+      expect(described_class.remaining(Time.now - 10)).to be 0
+    end
+  end
+
+  describe "#remaining!" do
+    it "returns nil when nil" do
+      expect(described_class.remaining!(nil)).to be_nil
+    end
+
+    it "returns time remaining when there is time remaining" do
+      expect(described_class.remaining!(Time.now + 10)).to be > 1
+    end
+
+    it "returns 0 when there is no time remaining" do
+      expect { described_class.remaining!(Time.now - 10) }.to raise_error(Timeout::Error)
+    end
+  end
+end

Library/Homebrew/utils/curl.rb

@@ -3,15 +3,13 @@
 
 require "open3"
 
-require "extend/time"
+require "utils/timer"
 
 module Utils
   # Helper function for interacting with `curl`.
   #
   # @api private
   module Curl
-    using TimeRemaining
-
     # This regex is used to extract the part of an ETag within quotation marks,
     # ignoring any leading weak validator indicator (`W/`). This simplifies
     # ETag comparison in `#curl_check_http_content`.
@@ -140,7 +138,7 @@ module Utils
       result = system_command curl_executable(use_homebrew_curl: use_homebrew_curl),
                               args:    curl_args(*args, **options),
                               env:     env,
-                              timeout: end_time&.remaining,
+                              timeout: Utils::Timer.remaining(end_time),
                               **command_options
 
       return result if result.success? || args.include?("--http1.1")
@@ -151,7 +149,7 @@ module Utils
       if result.exit_status == 16
         return curl_with_workarounds(
           *args, "--http1.1",
-          timeout: end_time&.remaining, **command_options, **options
+          timeout: Utils::Timer.remaining(end_time), **command_options, **options
         )
       end
 

Library/Homebrew/utils/timer.rb

@@ -0,0 +1,21 @@
+# typed: strong
+# frozen_string_literal: true
+
+module Utils
+  module Timer
+    sig { params(time: T.nilable(Time)).returns(T.any(Float, Integer, NilClass)) }
+    def self.remaining(time)
+      return unless time
+
+      [0, time - Time.now].max
+    end
+
+    sig { params(time: T.nilable(Time)).returns(T.any(Float, Integer, NilClass)) }
+    def self.remaining!(time)
+      r = remaining(time)
+      raise Timeout::Error if r && r <= 0
+
+      r
+    end
+  end
+end