[gentoo-dev] Re: New eclass osgi.eclass
Alistair Bush wrote: > Im sure Elvanor can't wait for you constructive feedback on his eclass > and depending on your feedback the eclass will enter the tree this > weekend. > A couple of very minor performance points, which I think are more significant in eclasses. Firstly the basename thing Donnie pointed out before: > > local feedfile=$(basename "${src}") > You could do this in pure bash, although it doesn't really matter: > local feedfile=${src##*/} [[ ${#} -lt 4 ]] && die "At least four arguments needed" Arithmetic context is quicker for this: (($#<4)) && die "At least four arguments needed" although in this case, _java-pkg_osgijar(), it looks it requires exactly 4: (($#==4)) || die 'Four arguments needed' You have a couple of functions that take, say, 4 or 5 arguments. It would be more robust to use a case, eg: case $# in 5)..;; 4)..;; *) die "Incorrect use of $FUNCNAME";; esac ..than if (($#>4)); then ..; else .. ;fi _java-pkg_osgi-plugin ${4} # this should be quoted (_java-pkg_osgijar) With regard to: debug-print-function ${FUNCNAME} $* if you want debug-print-function to get the arguments correctly, use "$@" not $* (cf 'Special Parameters' in man bash for a proper explanation: this applies to all arrays. http://wooledge.org/mywiki/BashFAQ/073 shows how to manipulate all members of an array, eg to add a prefix.) This use of counter in _java-pkg_osgijar-fromfile is odd: while [[ -n "${1}" ]]; do # while [[ $1 ]] or while (($#)) also work if [[ "${1}" == "--noversion" ]]; then noversion=1 else arguments[${counter}]="${1}" ((++counter)) fi shift 1 # just shift? done (([EMAIL PROTECTED] < 3)) && die "At least three arguments (not counting --noversion) are needed for java-pkg_osgijar-fromfile()" You can either just add to the array with: arguments+=("$1") or add using the counter: arguments[counter++]="$1" and then check: ((counter < 3)) && die .. Arithmetic context[1] applies in array indexes, so you don't need to use a $ for variables and you can post/pre-incr etc there. Yuu can also use it for C-style flags, with 0 as false and non-zero true: if [[ "${noversion}" == 1 ]]; can be if ((noversion)); This is handy since unset variables evaluate as zero, which is false inside ((..)), so a simple flag=1 is all that's needed, ie you don't have to set a default, although ofc it's more robust to do so, especially for functions. declare -i flag=0 # makes a local var which can only have integer values assigned (setting it to string typically evaluates to zero; arithmetic context is used for all assignments, so a string which happens to be a variable with an integer will return that value.) [1] http://wooledge.org/mywiki/ArithmeticExpression -- [EMAIL PROTECTED] mailing list
Re: [gentoo-dev] Re: New eclass osgi.eclass
Steve Long kirjoitti: > Alistair Bush wrote: >> Im sure Elvanor can't wait for you constructive feedback on his eclass >> and depending on your feedback the eclass will enter the tree this >> weekend. >> > A couple of very minor performance points, which I think are more > significant in eclasses. Firstly the basename thing Donnie pointed out > before: Well usually you don't noticy any differences. In global scope things could have some effect of course. > > You have a couple of functions that take, say, 4 or 5 arguments. It would be > more robust to use a case, eg: > case $# in > 5)..;; > 4)..;; > *) die "Incorrect use of $FUNCNAME";; > esac > ..than if (($#>4)); then ..; else .. ;fi > Well imho they are equivalent and eclass maintainers should go with what they prefer. > > With regard to: > debug-print-function ${FUNCNAME} $* > if you want debug-print-function to get the arguments correctly, use "$@" > not $* (cf 'Special Parameters' in man bash for a proper explanation: this > applies to all arrays. http://wooledge.org/mywiki/BashFAQ/073 shows how to > manipulate all members of an array, eg to add a prefix.) Doesn't matter as arguments are just written to a file and for some reason this form is used all over the base in eclasses but shouldn't hurt to use "[EMAIL PROTECTED]" with new code. > > This use of counter in _java-pkg_osgijar-fromfile is odd: > while [[ -n "${1}" ]]; do > # while [[ $1 ]] or while (($#)) also work >if [[ "${1}" == "--noversion" ]]; then > noversion=1 >else > arguments[${counter}]="${1}" > ((++counter)) >fi >shift 1 # just shift? > done > > (([EMAIL PROTECTED] < 3)) && die "At least three arguments (not > counting --noversion) are needed for java-pkg_osgijar-fromfile()" > > You can either just add to the array with: arguments+=("$1") > or add using the counter: arguments[counter++]="$1" > and then check: ((counter < 3)) && die .. > > Arithmetic context[1] applies in array indexes, so you don't need to use a $ > for variables and you can post/pre-incr etc there. > Yuu can also use it for C-style flags, with 0 as false and non-zero true: > if [[ "${noversion}" == 1 ]]; > can be > if ((noversion)); > > This is handy since unset variables evaluate as zero, which is false inside > ((..)), so a simple flag=1 is all that's needed, ie you don't have to set a > default, although ofc it's more robust to do so, especially for functions. > declare -i flag=0 # makes a local var which can only have integer values > assigned (setting it to string typically evaluates to zero; arithmetic > context is used for all assignments, so a string which happens to be a > variable with an integer will return that value.) > > [1] http://wooledge.org/mywiki/ArithmeticExpression > We really don't need to create the arguments array at all. We could just check if ${1} is --noversion and then use shift. After that we can use the positional arguments as usual so instead of ${arguments[0]} we can use just ${1}. Regards, Petteri signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] Re: New eclass osgi.eclass
Thanks all for this feedback. I think this is sufficient to warrant some modifications as suggested, and I propose to postpone the inclusion in tree a little bit. Regarding performance, there is an important point to note as IMHO it is much more penalizing than using basename or such. The fact that I use the jar utility to unzip a file, combined with the fact that jar does not have an option to specify a destination directory, is troublesome. I have to copy the .jar file to the destination directory and then remove it. This .jar file can be several MBs of course. Initially I was using zip but I replaced it with jar in order to avoid a dependency on zip. But maybe it's not worth the performance penalty. Regards, Elvanör On Dec 6, 2007 1:11 PM, Petteri Räty <[EMAIL PROTECTED]> wrote: > Steve Long kirjoitti: > > Alistair Bush wrote: > >> Im sure Elvanor can't wait for you constructive feedback on his eclass > >> and depending on your feedback the eclass will enter the tree this > >> weekend. > >> > > A couple of very minor performance points, which I think are more > > significant in eclasses. Firstly the basename thing Donnie pointed out > > before: > > Well usually you don't noticy any differences. In global scope things > could have some effect of course. > > > > > You have a couple of functions that take, say, 4 or 5 arguments. It would be > > more robust to use a case, eg: > > case $# in > > 5)..;; > > 4)..;; > > *) die "Incorrect use of $FUNCNAME";; > > esac > > ..than if (($#>4)); then ..; else .. ;fi > > > > Well imho they are equivalent and eclass maintainers should go with what > they prefer. > > > > > With regard to: > > debug-print-function ${FUNCNAME} $* > > if you want debug-print-function to get the arguments correctly, use "$@" > > not $* (cf 'Special Parameters' in man bash for a proper explanation: this > > applies to all arrays. http://wooledge.org/mywiki/BashFAQ/073 shows how to > > manipulate all members of an array, eg to add a prefix.) > > Doesn't matter as arguments are just written to a file and for some > reason this form is used all over the base in eclasses but shouldn't > hurt to use "[EMAIL PROTECTED]" with new code. > > > > > > This use of counter in _java-pkg_osgijar-fromfile is odd: > > while [[ -n "${1}" ]]; do > > # while [[ $1 ]] or while (($#)) also work > >if [[ "${1}" == "--noversion" ]]; then > > noversion=1 > >else > > arguments[${counter}]="${1}" > > ((++counter)) > >fi > >shift 1 # just shift? > > done > > > > (([EMAIL PROTECTED] < 3)) && die "At least three arguments (not > > counting --noversion) are needed for java-pkg_osgijar-fromfile()" > > > > You can either just add to the array with: arguments+=("$1") > > or add using the counter: arguments[counter++]="$1" > > and then check: ((counter < 3)) && die .. > > > > Arithmetic context[1] applies in array indexes, so you don't need to use a $ > > for variables and you can post/pre-incr etc there. > > Yuu can also use it for C-style flags, with 0 as false and non-zero true: > > if [[ "${noversion}" == 1 ]]; > > can be > > if ((noversion)); > > > > This is handy since unset variables evaluate as zero, which is false inside > > ((..)), so a simple flag=1 is all that's needed, ie you don't have to set a > > default, although ofc it's more robust to do so, especially for functions. > > declare -i flag=0 # makes a local var which can only have integer values > > assigned (setting it to string typically evaluates to zero; arithmetic > > context is used for all assignments, so a string which happens to be a > > variable with an integer will return that value.) > > > > [1] http://wooledge.org/mywiki/ArithmeticExpression > > > > We really don't need to create the arguments array at all. We could just > check if ${1} is --noversion and then use shift. After that we can use > the positional arguments as usual so instead of ${arguments[0]} we can > use just ${1}. > > Regards, > Petteri > > > ��í¢^����(� ��X��X�
Re: [gentoo-dev] Re: New eclass osgi.eclass
Jean-Noël Rivasseau kirjoitti: > > The fact that I use the jar utility to unzip a file, combined with the > fact that jar does not have an option to specify a destination > directory, is troublesome. I have to copy the .jar file to the > destination directory and then remove it. This .jar file can be > several MBs of course. > The jar utility supports writing to standard output and you can just use normal output redirection. See man jar. Regards, Petteri signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] Re: New eclass osgi.eclass
Hello all, I have a new version of the eclass ready, with much of the remarks addressed. It now goes by the name java-osgi and in the new form, should be ready to enter the tree. I fixed the performance problem I mentionned earlier, cleaned up the eclass API, and simplified the code almost everywhere. Waiting for your feedback. Cheers Elvanör java-osgi.eclass Description: Binary data