Make brew tap-info show commit hash for taps that don't use the JSON API (_i.e._, are cloned locally)

[benknoble]

Verification

• This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

Provide a detailed description of the proposed feature

Currently, brew tap-info <tap> provides a small amount of information about a tap; using --verbose does not provide more information, but using --json does.

In none of the above modes are tap versions shown, which can be helpful when troubleshooting an issue with a custom tap and formula that revolves around the version of both tap and formula.

The proposed feature is to add a version field to the JSON and text-based outputs of tap-info that indicates the Git commit hash of the currently checked-out tap, just as if git -C $(brew --repository)/Library/Taps/<tap> rev-parse HEAD were run.

What is the motivation for the feature?

As stated in the proposal, this provides useful debugging information:

• it indicates whether or not a tap is up-to-date
• it indicates the precise version of a tap being used to execute formula from that tap, which may reveal important differences

How will the feature be relevant to at least 90% of Homebrew users?

This feature can lead to quicker resolution of issues with formula from 3rd-party taps. For taps that use the JSON API, like homebrew/core, this will be less relevant (but brew tap-info homebrew/core reports "Not Installed" now, so I don't think that's a concern).

What alternatives to the feature have been considered?

I mentioned git -C $(brew --repository)/Library/Taps/<tap> rev-parse HEAD earlier: this is one way to request the relevant information. It adds an extra step for everyone involved in troubleshooting issues, however. Placing the information in brew tap-info makes the tap more self-describing and seems like a natural fit.

[MikeMcQuaid]

Thanks @benknoble, this loosely makes sense to me. Which would have been more useful to solve the problem you faced: the actual SHA of the commit, the date of the commit or both/neither?

[benknoble]

I think the SHA; I can get the date from that easily enough with access to the corresponding repo. (A way to include the result of git show -s <sha> would be nice but not necessary.)

[MikeMcQuaid]

I asked because in brew config we include:

ORIGIN: https://github.com/Homebrew/brew
HEAD: bd3c7f80530d21e791ccc01b02234b299941dd29
Last commit: 7 hours ago
Core tap HEAD: ae0af7c512095a35ffc933184d2c5108e6dfaaf1
Core tap last commit: 3 days ago
Core tap JSON: 20 Sep 11:55 UTC
Core cask tap HEAD: f5b6c9217e78b8f7b9e650bf1744027791b7052b
Core cask tap last commit: 3 days ago
Core cask tap JSON: 20 Sep 11:23 UTC

so the code is already there to do similar stuff so it may make sense for both consistency and given the existing helpers to do that

[MikeMcQuaid]

Will review PR for this. This code may be useful:

brew/Library/Homebrew/system_config.rb

Lines 110 to 133 in bd3c7f8

	def dump_tap_config(tap, out = $stdout)
	case tap
	when CoreTap
	tap_name = "Core tap"
	json_file_name = "formula.jws.json"
	when CoreCaskTap
	tap_name = "Core cask tap"
	json_file_name = "cask.jws.json"
	else
	raise ArgumentError, "Unknown tap: #{tap}"
	end
	if tap.installed?
	out.puts "#{tap_name} origin: #{tap.remote}" if tap.remote != tap.default_remote
	out.puts "#{tap_name} HEAD: #{tap.git_head || "(none)"}"
	out.puts "#{tap_name} last commit: #{tap.git_last_commit || "never"}"
	out.puts "#{tap_name} branch: #{tap.git_branch || "(none)"}" if tap.git_branch != "master"
	end
	if (json_file = Homebrew::API::HOMEBREW_CACHE_API/json_file_name) && json_file.exist?
	out.puts "#{tap_name} JSON: #{json_file.mtime.utc.strftime("%d %b %H:%M UTC")}"
	elsif !tap.installed?
	out.puts "#{tap_name}: N/A"
	end

Would suggest we do something consistently here both for JSON and Git taps.

[benknoble]

I asked because in brew config we include:

ORIGIN: https://github.com/Homebrew/brew
HEAD: bd3c7f80530d21e791ccc01b02234b299941dd29
Last commit: 7 hours ago
Core tap HEAD: ae0af7c512095a35ffc933184d2c5108e6dfaaf1
Core tap last commit: 3 days ago
Core tap JSON: 20 Sep 11:55 UTC
Core cask tap HEAD: f5b6c9217e78b8f7b9e650bf1744027791b7052b
Core cask tap last commit: 3 days ago
Core cask tap JSON: 20 Sep 11:23 UTC

so the code is already there to do similar stuff so it may make sense for both consistency and given the existing helpers to do that

Yep, if the date's already available easily I don't see any problems including it.

Will review PR for this. This code may be useful:

brew/Library/Homebrew/system_config.rb

Lines 110 to 133 in bd3c7f8

	def dump_tap_config(tap, out = $stdout)
	case tap
	when CoreTap
	tap_name = "Core tap"
	json_file_name = "formula.jws.json"
	when CoreCaskTap
	tap_name = "Core cask tap"
	json_file_name = "cask.jws.json"
	else
	raise ArgumentError, "Unknown tap: #{tap}"
	end
	if tap.installed?
	out.puts "#{tap_name} origin: #{tap.remote}" if tap.remote != tap.default_remote
	out.puts "#{tap_name} HEAD: #{tap.git_head || "(none)"}"
	out.puts "#{tap_name} last commit: #{tap.git_last_commit || "never"}"
	out.puts "#{tap_name} branch: #{tap.git_branch || "(none)"}" if tap.git_branch != "master"
	end
	if (json_file = Homebrew::API::HOMEBREW_CACHE_API/json_file_name) && json_file.exist?
	out.puts "#{tap_name} JSON: #{json_file.mtime.utc.strftime("%d %b %H:%M UTC")}"
	elsif !tap.installed?
	out.puts "#{tap_name}: N/A"
	end

Would suggest we do something consistently here both for JSON and Git taps.

Great! I'll find some time, hopefully soon, to work on this.

[benknoble]

So far, this seems to do something like what I want for both brew tap-info --installed and brew tap-info --installed --json (the latter optionally with | jq 'map({name, remote, HEAD, last_commit, branch})'):

diff --git i/Library/Homebrew/cmd/tap-info.rb w/Library/Homebrew/cmd/tap-info.rb
index 2d00a70cb..bca548660 100644
--- i/Library/Homebrew/cmd/tap-info.rb
+++ w/Library/Homebrew/cmd/tap-info.rb
@@ -76,6 +76,10 @@ def print_tap_info(taps)
               info += "\nPrivate" if tap.private?
               info += "\n#{tap.path} (#{tap.path.abv})"
               info += "\nFrom: #{tap.remote.presence || "N/A"}"
+              info += "\norigin: #{tap.remote}" if tap.remote != tap.default_remote
+              info += "\nHEAD: #{tap.git_head || "(none)"}"
+              info += "\nlast commit: #{tap.git_last_commit || "never"}"
+              info += "\nbranch: #{tap.git_branch || "(none)"}" if tap.git_branch != "master"
             else
               info += "Not installed"
             end
diff --git i/Library/Homebrew/tap.rb w/Library/Homebrew/tap.rb
index dd78d0a97..4bf635058 100644
--- i/Library/Homebrew/tap.rb
+++ w/Library/Homebrew/tap.rb
@@ -881,6 +881,9 @@ def to_hash
       hash["remote"] = remote
       hash["custom_remote"] = custom_remote?
       hash["private"] = private?
+      hash["HEAD"] = git_head || "(none)"
+      hash["last_commit"] = git_last_commit || "never"
+      hash["branch"] = git_branch || "(none)"
     end
 
     hash

Thoughts? It would be nice to avoid the duplicated code spread across 3 files, of course, but I'm not Ruby expert :) I did attempt a version of StringIO.open {|s| SystemConfig.dump_tap_config(tap, s); s.string } for the tap-info command, but (naturally) it errors when given non-Core taps. So any common code should be called by SystemConfig, not the other way around.

[MikeMcQuaid]

@benknoble That makes sense to me. I think the duplication here is fine. Can you open a PR? Thanks!