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

2022-01-19 Thread Sam James


> On 19 Jan 2022, at 06:35, Mike Frysinger  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 
)
- 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 
> 
> 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



signature.asc
Description: Message signed with OpenPGP


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

2022-01-17 Thread 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 
Signed-off-by: Sam James 
---
 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