merge Prefix changes of toolchain-binutils.eclass Reproducible: Always
Created attachment 354380 [details, diff] toolchain-binutils.patch Prefix support including solaris and mint
Fellows of toolchain herd, could you please have a look at the patch? They are straight forward like other prefixify efforts. Luca, please help. Giving toolchain eclasses Prefix support will make rap easier to get included. One exception is the TPREFIX var. I am not sure of its origin. I guess we needed it only for prefix-chaining. Correct me if I am wrong, @Fabian, @Michael. It's okay not to include TPREFIX at this point.
TPREFIX indeed stands for "Target Prefix", I remember introducing it, but not exactly what was the problem. It had to do with cross-prefix IIRC, but since that's broken for a while -- in fact we don't support it any more, I guess we can simplify into using EPREFIX again. A standing issue is whether or not the eclass should mangle the EPREFIX into an absolute path. Some say not doing so results in a GCC that doesn't lookup search paths correctly, others say it works fine. Since the bootstrap script tries to use an absolute path these days, I'd be happy with just warning, or something, but leaving the value of EPREFIX alone.
Created attachment 355742 [details, diff] toolchain-binutils.patch dropped TPREFIX as per Fabian's comment. The patch is ready to be included.
Looks fine, does it work with crossdev?
(In reply to Luca Barbato from comment #5) > Looks fine, does it work with crossdev? Yes, because it includes all the changes in Prefix overlay.
This needs Mike's approval to go in.
Comment on attachment 355742 [details, diff] toolchain-binutils.patch > # enable gold if available (installed as ld.gold) >- if use cxx ; then >+ # PREFIX LOCAL: Linux only (fails to compile on Solaris, MiNT #353410) >+ if [[ ${CHOST} == *-linux* ]] && use cxx ; then imo, this should be masked in the profiles rather than encoding targets in the eclass >+ [[ ${CHOST} == *"-solaris"* ]] && use nls && append-libs -lintl this should be fixed in the source rather than hacked in here >+ --infodir="${EPREFIX}"${DATAPATH}/info >+ --mandir="${EPREFIX}"${DATAPATH}/man does this work instead: --infodir='$(datadir)/info' --mandir='$(datadir)/man' >+ --libexecdir="${EPREFIX}"${LIBPATH} this would be: --libexecdir='$(libdir)' >+ --with-binutils-ldscript-dir="${EPREFIX}"/${LIBPATH}/ldscripts this would be: --with-binutils-ldscript-dir='$(libdir)/ldscripts' >- if [[ ! -e ${BINPATH}/ld ]] && [[ ${current_profile} == ${CTARGET}-${BVER} ]] ; then >+ if [[ ! -e ${EPREFIX}${BINPATH}/ld ]] && [[ ${current_profile} == ${CTARGET}-${BVER} ]] ; then > local choice=$(binutils-config -l | grep ${CTARGET} | awk '{print $2}') > choice=${choice//$'\n'/ } > choice=${choice/* } > if [[ -z ${choice} ]] ; then >- env -i ROOT="${ROOT}" binutils-config -u ${CTARGET} >+ env -i ROOT="${ROOT}" "${EPREFIX}"/usr/bin/binutils-config -u ${CTARGET} > else > binutils-config ${choice} > fi this doesn't make sense. binutils-config is in $PATH and that is where we should be executing it from. the other $D->$ED changes look fine
(In reply to SpanKY from comment #8) > Comment on attachment 355742 [details, diff] [details, diff] > toolchain-binutils.patch > > > # enable gold if available (installed as ld.gold) > >- if use cxx ; then > >+ # PREFIX LOCAL: Linux only (fails to compile on Solaris, MiNT #353410) > >+ if [[ ${CHOST} == *-linux* ]] && use cxx ; then > > imo, this should be masked in the profiles rather than encoding targets in > the eclass > > >+ [[ ${CHOST} == *"-solaris"* ]] && use nls && append-libs -lintl > > this should be fixed in the source rather than hacked in here > kicked out. > >+ --infodir="${EPREFIX}"${DATAPATH}/info > >+ --mandir="${EPREFIX}"${DATAPATH}/man > > does this work instead: > --infodir='$(datadir)/info' > --mandir='$(datadir)/man' > > >+ --libexecdir="${EPREFIX}"${LIBPATH} > > this would be: > --libexecdir='$(libdir)' > > >+ --with-binutils-ldscript-dir="${EPREFIX}"/${LIBPATH}/ldscripts > > this would be: > --with-binutils-ldscript-dir='$(libdir)/ldscripts' No they don't work. This results in configure --infodir=$(datadir)/info ... where bash tries to expand $(datadir) and fails. If we pass --libexecdir=\''$(libdir)'\' to ${myconf[@]}, it results in configure --libexecdir='$(libdir)' ... which gives an error of > configure: error: expected an absolute directory name for --libexecdir: $(libdir)' Therefore let me retain the original ugly ones. > >- if [[ ! -e ${BINPATH}/ld ]] && [[ ${current_profile} == ${CTARGET}-${BVER} ]] ; then > >+ if [[ ! -e ${EPREFIX}${BINPATH}/ld ]] && [[ ${current_profile} == ${CTARGET}-${BVER} ]] ; then > > local choice=$(binutils-config -l | grep ${CTARGET} | awk '{print $2}') > > choice=${choice//$'\n'/ } > > choice=${choice/* } > > if [[ -z ${choice} ]] ; then > >- env -i ROOT="${ROOT}" binutils-config -u ${CTARGET} > >+ env -i ROOT="${ROOT}" "${EPREFIX}"/usr/bin/binutils-config -u ${CTARGET} > > else > > binutils-config ${choice} > > fi > > this doesn't make sense. binutils-config is in $PATH and that is where we > should be executing it from. Applied. > the other $D->$ED changes look fine Thanks.
Created attachment 356280 [details, diff] toolchain-binutils.patch revised patch as per comment 8
(In reply to Benda Xu from comment #9) > > does this work instead: > > No they don't work. This results in > > configure --infodir=$(datadir)/info ... > > where bash tries to expand $(datadir) and fails. i think you misread the output. the ebuild does not expand $(datadir). that is what using arrays and quoting things fixes. whether the configure script itself expands things is a different story. i'm guessing it does considering the line you posted: > > configure: error: expected an absolute directory name for --libexecdir: $(libdir)'
(In reply to SpanKY from comment #11) > i think you misread the output. the ebuild does not expand $(datadir). > that is what using arrays and quoting things fixes. > > whether the configure script itself expands things is a different story. > i'm guessing it does considering the line you posted: > > > configure: error: expected an absolute directory name for --libexecdir: $(libdir)' Yes, you are right. The ebuild has no problem with this. While configure do execute it, treating it as bash expression: * QA Notice: command not found: * * /home/benda/gnto/bin/bash: line 4: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found * checking for suffix of executables... checking for C compiler default output file name... /home/benda/gnto/bin/bash: line 2: datadir: command n ot found * checking for strcasecmp... /home/benda/gnto/bin/bash: line 2: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found * /home/benda/gnto/bin/bash: line 4: datadir: command not found I guess we have no cleaner way to do it than the existing one.
Comment on attachment 356280 [details, diff] toolchain-binutils.patch > cat <<-EOF > env.d > TARGET="${CTARGET}" > VER="${BVER}" >- LIBPATH="${LIBPATH}" >+ LIBPATH="${EPREFIX}/${LIBPATH}" > FAKE_TARGETS="${FAKE_TARGETS}" > EOF doesn't this result in double slashes ? shouldn't it be just ${EPREFIX}${LIBPATH} ?
Created attachment 359724 [details, diff] toolchain-binutils.patch Yes, it should. Patch refreshed. extra: adding ED and EROOT fallback +: ${ED:=${D}} +: ${EROOT:=${ROOT}}
(In reply to Benda Xu from comment #14) this looks fine. feel free to commit.
+ 30 Sep 2013; Christoph Junghans <ottxor@gentoo.org> + toolchain-binutils.eclass: + add prefix support +
Thanks Mike and Chris.
