Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Alexandre Oliva via Gcc-patches]

On Aug  3, 2022, Eric Gallager  wrote:

> OK, after reviewing the surrounding code, I think it makes sense; do
> you want to commit it, or should I?

Please proceed, if you have the cycles to give it a round of meaningful
testing (as in, including reconfigure with configure-generated as, and
also with external as in place).

Adjusting the analogous test patterns covering the other tools and
generated scripts would surely be welcome as well ;-)

Thanks!

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Eric Gallager via Gcc-patches]

On Tue, Aug 2, 2022 at 11:33 PM Alexandre Oliva  wrote:
>
> On Aug  2, 2022, Eric Gallager  wrote:
>
> > On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva  wrote:
>
> >> -elif test -x as$build_exeext; then
> >> +elif test -x as$build_exeext \
> >> +   && { test "x$build_exeext" != "x" \
> >> +|| test "x`grep '^# Invoke as, ld or nm from the build tree' \
> >> + as`" = "x"; }; then
> >>
> >> WDYT?
>
> > Hi, thanks for the feedback; I'm a bit confused, though: where exactly
> > would this proposed change go?
>
> gcc/configure.ac, just before:
>
> # Build using assembler in the current directory.
> gcc_cv_as=./as$build_exeext
>

OK, after reviewing the surrounding code, I think it makes sense; do
you want to commit it, or should I?
Thanks,
Eric

> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Alexandre Oliva via Gcc-patches]

On Aug  2, 2022, Eric Gallager  wrote:

> On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva  wrote:

>> -elif test -x as$build_exeext; then
>> +elif test -x as$build_exeext \
>> +   && { test "x$build_exeext" != "x" \
>> +|| test "x`grep '^# Invoke as, ld or nm from the build tree' \
>> + as`" = "x"; }; then
>> 
>> WDYT?

> Hi, thanks for the feedback; I'm a bit confused, though: where exactly
> would this proposed change go?

gcc/configure.ac, just before:

# Build using assembler in the current directory.
gcc_cv_as=./as$build_exeext

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Eric Gallager via Gcc-patches]

On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva  wrote:
>
> Hello, Eric,
>
> Thanks for looking into this.
>
> On Aug  1, 2022, Eric Gallager via Gcc-patches  
> wrote:
>
> >> This just reassigns the value that was retrieved a couple of lines
> >> above from the very same variable.
>
> > Oh, ok, so I guess this isn't necessary after all?
>
> Yeah, I don't see how this patch could make any difference as to the
> reported problem.
>
> > In which case we can just close 43301 as INVALID then?
>
> AFAICT, with_build_time_tools is dealt with in the top level configure,
> setting up *_FOR_TARGET after searching for the tool names in the
> specified location.
>
> However, there's a potentially confusing consequence of the current
> code: gcc/configure looks for ./as$build_exeext in the build tree, and
> uses that without overwriting it if found, so if an earlier configure
> run created an 'as' script, a reconfigure will just use it, without
> creating the file again, even if it would have changed
> ORIGINAL_AS_FOR_TARGET in it.
>
> I suppose if the patch was tested by the original submitter on a clean
> build tree, so it would *appear* to have made a difference in fixing the
> problem, while it was actually just a no-op, and the apparent fix was a
> consequence of the clean build tree.
>
> So, the patch is not useful, but we may want to avoid the confusing
> scenario somehow.
>
> I suppose the point of not creating such a tiny script again is not to
> avoid unnecessary rebuilding of dependencies (I don't even see how
> dependencies on the script would come into play), so creating it again
> wouldn't hurt.  However, we wish to avoid overwriting an assembler
> copied into the build tree for use by gcc during the build.  Perhaps:
>
> -elif test -x as$build_exeext; then
> +elif test -x as$build_exeext \
> +   && { test "x$build_exeext" != "x" \
> +|| test "x`grep '^# Invoke as, ld or nm from the build tree' \
> + as`" = "x"; }; then
>
> WDYT?

Hi, thanks for the feedback; I'm a bit confused, though: where exactly
would this proposed change go?

>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Alexandre Oliva via Gcc-patches]

Hello, Eric,

Thanks for looking into this.

On Aug  1, 2022, Eric Gallager via Gcc-patches  wrote:

>> This just reassigns the value that was retrieved a couple of lines
>> above from the very same variable.

> Oh, ok, so I guess this isn't necessary after all?

Yeah, I don't see how this patch could make any difference as to the
reported problem.

> In which case we can just close 43301 as INVALID then?

AFAICT, with_build_time_tools is dealt with in the top level configure,
setting up *_FOR_TARGET after searching for the tool names in the
specified location.

However, there's a potentially confusing consequence of the current
code: gcc/configure looks for ./as$build_exeext in the build tree, and
uses that without overwriting it if found, so if an earlier configure
run created an 'as' script, a reconfigure will just use it, without
creating the file again, even if it would have changed
ORIGINAL_AS_FOR_TARGET in it.

I suppose if the patch was tested by the original submitter on a clean
build tree, so it would *appear* to have made a difference in fixing the
problem, while it was actually just a no-op, and the apparent fix was a
consequence of the clean build tree.

So, the patch is not useful, but we may want to avoid the confusing
scenario somehow.

I suppose the point of not creating such a tiny script again is not to
avoid unnecessary rebuilding of dependencies (I don't even see how
dependencies on the script would come into play), so creating it again
wouldn't hurt.  However, we wish to avoid overwriting an assembler
copied into the build tree for use by gcc during the build.  Perhaps:

-elif test -x as$build_exeext; then
+elif test -x as$build_exeext \
+   && { test "x$build_exeext" != "x" \
+|| test "x`grep '^# Invoke as, ld or nm from the build tree' \
+ as`" = "x"; }; then

WDYT?

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Eric Gallager via Gcc-patches]

On Mon, Aug 1, 2022 at 3:54 AM Andreas Schwab  wrote:
>
> On Jul 31 2022, Eric Gallager via Gcc-patches wrote:
>
> > It just makes the configure script respect the --with-build-time-tools
> > flag.
>
> Why does it make any difference?
>

See the original bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43301

> > diff --git a/configure b/configure
> > index 65d7078dbe7..4d46b94ebc4 100755
> > --- a/configure
> > +++ b/configure
> > @@ -12850,7 +12850,9 @@ fi
> >  # Check whether --with-build-time-tools was given.
> >  if test "${with_build_time_tools+set}" = set; then :
> >withval=$with_build_time_tools; case x"$withval" in
> > - x/*) ;;
> > + x/*)
> > +   with_build_time_tools=$withval
>
> This just reassigns the value that was retrieved a couple of lines
> above from the very same variable.
>

Oh, ok, so I guess this isn't necessary after all? In which case we
can just close 43301 as INVALID then?

> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Re: [PATCH] configure: respect --with-build-time-tools [PR43301]

[Andreas Schwab via Gcc-patches]

On Jul 31 2022, Eric Gallager via Gcc-patches wrote:

> It just makes the configure script respect the --with-build-time-tools
> flag.

Why does it make any difference?

> diff --git a/configure b/configure
> index 65d7078dbe7..4d46b94ebc4 100755
> --- a/configure
> +++ b/configure
> @@ -12850,7 +12850,9 @@ fi
>  # Check whether --with-build-time-tools was given.
>  if test "${with_build_time_tools+set}" = set; then :
>withval=$with_build_time_tools; case x"$withval" in
> - x/*) ;;
> + x/*)
> +   with_build_time_tools=$withval

This just reassigns the value that was retrieved a couple of lines
above from the very same variable.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] configure: respect --with-build-time-tools [PR43301]

[Eric Gallager via Gcc-patches]

Hi, there's been a patch sitting in bug 43301 for over a decade that I
think still makes sense to apply, so I rebased it against current
trunk and found that it still applies. It just makes the configure
script respect the --with-build-time-tools flag. OK to commit?

ChangeLog:

PR bootstrap/43301
* configure: Regenerate.
* configure.ac: Respect --with-build-time-tools flag.


patch-configure_1.diff
Description: Binary data