Merge pull request #8456 from reitermarkus/document-git

Refactor and document `Git`.

Library/Homebrew/dependency_collector.rb

@@ -54,7 +54,7 @@ class DependencyCollector
   end
 
   def git_dep_if_needed(tags)
-    return if Utils.git_available?
+    return if Utils::Git.available?
 
     Dependency.new("git", tags)
   end

Library/Homebrew/dev-cmd/audit.rb

@@ -1085,7 +1085,7 @@ module Homebrew
             problem http_content_problem
           end
         elsif strategy <= GitDownloadStrategy
-          problem "The URL #{url} is not a valid git URL" unless Utils.git_remote_exists? url
+          problem "The URL #{url} is not a valid git URL" unless Utils::Git.remote_exists? url
         elsif strategy <= SubversionDownloadStrategy
           next unless DevelopmentTools.subversion_handles_most_https_certificates?
           next unless Utils.svn_available?

Library/Homebrew/dev-cmd/bottle.rb

@@ -550,7 +550,7 @@ module Homebrew
         end
 
         unless args.no_commit?
-          Utils.set_git_name_email!
+          Utils::Git.set_name_email!
 
           short_name = formula_name.split("/", -1).last
           pkg_version = bottle_hash["formula"]["pkg_version"]

Library/Homebrew/dev-cmd/extract.rb

@@ -133,7 +133,7 @@ module Homebrew
       result = ""
       loop do
         rev = rev.nil? ? "HEAD" : "#{rev}~1"
-        rev, (path,) = Git.last_revision_commit_of_files(repo, pattern, before_commit: rev)
+        rev, (path,) = Utils::Git.last_revision_commit_of_files(repo, pattern, before_commit: rev)
         if rev.nil? && source_tap.shallow?
           odie <<~EOS
             Could not find #{name} but #{source_tap} is a shallow clone!
@@ -145,7 +145,7 @@ module Homebrew
         end
 
         file = repo/path
-        result = Git.last_revision_of_file(repo, file, before_commit: rev)
+        result = Utils::Git.last_revision_of_file(repo, file, before_commit: rev)
         if result.empty?
           odebug "Skipping revision #{rev} - file is empty at this revision"
           next
@@ -173,11 +173,11 @@ module Homebrew
 
       if files.empty?
         ohai "Searching repository history"
-        rev, (path,) = Git.last_revision_commit_of_files(repo, pattern)
+        rev, (path,) = Utils::Git.last_revision_commit_of_files(repo, pattern)
         odie "Could not find #{name}! The formula or version may not have existed." if rev.nil?
         file = repo/path
         version = formula_at_revision(repo, name, file, rev).version
-        result = Git.last_revision_of_file(repo, file)
+        result = Utils::Git.last_revision_of_file(repo, file)
       else
         file = files.first.realpath
         rev = "HEAD"
@@ -219,7 +219,7 @@ module Homebrew
   def formula_at_revision(repo, name, file, rev)
     return if rev.empty?
 
-    contents = Git.last_revision_of_file(repo, file, before_commit: rev)
+    contents = Utils::Git.last_revision_of_file(repo, file, before_commit: rev)
     contents.gsub!("@url=", "url ")
     contents.gsub!("require 'brewkit'", "require 'formula'")
     with_monkey_patch { Formulary.from_contents(name, file, contents) }

Library/Homebrew/dev-cmd/pull.rb

@@ -50,7 +50,7 @@ module Homebrew
     args = pull_args.parse
 
     # Passthrough Git environment variables for e.g. git am
-    Utils.set_git_name_email!(author: false, committer: true)
+    Utils::Git.set_name_email!(author: false, committer: true)
 
     # Depending on user configuration, git may try to invoke gpg.
     if Utils.popen_read("git config --get --bool commit.gpgsign").chomp == "true"

Library/Homebrew/diagnostic.rb

@@ -96,7 +96,7 @@ module Homebrew
       end
 
       def examine_git_origin(repository_path, desired_origin)
-        return if !Utils.git_available? || !repository_path.git?
+        return if !Utils::Git.available? || !repository_path.git?
 
         current_origin = repository_path.git_origin
 
@@ -482,13 +482,13 @@ module Homebrew
 
       def check_git_version
         minimum_version = ENV["HOMEBREW_MINIMUM_GIT_VERSION"]
-        return unless Utils.git_available?
-        return if Version.create(Utils.git_version) >= Version.create(minimum_version)
+        return unless Utils::Git.available?
+        return if Version.create(Utils::Git.version) >= Version.create(minimum_version)
 
         git = Formula["git"]
         git_upgrade_cmd = git.any_version_installed? ? "upgrade" : "install"
         <<~EOS
-          An outdated version (#{Utils.git_version}) of Git was detected in your PATH.
+          An outdated version (#{Utils::Git.version}) of Git was detected in your PATH.
           Git #{minimum_version} or newer is required for Homebrew.
           Please upgrade:
             brew #{git_upgrade_cmd} git
@@ -496,7 +496,7 @@ module Homebrew
       end
 
       def check_for_git
-        return if Utils.git_available?
+        return if Utils::Git.available?
 
         <<~EOS
           Git could not be found in your PATH.
@@ -507,7 +507,7 @@ module Homebrew
       end
 
       def check_git_newline_settings
-        return unless Utils.git_available?
+        return unless Utils::Git.available?
 
         autocrlf = HOMEBREW_REPOSITORY.cd { `git config --get core.autocrlf`.chomp }
         return unless autocrlf == "true"
@@ -543,7 +543,7 @@ module Homebrew
         return if ENV["CI"]
 
         coretap_path = CoreTap.instance.path
-        return if !Utils.git_available? || !(coretap_path/".git").exist?
+        return if !Utils::Git.available? || !(coretap_path/".git").exist?
 
         branch = coretap_path.git_branch
         return if branch.blank? || branch.include?("master")
@@ -642,7 +642,7 @@ module Homebrew
       end
 
       def check_git_status
-        return unless Utils.git_available?
+        return unless Utils::Git.available?
 
         message = nil
 

Library/Homebrew/extend/git_repository.rb

@@ -9,43 +9,43 @@ module GitRepositoryExtension
   end
 
   def git_origin
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "config", "--get", "remote.origin.url", chdir: self).chomp.presence
   end
 
   def git_origin=(origin)
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     safe_system "git", "remote", "set-url", "origin", origin, chdir: self
   end
 
   def git_head
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "rev-parse", "--verify", "-q", "HEAD", chdir: self).chomp.presence
   end
 
   def git_short_head
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "rev-parse", "--short=4", "--verify", "-q", "HEAD", chdir: self).chomp.presence
   end
 
   def git_last_commit
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "show", "-s", "--format=%cr", "HEAD", chdir: self).chomp.presence
   end
 
   def git_branch
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "rev-parse", "--abbrev-ref", "HEAD", chdir: self).chomp.presence
   end
 
   def git_last_commit_date
-    return unless git? && Utils.git_available?
+    return unless git? && Utils::Git.available?
 
     Utils.popen_read("git", "show", "-s", "--format=%cd", "--date=short", "HEAD", chdir: self).chomp.presence
   end

Library/Homebrew/formula.rb

@@ -1821,7 +1821,7 @@ class Formula
     test_env[:_JAVA_OPTIONS] += " -Djava.io.tmpdir=#{HOMEBREW_TEMP}"
 
     ENV.clear_sensitive_environment!
-    Utils.set_git_name_email!
+    Utils::Git.set_name_email!
 
     mktemp("#{name}-test") do |staging|
       staging.retain! if keep_tmp

Library/Homebrew/formula_installer.rb

@@ -722,8 +722,8 @@ class FormulaInstaller
     tab.runtime_dependencies = Tab.runtime_deps_hash(f_runtime_deps)
     tab.write
 
-    # let's reset Utils.git_available? if we just installed git
-    Utils.clear_git_available_cache if formula.name == "git"
+    # let's reset Utils::Git.available? if we just installed git
+    Utils::Git.clear_available_cache if formula.name == "git"
 
     # use installed curl when it's needed and available
     if formula.name == "curl" &&

Library/Homebrew/system_config.rb

@@ -98,9 +98,9 @@ module SystemConfig
     end
 
     def describe_git
-      return "N/A" unless Utils.git_available?
+      return "N/A" unless Utils::Git.available?
 
-      "#{Utils.git_version} => #{Utils.git_path}"
+      "#{Utils::Git.version} => #{Utils::Git.path}"
     end
 
     def describe_curl

Library/Homebrew/tap.rb

@@ -248,7 +248,7 @@ class Tap
     end
 
     # ensure git is installed
-    Utils.ensure_git_installed!
+    Utils::Git.ensure_installed!
 
     if installed?
       unless force_auto_update.nil?
@@ -707,7 +707,7 @@ class TapConfig
 
   def [](key)
     return unless tap.git?
-    return unless Utils.git_available?
+    return unless Utils::Git.available?
 
     tap.path.cd do
       Utils.popen_read("git", "config", "--get", "homebrew.#{key}").chomp.presence
@@ -716,7 +716,7 @@ class TapConfig
 
   def []=(key, value)
     return unless tap.git?
-    return unless Utils.git_available?
+    return unless Utils::Git.available?
 
     tap.path.cd do
       safe_system "git", "config", "--replace-all", "homebrew.#{key}", value.to_s

Library/Homebrew/test/tap_spec.rb

@@ -168,7 +168,7 @@ describe Tap do
 
     it "returns nil if Git is not available" do
       setup_git_repo
-      allow(Utils).to receive(:git_available?).and_return(false)
+      allow(Utils::Git).to receive(:available?).and_return(false)
       expect(subject.remote).to be nil
     end
   end

Library/Homebrew/test/utils/git_spec.rb

@@ -2,8 +2,10 @@
 
 require "utils/git"
 
-describe Git do
+describe Utils::Git do
   before do
+    described_class.clear_available_cache
+
     git = HOMEBREW_SHIMS_PATH/"scm/git"
 
     HOMEBREW_CACHE.cd do
@@ -88,49 +90,43 @@ describe Git do
       ).to eq("# README")
     end
   end
-end
 
-describe Utils do
-  before do
-    described_class.clear_git_available_cache
-  end
-
-  describe "::git_available?" do
+  describe "::available?" do
     it "returns true if git --version command succeeds" do
-      expect(described_class).to be_git_available
+      expect(described_class).to be_available
     end
 
     it "returns false if git --version command does not succeed" do
       stub_const("HOMEBREW_SHIMS_PATH", HOMEBREW_PREFIX/"bin/shim")
-      expect(described_class).not_to be_git_available
+      expect(described_class).not_to be_available
     end
   end
 
-  describe "::git_path" do
+  describe "::path" do
     it "returns nil when git is not available" do
       stub_const("HOMEBREW_SHIMS_PATH", HOMEBREW_PREFIX/"bin/shim")
-      expect(described_class.git_path).to eq(nil)
+      expect(described_class.path).to eq(nil)
     end
 
     it "returns path of git when git is available" do
-      expect(described_class.git_path).to end_with("git")
+      expect(described_class.path).to end_with("git")
     end
   end
 
-  describe "::git_version" do
+  describe "::version" do
     it "returns nil when git is not available" do
       stub_const("HOMEBREW_SHIMS_PATH", HOMEBREW_PREFIX/"bin/shim")
-      expect(described_class.git_version).to eq(nil)
+      expect(described_class.version).to eq(nil)
     end
 
     it "returns version of git when git is available" do
-      expect(described_class.git_version).not_to be_nil
+      expect(described_class.version).not_to be_nil
     end
   end
 
-  describe "::ensure_git_installed!" do
+  describe "::ensure_installed!" do
     it "returns nil if git already available" do
-      expect(described_class.ensure_git_installed!).to be_nil
+      expect(described_class.ensure_installed!).to be_nil
     end
 
     context "when git is not already available" do
@@ -140,25 +136,28 @@ describe Utils do
 
       it "can't install brewed git if homebrew/core is unavailable" do
         allow_any_instance_of(Pathname).to receive(:directory?).and_return(false)
-        expect { described_class.ensure_git_installed! }.to raise_error("Git is unavailable")
+        expect { described_class.ensure_installed! }.to raise_error("Git is unavailable")
       end
 
       it "raises error if can't install git" do
         stub_const("HOMEBREW_BREW_FILE", HOMEBREW_PREFIX/"bin/brew")
-        expect { described_class.ensure_git_installed! }.to raise_error("Git is unavailable")
+        expect { described_class.ensure_installed! }.to raise_error("Git is unavailable")
       end
 
       it "installs git" do
-        allow(Homebrew).to receive(:_system).with(any_args).and_return(true)
-        described_class.ensure_git_installed!
+        expect(described_class).to receive(:available?).and_return(false)
+        expect(described_class).to receive(:safe_system).with(HOMEBREW_BREW_FILE, "install", "git").and_return(true)
+        expect(described_class).to receive(:available?).and_return(true)
+
+        described_class.ensure_installed!
       end
     end
   end
 
-  describe "::git_remote_exists?" do
+  describe "::remote_exists?" do
     it "returns true when git is not available" do
       stub_const("HOMEBREW_SHIMS_PATH", HOMEBREW_PREFIX/"bin/shim")
-      expect(described_class).to be_git_remote_exists("blah")
+      expect(described_class).to be_remote_exists("blah")
     end
 
     context "when git is available" do
@@ -173,11 +172,11 @@ describe Utils do
           system git, "remote", "add", "origin", url
         end
 
-        expect(described_class).to be_git_remote_exists(url)
+        expect(described_class).to be_remote_exists(url)
       end
 
       it "returns false when git remote does not exist" do
-        expect(described_class).not_to be_git_remote_exists("blah")
+        expect(described_class).not_to be_remote_exists("blah")
       end
     end
   end

Library/Homebrew/utils/git.rb

@@ -2,9 +2,42 @@
 
 require "open3"
 
-module Git
+module Utils
+  # Helper functions for querying Git information.
+  #
+  # @api private
+  module Git
     module_function
 
+    def available?
+      version.present?
+    end
+
+    def version
+      return @version if defined?(@version)
+
+      stdout, _, status = system_command(HOMEBREW_SHIMS_PATH/"scm/git", args: ["--version"], print_stderr: false)
+      @version = status.success? ? stdout.chomp[/git version (\d+(?:\.\d+)*)/, 1] : nil
+    end
+
+    def path
+      return unless available?
+      return @path if defined?(@path)
+
+      @path = Utils.popen_read(HOMEBREW_SHIMS_PATH/"scm/git", "--homebrew=print-path").chomp.presence
+    end
+
+    def remote_exists?(url)
+      return true unless available?
+
+      quiet_system "git", "ls-remote", url
+    end
+
+    def clear_available_cache
+      remove_instance_variable(:@version) if defined?(@version)
+      remove_instance_variable(:@path) if defined?(@path)
+    end
+
     def last_revision_commit_of_file(repo, file, before_commit: nil)
       args = if before_commit.nil?
         ["--skip=1"]
@@ -52,31 +85,9 @@ module Git
       )
       out
     end
-end
 
-module Utils
-  def self.git_available?
-    @git_available ||= quiet_system HOMEBREW_SHIMS_PATH/"scm/git", "--version"
-  end
-
-  def self.git_path
-    return unless git_available?
-
-    @git_path ||= Utils.popen_read(
-      HOMEBREW_SHIMS_PATH/"scm/git", "--homebrew=print-path"
-    ).chomp.presence
-  end
-
-  def self.git_version
-    return unless git_available?
-
-    @git_version ||= Utils.popen_read(
-      HOMEBREW_SHIMS_PATH/"scm/git", "--version"
-    ).chomp[/git version (\d+(?:\.\d+)*)/, 1]
-  end
-
-  def self.ensure_git_installed!
-    return if git_available?
+    def ensure_installed!
+      return if available?
 
       # we cannot install brewed git if homebrew/core is unavailable.
       if CoreTap.instance.installed?
@@ -88,22 +99,10 @@ module Utils
         end
       end
 
-    raise "Git is unavailable" unless git_available?
+      raise "Git is unavailable" unless available?
     end
 
-  def self.clear_git_available_cache
-    @git_available = nil
-    @git_path = nil
-    @git_version = nil
-  end
-
-  def self.git_remote_exists?(url)
-    return true unless git_available?
-
-    quiet_system "git", "ls-remote", url
-  end
-
-  def self.set_git_name_email!(author: true, committer: true)
+    def set_name_email!(author: true, committer: true)
       if Homebrew::EnvConfig.git_name
         ENV["GIT_AUTHOR_NAME"] = Homebrew::EnvConfig.git_name if author
         ENV["GIT_COMMITTER_NAME"] = Homebrew::EnvConfig.git_name if committer
@@ -114,4 +113,5 @@ module Utils
       ENV["GIT_AUTHOR_EMAIL"] = Homebrew::EnvConfig.git_email if author
       ENV["GIT_COMMITTER_EMAIL"] = Homebrew::EnvConfig.git_email if committer
     end
+  end
 end