Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable IO and UpdateTest types #15104

Merged
merged 6 commits into from Mar 31, 2023
Merged

Enable IO and UpdateTest types #15104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
15582dd
Add Utils popen sigs, enable IO types
dduugg Mar 30, 2023
a4091b4
Just use a single args parameter for conciseness
dduugg Mar 30, 2023
80091b8
Fix tests
dduugg Mar 30, 2023
a20d28f
brew style --fix
dduugg Mar 30, 2023
4f5fc77
rm popen sigs
dduugg Mar 31, 2023
f9e512b
Enable update-test types
dduugg Mar 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions Library/Homebrew/dev-cmd/update-test.rb
View file
Open in desktop
@@ -1,14 +1,12 @@
# typed: false
# typed: true
# frozen_string_literal: true

require "cli/parser"
module Homebrew
extend T::Sig

module_function

sig { returns(CLI::Parser) }
def update_test_args
def self.update_test_args
Homebrew::CLI::Parser.new do
description <<~EOS
Run a test of `brew update` with a new repository clone.
Expand All @@ -27,7 +25,7 @@ def update_test_args
end
end

def update_test
def self.update_test
args = update_test_args.parse

# Avoid `update-report.rb` tapping Homebrew/homebrew-core
Expand All @@ -51,10 +49,12 @@ def update_test
"master"
end

start_commit = nil
# Utils.popen_read returns a String without a block argument, but that isn't easily typed. We thus label this
# as untyped for now.
start_commit = T.let("", T.untyped)
end_commit = "HEAD"
cd HOMEBREW_REPOSITORY do
start_commit = if (commit = args.commit)
start_commit = if (commit = T.let(args.commit, T.nilable(String)))
commit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really necessary? I tested it locally and it didn't seem to care.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, are some of these changes only needed for typed: strict @dduugg?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, this is a vestige of attempting to type the Utils.popen* methods. Would you like me to remove it in the next typing PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dduugg No, all good!

elsif (date = args.before)
Utils.popen_read("git", "rev-list", "-n1", "--before=#{date}", "origin/master").chomp
Expand All @@ -78,7 +78,7 @@ def update_test
start_commit = Utils.popen_read("git", "rev-parse", start_commit).chomp
odie "Could not find start commit!" if start_commit.empty?

end_commit = Utils.popen_read("git", "rev-parse", end_commit).chomp
end_commit = T.cast(Utils.popen_read("git", "rev-parse", end_commit).chomp, String)
odie "Could not find end commit!" if end_commit.empty?

if Utils.popen_read("git", "branch", "--list", "master").blank?
Expand Down Expand Up @@ -114,7 +114,7 @@ def update_test
safe_system "git", "reset", "--hard", start_commit

# update ENV["PATH"]
ENV["PATH"] = PATH.new(ENV.fetch("PATH")).prepend(curdir/"bin")
ENV["PATH"] = PATH.new(ENV.fetch("PATH")).prepend(curdir/"bin").to_s

# run brew help to install portable-ruby (if needed)
quiet_system "brew", "help"
Expand All @@ -139,7 +139,7 @@ def update_test
FileUtils.rm_rf "update-test" unless args.keep_tmp?
end

def git_tags
def self.git_tags
tags = Utils.popen_read("git", "tag", "--list", "--sort=-version:refname")
if tags.blank?
tags = if (HOMEBREW_REPOSITORY/".git/shallow").exist?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/git_repository.rb
View file
Open in desktop
Expand Up @@ -117,6 +117,6 @@ def popen_git(*args, safe: false, err: nil)
raise "Git is unavailable"
end

T.unsafe(Utils).popen_read(Utils::Git.git, *args, safe: safe, chdir: self, err: err).chomp.presence
Utils.popen_read(Utils::Git.git, *args, safe: safe, chdir: self, err: err).chomp.presence
end
end
22 changes: 12 additions & 10 deletions Library/Homebrew/extend/io.rb
View file
Open in desktop
@@ -1,22 +1,24 @@
# typed: false
# typed: true
# frozen_string_literal: true

class IO
def readline_nonblock(sep = $INPUT_RECORD_SEPARATOR)
line = +""
buffer = +""

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be outside the rescued block, so that line is provably a String when line.empty? is invoked below.

loop do
break if buffer == sep
begin
loop do
break if buffer == sep

read_nonblock(1, buffer)
line.concat(buffer)
end
read_nonblock(1, buffer)
line.concat(buffer)
end

line.freeze
rescue IO::WaitReadable, EOFError => e
raise e if line.empty?
line.freeze
rescue IO::WaitReadable, EOFError
raise if line.empty?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IO::WaitReadable is not technically an Exception, so it can't be passed to raise, but we can avoid the issue using an empty re-raise instead (e is not needed otherwise)

line.freeze
line.freeze
end
end
end