[gentoo-dev] [PATCH 1/4] autotools.eclass: don't inject -I${SYSROOT} to aclocal

[gentoo-dev] [PATCH 1/4] autotools.eclass: don't inject -I${SYSROOT} to aclocal

[Sam James]

When -I${SYSROOT} is injected, it'll override the default of -Im4, which
results in trying to install macros to ${SYSROOT} (a sandbox violation)
when they can't be found.

From aclocal(1):
```
       -I DIR add directory to search list for .m4 files

       --install
              copy third-party files to the first -I directory
```

The first directry is normally -Im4 if anything, whereas when injected
(when ${SYSROOT} is defined), it ends up being ${SYSROOT}, not m4 (so
we try to copy macros to somewhere outside of the build directory).

In EAPI 7+, this is almost always the case! We don't generally expect
to find macros (particularly things like autoconf-archive) in ${SYSROOT}
because that's for DEPEND-class dependencies, then they end up being
copied in unnecessarily and wrongly.

We could drop this just for < EAPI 7, but I'm not sure there's much
point there either. As Chewi observed in bug 677002, you can't
assume they'll be present in ${SYSROOT} anyway, and frankly,
the cross-compilation (and --root, --sysroot, and so on) situation
is rather bleak for earlier EAPIs, which is why we did all that
work for 7.

Bug: https://bugs.gentoo.org/710792
Closes: https://bugs.gentoo.org/677002
Closes: https://bugs.gentoo.org/738918
Thanks-to: James Le Cuirot <chewi@gentoo.org>
Signed-off-by: Sam James <sam@gentoo.org>
---
 eclass/autotools.eclass | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/eclass/autotools.eclass b/eclass/autotools.eclass
index 95c92cc6df8ca..e2572290f0cbe 100644
--- a/eclass/autotools.eclass
+++ b/eclass/autotools.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2021 Gentoo Authors
+# Copyright 1999-2022 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: autotools.eclass
@@ -666,12 +666,6 @@ autotools_m4sysdir_include() {
 	# First try to use the paths the system integrator has set up.
 	local paths=( $(eval echo ${AT_SYS_M4DIR}) )
 
-	if [[ ${#paths[@]} -eq 0 && -n ${SYSROOT} ]] ; then
-		# If they didn't give us anything, then default to the SYSROOT.
-		# This helps when cross-compiling.
-		local path="${SYSROOT}/usr/share/aclocal"
-		[[ -d ${path} ]] && paths+=( "${path}" )
-	fi
 	_autotools_m4dir_include "${paths[@]}"
 }
 
-- 
2.34.1

[gentoo-dev] [PATCH 2/4] autotools.eclass: use --system-acdir for aclocal

[Sam James]

We need to instruct aclocal that it might find macros in both
${BROOT} _and_ ${SYSROOT}.

- A classic example within BROOT is autoconf-archive.

- A classic example within SYSROOT is, say, libogg. A fair amount of
  codec software installs its own macro to help locating it (but this
  is in no ways limited to that genre/area).

  The correct position for a dependency like libogg is DEPEND, and yet
  the status quo doesn't mean that aclocal is obligated to check in ${ESYSROOT}
  which is where DEPEND-class dependencies are guaranteed to be installed.

  We can't rely on these being in BDEPEND -- in fact, most of the time,
  they won't be. If we wanted to rely on macros always being provided by
  BDEPEND, we'd have to duplicate a considerable number of dependencies
  in both BDEPEND + DEPEND, with the unnecessary cross-compilation that would
  entail too: it makes far more sense to just tell aclocal to look in the
  right place (an extra location).

Bug: https://bugs.gentoo.org/710792
Closes: https://bugs.gentoo.org/677002
Closes: https://bugs.gentoo.org/738918
Thanks-to: David Michael <fedora.dm0@gmail.com> (for the suggestion)
Thanks-to: James Le Cuirot <chewi@gentoo.org> (rubberducking & sounding board)
Signed-off-by: Sam James <sam@gentoo.org>
---
 eclass/autotools.eclass | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/eclass/autotools.eclass b/eclass/autotools.eclass
index e2572290f0cbe..2cf7c076d01ed 100644
--- a/eclass/autotools.eclass
+++ b/eclass/autotools.eclass
@@ -332,8 +332,22 @@ eaclocal_amflags() {
 # They also force installing the support files for safety.
 # Respects AT_M4DIR for additional directories to search for macros.
 eaclocal() {
+	# Feed in a list of paths:
+	# - ${BROOT}/usr/share/aclocal
+	# - ${ESYSROOT}/usr/share/aclocal
+	# See bug #677002
+	if [[ ! -f "${T}"/aclocal/dirlist ]] ; then
+		mkdir "${T}"/aclocal || die
+		cat <<- EOF > "${T}"/aclocal/dirlist || die
+			${BROOT}/usr/share/aclocal
+			${ESYSROOT}/usr/share/aclocal
+		EOF
+	fi
+
+	local system_acdir=" --system-acdir=${T}/aclocal"
+
 	[[ ! -f aclocal.m4 || -n $(grep -e 'generated.*by aclocal' aclocal.m4) ]] && \
-		autotools_run_tool --at-m4flags aclocal "$@" $(eaclocal_amflags)
+		autotools_run_tool --at-m4flags aclocal "$@" $(eaclocal_amflags) ${system_acdir}
 }
 
 # @FUNCTION: _elibtoolize
-- 
2.34.1

Re: [gentoo-dev] [PATCH 2/4] autotools.eclass: use --system-acdir for aclocal

[Sam James]

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



> On 17 Jan 2022, at 11:09, Sam James <sam@gentoo.org> wrote:
> 
> We need to instruct aclocal that it might find macros in both
> ${BROOT} _and_ ${SYSROOT}.
> 
> - A classic example within BROOT is autoconf-archive.
> 
> - A classic example within SYSROOT is, say, libogg. A fair amount of
>  codec software installs its own macro to help locating it (but this
>  is in no ways limited to that genre/area).
> 
>  The correct position for a dependency like libogg is DEPEND, and yet
>  the status quo doesn't mean that aclocal is obligated to check in ${ESYSROOT}
>  which is where DEPEND-class dependencies are guaranteed to be installed.
> 
>  We can't rely on these being in BDEPEND -- in fact, most of the time,
>  they won't be. If we wanted to rely on macros always being provided by
>  BDEPEND, we'd have to duplicate a considerable number of dependencies
>  in both BDEPEND + DEPEND, with the unnecessary cross-compilation that would
>  entail too: it makes far more sense to just tell aclocal to look in the
>  right place (an extra location).
> 
> Bug: https://bugs.gentoo.org/710792
> Closes: https://bugs.gentoo.org/677002
> Closes: https://bugs.gentoo.org/738918
> Thanks-to: David Michael <fedora.dm0@gmail.com> (for the suggestion)
> Thanks-to: James Le Cuirot <chewi@gentoo.org> (rubberducking & sounding board)
> Signed-off-by: Sam James <sam@gentoo.org>
> ---
> eclass/autotools.eclass | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/eclass/autotools.eclass b/eclass/autotools.eclass
> index e2572290f0cbe..2cf7c076d01ed 100644
> --- a/eclass/autotools.eclass
> +++ b/eclass/autotools.eclass
> @@ -332,8 +332,22 @@ eaclocal_amflags() {
> # They also force installing the support files for safety.
> # Respects AT_M4DIR for additional directories to search for macros.
> eaclocal() {
> +	# Feed in a list of paths:
> +	# - ${BROOT}/usr/share/aclocal
> +	# - ${ESYSROOT}/usr/share/aclocal
> +	# See bug #677002
> +	if [[ ! -f "${T}"/aclocal/dirlist ]] ; then
> +		mkdir "${T}"/aclocal || die
> +		cat <<- EOF > "${T}"/aclocal/dirlist || die
> +			${BROOT}/usr/share/aclocal
> +			${ESYSROOT}/usr/share/aclocal
> +		EOF
> +	fi
> +
> +	local system_acdir=" --system-acdir=${T}/aclocal"
> +
> 	[[ ! -f aclocal.m4 || -n $(grep -e 'generated.*by aclocal' aclocal.m4) ]] && \
> -		autotools_run_tool --at-m4flags aclocal "$@" $(eaclocal_amflags)
> +		autotools_run_tool --at-m4flags aclocal "$@" $(eaclocal_amflags) ${system_acdir}
> }

I've changed this locally to just skip the add-system-acdir-logic for older EAPIs. Forgot to amend the commit,
Chewi had already pointed this out privately.

best,
sam


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

[gentoo-dev] [PATCH 3/4] autotools.eclass: update for latest automake 1.16.4

[Sam James]

Signed-off-by: Sam James <sam@gentoo.org>
---
 eclass/autotools.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/autotools.eclass b/eclass/autotools.eclass
index 2cf7c076d01ed..5250b28042ee2 100644
--- a/eclass/autotools.eclass
+++ b/eclass/autotools.eclass
@@ -74,7 +74,7 @@ inherit gnuconfig libtool
 # Do NOT change this variable in your ebuilds!
 # If you want to force a newer minor version, you can specify the correct
 # WANT value by using a colon:  <PV>:<WANT_AUTOMAKE>
-_LATEST_AUTOMAKE=( 1.16.2-r1:1.16 )
+_LATEST_AUTOMAKE=( 1.16.4:1.16 )
 
 _automake_atom="sys-devel/automake"
 _autoconf_atom="sys-devel/autoconf"
-- 
2.34.1

[gentoo-dev] [PATCH 4/4] autotools.eclass: update for autoconf 2.71

[Sam James]

Closes: https://bugs.gentoo.org/827852
Signed-off-by: Sam James <sam@gentoo.org>
---
 eclass/autotools.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/autotools.eclass b/eclass/autotools.eclass
index 5250b28042ee2..5568cca505d78 100644
--- a/eclass/autotools.eclass
+++ b/eclass/autotools.eclass
@@ -95,7 +95,7 @@ if [[ -n ${WANT_AUTOCONF} ]] ; then
 		none)       _autoconf_atom="" ;; # some packages don't require autoconf at all
 		2.1)        _autoconf_atom="~sys-devel/autoconf-2.13" ;;
 		# if you change the "latest" version here, change also autotools_env_setup
-		latest|2.5) _autoconf_atom=">=sys-devel/autoconf-2.69" ;;
+		latest|2.5) _autoconf_atom=">=sys-devel/autoconf-2.71" ;;
 		*)          die "Invalid WANT_AUTOCONF value '${WANT_AUTOCONF}'" ;;
 	esac
 	export WANT_AUTOCONF
@@ -524,7 +524,7 @@ autotools_env_setup() {
 		[[ ${WANT_AUTOMAKE} == "latest" ]] && \
 			die "Cannot find the latest automake! Tried ${_LATEST_AUTOMAKE[*]}"
 	fi
-	[[ ${WANT_AUTOCONF} == "latest" ]] && export WANT_AUTOCONF=2.5
+	[[ ${WANT_AUTOCONF} == "latest" ]] && export WANT_AUTOCONF=2.71
 }
 
 # @FUNCTION: autotools_run_tool
-- 
2.34.1

[gentoo-dev] Re: [PATCH 1/4] autotools.eclass: don't inject -I${SYSROOT} to aclocal

[Mike Frysinger]

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

On 17 Jan 2022 11:09, Sam James wrote:
> When -I${SYSROOT} is injected, it'll override the default of -Im4, which
> results in trying to install macros to ${SYSROOT} (a sandbox violation)
> when they can't be found.
> 
> From aclocal(1):
> ```
>        -I DIR add directory to search list for .m4 files
> 
>        --install
>               copy third-party files to the first -I directory
> ```
> 
> The first directry is normally -Im4 if anything, whereas when injected
> (when ${SYSROOT} is defined), it ends up being ${SYSROOT}, not m4 (so
> we try to copy macros to somewhere outside of the build directory).

we should define the semantics we want and bring it upstream to get into
automake.  although it seems like ACLOCAL_PATH might work well enough
for us now to switch to that.

as a stop gap, it seems like the use of --install is pretty low ?  we're
cross-compiling about ~2.5k packages in CrOS every day and never seen a
failure here.  so the few packages which are running into troubles can
workaround it by setting AT_SYS_M4DIR right ?

> In EAPI 7+, this is almost always the case! We don't generally expect
> to find macros (particularly things like autoconf-archive) in ${SYSROOT}
> because that's for DEPEND-class dependencies, then they end up being
> copied in unnecessarily and wrongly.

i think this optimism is misplaced.  libraries often install m4 files
which is precisely why this logic is in here.
https://bugs.gentoo.org/677002#c10

deleting this check will break things.  prob more than we're fixing.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH 1/4] autotools.eclass: don't inject -I${SYSROOT} to aclocal

[Sam James]

[-- Attachment #1.1: Type: text/plain, Size: 2601 bytes --]



> On 19 Jan 2022, at 06:35, Mike Frysinger <vapier@gentoo.org> wrote:
> 
> On 17 Jan 2022 11:09, Sam James wrote:
>> When -I${SYSROOT} is injected, it'll override the default of -Im4, which
>> results in trying to install macros to ${SYSROOT} (a sandbox violation)
>> when they can't be found.
>> 
>> From aclocal(1):
>> ```
>>       -I DIR add directory to search list for .m4 files
>> 
>>       --install
>>              copy third-party files to the first -I directory
>> ```
>> 
>> The first directry is normally -Im4 if anything, whereas when injected
>> (when ${SYSROOT} is defined), it ends up being ${SYSROOT}, not m4 (so
>> we try to copy macros to somewhere outside of the build directory).
> 
> we should define the semantics we want and bring it upstream to get into
> automake.  although it seems like ACLOCAL_PATH might work well enough
> for us now to switch to that.
> 
> as a stop gap, it seems like the use of --install is pretty low ?  we're
> cross-compiling about ~2.5k packages in CrOS every day and never seen a
> failure here.  so the few packages which are running into troubles can
> workaround it by setting AT_SYS_M4DIR right ?

I've only seen it in the wild with:
- app-crypt/tpm2-tss (https://bugs.gentoo.org/756211 <https://bugs.gentoo.org/756211>)
- another package which I hit during "normal" use but I'm afraid I can't
recall what. I suspect I hit it before Python grew a BDEPEND on autoconf-archive
so we're less likely to hit it now.

But I accept it's niche. See below though, I think we agree that AT_SYS_M4DIR /
system acdir should be satisfactory here.

I don't mind keeping the old logic for < EAPI 7 if that'll help you in CrOS though.

> 
>> In EAPI 7+, this is almost always the case! We don't generally expect
>> to find macros (particularly things like autoconf-archive) in ${SYSROOT}
>> because that's for DEPEND-class dependencies, then they end up being
>> copied in unnecessarily and wrongly.
> 
> i think this optimism is misplaced.  libraries often install m4 files
> which is precisely why this logic is in here.
> https://bugs.gentoo.org/677002#c10 <https://bugs.gentoo.org/677002#c10>
> 
> deleting this check will break things.  prob more than we're fixing.

Sorry, you're absolutely right here!

But I think it's addressed by the system-acdir commit that follows it up.

i.e. the commit message is totally wrong (and I'll fix this), but the change is correct
given we follow it up with seemingly a better way of handling
the original case.

Does that sound right?

Best,
sam


[-- Attachment #1.2: Type: text/html, Size: 4768 bytes --]

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]