From bd4124c10fc5a281fe38cb7566b14dee14cb6368 Mon Sep 17 00:00:00 2001
From: Sam Ford <1584702+samford@users.noreply.github.com>
Date: Fri, 13 Aug 2021 16:29:06 -0400
Subject: [PATCH] Livecheck: Avoid duplicate URLs

Some formulae/casks contain duplicate checkable URLs or contain
URLs that become duplicates after `#preprocess_url` is used. With
the former, a formula's `stable` and `head` URLs can be the same if
they both use the Git repository. With the latter, a formula's
`homepage` and `stable` URLs are different but `#preprocess_url` can
return the same Git repository URL for each (which can also be
a duplicate of the `head` URL).

The `fabric-completion` formula is a worst case scenario, as it
contains both of these conditions but the repository has no tags.
By default, livecheck would needlessly check the repository three
times (i.e., no versions are found so livecheck tries the next URL,
which happens to be the same URL).

This commit avoids duplicate URLs in `#checkable_urls` and keeps
track of checked URLs in `#latest_version` to avoid a duplicate
caused by `#preprocess_url`. This should effectively resolve the
issue of checking the same URL more than once for a given
formula/cask. Checking the same URL only once across all the
formulae/casks in a given livecheck run will be handled in a later
commit (though I've done most of the groundwork already in previous
PRs).
---
 Library/Homebrew/livecheck/livecheck.rb       | 22 +++++++++----------
 .../Homebrew/test/livecheck/livecheck_spec.rb | 10 +++++++++
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb
index 63019e7d3a..306934d7ed 100644
--- a/Library/Homebrew/livecheck/livecheck.rb
+++ b/Library/Homebrew/livecheck/livecheck.rb
@@ -484,7 +484,7 @@ module Homebrew
         T.absurd(formula_or_cask)
       end
 
-      urls.compact
+      urls.compact.uniq
     end
 
     # Preprocesses and returns the URL used by livecheck.
@@ -609,24 +609,16 @@ module Homebrew
         end
       end
 
+      checked_urls = []
       # rubocop:disable Metrics/BlockLength
       urls.each_with_index do |original_url, i|
-        if debug
-          puts
-          if livecheck_url.is_a?(Symbol)
-            # This assumes the URL symbol will fit within the available space
-            puts "URL (#{livecheck_url}):".ljust(18, " ") + original_url
-          else
-            puts "URL:              #{original_url}"
-          end
-        end
-
         # Only preprocess the URL when it's appropriate
         url = if STRATEGY_SYMBOLS_TO_SKIP_PREPROCESS_URL.include?(livecheck_strategy)
           original_url
         else
           preprocess_url(original_url)
         end
+        next if checked_urls.include?(url)
 
         strategies = Strategy.from_url(
           url,
@@ -639,6 +631,13 @@ module Homebrew
         strategy_name = livecheck_strategy_names[strategy]
 
         if debug
+          puts
+          if livecheck_url.is_a?(Symbol)
+            # This assumes the URL symbol will fit within the available space
+            puts "URL (#{livecheck_url}):".ljust(18, " ") + original_url
+          else
+            puts "URL:              #{original_url}"
+          end
           puts "URL (processed):  #{url}" if url != original_url
           if strategies.present? && verbose
             puts "Strategies:       #{strategies.map { |s| livecheck_strategy_names[s] }.join(", ")}"
@@ -674,6 +673,7 @@ module Homebrew
         match_version_map = strategy_data[:matches]
         regex = strategy_data[:regex]
         messages = strategy_data[:messages]
+        checked_urls << url
 
         if messages.is_a?(Array) && match_version_map.blank?
           puts messages unless json
diff --git a/Library/Homebrew/test/livecheck/livecheck_spec.rb b/Library/Homebrew/test/livecheck/livecheck_spec.rb
index b3a11abdd8..72e529f19e 100644
--- a/Library/Homebrew/test/livecheck/livecheck_spec.rb
+++ b/Library/Homebrew/test/livecheck/livecheck_spec.rb
@@ -44,6 +44,15 @@ describe Homebrew::Livecheck do
     RUBY
   end
 
+  let(:f_duplicate_urls) do
+    formula("test_duplicate_urls") do
+      desc "Test formula with a duplicate URL"
+      homepage "https://github.com/Homebrew/brew.git"
+      url "https://brew.sh/test-0.0.1.tgz"
+      head "https://github.com/Homebrew/brew.git"
+    end
+  end
+
   describe "::resolve_livecheck_reference" do
     context "when a formula/cask has a livecheck block without formula/cask methods" do
       it "returns [nil, []]" do
@@ -144,6 +153,7 @@ describe Homebrew::Livecheck do
     it "returns the list of URLs to check" do
       expect(livecheck.checkable_urls(f)).to eq([stable_url, head_url, homepage_url])
       expect(livecheck.checkable_urls(c)).to eq([cask_url, homepage_url])
+      expect(livecheck.checkable_urls(f_duplicate_urls)).to eq([stable_url, head_url])
     end
   end