Skip to content

Commit

Permalink
Replace Time refinement with Timer Util
Browse files Browse the repository at this point in the history
  • Loading branch information
dduugg committed Jan 30, 2024
1 parent 2cb8efc commit e00d066
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 59 deletions.
50 changes: 26 additions & 24 deletions Library/Homebrew/download_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ class Mechanize; end

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
Expand Down Expand Up @@ -427,7 +425,7 @@ def fetch(timeout: nil)
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
Expand All @@ -442,7 +440,7 @@ def fetch(timeout: nil)
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
Expand Down Expand Up @@ -802,9 +800,9 @@ def fetch_repo(target, url, revision = nil, ignore_externals: false, timeout: ni
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

Expand Down Expand Up @@ -998,23 +996,23 @@ def update_repo(timeout: nil)
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?

Expand All @@ -1025,7 +1023,8 @@ def clone_repo(timeout: nil)
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 }
Expand All @@ -1047,11 +1046,11 @@ def update_submodules(timeout: nil)
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

Expand Down Expand Up @@ -1214,20 +1213,23 @@ def quiet_flag
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 }
def update(timeout: nil)
command! "cvs",
args: [*quiet_flag, "update"],
chdir: cached_location,
timeout: timeout&.remaining
timeout: Utils::Timer.remaining(timeout)
end

def split_url(in_url)
Expand Down Expand Up @@ -1292,7 +1294,7 @@ def clone_repo(timeout: nil)
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)

Check warning on line 1297 in Library/Homebrew/download_strategy.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/download_strategy.rb#L1297

Added line #L1297 was not covered by tests
end

sig { params(timeout: T.nilable(Time)).void }
Expand All @@ -1306,7 +1308,7 @@ def update(timeout: nil)
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)

Check warning on line 1311 in Library/Homebrew/download_strategy.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/download_strategy.rb#L1311

Added line #L1311 was not covered by tests

update_args = %w[update --clean]
update_args << if @ref_type && @ref
Expand All @@ -1316,7 +1318,7 @@ def update(timeout: nil)
"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)

Check warning on line 1321 in Library/Homebrew/download_strategy.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/download_strategy.rb#L1321

Added line #L1321 was not covered by tests
end

def current_revision
Expand Down Expand Up @@ -1376,15 +1378,15 @@ def clone_repo(timeout: nil)
# "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 }
def update(timeout: nil)
command! "bzr",
args: ["update"],
chdir: cached_location,
timeout: timeout&.remaining
timeout: Utils::Timer.remaining(timeout)
end
end

Expand Down Expand Up @@ -1430,12 +1432,12 @@ def cache_tag

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)

Check warning on line 1435 in Library/Homebrew/download_strategy.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/download_strategy.rb#L1435

Added line #L1435 was not covered by tests
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)

Check warning on line 1440 in Library/Homebrew/download_strategy.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/download_strategy.rb#L1440

Added line #L1440 was not covered by tests
end
end

Expand Down
19 changes: 2 additions & 17 deletions Library/Homebrew/extend/time.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,7 @@
# typed: true
# typed: strong
# frozen_string_literal: true

module TimeRemaining
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
require "time"

Check warning on line 4 in Library/Homebrew/extend/time.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/extend/time.rb#L4

Added line #L4 was not covered by tests

class Time
# Backwards compatibility for formulae that used this ActiveSupport extension
Expand Down
9 changes: 0 additions & 9 deletions Library/Homebrew/extend/time.rbi

This file was deleted.

6 changes: 2 additions & 4 deletions Library/Homebrew/system_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@
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)
Expand Down Expand Up @@ -259,7 +257,7 @@ def each_output_line(&block)
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

Expand Down
33 changes: 33 additions & 0 deletions Library/Homebrew/test/utils/timer_spec.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 3 additions & 5 deletions Library/Homebrew/utils/curl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -140,7 +138,7 @@ def curl_with_workarounds(
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")
Expand All @@ -151,7 +149,7 @@ def curl_with_workarounds(
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

Expand Down
21 changes: 21 additions & 0 deletions Library/Homebrew/utils/timer.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e00d066

Please sign in to comment.