* [PATCH] configure: respect --with-build-time-tools [PR43301] @ 2022-07-31 20:55 Eric Gallager 2022-08-01 7:54 ` Andreas Schwab 0 siblings, 1 reply; 8+ messages in thread From: Eric Gallager @ 2022-07-31 20:55 UTC (permalink / raw) To: gcc-patches; +Cc: Paolo Bonzini, neroden, aoliva, Ralf.Wildenhues [-- Attachment #1: Type: text/plain, Size: 390 bytes --] 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. [-- Attachment #2: patch-configure_1.diff --] [-- Type: application/octet-stream, Size: 1040 bytes --] 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 + ;; *) with_build_time_tools= { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: argument to --with-build-time-tools must be an absolute path" >&5 diff --git a/configure.ac b/configure.ac index 371def9b421..96a878a14cb 100644 --- a/configure.ac +++ b/configure.ac @@ -3600,7 +3600,9 @@ AC_ARG_WITH([build-time-tools], [AS_HELP_STRING([--with-build-time-tools=PATH], [use given path to find target tools during the build])], [case x"$withval" in - x/*) ;; + x/*) + with_build_time_tools=$withval + ;; *) with_build_time_tools= AC_MSG_WARN([argument to --with-build-time-tools must be an absolute path]) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-07-31 20:55 [PATCH] configure: respect --with-build-time-tools [PR43301] Eric Gallager @ 2022-08-01 7:54 ` Andreas Schwab 2022-08-01 15:40 ` Eric Gallager 0 siblings, 1 reply; 8+ messages in thread From: Andreas Schwab @ 2022-08-01 7:54 UTC (permalink / raw) To: Eric Gallager via Gcc-patches Cc: Eric Gallager, Paolo Bonzini, aoliva, neroden 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, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-01 7:54 ` Andreas Schwab @ 2022-08-01 15:40 ` Eric Gallager 2022-08-02 5:24 ` Alexandre Oliva 0 siblings, 1 reply; 8+ messages in thread From: Eric Gallager @ 2022-08-01 15:40 UTC (permalink / raw) To: Andreas Schwab Cc: Eric Gallager via Gcc-patches, Paolo Bonzini, aoliva, neroden On Mon, Aug 1, 2022 at 3:54 AM Andreas Schwab <schwab@suse.de> 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, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different." ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-01 15:40 ` Eric Gallager @ 2022-08-02 5:24 ` Alexandre Oliva 2022-08-02 12:19 ` Eric Gallager 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Oliva @ 2022-08-02 5:24 UTC (permalink / raw) To: Eric Gallager via Gcc-patches Cc: Andreas Schwab, Eric Gallager, Paolo Bonzini, neroden Hello, Eric, Thanks for looking into this. On Aug 1, 2022, Eric Gallager via Gcc-patches <gcc-patches@gcc.gnu.org> 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 hacker https://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 <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-02 5:24 ` Alexandre Oliva @ 2022-08-02 12:19 ` Eric Gallager 2022-08-03 3:33 ` Alexandre Oliva 0 siblings, 1 reply; 8+ messages in thread From: Eric Gallager @ 2022-08-02 12:19 UTC (permalink / raw) To: Alexandre Oliva Cc: Eric Gallager via Gcc-patches, Andreas Schwab, Paolo Bonzini, neroden On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva <oliva@gnu.org> wrote: > > Hello, Eric, > > Thanks for looking into this. > > On Aug 1, 2022, Eric Gallager via Gcc-patches <gcc-patches@gcc.gnu.org> 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 hacker https://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 <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-02 12:19 ` Eric Gallager @ 2022-08-03 3:33 ` Alexandre Oliva 2022-08-03 11:51 ` Eric Gallager 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Oliva @ 2022-08-03 3:33 UTC (permalink / raw) To: Eric Gallager Cc: Eric Gallager via Gcc-patches, Andreas Schwab, Paolo Bonzini, neroden On Aug 2, 2022, Eric Gallager <egall@gwmail.gwu.edu> wrote: > On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva <oliva@gnu.org> 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 hacker https://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 <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-03 3:33 ` Alexandre Oliva @ 2022-08-03 11:51 ` Eric Gallager 2022-08-05 3:18 ` Alexandre Oliva 0 siblings, 1 reply; 8+ messages in thread From: Eric Gallager @ 2022-08-03 11:51 UTC (permalink / raw) To: Alexandre Oliva Cc: Eric Gallager via Gcc-patches, Andreas Schwab, Paolo Bonzini, neroden On Tue, Aug 2, 2022 at 11:33 PM Alexandre Oliva <oliva@gnu.org> wrote: > > On Aug 2, 2022, Eric Gallager <egall@gwmail.gwu.edu> wrote: > > > On Tue, Aug 2, 2022 at 1:24 AM Alexandre Oliva <oliva@gnu.org> 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 hacker https://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 <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] configure: respect --with-build-time-tools [PR43301] 2022-08-03 11:51 ` Eric Gallager @ 2022-08-05 3:18 ` Alexandre Oliva 0 siblings, 0 replies; 8+ messages in thread From: Alexandre Oliva @ 2022-08-05 3:18 UTC (permalink / raw) To: Eric Gallager Cc: Eric Gallager via Gcc-patches, Andreas Schwab, Paolo Bonzini, neroden On Aug 3, 2022, Eric Gallager <egall@gwmail.gwu.edu> 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 hacker https://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 <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-08-05 3:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-31 20:55 [PATCH] configure: respect --with-build-time-tools [PR43301] Eric Gallager 2022-08-01 7:54 ` Andreas Schwab 2022-08-01 15:40 ` Eric Gallager 2022-08-02 5:24 ` Alexandre Oliva 2022-08-02 12:19 ` Eric Gallager 2022-08-03 3:33 ` Alexandre Oliva 2022-08-03 11:51 ` Eric Gallager 2022-08-05 3:18 ` Alexandre Oliva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).