[gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: rox-0install.eclass

[gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: rox-0install.eclass

[Donnie Berkholz]

On 17:07 Tue 04 Dec     , Jim Ramsay (lack) wrote:
> Revision  Changes    Path
> 1.1                  eclass/rox-0install.eclass
> 
> file : http://sources.gentoo.org/viewcvs.py/gentoo-x86/eclass/rox-0install.eclass?rev=1.1&view=markup
> plain: http://sources.gentoo.org/viewcvs.py/gentoo-x86/eclass/rox-0install.eclass?rev=1.1&content-type=text/plain

I don't remember this going by gentoo-dev. Always send eclasses to 
gentoo-dev before committing them.

> Index: rox-0install.eclass
> ===================================================================
> # Copyright 1999-2004 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: /var/cvsroot/gentoo-x86/eclass/rox-0install.eclass,v 1.1 2007/12/04 17:07:52 lack Exp $
> 
> # ROX-0install eclass Version 1
> 
> # Created by Jim Ramsay (lack@gentoo.org) to ease installation of ROX desktop
> # applications and integrate this with zeroinstall-injector
> # (http://0install.net)
> 
> # These variables are only used inside functions, and so may be set anywhere in
> # the ebuild:
> #
> # ZEROINSTALL_STRIP_REQUIRES - this flag, if set, will force the local
> #    zeroinstall feed to have all its 'requires' directives stripped out
> # LOCAL_FEED_SRC - The ebuild-supplied native feed, for those packages which do
> #    not already contain one.  By default we check for ${APPNAME}.xml and
> #    ${APPNAME}/${APPNAME}.xml
> 
> # This is an extension of rox.eclass
> inherit rox
> 
> DEPEND="${DEPEND}
> 	rox-base/zeroinstall-injector"
> 
> # Some locations for ZEROINSTALL
> NATIVE_FEED_DIR="/usr/share/0install.net/native_feeds"
> ICON_CACHE_DIR="/var/cache/0install.net/interface_icons"
> 
> # Does all the 0install local feed magic you could want:
> #   - Parses the input file to get the interface URI
> #   - Edits the input file and installs it to the final location
> #   - Installs a local feed pointer
> #
> # Environment variables:
> #  ZEROINSTALL_STRIP_REQUIRES - If set, strips all 'requires' sections from the XML
> #                            on editing.  Default: Not set
> #
> # 0install_native_feed <src> <destpath>
> #  src   - The XML file we will edit, install, and point at
> #  path  - The path where the implementation will be installed 
> #          IE, the final edited xml will be at <path>/<basename of src>
> 0install_native_feed() {
> 	local src="${1}"; shift
> 	local path="${1}"; shift

This is a rather strange paradigm to me, instead of:

local src=$1 path=$2
shift 2

> 	local feedfile=$(basename "${src}")

You could do this in pure bash, although it doesn't really matter:

local feedfile=${src##*/}

> 	local dest="${path}/$feedfile"

How do you decide when not to use braces { } around variables?

> 	0distutils "${src}" > tmp.native_feed || die "0distutils feed edit failed"
> 
> 	if [[ ${ZEROINSTALL_STRIP_REQUIRES} ]]; then
> 		# Strip out all 'requires' sections
> 		sed -i -e '/<requires.*\/>/d' \
> 			-e '/<requires.*\>/,/<\/requires>/d' tmp.native_feed

What happens if the contents of a <requires> section are on a separate 
line? Is this a concern?

> 	fi
> 
> 	(
> 		insinto ${path}
> 		newins tmp.native_feed ${feedfile}
> 	)
> 
> 	local feedname

You could just declare feedname local and set it in the same line.

> 	feedname=$(0distutils -e "${src}") || "0distutils URI escape failed"

What's the || doing? You've got a string sitting there. Is 'die' 
missing?

> 	dosym "${dest}" "${NATIVE_FEED_DIR}/${feedname}"
> 
> 	local cachedname
> 	cachedname=$(0distutils -c "${src}") || "0distutils URI escape failed"

Same questions.

> 	dosym "${path}/.DirIcon" "${ICON_CACHE_DIR}/${cachedname}"
> }
> 
> # Exported functions
> rox-0install_src_install() {
> 	# First do the regular Rox install
> 	rox_src_install
> 
> 	# Now search for the feed, and install it if found.
> 	local search_list="${LOCAL_FEED_SRC} ${APPNAME}/${APPNAME}.xml ${APPNAME}.xml"
> 	local installed=false
> 	for feed in ${search_list}; do
> 		if [[ -f "${feed}" ]]; then
> 			0install_native_feed "${feed}" "${APPDIR}/${APPNAME}"
> 			installed=true
> 			break
> 		fi
> 	done
> 
> 	if ! $installed; then

This is kind of a weird way to do it. I'd check instead for
[[ -n ${installed} ]] and initialize it to empty.

> 		ewarn "No native feed found - This application will not be found by 0launch."
> 	fi
> }
> 
> EXPORT_FUNCTIONS src_install

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

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: rox-0install.eclass

[Jim Ramsay]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

Donnie Berkholz wrote:
> I don't remember this going by gentoo-dev. Always send eclasses to 
> gentoo-dev before committing them.

My apologies, I was just so excited that I'd finally finished it!

Apparently I missed a few things, thanks very much for catching
it.

> > 0install_native_feed() {
> > 	local src="${1}"; shift
> > 	local path="${1}"; shift
> 
> This is a rather strange paradigm to me, instead of:
> 
> local src=$1 path=$2
> shift 2

This is cleaner, thanks.  In fact, I don't even really need the
shift at all.

> > 	local feedfile=$(basename "${src}")
> 
> You could do this in pure bash, although it doesn't really matter:
> 
> local feedfile=${src##*/}

Sure, may as well, save a subshell.

> > 	local dest="${path}/$feedfile"
> 
> How do you decide when not to use braces { } around variables?

In general I've used them everywhere... except there :)  Fixed.

> > 	0distutils "${src}" > tmp.native_feed || die "0distutils feed edit failed"
> > 
> > 	if [[ ${ZEROINSTALL_STRIP_REQUIRES} ]]; then
> > 		# Strip out all 'requires' sections
> > 		sed -i -e '/<requires.*\/>/d' \
> > 			-e '/<requires.*\>/,/<\/requires>/d' tmp.native_feed
> 
> What happens if the contents of a <requires> section are on a separate 
> line? Is this a concern?

It hasn't been so far - The convention in all known cases is
either it's all on one line, or a multi-line as is caught  by the
second sed expression.  This is just a stopgap measure until I
rework 0distutils to do this optional stripping on its own.

> > 	local feedname
> 
> You could just declare feedname local and set it in the same line.

Not if I want to potentially die on the assignment.  As I found
out in #gentoo-dev-help today, try this:

$ t() { local a=$(false) || echo "Die"; }; t

Versus this:

$ t() { local a; a=$(false) || echo "Die"; }; t
Die

> > 	feedname=$(0distutils -e "${src}") || "0distutils URI escape failed"
> 
> What's the || doing? You've got a string sitting there. Is 'die' 
> missing?

Verily - nice catch!

> > 	if ! $installed; then
> 
> This is kind of a weird way to do it. I'd check instead for
> [[ -n ${installed} ]] and initialize it to empty.

Sure, that looks nicer.

I'll be committing these changes right away, since the "die" ones
at least are very important.  But I'll be submitting further
changes to the list first.  Sorry about that :)

Luckily only 1 ebuild so far which uses this eclass has actually
hit the tree!

-- 
Jim Ramsay
Gentoo/Linux Developer (rox/fluxbox/gkrellm)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: rox-0install.eclass

[Donnie Berkholz]

On 21:24 Tue 04 Dec     , Jim Ramsay wrote:
> I'll be committing these changes right away, since the "die" ones
> at least are very important.  But I'll be submitting further
> changes to the list first.  Sorry about that :)
> 
> Luckily only 1 ebuild so far which uses this eclass has actually
> hit the tree!

No need to submit all your eclass diffs; that rule is just for addition 
of new eclasses, since they're kind of hard to remove.

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