Speedup pass on macOS and brew

[pass-maillinglist at artursterz.de]

Thank you for your review

You are right, your code looks much cleaner and does not contain redundancies. I also think that testing for exit status is better, since this also does not spawn the test process. I created a git diff, which would be nice if it goes upstream.

diff --git a/src/platform/darwin.sh b/src/platform/darwin.sh
index 342ecce..5d0e4de 100644
--- a/src/platform/darwin.sh
+++ b/src/platform/darwin.sh
@@ -43,6 +43,12 @@ qrcode() {
        fi
 }

-GETOPT="$(brew --prefix gnu-getopt 2>/dev/null || { which port &>/dev/null && echo /opt/local; } || echo /usr/local)/bin/getopt"
+if command -v brew >/dev/null; then
+    GETOPT_PREFIX="$(brew --prefix)/opt/gnu-getopt"
+elif command -v port >/dev/null; then
+    GETOPT_PREFIX="/opt/local"
+fi
+GETOPT="${GETOPT_PREFIX:-/usr/local}/bin/getopt"
+
 SHRED="srm -f -z"
 BASE64="openssl base64"

> Am 23.10.2018 um 05:54 schrieb Allan Odgaard <lists+pass at simplit.com>:
> 
> On 22 Oct 2018, at 23:50, pass-maillinglist at artursterz.de <mailto:pass-maillinglist at artursterz.de> wrote:
> 
> currently (and as I can tell also in the past, see [1],[2],[3],[4],[5]), brew —prefix <FORMULA> is really slow (see the example output below), which makes it somewhat annoying using pass on macOS. I hunted this down and found that the problem is the following: brew runs git rev-parse --short=4 --verify -q HEAD every time to find the currently installed version of the formula (which seems somewhat unreasonable). Depending on your git and network connectivity, this may take longer. Using brew —prefix without a formula is reasonably fast. As far as I can tell, there is currently no way to change the prefix path in brew, thus using $(brew —prefix)/opt/gnu-getopt should be a save way for determining the path to gnu-getopt.
> 
> Great find! I’ve been somewhat annoyed myself by the ~1s delay when using pass, but always assumed it was just GPG being slow.
> 
> I patched this and attached the patch to this mail. Since my implementation is somewhat longer, I think it should not be implemented in one line anymore, but split it up as I did (see the patch). If you come up with something better let me know (or, of course, patch it yourself). Additionally, since I am not using Macports, I can not test if this also works with ports. Before considering pushing my patch upstream, someone needs to test this! For brew, it works fine.
> 
> The elif and else branches look wrong, as we are not in a command context, so shouldn’t prefix with echo, plus including the which line is redundant when the elif already test for the presence of port.
> 
> I also find it cleaner to test the exit status of command instead of testing against non-zero output.
> 
> Here’s my revised proposal for the relevant code to setup GETOPT:
> 
> if command -v brew >/dev/null; then
>     GETOPT_PREFIX="$(brew --prefix)/opt/gnu-getopt"
> elif command -v port >/dev/null; then
>     GETOPT_PREFIX="/opt/local"
> fi
> GETOPT="${GETOPT_PREFIX:-/usr/local}/bin/getopt"
> If we find neither brew nor port then we use GETOPT_PREFIX before falling back to /usr/local.
> 
> Don’t think we actually need this, but it makes the code look nicer :)
> 
> _______________________________________________
> Password-Store mailing list
> Password-Store at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/password-store

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/password-store/attachments/20181023/a9905880/attachment-0001.html>