Homebrew-core: gopass formula adds warning to bash completion

[janrito]

These:

output = Utils.popen_read("#{bin}/gopass completion bash")
(bash_completion/"gopass-completion").write output

captures the output of gopass completion bash and prints it into /usr/local/etc/bash_completion.d/gopass-completion

I think that because it's running the command in a limited shell, gopass outputs a warning because it cannot find gpg. So the completion file actually looks like this:

Warning: GPG not found: no gpg binary found
_gopass_bash_autocomplete() {
     local cur opts base
     COMPREPLY=()
     cur="${COMP_WORDS[COMP_CWORD]}"
     opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion )
     local IFS=$'\n'
     COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) )
     return 0
 }

complete -F _gopass_bash_autocomplete gopass

This throws an error later, because Warning is not a valid bash command

[janrito]

@justwatchcom

[DomT4]

Pretty sure this was broken by the new ENV filtering.

diff --git a/Formula/gopass.rb b/Formula/gopass.rb
index 662c8223b4..2cc6001d93 100644
--- a/Formula/gopass.rb
+++ b/Formula/gopass.rb
@@ -24,7 +24,12 @@ class Gopass < Formula
       ENV["PREFIX"] = prefix
       system "make", "install"
     end
+  end

+  def post_install
+    # Checks for GPG executable which isn't available inside
+    # install under Homebrew's ENV filtering.
+    # https://github.com/Homebrew/homebrew-core/issues/21903
     output = Utils.popen_read("#{bin}/gopass completion bash")
     (bash_completion/"gopass-completion").write output
   end

resolves the problem but also doesn't link the completions to the right place, so it's not a particularly viable solution.

[ilovezfs]

@DomT4 yeah I think ENV_FILTERING pretty much broke the gpg requirement altogether.

[ilovezfs]

bash-3.2$ echo $PATH
/usr/local/Homebrew/Library/Homebrew/shims/super:/usr/local/opt/go/bin:/usr/local/opt/gnupg/bin:/usr/bin:/bin:/usr/sbin:/sbin
bash-3.2$ 

vs.

bash-3.2$ echo $PATH
/usr/local/Homebrew/Library/Homebrew/shims/super:/usr/local/opt/go/bin:/usr/bin:/bin:/usr/sbin:/sbin
bash-3.2$ 

[DomT4]

yeah I think ENV_FILTERING pretty much broke the gpg requirement altogether.

Ace. I'll look into it 😓.

[DomT4]

Yeah, Gpg.available? just returns a flat false now, even inside things like brew irb.

[ilovezfs]

I guess technically it's not lying given the outcome.

[DomT4]

Should be easy enough to fix.

[DomT4]

_(He says, optimistically)_

[ilovezfs]

If satisfy returns a pathname, then the parent directory is supposed to get prepended to the PATH, unless the parent is /usr/local/bin, in which case you're SOL.

But satisfy doesn't return a pathname anymore. And even if it did, the non-formulae satisfiers would have a parent of /usr/local/bin.

So … yeah

[DomT4]

diff --git a/Library/Homebrew/gpg.rb b/Library/Homebrew/gpg.rb
index 83b525b44..691e3609e 100644
--- a/Library/Homebrew/gpg.rb
+++ b/Library/Homebrew/gpg.rb
@@ -2,12 +2,14 @@ require "utils"

 class Gpg
   def self.find_gpg(executable)
-    which_all(executable).detect do |gpg|
-      gpg_short_version = Utils.popen_read(gpg, "--version")[/\d\.\d/, 0]
-      next unless gpg_short_version
-      gpg_version = Version.create(gpg_short_version.to_s)
-      @version = gpg_version
-      gpg_version >= Version.create("2.0")
+    with_homebrew_path do
+      which_all(executable).detect do |gpg|
+        gpg_short_version = Utils.popen_read(gpg, "--version")[/\d\.\d/, 0]
+        next unless gpg_short_version
+        gpg_version = Version.create(gpg_short_version.to_s)
+        @version = gpg_version
+        gpg_version >= Version.create("2.0")
+      end
     end
   end

Fixed actually finding it like that locally.

[ilovezfs]

That is kind of weird though since it reinvents the Requirement#which and Requirement#which_all wheel.

[ilovezfs]

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/requirement.rb#L158-L164

[DomT4]

Yeah, it's what I'd politely call a hack 🙃. I'm tempted to submit it as a workaround whilst I look at a broader fix so the immediate problem is resolved, but.

[ilovezfs]

yep. hack > no hack :)

[DomT4]

It fixes this use case in the short-term, although presumably the formula will need rebottling here:

==> Summary
🍺  /usr/local/Cellar/gopass/1.6.5: 7 files, 9.9MB, built in 22 seconds

~> cat /usr/local/Cellar/gopass/1.6.5/etc/bash_completion.d/gopass-completion
_gopass_bash_autocomplete() {
     local cur opts base
     COMPREPLY=()
     cur="${COMP_WORDS[COMP_CWORD]}"
     opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion )
     local IFS=$'\n'
     COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) )
     return 0
 }

complete -F _gopass_bash_autocomplete gopass

[ilovezfs]

Presumably this use case is still broken for non-formulae satisfiers though …

[DomT4]

Ah I see https://github.com/Homebrew/homebrew-core/pull/21945 is pending, so 👍.

non-formulae satisfiers though …

Unsure immediately. Haven't done a massive dive yet. Gpg.available? and Gpg.executable return the correct values in brew irb but I don't know how representative that is.

[ilovezfs]

It might be possible to patch the gopass completion bash so that it works with a full path instead of needing the parent directory to actually be in the PATH but I'm not sure I care whether this particular case works with non-formulae satisfiers anyway.

[DomT4]

Still, if we were going to blow up the GPG Requirement now I'm home for the Christmas holiday is the time to do so 😸. Have more than half a second to look things over for a change.

[ilovezfs]

ilovezfs dreams up schemes to keep DomT4 busy

[DomT4]

I'm adding to my list a bit 🙈. Already have the gem situation and now this as priority projects, heh.

[ilovezfs]

I'm assuming this will take care of itself once one of the brew-side fixes lands.

[DomT4]

Aye, either https://github.com/Homebrew/brew/pull/3603 or https://github.com/Homebrew/brew/pull/3591 will handle this, albeit in different ways.