Mailing List Archive

On 22:26 Wed 05 Dec , Alistair Bush wrote:
> # @ebuild-function _java-pkg_osgi-plugin

I just picked this one at random, although my question applies to the
whole eclass. Why is everything tagged with a "java-pkg_" prefix?

Thanks,
Donnie
--
gentoo-dev@gentoo.org mailing list

Donnie Berkholz kirjoitti:
> On 22:26 Wed 05 Dec , Alistair Bush wrote:
>> # @ebuild-function _java-pkg_osgi-plugin
>
> I just picked this one at random, although my question applies to the
> whole eclass. Why is everything tagged with a "java-pkg_" prefix?
>
> Thanks,
> Donnie

Probably used the same prefix as the traditional java stuff but yes
these should use the osgi prefix. I was also thinking whether we should
be naming this java-osgi as we already have java-ant instead of ant etc.

Regards,
Petteri

Regards,
Petteri

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 "${@}" 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
>
> ((${#arguments[@]} < 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

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 <betelgeuse@gentoo.org> 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 "${@}" 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
> >
> > ((${#arguments[@]} < 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¬

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

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

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

((${#arguments[@]} < 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


--
gentoo-dev@gentoo.org mailing list