From dd7d91bc6f761c24e614365451cde6765cd02200 Mon Sep 17 00:00:00 2001
From: Mike McQuaid <mike@mikemcquaid.com>
Date: Thu, 17 Oct 2024 08:34:03 +0100
Subject: [PATCH] Improve/fix HOMEBREW_FORBIDDEN_LICENSES handling

`HOMEBREW_FORBIDDEN_LICENSES` now actually checks for valid SPDX license
identifiers rather than requiring the user to guess.

When an identifier is invalid, it will be ignore and warned about
instead.
---
 Library/Homebrew/env_config.rb        |  2 +-
 Library/Homebrew/formula_installer.rb | 25 +++++++++++++++++++++----
 docs/Manpage.md                       |  4 ++--
 manpages/brew.1                       |  2 +-
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/Library/Homebrew/env_config.rb b/Library/Homebrew/env_config.rb
index bb185001b30a5..a3b3124e36f0f 100644
--- a/Library/Homebrew/env_config.rb
+++ b/Library/Homebrew/env_config.rb
@@ -204,7 +204,7 @@ module EnvConfig
                      "formula or cask if it or any of its dependencies is on this list.",
       },
       HOMEBREW_FORBIDDEN_LICENSES:               {
-        description: "A space-separated list of licenses. Homebrew will refuse to install a " \
+        description: "A space-separated list of SPDX license identifiers. Homebrew will refuse to install a " \
                      "formula if it or any of its dependencies has a license on this list.",
       },
       HOMEBREW_FORBIDDEN_OWNER:                  {
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index be3be998def58..da443e8479f97 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -1482,8 +1482,25 @@ def forbidden_license_check
       pattern = /#{s.to_s.tr("_", " ")}/i
       forbidden_licenses.sub!(pattern, s.to_s)
     end
-    forbidden_licenses = forbidden_licenses.split.to_h do |license|
-      [license, SPDX.license_version_info(license)]
+
+    invalid_licenses = []
+    forbidden_licenses = forbidden_licenses.split.each_with_object({}) do |license, hash|
+      unless SPDX.valid_license?(license)
+        invalid_licenses << license
+        next
+      end
+
+      hash[license] = SPDX.license_version_info(license)
+    end
+
+    if invalid_licenses.present?
+      opoo <<~EOS
+        HOMEBREW_FORBIDDEN_LICENSES contains invalid license identifiers: #{invalid_licenses.to_sentence}
+        These licenses will not be forbidden. See the valid SPDX license identifiers at:
+          #{Formatter.url("https://spdx.org/licenses/")}
+        And the licenses for a formula with:
+          brew info <formula>
+      EOS
     end
 
     return if forbidden_licenses.blank?
@@ -1501,7 +1518,7 @@ def forbidden_license_check
         raise CannotInstallFormulaError, <<~EOS
           The installation of #{formula.name} has a dependency on #{dep.name} where all
           its licenses were forbidden by #{owner} in `HOMEBREW_FORBIDDEN_LICENSES`:
-            #{SPDX.license_expression_to_string dep_f.license}.#{owner_contact}
+            #{SPDX.license_expression_to_string dep_f.license}#{owner_contact}
         EOS
       end
     end
@@ -1512,7 +1529,7 @@ def forbidden_license_check
 
     raise CannotInstallFormulaError, <<~EOS
       #{formula.name}'s licenses are all forbidden by #{owner} in `HOMEBREW_FORBIDDEN_LICENSES`:
-        #{SPDX.license_expression_to_string formula.license}.#{owner_contact}
+        #{SPDX.license_expression_to_string formula.license}#{owner_contact}
     EOS
   end
 
diff --git a/docs/Manpage.md b/docs/Manpage.md
index e73cd8b46a29b..7c8b8bac0907e 100644
--- a/docs/Manpage.md
+++ b/docs/Manpage.md
@@ -3798,8 +3798,8 @@ command execution e.g. `$(cat file)`.
 
 `HOMEBREW_FORBIDDEN_LICENSES`
 
-: A space-separated list of licenses. Homebrew will refuse to install a formula
-  if it or any of its dependencies has a license on this list.
+: A space-separated list of SPDX license identifiers. Homebrew will refuse to
+  install a formula if it or any of its dependencies has a license on this list.
 
 `HOMEBREW_FORBIDDEN_OWNER`
 
diff --git a/manpages/brew.1 b/manpages/brew.1
index 129c37e8308bd..872f971c09d1c 100644
--- a/manpages/brew.1
+++ b/manpages/brew.1
@@ -2470,7 +2470,7 @@ A space\-separated list of casks\. Homebrew will refuse to install a cask if it
 A space\-separated list of formulae\. Homebrew will refuse to install a formula or cask if it or any of its dependencies is on this list\.
 .TP
 \fBHOMEBREW_FORBIDDEN_LICENSES\fP
-A space\-separated list of licenses\. Homebrew will refuse to install a formula if it or any of its dependencies has a license on this list\.
+A space\-separated list of SPDX license identifiers\. Homebrew will refuse to install a formula if it or any of its dependencies has a license on this list\.
 .TP
 \fBHOMEBREW_FORBIDDEN_OWNER\fP
 The person who has set any \fBHOMEBREW_FORBIDDEN_*\fP variables\.