Created attachment 359048 [details, diff] patch against gx86 Just following the idea of other eclasses.
'use' should not be called in global scope.
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #1) > 'use' should not be called in global scope. True, ruby-ng-gnome2.eclass does it the same way.
Created attachment 359136 [details, diff] patch against gx86 Make calls to "use prefix" local.
Setting DYLD_LIBRARY_PATH is not necessary on non-Darwin systems. if [[ -n ${libdir} ]] ; then if [[ ${CHOST} == *-darwin* ]] ; then var=DYLD_LIBRARY_PATH else var=LD_LIBRARY_PATH fi cat <<-EOF if [ "\${${var}+set}" = "set" ] ; then export ${var}="\${${var}}:${EPREFIX}${libdir}" else export ${var}="${EPREFIX}${libdir}" fi EOF fi ('local var' is not required due to subshell.)
Created attachment 359138 [details, diff] patch against gx86 (In reply to Arfrever Frehtes Taifersar Arahesis from comment #4) > Setting DYLD_LIBRARY_PATH is not necessary on non-Darwin systems. > > if [[ -n ${libdir} ]] ; then > if [[ ${CHOST} == *-darwin* ]] ; then > var=DYLD_LIBRARY_PATH > else > var=LD_LIBRARY_PATH > fi > > cat <<-EOF > if [ "\${${var}+set}" = "set" ] ; then > export ${var}="\${${var}}:${EPREFIX}${libdir}" > else > export ${var}="${EPREFIX}${libdir}" > fi > EOF > fi > > ('local var' is not required due to subshell.) That is actually quite a bit nicer.
Comment on attachment 359138 [details, diff] patch against gx86 >+ has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}" && EPREFIX= wouldn't this be simpler: has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} then you can copy & paste that line everywhere below. or maybe make an internal func: _eutils_eprefix_init() { has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} } >+ # cannot preserve-libs on aix yet. >+ [[ ${CHOST} == *-aix* ]] && { >+ einfo "Not preserving libs on AIX (yet), not implemented." >+ return 0 >+ } no need for this funky syntax. use standard `if ... ; then` rather than {...}
(In reply to SpanKY from comment #6) > >+ # cannot preserve-libs on aix yet. > >+ [[ ${CHOST} == *-aix* ]] && { > >+ einfo "Not preserving libs on AIX (yet), not implemented." > >+ return 0 > >+ } "AIX" triggers my attention here: IMO, the comment+message is not true any more and wrong now - better tell to always enable (or relying on) FEATURES=preserve-libs on AIX instead (who would want to disable it anyway?), it /is/ implemented in portage.
(In reply to SpanKY from comment #6) > Comment on attachment 359138 [details, diff] [details, diff] > patch against gx86 > > >+ has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}" && EPREFIX= > > wouldn't this be simpler: > has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} This not exactly the same. If the user has ED to something odd on EAPI<2, we are screwed. The above variant will always set ED="${D}". > > then you can copy & paste that line everywhere below. or maybe make an > internal func: > _eutils_eprefix_init() { > has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} > } What ever you prefer, I will one extra explicit line per function. > > >+ # cannot preserve-libs on aix yet. > >+ [[ ${CHOST} == *-aix* ]] && { > >+ einfo "Not preserving libs on AIX (yet), not implemented." > >+ return 0 > >+ } > no need for this funky syntax. use standard `if ... ; then` rather than > {...} Can go due to haubi's comments.
Created attachment 361676 [details, diff] patch against gx86 Updated version of the patch, if preferred we can also go for the sloppy version to set EPREFIX and ED.
(In reply to Christoph Junghans from comment #8) if the user is setting ED in their environment, then they should be failing. we've used this style elsewhere in the tree, so they're already screwed. then again, it's already conditional on EAPI level, so no need for the : ${:=} logic.
Comment on attachment 361676 [details, diff] patch against gx86 merge the EAPI default var logic into a single func as suggested earlier
Created attachment 364366 [details, diff] patch against gx86 Here we go again!
Comment on attachment 364366 [details, diff] patch against gx86 >+# @FUNCTION: _eutils_eprefix_init >+# @USAGE: >+# @DESCRIPTION: >+# Initialized prefix variables for EAPI<3. drop the @USAGE and add @INTERNAL >+_eutils_eprefix_init() { >+ has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} >+} to be clear, i just wanted one func. if you want to drop the : logic, that's fine (as discussed earlier). has "${EAPI:-0}" 0 1 2 && ED=${D} EPREFIX= EROOT=${ROOT} i'm assuming that means you don't support prefix with EAPI=2 and older. >- local USEFILE=${ROOT}/var/db/pkg/${PKG}/USE >- local IUSEFILE=${ROOT}/var/db/pkg/${PKG}/IUSE >+ local USEFILE="${EROOT}"/var/db/pkg/${PKG}/USE >+ local IUSEFILE="${EROOT}"/var/db/pkg/${PKG}/IUSE the quotes are pointless here >@@ -1408,17 +1422,22 @@ > echo '#!/bin/sh' > [[ -n ${chdir} ]] && printf 'cd "%s"\n' "${chdir}" > if [[ -n ${libdir} ]] ; then >+ if [[ ${CHOST} == *-darwin* ]] ; then >+ var=DYLD_LIBRARY_PATH >+ else >+ var=LD_LIBRARY_PATH >+ fi missing `local var` decl > # We don't want to quote ${bin} so that people can pass complex > # things as ${bin} ... "./someprog --args" >- printf 'exec %s "$@"\n' "${bin}" >+ printf 'exec %s "$@"\n' "${bin/#\//${EPREFIX}\/}" hmm, you correctly handle the prefix in the program name, but i think it's missing handling when the chdir option is enabled
Created attachment 364446 [details, diff] patch against gx86 (In reply to SpanKY from comment #13) > Comment on attachment 364366 [details, diff] [details, diff] > patch against gx86 > > >+# @FUNCTION: _eutils_eprefix_init > >+# @USAGE: > >+# @DESCRIPTION: > >+# Initialized prefix variables for EAPI<3. > > drop the @USAGE and add @INTERNAL Done! > > >+_eutils_eprefix_init() { > >+ has "${EAPI:-0}" 0 1 2 && : ${ED:=${D}} ${EPREFIX:=} ${EROOT:=${ROOT}} > >+} > > to be clear, i just wanted one func. if you want to drop the : logic, > that's fine (as discussed earlier). > has "${EAPI:-0}" 0 1 2 && ED=${D} EPREFIX= EROOT=${ROOT} > > i'm assuming that means you don't support prefix with EAPI=2 and older. Huh, there are EAPI<=2 ebuilds in the prefix overlay, so the := is needed as we only want to set it if it isn't already set by portage otherwise we would end up with empty EPREFIX for EAPI<=2. > > >- local USEFILE=${ROOT}/var/db/pkg/${PKG}/USE > >- local IUSEFILE=${ROOT}/var/db/pkg/${PKG}/IUSE > >+ local USEFILE="${EROOT}"/var/db/pkg/${PKG}/USE > >+ local IUSEFILE="${EROOT}"/var/db/pkg/${PKG}/IUSE > > the quotes are pointless here Done. > > >@@ -1408,17 +1422,22 @@ > > echo '#!/bin/sh' > > [[ -n ${chdir} ]] && printf 'cd "%s"\n' "${chdir}" > > if [[ -n ${libdir} ]] ; then > >+ if [[ ${CHOST} == *-darwin* ]] ; then > >+ var=DYLD_LIBRARY_PATH > >+ else > >+ var=LD_LIBRARY_PATH > >+ fi > > missing `local var` decl Added, but "('local var' is not required due to subshell.)" (comment #4). > > > # We don't want to quote ${bin} so that people can pass complex > > # things as ${bin} ... "./someprog --args" > >- printf 'exec %s "$@"\n' "${bin}" > >+ printf 'exec %s "$@"\n' "${bin/#\//${EPREFIX}\/}" > > hmm, you correctly handle the prefix in the program name, but i think it's > missing handling when the chdir option is enabled True, it seems we never used that in prefix.
(In reply to Christoph Junghans from comment #14) well i'm completely lost then on _eutils_eprefix_init as it seems you've argued it both ways. but it doesn't matter as this version is fine by me and fine by you. committed: http://sources.gentoo.org/eclass/eutils.eclass?r1=1.427&r2=1.428
(In reply to SpanKY from comment #15) > (In reply to Christoph Junghans from comment #14) > > well i'm completely lost then on _eutils_eprefix_init as it seems you've > argued it both ways. I got a bit confused, too ;-) > but it doesn't matter as this version is fine by me > and fine by you. committed: > http://sources.gentoo.org/eclass/eutils.eclass?r1=1.427&r2=1.428 Great, thanks.
