* [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
[not found] <20100927113752.CF4CE20051@flycatcher.gentoo.org>
@ 2010-10-01 13:07 ` Peter Volkov
2010-10-01 13:34 ` Thomas Sachau
` (2 more replies)
0 siblings, 3 replies; 37+ messages in thread
From: Peter Volkov @ 2010-10-01 13:07 UTC (permalink / raw
To: gentoo-dev, djc
В Пнд, 27/09/2010 в 11:37 +0000, Dirkjan Ochtman (djc) пишет:
> src_prepare() {
> epatch "${FILESDIR}/${PN}-2.1_rc13-peercred.patch"
> epatch "${FILESDIR}/${PN}-2.1_rc20-pkcs11.patch"
> use ipv6 && epatch "${WORKDIR}/${PN}-2.1.1-ipv6-${IPV6_VERSION}.patch"
> use eurephia && epatch "${DISTDIR}/${PN}-2.1.0_eurephia.patch"
> sed -i \
> -e "s/gcc \${CC_FLAGS}/\${CC} \${CFLAGS} -Wall/" \
> -e "s/-shared/-shared \${LDFLAGS}/" \
> plugin/*/Makefile || die "sed failed"
> eautoreconf
Is autoreconf really necessary here? Looks like it is redundant in case
ipv6 or eurephia are disabled.
> src_configure() {
> econf ${myconf} \
> $(use_enable passwordsave password-save) \
> $(use_enable ssl) \
> $(use_enable ssl crypto) \
> $(use_enable iproute2) \
> || die "configure failed"
|| die is redundant here.
> src_compile() {
> use static && sed -i -e '/^LIBS/s/LIBS = /LIBS = -static /' Makefile
>
> emake || die "make failed"
>
> if ! use minimal ; then
> cd plugin
> for i in $( ls 2>/dev/null ); do
This is bad construction:
http://mywiki.wooledge.org/BashPitfalls#for_i_in_.24.28ls_.2A.mp3.29
> [[ ${i} == "README" || ${i} == "examples" || ${i} == "defer" ]] && continue
> [[ ${i} == "auth-pam" ]] && ! use pam && continue
> einfo "Building ${i} plugin"
> cd "${i}"
> emake CC=$(tc-getCC) || die "make failed"
> cd ..
I think pushd/popd are better suited for this then cd "${dir}" / cd ..
> done
> cd ..
> fi
> }
> # remove empty dir
> rmdir "${D}/usr/share/doc/openvpn"
|| die is prudent here.
> # Empty dir
> dodir /etc/openvpn
> keepdir /etc/openvpn
dodir is redundant: keepdir will create directory.
> # Install some helper scripts
> exeinto /etc/openvpn
> doexe "${FILESDIR}/up.sh"
> doexe "${FILESDIR}/down.sh"
|| die after doexe is really good idea.
> # Install the init script and config file
> newinitd "${FILESDIR}/${PN}-2.1.init" openvpn
> newconfd "${FILESDIR}/${PN}-2.1.conf" openvpn
|| die absent
> # install examples, controlled by the respective useflag
> if use examples ; then
> # dodoc does not supportly support directory traversal, #15193
> insinto /usr/share/doc/${PF}/examples
> doins -r sample-{config-files,keys,scripts} contrib
> prepalldocs
> fi
> # Install plugins and easy-rsa
> if ! use minimal ; then
> cd easy-rsa/2.0
> make install "DESTDIR=${D}/usr/share/${PN}/easy-rsa"
> cd ../..
>
> exeinto "/usr/$(get_libdir)/${PN}"
> doexe plugin/*/*.so
> fi
> }
>
> pkg_postinst() {
> # Add openvpn user so openvpn servers can drop privs
> # Clients should run as root so they can change ip addresses,
> # dns information and other such things.
> enewgroup openvpn
> enewuser openvpn "" "" "" openvpn
>
> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ; then
I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably there
are better alternatives. Also ${ROOT} is missed here.
> ewarn "WARNING: The openvpn init script has changed"
> ewarn ""
> fi
>
> einfo "The openvpn init script expects to find the configuration file"
> einfo "openvpn.conf in /etc/openvpn along with any extra files it may need."
This information is for users, so, please, use elog here.
> einfo ""
> einfo "To create more VPNs, simply create a new .conf file for it and"
> einfo "then create a symlink to the openvpn init script from a link called"
> einfo "openvpn.newconfname - like so"
> einfo " cd /etc/openvpn"
> einfo " ${EDITOR##*/} foo.conf"
> einfo " cd /etc/init.d"
> einfo " ln -s openvpn openvpn.foo"
> einfo ""
> einfo "You can then treat openvpn.foo as any other service, so you can"
> einfo "stop one vpn and start another if you need to."
>
> if grep -Eq "^[ \t]*(up|down)[ \t].*" "${ROOT}/etc/openvpn"/*.conf 2>/dev/null ; then
> ewarn ""
> ewarn "WARNING: If you use the remote keyword then you are deemed to be"
> ewarn "a client by our init script and as such we force up,down scripts."
> ewarn "These scripts call /etc/openvpn/\$SVCNAME-{up,down}.sh where you"
> ewarn "can move your scripts to."
> fi
>
> if ! use minimal ; then
> einfo ""
> einfo "plugins have been installed into /usr/$(get_libdir)/${PN}"
> fi
>
> if use ipv6 ; then
> einfo ""
> einfo "This build contains IPv6-Patch from JuanJo Ciarlante."
> einfo "For more information please visit:"
> einfo "http://github.com/jjo/openvpn-ipv6"
> fi
>
> if use eurephia ; then
> einfo ""
> einfo "This build contains eurephia patch."
> einfo "For more information please visit:"
> einfo "http://www.eurephia.net/"
> fi
> }
--
Peter.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-01 13:07 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild Peter Volkov
@ 2010-10-01 13:34 ` Thomas Sachau
2010-10-01 15:17 ` Nirbheek Chauhan
2010-10-12 20:09 ` Dirkjan Ochtman
2 siblings, 0 replies; 37+ messages in thread
From: Thomas Sachau @ 2010-10-01 13:34 UTC (permalink / raw
To: gentoo-dev; +Cc: Peter Volkov, djc
[-- Attachment #1: Type: text/plain, Size: 627 bytes --]
Am 01.10.2010 15:07, schrieb Peter Volkov:
> В Пнд, 27/09/2010 в 11:37 +0000, Dirkjan Ochtman (djc) пишет:
>
>> [[ ${i} == "README" || ${i} == "examples" || ${i} == "defer" ]] && continue
>> [[ ${i} == "auth-pam" ]] && ! use pam && continue
>> einfo "Building ${i} plugin"
>> cd "${i}"
>> emake CC=$(tc-getCC) || die "make failed"
>> cd ..
>
> I think pushd/popd are better suited for this then cd "${dir}" / cd ..
>
You can make it even shorter by using the -C switch of (e)make, so you dont have to manually
enter/exit the dir.
--
Thomas Sachau
Gentoo Linux Developer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-01 13:07 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild Peter Volkov
2010-10-01 13:34 ` Thomas Sachau
@ 2010-10-01 15:17 ` Nirbheek Chauhan
2010-10-02 3:38 ` Ryan Hill
2010-10-05 4:20 ` Jeroen Roovers
2010-10-12 20:09 ` Dirkjan Ochtman
2 siblings, 2 replies; 37+ messages in thread
From: Nirbheek Chauhan @ 2010-10-01 15:17 UTC (permalink / raw
To: gentoo-dev; +Cc: djc
On Fri, Oct 1, 2010 at 6:37 PM, Peter Volkov <pva@gentoo.org> wrote:
> В Пнд, 27/09/2010 в 11:37 +0000, Dirkjan Ochtman (djc) пишет:
>> src_compile() {
>> use static && sed -i -e '/^LIBS/s/LIBS = /LIBS = -static /' Makefile
>>
>> emake || die "make failed"
>>
>> if ! use minimal ; then
>> cd plugin
>> for i in $( ls 2>/dev/null ); do
>
> This is bad construction:
> http://mywiki.wooledge.org/BashPitfalls#for_i_in_.24.28ls_.2A.mp3.29
>
A nice way around this is to do the following:
ls -1 | while read i; do
`read` delimits on newline, so you're safe.
--
~Nirbheek Chauhan
Gentoo GNOME+Mozilla Team
^ permalink raw reply [flat|nested] 37+ messages in thread
* [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-01 15:17 ` Nirbheek Chauhan
@ 2010-10-02 3:38 ` Ryan Hill
2010-10-02 12:32 ` Nirbheek Chauhan
2010-10-05 4:20 ` Jeroen Roovers
1 sibling, 1 reply; 37+ messages in thread
From: Ryan Hill @ 2010-10-02 3:38 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]
On Fri, 1 Oct 2010 20:47:38 +0530
Nirbheek Chauhan <nirbheek@gentoo.org> wrote:
> On Fri, Oct 1, 2010 at 6:37 PM, Peter Volkov <pva@gentoo.org> wrote:
> > В Пнд, 27/09/2010 в 11:37 +0000, Dirkjan Ochtman (djc) пишет:
> >> src_compile() {
> >> use static && sed -i -e '/^LIBS/s/LIBS = /LIBS = -static /' Makefile
> >>
> >> emake || die "make failed"
> >>
> >> if ! use minimal ; then
> >> cd plugin
> >> for i in $( ls 2>/dev/null ); do
> >
> > This is bad construction:
> > http://mywiki.wooledge.org/BashPitfalls#for_i_in_.24.28ls_.2A.mp3.29
> >
>
> A nice way around this is to do the following:
>
> ls -1 | while read i; do
>
> `read` delimits on newline, so you're safe.
This strips leading and trailing whitespace.
$ touch " test1"
$ touch "test2 "
$ touch "test 3"
$ touch " test 4 "
$ ls -1 | while read i; do echo "###${i}###"; done
###test2###
###test 3###
###test1###
###test 4###
instead use:
$ ls -1 | while IFS= read i; do echo "###${i}###"; done
###test2 ###
###test 3###
### test1###
### test 4 ###
or recursively:
$ find . -type f -print0 \
| while IFS= read -d $'\0' i; do echo "###${i}###"; done
###./ test1###
###./ test 4 ###
###./test 3###
###./test2 ###
or just make things simple on yourself :)
$ for i in *; do echo "###${i}###"; done
### test1###
###test2 ###
###test 3###
### test 4 ###
--
fonts, gcc-porting, we hold our breath, we spin around the world
toolchain, wxwidgets you and me cling to the outside of the earth
@ gentoo.org EFFD 380E 047A 4B51 D2BD C64F 8AA8 8346 F9A4 0662
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-02 3:38 ` Ryan Hill
@ 2010-10-02 12:32 ` Nirbheek Chauhan
0 siblings, 0 replies; 37+ messages in thread
From: Nirbheek Chauhan @ 2010-10-02 12:32 UTC (permalink / raw
To: gentoo-dev
On Sat, Oct 2, 2010 at 9:08 AM, Ryan Hill <dirtyepic@gentoo.org> wrote:
> On Fri, 1 Oct 2010 20:47:38 +0530
> Nirbheek Chauhan <nirbheek@gentoo.org> wrote:
>>
>> A nice way around this is to do the following:
>>
>> ls -1 | while read i; do
>>
>> `read` delimits on newline, so you're safe.
>
> This strips leading and trailing whitespace.
>
> $ touch " test1"
> $ touch "test2 "
> $ touch "test 3"
> $ touch " test 4 "
>
> $ ls -1 | while read i; do echo "###${i}###"; done
> ###test2###
> ###test 3###
> ###test1###
> ###test 4###
>
Damn, I never knew that.
> instead use:
>
> $ ls -1 | while IFS= read i; do echo "###${i}###"; done
> ###test2 ###
> ###test 3###
> ### test1###
> ### test 4 ###
>
> or recursively:
>
> $ find . -type f -print0 \
> | while IFS= read -d $'\0' i; do echo "###${i}###"; done
> ###./ test1###
> ###./ test 4 ###
> ###./test 3###
> ###./test2 ###
>
> or just make things simple on yourself :)
>
> $ for i in *; do echo "###${i}###"; done
> ### test1###
> ###test2 ###
> ###test 3###
> ### test 4 ###
>
I wonder how many scripts I wrote now have this subtle bug. Thanks!
--
~Nirbheek Chauhan
Gentoo GNOME+Mozilla Team
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-01 15:17 ` Nirbheek Chauhan
2010-10-02 3:38 ` Ryan Hill
@ 2010-10-05 4:20 ` Jeroen Roovers
1 sibling, 0 replies; 37+ messages in thread
From: Jeroen Roovers @ 2010-10-05 4:20 UTC (permalink / raw
To: gentoo-dev
On Fri, 1 Oct 2010 20:47:38 +0530
Nirbheek Chauhan <nirbheek@gentoo.org> wrote:
> >> for i in $( ls 2>/dev/null ); do
[...]
> A nice way around this is to do the following:
>
> ls -1 | while read i; do
What pva carelessly omitted to point out was that using ls(1) is
incredibly bad bash programming, which is probably why stderr is being
redirected in this case, too. :)
jer
^ permalink raw reply [flat|nested] 37+ messages in thread
* [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-01 13:07 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild Peter Volkov
2010-10-01 13:34 ` Thomas Sachau
2010-10-01 15:17 ` Nirbheek Chauhan
@ 2010-10-12 20:09 ` Dirkjan Ochtman
2010-10-12 20:17 ` Mike Frysinger
` (2 more replies)
2 siblings, 3 replies; 37+ messages in thread
From: Dirkjan Ochtman @ 2010-10-12 20:09 UTC (permalink / raw
To: Peter Volkov; +Cc: gentoo-dev
On Fri, Oct 1, 2010 at 15:07, Peter Volkov <pva@gentoo.org> wrote:
> [a very thorough review of the openvpn ebuild]
Thanks for reviewing, I've fixed most of the issues.
>> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ; then
>
> I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably there
> are better alternatives. Also ${ROOT} is missed here.
I've put ${ROOT} in, are there no better alternatives? I don't think
anyone mentioned any.
Cheers,
Dirkjan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-12 20:09 ` Dirkjan Ochtman
@ 2010-10-12 20:17 ` Mike Frysinger
2010-10-12 20:26 ` Jeroen Roovers
2010-10-17 21:35 ` Mike Frysinger
2 siblings, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-12 20:17 UTC (permalink / raw
To: gentoo-dev; +Cc: Dirkjan Ochtman, Peter Volkov
[-- Attachment #1: Type: Text/Plain, Size: 725 bytes --]
On Tuesday, October 12, 2010 16:09:06 Dirkjan Ochtman wrote:
> On Fri, Oct 1, 2010 at 15:07, Peter Volkov wrote:
> >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ; then
> >
> > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably there
> > are better alternatives. Also ${ROOT} is missed here.
>
> I've put ${ROOT} in, are there no better alternatives? I don't think
> anyone mentioned any.
[ -e ] will break with multiple files (which is the whole point of the glob ?)
you could use find, but that doesnt really seem any better than the ls (which
btw will spill errors if no local.conf exists):
find "${ROOT}"/etc/openvpn/*/ -maxdepth 1 -name local.conf >/dev/null
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-12 20:09 ` Dirkjan Ochtman
2010-10-12 20:17 ` Mike Frysinger
@ 2010-10-12 20:26 ` Jeroen Roovers
2010-10-12 20:57 ` Mike Frysinger
2010-10-17 21:35 ` Mike Frysinger
2 siblings, 1 reply; 37+ messages in thread
From: Jeroen Roovers @ 2010-10-12 20:26 UTC (permalink / raw
To: gentoo-dev
On Tue, 12 Oct 2010 22:09:06 +0200
Dirkjan Ochtman <djc@gentoo.org> wrote:
> On Fri, Oct 1, 2010 at 15:07, Peter Volkov <pva@gentoo.org> wrote:
> > [a very thorough review of the openvpn ebuild]
>
> Thanks for reviewing, I've fixed most of the issues.
>
> >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ;
> >> then
> >
> > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably
> > there are better alternatives. Also ${ROOT} is missed here.
>
> I've put ${ROOT} in, are there no better alternatives? I don't think
> anyone mentioned any.
for foo in ${ROOT}/etc/openvpn/*/local.conf; do
[ -e ${foo} ] && bar ${foo}
done
If no ${ROOT}/etc/openvpn/*/local.conf is found, it returns the exact
string; which doesn't exist so Nothing Happens.
jer
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-12 20:26 ` Jeroen Roovers
@ 2010-10-12 20:57 ` Mike Frysinger
2010-10-13 8:23 ` Amadeusz Żołnowski
0 siblings, 1 reply; 37+ messages in thread
From: Mike Frysinger @ 2010-10-12 20:57 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 990 bytes --]
On Tuesday, October 12, 2010 16:26:31 Jeroen Roovers wrote:
> On Tue, 12 Oct 2010 22:09:06 +0200 Dirkjan Ochtman wrote:
> > On Fri, Oct 1, 2010 at 15:07, Peter Volkov wrote:
> > > [a very thorough review of the openvpn ebuild]
> >
> > Thanks for reviewing, I've fixed most of the issues.
> >
> > >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ;
> > >>
> > >> then
> > >
> > > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably
> > > there are better alternatives. Also ${ROOT} is missed here.
> >
> > I've put ${ROOT} in, are there no better alternatives? I don't think
> > anyone mentioned any.
>
> for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> [ -e ${foo} ] && bar ${foo}
> done
>
> If no ${ROOT}/etc/openvpn/*/local.conf is found, it returns the exact
> string; which doesn't exist so Nothing Happens.
i'd say doing a loop is worse than a `ls` hack. and this has quoting
problems, but that's ancillary ...
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
[not found] ` <fCUgF-1kl-15@gated-at.bofh.it>
@ 2010-10-13 7:08 ` Vaeth
2010-10-13 14:15 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Vaeth @ 2010-10-13 7:08 UTC (permalink / raw
To: gentoo-dev
Mike Frysinger wrote:
> > for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> > [ -e ${foo} ] && bar ${foo}
> > done
>
> i'd say doing a loop is worse than a `ls` hack.
Why do you think so? No external program on which you must rely,
and if you put a "break" in there, the loop is just syntactical.
Here is another possibility if you do not need positional
parameters anymore:
set -- "${ROOT}"/etc/openvpn/*/local.conf
if test -e "${1}"
then ...
fi
And if you do need, you can write a function:
Test2() {
test "${1}" "${2}"
}
if Test2 -e "${ROOT}"/etc/openvpn/*/local.conf
then ...
fi
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-12 20:57 ` Mike Frysinger
@ 2010-10-13 8:23 ` Amadeusz Żołnowski
2010-10-13 14:13 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 8:23 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]
Excerpts from Mike Frysinger's message of Tue Oct 12 22:57:11 +0200 2010:
> On Tuesday, October 12, 2010 16:26:31 Jeroen Roovers wrote:
> > On Tue, 12 Oct 2010 22:09:06 +0200 Dirkjan Ochtman wrote:
> > > On Fri, Oct 1, 2010 at 15:07, Peter Volkov wrote:
> > > > [a very thorough review of the openvpn ebuild]
> > >
> > > Thanks for reviewing, I've fixed most of the issues.
> > >
> > > >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ;
> > > >>
> > > >> then
> > > >
> > > > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably
> > > > there are better alternatives. Also ${ROOT} is missed here.
> > >
> > > I've put ${ROOT} in, are there no better alternatives? I don't think
> > > anyone mentioned any.
> >
> > for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> > [ -e ${foo} ] && bar ${foo}
> > done
> >
> > If no ${ROOT}/etc/openvpn/*/local.conf is found, it returns the exact
> > string; which doesn't exist so Nothing Happens.
>
> i'd say doing a loop is worse than a `ls` hack. and this has quoting
> problems, but that's ancillary ...
> -mike
What about defining following function?
any_exists() {
local f
for f; do
[[ -e $f ]] && return 0
done
return 1
}
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 8:23 ` Amadeusz Żołnowski
@ 2010-10-13 14:13 ` Mike Frysinger
2010-10-13 16:36 ` Amadeusz Żołnowski
2010-10-13 16:41 ` Michał Górny
0 siblings, 2 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 14:13 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 1508 bytes --]
On Wednesday, October 13, 2010 04:23:16 Amadeusz Żołnowski wrote:
> Excerpts from Mike Frysinger's message of Tue Oct 12 22:57:11 +0200 2010:
> > On Tuesday, October 12, 2010 16:26:31 Jeroen Roovers wrote:
> > > On Tue, 12 Oct 2010 22:09:06 +0200 Dirkjan Ochtman wrote:
> > > > On Fri, Oct 1, 2010 at 15:07, Peter Volkov wrote:
> > > > > [a very thorough review of the openvpn ebuild]
> > > >
> > > > Thanks for reviewing, I've fixed most of the issues.
> > > >
> > > > >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ;
> > > > >>
> > > > >> then
> > > > >
> > > > > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably
> > > > > there are better alternatives. Also ${ROOT} is missed here.
> > > >
> > > > I've put ${ROOT} in, are there no better alternatives? I don't think
> > > > anyone mentioned any.
> > >
> > > for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> > >
> > > [ -e ${foo} ] && bar ${foo}
> > >
> > > done
> > >
> > > If no ${ROOT}/etc/openvpn/*/local.conf is found, it returns the exact
> > > string; which doesn't exist so Nothing Happens.
> >
> > i'd say doing a loop is worse than a `ls` hack. and this has quoting
> > problems, but that's ancillary ...
> What about defining following function?
>
> any_exists() {
> local f
>
> for f; do
> [[ -e $f ]] && return 0
> done
>
> return 1
> }
perhaps if it had a better name and were in a common location (eclass)
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 7:08 ` Vaeth
@ 2010-10-13 14:15 ` Mike Frysinger
0 siblings, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 14:15 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 954 bytes --]
On Wednesday, October 13, 2010 03:08:24 Vaeth wrote:
> Mike Frysinger wrote:
> > > for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> > >
> > > [ -e ${foo} ] && bar ${foo}
> > >
> > > done
> >
> > i'd say doing a loop is worse than a `ls` hack.
>
> Why do you think so? No external program on which you must rely,
> and if you put a "break" in there, the loop is just syntactical.
> Here is another possibility if you do not need positional
> parameters anymore:
>
> set -- "${ROOT}"/etc/openvpn/*/local.conf
> if test -e "${1}"
> then ...
> fi
>
> And if you do need, you can write a function:
>
> Test2() {
> test "${1}" "${2}"
> }
>
> if Test2 -e "${ROOT}"/etc/openvpn/*/local.conf
> then ...
> fi
relying on an external program to get a single line of readable code is better
than multiple lines of code that attempt to do the same thing. your further
examples here are even worse on many levels.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
[not found] ` <fDbhw-4Y2-5@gated-at.bofh.it>
@ 2010-10-13 16:35 ` Vaeth
2010-10-13 18:07 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Vaeth @ 2010-10-13 16:35 UTC (permalink / raw
To: gentoo-dev
Mike Frysinger wrote:
> relying on an external program
The point is that external programs can have all sorts of undesired
side effects. For instance, if an directory is not readable (or
readable but not executable or is on $FILESYSTEM with who-knows-what
permissions or accessability problems) it can cause unexpected
errors which otherwise are treated by "standard" shell behavior:
*This* is why the ls is hackish here, not the readability.
About the readability, one can always have different opinions,
but your attack is inappropriate:
> to get a single line of readable code is better than
> multiple lines of code that attempt to do the same thing.
First, my suggestions are not "multiple lines" but only
exactly *two* lines (i.e. only one additional line):
Please compare
set -- "${ROOT}"/etc/openvpn/*/local.conf
if test -e "${1}"
with the "single line of readable code" which contains not less code,
but is only squashed into one line:
if [[ -e $(ls -1 -- "${ROOT}"/etc/openvpn/*/local.conf) ]]
Is this really so much more readable?
(The "--" can be omitted in both code pieces iff portage has a test
that ROOT does not start with "-").
And if you really want only readability, you should like even much
more my second suggestion which could also be squashed as two lines:
Exists() { test -e "${1}"; }
if Exists "${ROOT}"/etc/openvpn/*/local.conf
I would say this is *way* more readable than the complex 1-liner.
Of course, opinions may differ, but I think an unfounded attack as
> your further examples here are even worse on many levels.
is not appropriate here.
Regards
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 14:13 ` Mike Frysinger
@ 2010-10-13 16:36 ` Amadeusz Żołnowski
2010-10-13 16:41 ` Michał Górny
1 sibling, 0 replies; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 16:36 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]
Excerpts from Mike Frysinger's message of Wed Oct 13 16:13:58 +0200 2010:
> On Wednesday, October 13, 2010 04:23:16 Amadeusz Żołnowski wrote:
> > Excerpts from Mike Frysinger's message of Tue Oct 12 22:57:11 +0200 2010:
> > > On Tuesday, October 12, 2010 16:26:31 Jeroen Roovers wrote:
> > > > On Tue, 12 Oct 2010 22:09:06 +0200 Dirkjan Ochtman wrote:
> > > > > On Fri, Oct 1, 2010 at 15:07, Peter Volkov wrote:
> > > > > > [a very thorough review of the openvpn ebuild]
> > > > >
> > > > > Thanks for reviewing, I've fixed most of the issues.
> > > > >
> > > > > >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ;
> > > > > >>
> > > > > >> then
> > > > > >
> > > > > > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably
> > > > > > there are better alternatives. Also ${ROOT} is missed here.
> > > > >
> > > > > I've put ${ROOT} in, are there no better alternatives? I don't think
> > > > > anyone mentioned any.
> > > >
> > > > for foo in ${ROOT}/etc/openvpn/*/local.conf; do
> > > >
> > > > [ -e ${foo} ] && bar ${foo}
> > > >
> > > > done
> > > >
> > > > If no ${ROOT}/etc/openvpn/*/local.conf is found, it returns the exact
> > > > string; which doesn't exist so Nothing Happens.
> > >
> > > i'd say doing a loop is worse than a `ls` hack. and this has quoting
> > > problems, but that's ancillary ...
>
> > What about defining following function?
> >
> > any_exists() {
> > local f
> >
> > for f; do
> > [[ -e $f ]] && return 0
> > done
> >
> > return 1
> > }
>
> perhaps if it had a better name and were in a common location (eclass)
> -mike
So give it a better name. :-) In this case 'ls' shouldn't hurt anybody,
but such function solves problem in much more elegant manner -
regardless it's definied in an ebuild or eclass.
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 14:13 ` Mike Frysinger
2010-10-13 16:36 ` Amadeusz Żołnowski
@ 2010-10-13 16:41 ` Michał Górny
2010-10-13 16:51 ` Amadeusz Żołnowski
1 sibling, 1 reply; 37+ messages in thread
From: Michał Górny @ 2010-10-13 16:41 UTC (permalink / raw
To: gentoo-dev; +Cc: vapier
[-- Attachment #1: Type: text/plain, Size: 356 bytes --]
On Wed, 13 Oct 2010 10:13:58 -0400
Mike Frysinger <vapier@gentoo.org> wrote:
> > any_exists() {
> > local f
> >
> > for f; do
> > [[ -e $f ]] && return 0
> > done
> >
> > return 1
> > }
>
> perhaps if it had a better name and were in a common location (eclass)
has_file()?
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 16:41 ` Michał Górny
@ 2010-10-13 16:51 ` Amadeusz Żołnowski
2010-10-13 17:26 ` Michał Górny
0 siblings, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 16:51 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 530 bytes --]
Excerpts from Michał Górny's message of Wed Oct 13 18:41:54 +0200 2010:
> On Wed, 13 Oct 2010 10:13:58 -0400
> Mike Frysinger <vapier@gentoo.org> wrote:
>
> > > any_exists() {
> > > local f
> > >
> > > for f; do
> > > [[ -e $f ]] && return 0
> > > done
> > >
> > > return 1
> > > }
> >
> > perhaps if it had a better name and were in a common location (eclass)
>
> has_file()?
What it would mean? „Has”?
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 16:51 ` Amadeusz Żołnowski
@ 2010-10-13 17:26 ` Michał Górny
2010-10-13 18:04 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Michał Górny @ 2010-10-13 17:26 UTC (permalink / raw
To: gentoo-dev; +Cc: aidecoe
[-- Attachment #1: Type: text/plain, Size: 289 bytes --]
On Wed, 13 Oct 2010 18:51:31 +0200
Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> > has_file()?
>
> What it would mean? „Has”?
It's reference to has() function specified by PMS. 'The file system has
a file named one-of ${@}' :P.
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 17:26 ` Michał Górny
@ 2010-10-13 18:04 ` Mike Frysinger
2010-10-13 18:51 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 18:04 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 443 bytes --]
On Wednesday, October 13, 2010 13:26:24 Michał Górny wrote:
> On Wed, 13 Oct 2010 18:51:31 +0200 Amadeusz Żołnowski wrote:
> > > has_file()?
> >
> > What it would mean? „Has”?
>
> It's reference to has() function specified by PMS. 'The file system has
> a file named one-of ${@}' :P.
the has functions are typically used to look up an item in a list. i'd go
with "path_exists" since this is doing -e and not -f.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 16:35 ` Vaeth
@ 2010-10-13 18:07 ` Mike Frysinger
0 siblings, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 18:07 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 947 bytes --]
On Wednesday, October 13, 2010 12:35:20 Vaeth wrote:
> Mike Frysinger wrote:
> > relying on an external program
>
> The point is that external programs can have all sorts of undesired
> side effects. For instance, if an directory is not readable (or
> readable but not executable or is on $FILESYSTEM with who-knows-what
> permissions or accessability problems) it can cause unexpected
> errors which otherwise are treated by "standard" shell behavior:
> *This* is why the ls is hackish here, not the readability.
and if you had read all my replies, you would see i already pointed out this
problem with the original code
> but your attack is inappropriate:
might want to review the meanings of these words. i said your examples were
worse, not better. i didnt say anything about you personally.
the rest of your e-mail isnt worth responding to, so i wont bother. as most
likely will any follow up to this.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 18:04 ` Mike Frysinger
@ 2010-10-13 18:51 ` Mike Frysinger
2010-10-13 19:20 ` Amadeusz Żołnowski
2010-10-13 19:37 ` Ulrich Mueller
0 siblings, 2 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 18:51 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 845 bytes --]
here's what i'll commit to eutils.eclass:
# @FUNCTION: path_exists
# @USAGE: [-a|-o] <paths>
# @DESCRIPTION:
# Check if the specified paths exist. Works for all types of paths
# (files/dirs/etc...). The -a and -o flags control the requirements
# of the paths. They correspond to "and" and "or" logic. So the -a
# flag means all the paths must exist while the -o flag means at least
# one of the paths must exist. The default behavior is "and". If no
# paths are specified, then the return value is "false".
path_exists() {
local opt=$1
[[ ${opt} == -[ao] ]] && shift || opt="-a"
# no paths -> return false
# same behavior as: [[ -e "" ]]
[[ $# -eq 0 ]] && return 1
local p r=0
for p in "$@" ; do
[[ -e ${p} ]]
: $(( r += $? ))
done
case ${opt} in
-a) return $(( r != 0 )) ;;
-o) return $(( r == $# )) ;;
esac
}
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 18:51 ` Mike Frysinger
@ 2010-10-13 19:20 ` Amadeusz Żołnowski
2010-10-13 19:57 ` Amadeusz Żołnowski
2010-10-13 19:37 ` Ulrich Mueller
1 sibling, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 19:20 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
Excerpts from Mike Frysinger's message of Wed Oct 13 20:51:35 +0200 2010:
> path_exists() {
> local opt=$1
> [[ ${opt} == -[ao] ]] && shift || opt="-a"
>
> # no paths -> return false
> # same behavior as: [[ -e "" ]]
> [[ $# -eq 0 ]] && return 1
>
> local p r=0
> for p in "$@" ; do
> [[ -e ${p} ]]
> : $(( r += $? ))
> done
1) Why check every path in both "and" and "or" cases?
2) Even simpler:
for p; do
[[ -e $p ]]
((r+=$?))
done
>
> case ${opt} in
> -a) return $(( r != 0 )) ;;
> -o) return $(( r == $# )) ;;
> esac
> }
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 18:51 ` Mike Frysinger
2010-10-13 19:20 ` Amadeusz Żołnowski
@ 2010-10-13 19:37 ` Ulrich Mueller
2010-10-13 21:45 ` Mike Frysinger
1 sibling, 1 reply; 37+ messages in thread
From: Ulrich Mueller @ 2010-10-13 19:37 UTC (permalink / raw
To: gentoo-dev
>>>>> On Wed, 13 Oct 2010, Mike Frysinger wrote:
> # If no paths are specified, then the return value is "false".
For the "or" case that's fine. But for the "and" case, I would expect
that the function returns true if called with no arguments.
Ulrich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 19:20 ` Amadeusz Żołnowski
@ 2010-10-13 19:57 ` Amadeusz Żołnowski
2010-10-13 21:46 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 19:57 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]
Excerpts from Amadeusz Żołnowski's message of Wed Oct 13 21:20:01 +0200 2010:
> Excerpts from Mike Frysinger's message of Wed Oct 13 20:51:35 +0200 2010:
> > path_exists() {
> > local opt=$1
> > [[ ${opt} == -[ao] ]] && shift || opt="-a"
> >
> > # no paths -> return false
> > # same behavior as: [[ -e "" ]]
> > [[ $# -eq 0 ]] && return 1
> >
> > local p r=0
> > for p in "$@" ; do
> > [[ -e ${p} ]]
> > : $(( r += $? ))
> > done
>
> 1) Why check every path in both "and" and "or" cases?
>
> 2) Even simpler:
> for p; do
> [[ -e $p ]]
> ((r+=$?))
> done
>
> >
> > case ${opt} in
> > -a) return $(( r != 0 )) ;;
> > -o) return $(( r == $# )) ;;
> > esac
> > }
And why putting different tasks into one function? My suggestion:
any_paths() {
local f
for f; do
[[ -e $f ]] && return 0
done
return 1
}
all_paths() {
local f
for f; do
[[ -e $f ]] || return 1
done
return 0
}
Isn't it simpler approach? And have benfits over 2in1:
1) More readable in use.
2) More efficient.
3) 1 + 1 < 2 in this case ;-)
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 19:37 ` Ulrich Mueller
@ 2010-10-13 21:45 ` Mike Frysinger
0 siblings, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 21:45 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 571 bytes --]
On Wednesday, October 13, 2010 15:37:02 Ulrich Mueller wrote:
> >>>>> On Wed, 13 Oct 2010, Mike Frysinger wrote:
> > # If no paths are specified, then the return value is "false".
>
> For the "or" case that's fine. But for the "and" case, I would expect
> that the function returns true if called with no arguments.
i disagree as the comment says: it's sticking to the bash behavior.
f=
[[ -e ${f} ]] && [[ -e ${f} ]]
[[ -e ${f} ]] || [[ -e ${f} ]]
these both return false. as does this function:
f=
path_exists -a ${f}
path_exists -o ${f}
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 19:57 ` Amadeusz Żołnowski
@ 2010-10-13 21:46 ` Mike Frysinger
2010-10-13 22:13 ` Amadeusz Żołnowski
2010-10-18 7:08 ` Michał Górny
0 siblings, 2 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 21:46 UTC (permalink / raw
To: gentoo-dev; +Cc: Amadeusz Żołnowski
[-- Attachment #1: Type: Text/Plain, Size: 296 bytes --]
On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> And why putting different tasks into one function?
for the same reason we dont have separate test binaries: test_exist,
test_file, test_dir, etc...
it makes more sense in my mind to combine the functionality.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 21:46 ` Mike Frysinger
@ 2010-10-13 22:13 ` Amadeusz Żołnowski
2010-10-13 22:32 ` Mike Frysinger
2010-10-18 7:08 ` Michał Górny
1 sibling, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 22:13 UTC (permalink / raw
To: Mike Frysinger; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 796 bytes --]
Excerpts from Mike Frysinger's message of Wed Oct 13 23:46:43 +0200 2010:
> On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > And why putting different tasks into one function?
>
> for the same reason we dont have separate test binaries: test_exist,
> test_file, test_dir, etc...
>
> it makes more sense in my mind to combine the functionality.
> -mike
So the only argument for having more complicated, less intuitive and
less readable function is the old 'test' program? Please, reconsider my
solution with more reason. Moreover we're using 'test' as '[[ … ]]'
which changes much in readability. Next thing is that '-a' and '-o'
don't strictly conform to 'test' convention.
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 22:13 ` Amadeusz Żołnowski
@ 2010-10-13 22:32 ` Mike Frysinger
2010-10-13 23:08 ` Amadeusz Żołnowski
0 siblings, 1 reply; 37+ messages in thread
From: Mike Frysinger @ 2010-10-13 22:32 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 1004 bytes --]
On Wednesday, October 13, 2010 18:13:18 Amadeusz Żołnowski wrote:
> Excerpts from Mike Frysinger's message of Wed Oct 13 23:46:43 +0200 2010:
> > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > And why putting different tasks into one function?
> >
> > for the same reason we dont have separate test binaries: test_exist,
> > test_file, test_dir, etc...
> >
> > it makes more sense in my mind to combine the functionality.
>
> So the only argument for having more complicated, less intuitive and
> less readable function is the old 'test' program? Please, reconsider my
> solution with more reason.
we prioritize differently. i prefer unified code with options. you preferred
unrolled duplicated code.
> Moreover we're using 'test' as '[[ … ]]' which changes much in readability.
what are you talking about ? no one is using `test` in their code and if they
are, their code is broken. none of the stuff ive posted is running `test`.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 22:32 ` Mike Frysinger
@ 2010-10-13 23:08 ` Amadeusz Żołnowski
2010-10-17 21:33 ` Mike Frysinger
0 siblings, 1 reply; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-13 23:08 UTC (permalink / raw
To: Mike Frysinger; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
Excerpts from Mike Frysinger's message of Thu Oct 14 00:32:40 +0200 2010:
> On Wednesday, October 13, 2010 18:13:18 Amadeusz Żołnowski wrote:
> > Excerpts from Mike Frysinger's message of Wed Oct 13 23:46:43 +0200 2010:
> > > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > > And why putting different tasks into one function?
> > >
> > > for the same reason we dont have separate test binaries: test_exist,
> > > test_file, test_dir, etc...
> > >
> > > it makes more sense in my mind to combine the functionality.
> >
> > So the only argument for having more complicated, less intuitive and
> > less readable function is the old 'test' program? Please, reconsider my
> > solution with more reason.
>
> we prioritize differently. i prefer unified code with options.
In which part it's unified? As I said it doesn't conform much to 'test'
convention.
> you preferred unrolled duplicated code.
What I'm duplicating? Lines "local f", "for f; do" or "}"? And what's
bad about it while still having less LOC?
> > Moreover we're using 'test' as '[[ … ]]' which changes much in readability.
>
> what are you talking about ? no one is using `test` in their code and if they
> are, their code is broken. none of the stuff ive posted is running `test`.
I've messed up with that paragraph. The point was that "path_exists -a"
is even less readable than "test -e something". As you've said you
wouldn't use "test -e" syntax instead of "[[ -e ]]", but let's skip that
part…
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 23:08 ` Amadeusz Żołnowski
@ 2010-10-17 21:33 ` Mike Frysinger
2010-10-17 23:06 ` Amadeusz Żołnowski
0 siblings, 1 reply; 37+ messages in thread
From: Mike Frysinger @ 2010-10-17 21:33 UTC (permalink / raw
To: Amadeusz Żołnowski; +Cc: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 1344 bytes --]
On Wednesday, October 13, 2010 19:08:55 Amadeusz Żołnowski wrote:
> Excerpts from Mike Frysinger's message of Thu Oct 14 00:32:40 +0200 2010:
> > On Wednesday, October 13, 2010 18:13:18 Amadeusz Żołnowski wrote:
> > > Mike Frysinger's message of Wed Oct 13 23:46:43 +0200 2010:
> > > > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > > > And why putting different tasks into one function?
> > > >
> > > > for the same reason we dont have separate test binaries: test_exist,
> > > > test_file, test_dir, etc...
> > > >
> > > > it makes more sense in my mind to combine the functionality.
> > >
> > > So the only argument for having more complicated, less intuitive and
> > > less readable function is the old 'test' program? Please, reconsider
> > > my solution with more reason.
> >
> > we prioritize differently. i prefer unified code with options.
>
> In which part it's unified?
the file checking & status accumulation. extending my code to add more
options in the future is easier as well.
> As I said it doesn't conform much to 'test' convention.
sure it does
this thread is going nowhere. i believe my proposal is the way to go, and we
arent arguing over anything of too much value (i.e. bike shedding). no one
else has an opinion, so ive gone my route.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-12 20:09 ` Dirkjan Ochtman
2010-10-12 20:17 ` Mike Frysinger
2010-10-12 20:26 ` Jeroen Roovers
@ 2010-10-17 21:35 ` Mike Frysinger
2 siblings, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-17 21:35 UTC (permalink / raw
To: gentoo-dev; +Cc: Dirkjan Ochtman, Peter Volkov
[-- Attachment #1: Type: Text/Plain, Size: 513 bytes --]
On Tuesday, October 12, 2010 16:09:06 Dirkjan Ochtman wrote:
> On Fri, Oct 1, 2010 at 15:07, Peter Volkov <pva@gentoo.org> wrote:
> >> if [[ -n $(ls /etc/openvpn/*/local.conf 2>/dev/null) ]] ; then
> >
> > I'd suggested [ -e /etc/openvpn/*/local.conf ] here, but probably there
> > are better alternatives. Also ${ROOT} is missed here.
>
> I've put ${ROOT} in, are there no better alternatives? I don't think
> anyone mentioned any.
please to convert to new eutils.eclass:path_exists()
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-17 21:33 ` Mike Frysinger
@ 2010-10-17 23:06 ` Amadeusz Żołnowski
0 siblings, 0 replies; 37+ messages in thread
From: Amadeusz Żołnowski @ 2010-10-17 23:06 UTC (permalink / raw
To: Mike Frysinger; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]
Excerpts from Mike Frysinger's message of Sun Oct 17 23:33:57 +0200 2010:
> On Wednesday, October 13, 2010 19:08:55 Amadeusz Żołnowski wrote:
> > Excerpts from Mike Frysinger's message of Thu Oct 14 00:32:40 +0200 2010:
> > > On Wednesday, October 13, 2010 18:13:18 Amadeusz Żołnowski wrote:
> > > > Mike Frysinger's message of Wed Oct 13 23:46:43 +0200 2010:
> > > > > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > > > > And why putting different tasks into one function?
> > > > >
> > > > > for the same reason we dont have separate test binaries: test_exist,
> > > > > test_file, test_dir, etc...
> > > > >
> > > > > it makes more sense in my mind to combine the functionality.
> > > >
> > > > So the only argument for having more complicated, less intuitive and
> > > > less readable function is the old 'test' program? Please, reconsider
> > > > my solution with more reason.
> > >
> > > we prioritize differently. i prefer unified code with options.
> >
> > In which part it's unified?
>
> the file checking & status accumulation. extending my code to add more
> options in the future is easier as well.
The name doesn't allow for any sensible extension. I'd advise to change
it to 'mtest' (which would stand for "multiple test") or something like
that and give up default behaviour. OK, and then your /options way/
starts to make sense. (Although status accumulation has still no benefit
here.)
Having such an 'mtest' we might use it in following way:
mtest -[oa] -[fredcw...] path1 path2 ... pathn
And stil we can do it simpler:
mtest() {
local f log_op=$1 test_op=$2; shift 2
case $log_op in
-a) for f; do
eval "[[ $test_op '$f' ]]" || return 1
done
return 0
;;
-o) for f; do
eval "[[ $test_op '$f' ]]" && return 0
done
return 1
;;
esac
return -1
}
– it's just a rough draft, proof of concept. You've already gone your
way…
> this thread is going nowhere. i believe my proposal is the way to go, and we
> arent arguing over anything of too much value (i.e. bike shedding). no one
> else has an opinion, so ive gone my route.
Cheers,
--
Amadeusz Żołnowski
PGP key fpr: C700 CEDE 0C18 212E 49DA 4653 F013 4531 E1DB FAB5
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-13 21:46 ` Mike Frysinger
2010-10-13 22:13 ` Amadeusz Żołnowski
@ 2010-10-18 7:08 ` Michał Górny
2010-10-18 18:06 ` Mike Frysinger
1 sibling, 1 reply; 37+ messages in thread
From: Michał Górny @ 2010-10-18 7:08 UTC (permalink / raw
To: gentoo-dev; +Cc: vapier, Amadeusz Żołnowski
[-- Attachment #1: Type: text/plain, Size: 613 bytes --]
On Wed, 13 Oct 2010 17:46:43 -0400
Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > And why putting different tasks into one function?
>
> for the same reason we dont have separate test binaries: test_exist,
> test_file, test_dir, etc...
>
> it makes more sense in my mind to combine the functionality.
And we finally reach the point where there is no reason to implement
a new function when it's supposed to be basically longer and more
unclear to use than:
[[ -f a || -f b || -f c ]]
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-18 7:08 ` Michał Górny
@ 2010-10-18 18:06 ` Mike Frysinger
2010-10-18 18:25 ` Mike Frysinger
2010-10-19 4:03 ` Michał Górny
0 siblings, 2 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-18 18:06 UTC (permalink / raw
To: Michał Górny; +Cc: gentoo-dev, Amadeusz Żołnowski
[-- Attachment #1: Type: Text/Plain, Size: 727 bytes --]
On Monday, October 18, 2010 03:08:15 Michał Górny wrote:
> On Wed, 13 Oct 2010 17:46:43 -0400 Mike Frysinger wrote:
> > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > And why putting different tasks into one function?
> >
> > for the same reason we dont have separate test binaries: test_exist,
> > test_file, test_dir, etc...
> >
> > it makes more sense in my mind to combine the functionality.
>
> And we finally reach the point where there is no reason to implement
> a new function when it's supposed to be basically longer and more
> unclear to use than:
>
> [[ -f a || -f b || -f c ]]
except that we're using globs, so our example isnt applicable.
[[ -f */a ]]
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-18 18:06 ` Mike Frysinger
@ 2010-10-18 18:25 ` Mike Frysinger
2010-10-19 4:03 ` Michał Górny
1 sibling, 0 replies; 37+ messages in thread
From: Mike Frysinger @ 2010-10-18 18:25 UTC (permalink / raw
To: gentoo-dev; +Cc: Michał Górny, Amadeusz Żołnowski
[-- Attachment #1: Type: Text/Plain, Size: 830 bytes --]
On Monday, October 18, 2010 14:06:26 Mike Frysinger wrote:
> On Monday, October 18, 2010 03:08:15 Michał Górny wrote:
> > On Wed, 13 Oct 2010 17:46:43 -0400 Mike Frysinger wrote:
> > > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > > And why putting different tasks into one function?
> > >
> > > for the same reason we dont have separate test binaries: test_exist,
> > > test_file, test_dir, etc...
> > >
> > > it makes more sense in my mind to combine the functionality.
> >
> > And we finally reach the point where there is no reason to implement
> > a new function when it's supposed to be basically longer and more
> > unclear to use than:
> >
> > [[ -f a || -f b || -f c ]]
>
> except that we're using globs, so our example isnt applicable.
err, "our" -> "your"
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild
2010-10-18 18:06 ` Mike Frysinger
2010-10-18 18:25 ` Mike Frysinger
@ 2010-10-19 4:03 ` Michał Górny
1 sibling, 0 replies; 37+ messages in thread
From: Michał Górny @ 2010-10-19 4:03 UTC (permalink / raw
To: gentoo-dev; +Cc: vapier, Amadeusz Żołnowski
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
On Mon, 18 Oct 2010 14:06:26 -0400
Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday, October 18, 2010 03:08:15 Michał Górny wrote:
> > On Wed, 13 Oct 2010 17:46:43 -0400 Mike Frysinger wrote:
> > > On Wednesday, October 13, 2010 15:57:17 Amadeusz Żołnowski wrote:
> > > > And why putting different tasks into one function?
> > >
> > > for the same reason we dont have separate test binaries:
> > > test_exist, test_file, test_dir, etc...
> > >
> > > it makes more sense in my mind to combine the functionality.
> >
> > And we finally reach the point where there is no reason to implement
> > a new function when it's supposed to be basically longer and more
> > unclear to use than:
> >
> > [[ -f a || -f b || -f c ]]
>
> except that we're using globs, so our example isnt applicable.
> [[ -f */a ]]
Ok. So, let's sum up what your function does with wildcards:
1) when passed a single wildcard, the OR and AND variants basically do
the same (unless one of the matched files disappears during the
check), so their co-existence is useless in that case.
2) When passed multiple wildcards (or a wildcard and filenames), these
variants indeed are both useful but the AND variant does unnecessary
amount of checking (checking each filename matched by each wildcard)
and is fragile to a single matched file disappearing during the runtime.
So, it's basically safer to use:
if path_exists -o a/* && path_exists -o b/*; then
...
fi
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2010-10-19 4:05 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20100927113752.CF4CE20051@flycatcher.gentoo.org>
2010-10-01 13:07 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in net-misc/openvpn: ChangeLog openvpn-2.1.3.ebuild Peter Volkov
2010-10-01 13:34 ` Thomas Sachau
2010-10-01 15:17 ` Nirbheek Chauhan
2010-10-02 3:38 ` Ryan Hill
2010-10-02 12:32 ` Nirbheek Chauhan
2010-10-05 4:20 ` Jeroen Roovers
2010-10-12 20:09 ` Dirkjan Ochtman
2010-10-12 20:17 ` Mike Frysinger
2010-10-12 20:26 ` Jeroen Roovers
2010-10-12 20:57 ` Mike Frysinger
2010-10-13 8:23 ` Amadeusz Żołnowski
2010-10-13 14:13 ` Mike Frysinger
2010-10-13 16:36 ` Amadeusz Żołnowski
2010-10-13 16:41 ` Michał Górny
2010-10-13 16:51 ` Amadeusz Żołnowski
2010-10-13 17:26 ` Michał Górny
2010-10-13 18:04 ` Mike Frysinger
2010-10-13 18:51 ` Mike Frysinger
2010-10-13 19:20 ` Amadeusz Żołnowski
2010-10-13 19:57 ` Amadeusz Żołnowski
2010-10-13 21:46 ` Mike Frysinger
2010-10-13 22:13 ` Amadeusz Żołnowski
2010-10-13 22:32 ` Mike Frysinger
2010-10-13 23:08 ` Amadeusz Żołnowski
2010-10-17 21:33 ` Mike Frysinger
2010-10-17 23:06 ` Amadeusz Żołnowski
2010-10-18 7:08 ` Michał Górny
2010-10-18 18:06 ` Mike Frysinger
2010-10-18 18:25 ` Mike Frysinger
2010-10-19 4:03 ` Michał Górny
2010-10-13 19:37 ` Ulrich Mueller
2010-10-13 21:45 ` Mike Frysinger
2010-10-17 21:35 ` Mike Frysinger
[not found] <fyNx7-2Qx-7@gated-at.bofh.it>
[not found] ` <fCTuh-8on-5@gated-at.bofh.it>
[not found] ` <fCTDX-aL-5@gated-at.bofh.it>
[not found] ` <fCUgF-1kl-15@gated-at.bofh.it>
2010-10-13 7:08 ` Vaeth
2010-10-13 14:15 ` Mike Frysinger
[not found] <fD3Dj-k9-9@gated-at.bofh.it>
[not found] ` <fD3Dj-k9-15@gated-at.bofh.it>
[not found] ` <fD3Dj-k9-7@gated-at.bofh.it>
[not found] ` <fDbhw-4Y2-5@gated-at.bofh.it>
2010-10-13 16:35 ` Vaeth
2010-10-13 18:07 ` Mike Frysinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox