Re: [gentoo-dev] [PATCH 2/2] Support wrapping headers for multilib ABIs.

[Alexis Ballier]

On Sat, 23 Mar 2013 17:26:38 +0100
Michał Górny <mgorny@g.o> wrote:

> ---
>  gx86/eclass/autotools-multilib.eclass | 82
> +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+)
>
> diff --git a/gx86/eclass/autotools-multilib.eclass
> b/gx86/eclass/autotools-multilib.eclass index d7372b0..c65c777 100644
> --- a/gx86/eclass/autotools-multilib.eclass
> +++ b/gx86/eclass/autotools-multilib.eclass


why not multilib-build ?


> @@ -33,6 +33,28 @@ inherit autotools-utils multilib-build
>
>  EXPORT_FUNCTIONS src_prepare src_configure src_compile src_test
> src_install
> +# @ECLASS-VARIABLE: MULTILIB_WRAPPED_HEADERS
> +# @DESCRIPTION:
> +# A list of headers to wrap for multilib support. The listed headers
> +# will be moved to a non-standard location and replace with a file
> +# including them conditionally to current ABI.
> +#
> +# This variable has to be a bash array. Paths shall be relative to
> +# installation root (${D}), and name regular files. Recursive
> wrapping +# is not supported.
> +#
> +# Please note that header wrapping is *discouraged*. It is preferred
> to +# install all headers in a subdirectory of libdir and use
> pkg-config to +# locate the headers. Some C preprocessors will not
> work with wrapped +# headers.
> +#
> +# Example:
> +# @CODE
> +# MULTILIB_WRAPPED_HEADERS=(
> +#	/usr/include/foobar/config.h
> +# )
> +# @CODE
> +
>  autotools-multilib_src_prepare() {
>  	autotools-utils_src_prepare "${@}"
>  }
> @@ -49,13 +71,73 @@ autotools-multilib_src_test() {
>  	multilib_foreach_abi autotools-utils_src_test "${@}"
>  }
>
> +_autotools-multilib_wrap_headers() {
> +	debug-print-function ${FUNCNAME} "$@"
> +	local f
> +
> +	for f in "${MULTILIB_WRAPPED_HEADERS[@]}"; do
> +		# drop leading slash if it's there
> +		f=${f#/}
> +
> +		if [[ ${f} != usr/include/* ]]; then
> +			die "Wrapping headers outside
> of /usr/include is not supported at the moment."
> +		fi

Do you really want to support this at some point? Why?
I'd just go for paths relative to $EPREFIX/usr/include (or
$ED/usr/include) for MULTILIB_WRAPPED_HEADERS. That would simplify the
code.


> +		# and then usr/include
> +		f=${f#usr/include/}
> +
> +		local dir=${f%/*}
> +
> +		# $CHOST shall be set by multilib_toolchain_setup
> +		dodir "/tmp/multilib-include/${CHOST}/${dir}"
> +		mv "${ED}/usr/include/${f}"
> "${ED}/tmp/multilib-include/${CHOST}/${dir}/" || die +

why not use $T rather than $ED/tmp ?

> +		if [[ ! -f ${ED}/tmp/multilib-include/${f} ]]; then
> +			dodir "/tmp/multilib-include/${dir}"
> +			cat > "${ED}/tmp/multilib-include/${f}"
> <<_EOF_ || die +/* This file is auto-generated by
> autotools-multilib.eclass
> + * as a multilib-friendly wrapper to /${f}. For the original content,
> + * please see the files that are #included below.
> + */
> +_EOF_
> +		fi
> +
> +		local defs
> +		case "${ABI}" in

didn't you say $ABI may have name collisions?
considering the code below, it seems much safer to match on $CHOST

[...]

It'd be nice to have an attempt to support all the ABIs gentoo supports
in that file: I'd prefer to spot possible problems with this solution
vs. sedding a template for example to be seen before having to rewrite
it.
For example, IIRC, ppc64 defines both __powerpc__ and __powerpc64__,
the natural way would be:
#if defined(__powerpc64__)
ppc64 stuff
#elif defined(__powerpc__)
ppc stuff
#endif

with your approach that'd be:
#if defined(__powerpc__) && !defined(__powerpc64__)
ppc stuff
#elif defined(__powerpc64__)
ppc64 stuff
#endif


doing with a template has its disadvantages but allows more flexibility
in how the #ifery is written; I don't want to realize your approach
cannot deal with some weird arches after it has been deployed.


thanks for working on this,

Alexis.