kryptco / homebrew-tap

Other
6 stars 5 forks source link

Formula assumes install location is `/usr/local` #1

Closed daenney closed 7 years ago

daenney commented 7 years ago

I've got my Homebrew installation somewhere else than /usr/local and as a consequence during the installation I get:

Last 15 lines from /Users/daenney/Library/Logs/Homebrew/kr/post_install.05.LaunchAgents:
2017-05-02 13:50:30 +0200

mkdir -p ~/Library/LaunchAgents; cp /usr/local/share/kr/co.krypt.krd.plist ~/Library/LaunchAgents

cp: /usr/local/share/kr/co.krypt.krd.plist: No such file or directory

It shouldn't hardcode the path but use the prefix variable that's available in formulae.

daenney commented 7 years ago

Similarly it inserts the wrong paths in my ~/.ssh/config because of it:

PKCS11Provider /usr/local/lib/kr-pkcs11.so
ProxyCommand /usr/local/bin/krssh %h %p
halostatue commented 7 years ago

This appears to have been mostly fixed, but co.krypt.krd.plist still assumes/usr/local/bin/krd; instead of just copying the plist, the formula should modify the plist with the correct prefix value.

daenney commented 7 years ago

It seems to still be used in the post_install lines, the perl invocations. So the configuration written would also not work as it currently stands.

kcking commented 7 years ago

Nice catch! I just fixed the perl invocations and plist prefix (as a sed invocation for now).

Let me know if you run into anything else.

daenney commented 7 years ago

@kcking I think you've missed one: https://github.com/kryptco/homebrew-tap/blob/master/kr.rb#L63

system "perl -0777 -p -i.bak1 -e 's/\s*#Added by Kryptonite\\nHost \\*\\n\\tPKCS11Provider \\/usr\\/local\\/lib\\/kr-pkcs11.so\\n\\tProxyCommand `find \\/usr\\/local\\/bin\\/krssh 2>\\/dev\\/null \\|\\| which nc` %h %p\\n\\tIdentityFile ~\\/.ssh\\/id_kryptonite\\n\\tIdentityFile ~\\/.ssh\\/id_ed25519\\n\\tIdentityFile ~\\/.ssh\\/id_rsa\\n\\tIdentityFile ~\\/.ssh\\/id_ecdsa\\n\\tIdentityFile ~\\/.ssh\\/id_dsa//g' ~/.ssh/config"
daenney commented 7 years ago

Oh mm, I might be reading the intent of that wrong though.

kcking commented 7 years ago

yes that line is to remove the old ssh_config, so it should stay hard-coded (and will be removed in the future).

daenney commented 7 years ago

Ah yeah, that makes sense. Sorry for the noise and thanks for fixing it!

kcking commented 7 years ago

No problem!