Re: [PATCHv2] top-level configure: setup target_configdirs based on repository

2021-09-28 Thread Andrew Burgess
* Richard Biener  [2021-09-27 10:23:50 +0200]:

> On Fri, Sep 24, 2021 at 12:34 PM Andrew Burgess
>  wrote:
> >
> > * Thomas Schwinge  [2021-09-23 11:29:05 +0200]:
> >
> > > Hi!
> > >
> > > I only had a curious look here; hope that's still useful.
> > >
> > > On 2021-09-22T16:30:42+0100, Andrew Burgess  
> > > wrote:
> > > > The top-level configure script is shared between the gcc repository
> > > > and the binutils-gdb repository.
> > > >
> > > > The target_configdirs variable in the configure.ac script, defines
> > > > sub-directories that contain components that should be built for the
> > > > target using the target tools.
> > > >
> > > > Some components, e.g. zlib, are built as both host and target
> > > > libraries.
> > > >
> > > > This causes problems for binutils-gdb.  If we run 'make all' in the
> > > > binutils-gdb repository we end up trying to build a target version of
> > > > the zlib library, which requires the target compiler be available.
> > > > Often the target compiler isn't immediately available, and so the
> > > > build fails.
> > >
> > > I did wonder: shouldn't normally these target libraries be masked out via
> > > 'noconfigdirs' (see 'Handle --disable- generically' section),
> > > via 'enable_[...]' being set to 'no'?  But I think I now see the problem
> > > here: the 'enable_[...]' variables guard both the host and target library
> > > build!  (... if I'm quickly understanding that correctly...)
> > >
> > > ... and you do need the host zlib, thus '$enable_zlib != no'.
> > >
> > > > The problem with zlib impacted a previous attempt to synchronise the
> > > > top-level configure scripts from gcc to binutils-gdb, see this thread:
> > > >
> > > >   https://sourceware.org/pipermail/binutils/2019-May/107094.html
> > > >
> > > > And I'm in the process of importing libbacktrace in to binutils-gdb,
> > > > which is also a host and target library, and triggers the same issues.
> > > >
> > > > I believe that for binutils-gdb, at least at the moment, there are no
> > > > target libraries that we need to build.
> > > >
> > > > My proposal then is to make the value of target_libraries change based
> > > > on which repository we are building in.  Specifically, if the source
> > > > tree has a gcc/ directory then we should set the target_libraries
> > > > variable, otherwise this variable is left entry.
> > > >
> > > > I think that if someone tries to create a single unified tree (gcc +
> > > > binutils-gdb in a single source tree) and then build, this change will
> > > > not have a negative impact, the tree still has gcc/ so we'd expect the
> > > > target compiler to be built, which means building the target_libraries
> > > > should work just fine.
> > > >
> > > > However, if the source tree lacks gcc/ then we assume the target
> > > > compiler isn't built/available, and so target_libraries shouldn't be
> > > > built.
> > > >
> > > > There is already precedent within configure.ac for check on the
> > > > existence of gcc/ in the source tree, see the handling of
> > > > -enable-werror around line 3658.
> > >
> > > (I understand that one to just guard the 'cat $srcdir/gcc/DEV-PHASE',
> > > tough.)
> > >
> > > > I've tested a build of gcc on x86-64, and the same set of target
> > > > libraries still seem to get built.  On binutils-gdb this change
> > > > resolves the issues with 'make all'.
> > > >
> > > > Any thoughts?
> > >
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -180,9 +180,17 @@ target_tools="target-rda"
> > > >  ## We assign ${configdirs} this way to remove all embedded newlines.  
> > > > This
> > > >  ## is important because configure will choke if they ever get through.
> > > >  ## ${configdirs} is directories we build using the host tools.
> > > > -## ${target_configdirs} is directories we build using the target tools.
> > > > +##
> > > > +## ${target_configdirs} is directories we build using the target
> > > > +## tools, these are only needed when working in the gcc tree.  This
> > > > +## file is also reused in the binutils-gdb tree, where building any
> > > > +## target stuff doesn't make sense.
> > > >  configdirs=`echo ${host_libs} ${host_tools}`
> > > > -target_configdirs=`echo ${target_libraries} ${target_tools}`
> > > > +if test -d ${srcdir}/gcc; then
> > > > +  target_configdirs=`echo ${target_libraries} ${target_tools}`
> > > > +else
> > > > +  target_configdirs=""
> > > > +fi
> > > >  build_configdirs=`echo ${build_libs} ${build_tools}`
> > >
> > > What I see is that after this, there are still occasions where inside
> > > 'case "${target}"', 'target_configdirs' gets amended, so those won't be
> > > caught by your approach?
> >
> > Good point, I'd failed to spot these.
> >
> > >
> > > Instead of erasing 'target_configdirs' as you've posted, and
> > > understanding that we can't just instead add all the "offending" ones to
> > > 'noconfigdirs' for '! test -d "$srcdir"/gcc/' (because that would also
> > > disable them for host usage),
> >
> > Great idea, 

Re: [PATCHv2] top-level configure: setup target_configdirs based on repository

2021-09-27 Thread Richard Biener via Gcc-patches
On Fri, Sep 24, 2021 at 12:34 PM Andrew Burgess
 wrote:
>
> * Thomas Schwinge  [2021-09-23 11:29:05 +0200]:
>
> > Hi!
> >
> > I only had a curious look here; hope that's still useful.
> >
> > On 2021-09-22T16:30:42+0100, Andrew Burgess  
> > wrote:
> > > The top-level configure script is shared between the gcc repository
> > > and the binutils-gdb repository.
> > >
> > > The target_configdirs variable in the configure.ac script, defines
> > > sub-directories that contain components that should be built for the
> > > target using the target tools.
> > >
> > > Some components, e.g. zlib, are built as both host and target
> > > libraries.
> > >
> > > This causes problems for binutils-gdb.  If we run 'make all' in the
> > > binutils-gdb repository we end up trying to build a target version of
> > > the zlib library, which requires the target compiler be available.
> > > Often the target compiler isn't immediately available, and so the
> > > build fails.
> >
> > I did wonder: shouldn't normally these target libraries be masked out via
> > 'noconfigdirs' (see 'Handle --disable- generically' section),
> > via 'enable_[...]' being set to 'no'?  But I think I now see the problem
> > here: the 'enable_[...]' variables guard both the host and target library
> > build!  (... if I'm quickly understanding that correctly...)
> >
> > ... and you do need the host zlib, thus '$enable_zlib != no'.
> >
> > > The problem with zlib impacted a previous attempt to synchronise the
> > > top-level configure scripts from gcc to binutils-gdb, see this thread:
> > >
> > >   https://sourceware.org/pipermail/binutils/2019-May/107094.html
> > >
> > > And I'm in the process of importing libbacktrace in to binutils-gdb,
> > > which is also a host and target library, and triggers the same issues.
> > >
> > > I believe that for binutils-gdb, at least at the moment, there are no
> > > target libraries that we need to build.
> > >
> > > My proposal then is to make the value of target_libraries change based
> > > on which repository we are building in.  Specifically, if the source
> > > tree has a gcc/ directory then we should set the target_libraries
> > > variable, otherwise this variable is left entry.
> > >
> > > I think that if someone tries to create a single unified tree (gcc +
> > > binutils-gdb in a single source tree) and then build, this change will
> > > not have a negative impact, the tree still has gcc/ so we'd expect the
> > > target compiler to be built, which means building the target_libraries
> > > should work just fine.
> > >
> > > However, if the source tree lacks gcc/ then we assume the target
> > > compiler isn't built/available, and so target_libraries shouldn't be
> > > built.
> > >
> > > There is already precedent within configure.ac for check on the
> > > existence of gcc/ in the source tree, see the handling of
> > > -enable-werror around line 3658.
> >
> > (I understand that one to just guard the 'cat $srcdir/gcc/DEV-PHASE',
> > tough.)
> >
> > > I've tested a build of gcc on x86-64, and the same set of target
> > > libraries still seem to get built.  On binutils-gdb this change
> > > resolves the issues with 'make all'.
> > >
> > > Any thoughts?
> >
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -180,9 +180,17 @@ target_tools="target-rda"
> > >  ## We assign ${configdirs} this way to remove all embedded newlines.  
> > > This
> > >  ## is important because configure will choke if they ever get through.
> > >  ## ${configdirs} is directories we build using the host tools.
> > > -## ${target_configdirs} is directories we build using the target tools.
> > > +##
> > > +## ${target_configdirs} is directories we build using the target
> > > +## tools, these are only needed when working in the gcc tree.  This
> > > +## file is also reused in the binutils-gdb tree, where building any
> > > +## target stuff doesn't make sense.
> > >  configdirs=`echo ${host_libs} ${host_tools}`
> > > -target_configdirs=`echo ${target_libraries} ${target_tools}`
> > > +if test -d ${srcdir}/gcc; then
> > > +  target_configdirs=`echo ${target_libraries} ${target_tools}`
> > > +else
> > > +  target_configdirs=""
> > > +fi
> > >  build_configdirs=`echo ${build_libs} ${build_tools}`
> >
> > What I see is that after this, there are still occasions where inside
> > 'case "${target}"', 'target_configdirs' gets amended, so those won't be
> > caught by your approach?
>
> Good point, I'd failed to spot these.
>
> >
> > Instead of erasing 'target_configdirs' as you've posted, and
> > understanding that we can't just instead add all the "offending" ones to
> > 'noconfigdirs' for '! test -d "$srcdir"/gcc/' (because that would also
> > disable them for host usage),
>
> Great idea, this is what I've done in the revised patch below.
>
> >I wonder if it'd make sense to turn all
> > existing 'target_libraries=[...]' and 'target_tools=[...]' assignments
> > and later amendments into '[...]_gcc=[...]' variants, with 

Re: [PATCHv2] top-level configure: setup target_configdirs based on repository

2021-09-27 Thread Thomas Schwinge
Hi!

On 2021-09-24T11:34:34+0100, Andrew Burgess  wrote:
> The V2 patch below:
>
>   - Moves the check for gcc/ to much later in the configure script,
> after we've finished building target_configdirs,
>
>   - Makes use of skipdirs to avoid building anything from
> target_configdirs if we're not also building gcc.

Thanks, that looks better in line with how that script generally appears
to work (... per my not-in-depth understanding).  (But I can't formally
approve.)

Reviewed-by: Thomas Schwinge 


Grüße
 Thomas


> commit 84c8b7f1605c8f2840d3c857a4d86abc7dde0668
> Author: Andrew Burgess 
> Date:   Wed Sep 22 15:15:41 2021 +0100
>
> top-level configure: setup target_configdirs based on repository
>
> The top-level configure script is shared between the gcc repository
> and the binutils-gdb repository.
>
> The target_configdirs variable in the configure.ac script, defines
> sub-directories that contain components that should be built for the
> target using the target tools.
>
> Some components, e.g. zlib, are built as both host and target
> libraries.
>
> This causes problems for binutils-gdb.  If we run 'make all' in the
> binutils-gdb repository we end up trying to build a target version of
> the zlib library, which requires the target compiler be available.
> Often the target compiler isn't immediately available, and so the
> build fails.
>
> The problem with zlib impacted a previous attempt to synchronise the
> top-level configure scripts from gcc to binutils-gdb, see this thread:
>
>   https://sourceware.org/pipermail/binutils/2019-May/107094.html
>
> And I'm in the process of importing libbacktrace in to binutils-gdb,
> which is also a host and target library, and triggers the same issues.
>
> I believe that for binutils-gdb, at least at the moment, there are no
> target libraries that we need to build.
>
> In the configure script we build three lists of things we want to
> build, $configdirs, $build_configdirs, and $target_configdirs, we also
> build two lists of things we don't want to build, $skipdirs and
> $noconfigdirs.  We then remove anything that is in the lists of things
> not to build, from the list of things that should be built.
>
> My proposal is to add everything in target_configdirs into skipdirs,
> if the source tree doesn't contain a gcc/ sub-directory.  The result
> is that for binutils-gdb no target tools or libraries will be built,
> while for the gcc repository, nothing should change.
>
> If a user builds a unified source tree, then the target tools and
> libraries should still be built as the gcc/ directory will be present.
>
> I've tested a build of gcc on x86-64, and the same set of target
> libraries still seem to get built.  On binutils-gdb this change
> resolves the issues with 'make all'.
>
> ChangeLog:
>
> * configure: Regenerate.
> * configure.ac (skipdirs): Add the contents of target_configdirs 
> if
> we are not building gcc.
>
> diff --git a/configure b/configure
> index 85ab9915402..785498efff5 100755
> --- a/configure
> +++ b/configure
> @@ -8874,6 +8874,16 @@ case ,${enable_languages}, in
>  ;;
>  esac
>
> +# If gcc/ is not in the source tree then we'll not be building a
> +# target compiler, assume in that case we don't want to build any
> +# target libraries or tools.
> +#
> +# This was added primarily for the benefit for binutils-gdb who reuse
> +# this configure script, but don't always have target tools available.
> +if test ! -d ${srcdir}/gcc; then
> +   skipdirs="${skipdirs} ${target_configdirs}"
> +fi
> +
>  # Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
>  # $build_configdirs and $target_configdirs.
>  # If we have the source for $noconfigdirs entries, add them to $notsupp.
> diff --git a/configure.ac b/configure.ac
> index 1df038b04f3..c523083c346 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2272,6 +2272,16 @@ case ,${enable_languages}, in
>  ;;
>  esac
>
> +# If gcc/ is not in the source tree then we'll not be building a
> +# target compiler, assume in that case we don't want to build any
> +# target libraries or tools.
> +#
> +# This was added primarily for the benefit for binutils-gdb who reuse
> +# this configure script, but don't always have target tools available.
> +if test ! -d ${srcdir}/gcc; then
> +   skipdirs="${skipdirs} ${target_configdirs}"
> +fi
> +
>  # Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
>  # $build_configdirs and $target_configdirs.
>  # If we have the source for $noconfigdirs entries, add them to $notsupp.
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955


[PATCHv2] top-level configure: setup target_configdirs based on repository

2021-09-24 Thread Andrew Burgess
* Thomas Schwinge  [2021-09-23 11:29:05 +0200]:

> Hi!
> 
> I only had a curious look here; hope that's still useful.
> 
> On 2021-09-22T16:30:42+0100, Andrew Burgess  
> wrote:
> > The top-level configure script is shared between the gcc repository
> > and the binutils-gdb repository.
> >
> > The target_configdirs variable in the configure.ac script, defines
> > sub-directories that contain components that should be built for the
> > target using the target tools.
> >
> > Some components, e.g. zlib, are built as both host and target
> > libraries.
> >
> > This causes problems for binutils-gdb.  If we run 'make all' in the
> > binutils-gdb repository we end up trying to build a target version of
> > the zlib library, which requires the target compiler be available.
> > Often the target compiler isn't immediately available, and so the
> > build fails.
> 
> I did wonder: shouldn't normally these target libraries be masked out via
> 'noconfigdirs' (see 'Handle --disable- generically' section),
> via 'enable_[...]' being set to 'no'?  But I think I now see the problem
> here: the 'enable_[...]' variables guard both the host and target library
> build!  (... if I'm quickly understanding that correctly...)
> 
> ... and you do need the host zlib, thus '$enable_zlib != no'.
> 
> > The problem with zlib impacted a previous attempt to synchronise the
> > top-level configure scripts from gcc to binutils-gdb, see this thread:
> >
> >   https://sourceware.org/pipermail/binutils/2019-May/107094.html
> >
> > And I'm in the process of importing libbacktrace in to binutils-gdb,
> > which is also a host and target library, and triggers the same issues.
> >
> > I believe that for binutils-gdb, at least at the moment, there are no
> > target libraries that we need to build.
> >
> > My proposal then is to make the value of target_libraries change based
> > on which repository we are building in.  Specifically, if the source
> > tree has a gcc/ directory then we should set the target_libraries
> > variable, otherwise this variable is left entry.
> >
> > I think that if someone tries to create a single unified tree (gcc +
> > binutils-gdb in a single source tree) and then build, this change will
> > not have a negative impact, the tree still has gcc/ so we'd expect the
> > target compiler to be built, which means building the target_libraries
> > should work just fine.
> >
> > However, if the source tree lacks gcc/ then we assume the target
> > compiler isn't built/available, and so target_libraries shouldn't be
> > built.
> >
> > There is already precedent within configure.ac for check on the
> > existence of gcc/ in the source tree, see the handling of
> > -enable-werror around line 3658.
> 
> (I understand that one to just guard the 'cat $srcdir/gcc/DEV-PHASE',
> tough.)
> 
> > I've tested a build of gcc on x86-64, and the same set of target
> > libraries still seem to get built.  On binutils-gdb this change
> > resolves the issues with 'make all'.
> >
> > Any thoughts?
> 
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -180,9 +180,17 @@ target_tools="target-rda"
> >  ## We assign ${configdirs} this way to remove all embedded newlines.  This
> >  ## is important because configure will choke if they ever get through.
> >  ## ${configdirs} is directories we build using the host tools.
> > -## ${target_configdirs} is directories we build using the target tools.
> > +##
> > +## ${target_configdirs} is directories we build using the target
> > +## tools, these are only needed when working in the gcc tree.  This
> > +## file is also reused in the binutils-gdb tree, where building any
> > +## target stuff doesn't make sense.
> >  configdirs=`echo ${host_libs} ${host_tools}`
> > -target_configdirs=`echo ${target_libraries} ${target_tools}`
> > +if test -d ${srcdir}/gcc; then
> > +  target_configdirs=`echo ${target_libraries} ${target_tools}`
> > +else
> > +  target_configdirs=""
> > +fi
> >  build_configdirs=`echo ${build_libs} ${build_tools}`
> 
> What I see is that after this, there are still occasions where inside
> 'case "${target}"', 'target_configdirs' gets amended, so those won't be
> caught by your approach?

Good point, I'd failed to spot these.

> 
> Instead of erasing 'target_configdirs' as you've posted, and
> understanding that we can't just instead add all the "offending" ones to
> 'noconfigdirs' for '! test -d "$srcdir"/gcc/' (because that would also
> disable them for host usage),

Great idea, this is what I've done in the revised patch below.

>I wonder if it'd make sense to turn all
> existing 'target_libraries=[...]' and 'target_tools=[...]' assignments
> and later amendments into '[...]_gcc=[...]' variants, with potentially
> further variants existing -- but probably not, because won't you always
> need the target GCC to be able to build target libraries ;-) -- and then,
> where we finally evalue '$target_libraries' and '$target_tools', only
> evaluate the '[...]_gcc' variants