Re: First attempt at ports: password-store
Am Donnerstag, den 08.01.2015, 16:33 + schrieb Mikolaj Kucharski:
As tree(1) is already run depends, I would stay below with it. Any
reason you are changing it to find(1)?
Yes. The tree in OpenBSD ports is http://spootnik.org/tree/tree-0.61.tgz
by Pierre-Ives Ritschard, the password-store developers depend according
to their documentation on tree = 1.7.0 from Steve Baker
http://mama.indstate.edu/users/ice/tree, whose feature set it much
bigger than the one of the ports version.
The ports version is good enough for the base functionality (display a
fancy directory tree in ascii-art), but it cannot do stuff like pattern
matching. This is the reason, why I did say, that that particular line
needs some consideration whether there might be a more satisfying
solution. Be it in the upstream or the port.
The reason for me to use find(1) is that it is the straight-forward way
to display the expected (good enough) result for pass find foo.
Drawbacks: The format is different:
| Password Store
| | bar/foo
| | baz/foo
instead of the original
| Password Store
| |-- bar
| | `-- foo
| `-- baz
| `-- foo
And because of that of course it breaks the regression test
test/t0400-grep.sh. Although, the output format could graphics
could be magicked to to resemble the original, I do not think
consider it to be worthwhile.
What do you think about below change? Be aware, didn't tested it with
pass(1)
--- files/openbsd.sh Thu Jan 8 12:33:37 2015
+++ files/openbsd.sh Thu Jan 8 16:24:39 2015
@@ -6,7 +6,7 @@
local warn=1
[[ $1 == nowarn ]] warn=0
local template=$PROGRAM.X
- if sysctl kern.usermount | grep -q =1$; then
+ if [ `sysctl -n kern.usermount` == 1 ]; then
SECURE_TMPDIR=$(mktemp -d ${TMPDIR:-/tmp}/$template)
mount -t tmpfs -o -s16M tmpfs $SECURE_TMPDIR || die
Error: could not create tmpfs.
unmount_tmpdir() {
Surely. And still works of course.
As a side note, in OpenBSD
ports, examples are usually placed under:
${PREFIX}/share/examples/${PKGNAME}
Yes. I noticed (and changed) that aready locally. Also that one can
install multiple files/dirs with {foo,bar,baz}.
Scanning other ports, some place more or less the same kinds of files
to doc/$PACKAGE/examples, some place them to examples/$PACKAGE. As
the documentation lists examples/$PACKAGE, I consider this to be the
right place though.
--
David Dahlberg
Fraunhofer FKIE, Dept. Communication Systems (KOM) | Tel: +49-228-9435-845
Fraunhoferstr. 20, 53343 Wachtberg, Germany| Fax: +49-228-856277
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 09:52:31AM +, David Dahlberg wrote: Hi *, this is my first attempt at porting an application (password-store) to OpenBSD. Would you please comment on whether it is usable and/or where and how to improve it? Cheers David Hi David, the Makefile is missing $OpenBSD$ in the first line, for CVS. I wouldn't override PKGNAME for this and let it default to DISTNAME so the package is called 'password-store-1.6.3' instead of 'pass-1.6.3' I think the longer name is more informative and has less potential for ambiguity. We usually split *_DEPENDS list items onto separate lines: BUILD_DEPENDS = archivers/xz \ devel/gmake instead of BUILD_DEPENDS = devel/gmake archivers/xz The lines should also be sorted alphabetically. Same for RUN_DEPENDS. archivers/xz seems to be added to BUILD_DEPENDS by default with EXTRACT_SUFX = .tar.xz so you can probably drop it from BUILD_DEPENDS in your Makefile. Have you tried talking to upstream about using more portable command line flags in their Makefile so you can drop the -v flag patches in a future update? Perhaps they could at least put in some abstraction to disable these flags (e.g. with 'make QUIET=yes' or 'make V=0' don't use -v options at all). Otherwise this looks good to me.
Re: First attempt at ports: password-store
On 2015/01/08 13:17, Stefan Sperling wrote: On Thu, Jan 08, 2015 at 09:52:31AM +, David Dahlberg wrote: Hi *, this is my first attempt at porting an application (password-store) to OpenBSD. Would you please comment on whether it is usable and/or where and how to improve it? Cheers David Hi David, the Makefile is missing $OpenBSD$ in the first line, for CVS. I wouldn't override PKGNAME for this and let it default to DISTNAME so the package is called 'password-store-1.6.3' instead of 'pass-1.6.3' I think the longer name is more informative and has less potential for ambiguity. We usually split *_DEPENDS list items onto separate lines: BUILD_DEPENDS = archivers/xz \ devel/gmake instead of BUILD_DEPENDS = devel/gmake archivers/xz The lines should also be sorted alphabetically. Same for RUN_DEPENDS. archivers/xz seems to be added to BUILD_DEPENDS by default with EXTRACT_SUFX = .tar.xz so you can probably drop it from BUILD_DEPENDS in your Makefile. Same for gmake with USE_GMAKE=Yes, so I don't think there should be any BUILD_DEPENDS. Check for whitespace nits as well, mixed tabs/spaces etc.
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 01:17:31PM +0100, Stefan Sperling wrote:
BUILD_DEPENDS = devel/gmake archivers/xz
Also, I see you have USE_GMAKE=Yes, so I don't think you need to build
depends on devel/gmake. That dependency will be added automatically by
bsd.port.mk(5).
I also concur what Stefan wrote about package name. I would leave it as
password-store.
Looking at README file in password-store's git repo, it looks like
content for pkg/DESCR and you have already good description for it, so I
don't think there is a need for post-install part at all, as I wouldn't
include license file too.
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Please, use ${FILESDIR}/openbsd.sh instead of files/openbsd.sh to copy
the file to target location.
--
best regards
q#
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 09:52:31AM +, David Dahlberg wrote: Hi *, this is my first attempt at porting an application (password-store) to OpenBSD. Would you please comment on whether it is usable and/or where and how to improve it? I didn't look at the distfile of password-store, but only to upstream's git repo. You may considering to include the completion files for bash and zsh. I don't think we have fish in ports. However completion files can wait for another update if you wish. Did you consider to take maintainership of the port, while you are submitting it? -- best regards q#
Re: First attempt at ports: password-store
On 2015/01/08 12:38, Mikolaj Kucharski wrote:
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Not for post-extract, ${INSTALL_xx} sets permissions/ownership which is
not wanted in extract.
Manpages are installed to the wrong place.
Looking at the script ...
http://git.zx2c4.com/password-store/tree/src/password-store.sh
(only briefly, I can't handle reading 160-column-wide shell scripts for very
long ;)
#!/usr/bin/env bash
should be patched to use the explicit path (which will need to use ${LOCALBASE}
and SUBST_CMD, or some other method of substituting, rather than hardcoding
/usr/local).
local
passfile_temp=${passfile}.tmp.${RANDOM}.${RANDOM}.${RANDOM}.${RANDOM}.--
...blech.
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 12:56:30PM +, Stuart Henderson wrote:
On 2015/01/08 12:38, Mikolaj Kucharski wrote:
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Not for post-extract, ${INSTALL_xx} sets permissions/ownership which is
not wanted in extract.
Manpages are installed to the wrong place.
Looking at the script ...
http://git.zx2c4.com/password-store/tree/src/password-store.sh
(only briefly, I can't handle reading 160-column-wide shell scripts for very
long ;)
#!/usr/bin/env bash
should be patched to use the explicit path (which will need to use
${LOCALBASE}
Out of curiosity, why?
--
Antoine
Re: First attempt at ports: password-store
Am Donnerstag, den 08.01.2015, 13:17 +0100 schrieb Stefan Sperling: [some remarks] [x] changed it. Have you tried talking to upstream about using more portable command line flags in their Makefile so you can drop the -v flag patches in a future update? Yes, I am in conversation with him. Yet I keep on finding new things in the code paths that are seldom used by me ;-) Perhaps they could at least put in some abstraction to disable these flags (e.g. with 'make QUIET=yes' or 'make V=0' don't use -v options at all). It is not only the Makefile, you'll find this in the base shell script as well. For some command there already anchors for platform specific substitutions (i.e. getopt and shred). Actually, the verbose and colour switches do not give me much headaches. There are multiple possibilities to address this. Worse is the usage tree. For now, I had to substitute it somehow with find | grep which of course is not as fancy and breaks the unit-test. -dd -- David Dahlberg Fraunhofer FKIE, Dept. Communication Systems (KOM) | Tel: +49-228-9435-845 Fraunhoferstr. 20, 53343 Wachtberg, Germany| Fax: +49-228-856277
Re: First attempt at ports: password-store
Am Donnerstag, den 08.01.2015, 12:38 + schrieb Mikolaj Kucharski:
Looking at README file in password-store's git repo, it looks like
content for pkg/DESCR and you have already good description for it, so I
don't think there is a need for post-install part at all, as I wouldn't
include license file too.
IMHO the point is: Is it required to ship the licence file with the
package? What would the bare-footed guy say?
If documenting the licence is not required, the whole directory is
unnecessary, agreed.
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Please, use ${FILESDIR}/openbsd.sh instead of files/openbsd.sh to copy
the file to target location.
Indeed. Thanks for pointing me at it.
--
David Dahlberg
Fraunhofer FKIE, Dept. Communication Systems (KOM) | Tel: +49-228-9435-845
Fraunhoferstr. 20, 53343 Wachtberg, Germany| Fax: +49-228-856277
Re: First attempt at ports: password-store
On 2015/01/08 13:58, Antoine Jacoutot wrote:
On Thu, Jan 08, 2015 at 12:56:30PM +, Stuart Henderson wrote:
On 2015/01/08 12:38, Mikolaj Kucharski wrote:
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Not for post-extract, ${INSTALL_xx} sets permissions/ownership which is
not wanted in extract.
Manpages are installed to the wrong place.
Looking at the script ...
http://git.zx2c4.com/password-store/tree/src/password-store.sh
(only briefly, I can't handle reading 160-column-wide shell scripts for
very long ;)
#!/usr/bin/env bash
should be patched to use the explicit path (which will need to use
${LOCALBASE}
Out of curiosity, why?
In case people have stupid PATHs..
Re: First attempt at ports: password-store
Out of curiosity, why? In case people have stupid PATHs.. Then you can start to fix the ports tree for constructs like: #!/usr/bin/env perl (and probably others) IMHO it's not worth patching for that. (#!/bin/bash is another matter of course) -- Antoine
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 01:11:59PM +, Dahlberg, David wrote:
In post-extract I would use ${INSTALL_SCRIPT} or ${INSTALL_DATA} instead
of cp(1), depending should openbsd.sh have executable bit set or not.
Please, use ${FILESDIR}/openbsd.sh instead of files/openbsd.sh to copy
the file to target location.
Indeed. Thanks for pointing me at it.
Please ignore ${INSTALL_xxx} part above, per Stuart Henderson's comments
in other email. Using cp(1) should be fine while copying into ${WRKDIST}
--
best regards
q#
Re: First attempt at ports: password-store
Am Donnerstag, den 08.01.2015, 12:48 + schrieb Mikolaj Kucharski: I didn't look at the distfile of password-store, but only to upstream's git repo. You may considering to include the completion files for bash and zsh. For now, I packaged those scripts to /usr/local/share/doc/password-store/examples/completion. If there is a *-completion infrastructure, such as /etc/bash_completion.d in Debian, I have not found it yet, not even with the shells/bash PLIST. But if there is a way, please do not hesitate to point me at it. Did you consider to take maintainership of the port, while you are submitting it? If you insist, of course I will do ;-) Attached is a new version, which addresses all (non-controversial) remarks. As for Linux and OSX, the OpenBSD-specific script is now also able to edit temporarily decypted passwords in a ramdisk instead of regular /tmp. -dd -- David Dahlberg Fraunhofer FKIE, Dept. Communication Systems (KOM) | Tel: +49-228-9435-845 Fraunhoferstr. 20, 53343 Wachtberg, Germany| Fax: +49-228-856277 password-store.tar.gz Description: password-store.tar.gz
Re: First attempt at ports: password-store
On Thu, Jan 08, 2015 at 03:44:40PM +, David Dahlberg wrote:
Attached is a new version, which addresses all (non-controversial)
remarks. As for Linux and OSX, the OpenBSD-specific script is now also
able to edit temporarily decypted passwords in a ramdisk instead of
regular /tmp.
Empty line in Makefile after $OpenBSD$ keyword.
As tree(1) is already run depends, I would stay below with it. Any
reason you are changing it to find(1)?
@@ -343,8 +343,8 @@ cmd_show() {
cmd_find() {
[[ -z $@ ]] die Usage: $PROGRAM $COMMAND pass-names...
IFS=, eval 'echo Search Terms: $*'
- local terms=*$(printf '%s*|*' $@)
- tree -C -l --noreport -P ${terms%|*} --prune --matchdirs
--ignore-case $PREFIX | tail -n +2 | sed 's/\.gpg$//'
+local esc_prefix=`echo $PREFIX | sed 's#/#\\\/#g'`
+find $PREFIX -name *$@* | sed -e s/^$esc_prefix\//\| / -e
's/\.gpg$//'
}
cmd_grep() {
@@ -352,7 +352,7 @@ cmd_grep() {
What do you think about below change? Be aware, didn't tested it with
pass(1)
--- files/openbsd.shThu Jan 8 12:33:37 2015
+++ files/openbsd.shThu Jan 8 16:24:39 2015
@@ -6,7 +6,7 @@
local warn=1
[[ $1 == nowarn ]] warn=0
local template=$PROGRAM.X
- if sysctl kern.usermount | grep -q =1$; then
+ if [ `sysctl -n kern.usermount` == 1 ]; then
SECURE_TMPDIR=$(mktemp -d ${TMPDIR:-/tmp}/$template)
mount -t tmpfs -o -s16M tmpfs $SECURE_TMPDIR || die Error:
could not create tmpfs.
unmount_tmpdir() {
In general, your port reads fine. I'm not sure where would be better
place to put completion and other files. As a side note, in OpenBSD
ports, examples are usually placed under:
${PREFIX}/share/examples/${PKGNAME}
However, not sure is that applicable for this port. I will let othes
comment on this.
--
best regards
q#
