diff mbox series

[v1,1/1] configure: correct tooldir install path

Message ID 20231215095529.739093-1-neal.frager@amd.com
State New
Headers show
Series [v1,1/1] configure: correct tooldir install path | expand

Commit Message

Neal Frager Dec. 15, 2023, 9:55 a.m. UTC
This patch is required to fix how the newlib headers are installed
when using a sysroot install directory.

The cross compiler expects headers to be in
.../host/usr/arm-none-eabi/sysroot/usr/include/newlib.h
by default newlib installed the headers into
.../host/usr/arm-none-eabi/sysroot/usr/arm-none-eabi/include/newlib.h

${exec_prefix} provides the .../host/usr/arm-none-eabi/sysroot path
${target_noncanonical} provides an extra arm-none-eabi/ that must be removed.

Signed-off-by: Neal Frager <neal.frager@amd.com>
Co-developed-by: Chris Wardman <cjwfirmware@vxmdesign.com>
---
 configure | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Thomas Petazzoni Dec. 15, 2023, 10:33 a.m. UTC | #1
Hello Neal,

On Fri, 15 Dec 2023 09:55:29 +0000
Neal Frager <neal.frager@amd.com> wrote:

> This patch is required to fix how the newlib headers are installed
> when using a sysroot install directory.
> 
> The cross compiler expects headers to be in
> .../host/usr/arm-none-eabi/sysroot/usr/include/newlib.h
> by default newlib installed the headers into
> .../host/usr/arm-none-eabi/sysroot/usr/arm-none-eabi/include/newlib.h
> 
> ${exec_prefix} provides the .../host/usr/arm-none-eabi/sysroot path
> ${target_noncanonical} provides an extra arm-none-eabi/ that must be removed.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> Co-developed-by: Chris Wardman <cjwfirmware@vxmdesign.com>
> ---
>  configure | 7 +++++++
>  1 file changed, 7 insertions(+)

The configure script is generated from configure.ac, so you should not
patch configure but configure.ac.

> +  --with-install-sysroot  use sysroot install directory
>    --with-debug-prefix-map='A=B C=D ...'
>                            map A to B, C to D ... in debug information
>    --with-build-config='NAME NAME2...'
> @@ -7057,7 +7059,12 @@ esac
>  
>  # Some systems (e.g., one of the i386-aix systems the gas testers are
>  # using) don't handle "\$" correctly, so don't use it here.
> +# Check whether --with-install-sysroot was given.
> +if test "${with_install_sysroot+set}" = set; then :

--with/--without options are usually not used for boolean things,
instead --enable/--disable option are used for boolean things.

However here, I think that what would make sense is a --with-tooldir
option. When not passed, the value is
'${exec_prefix}'/${target_noncanonical}, and when passed, the value is
the one passed in the option:

 ./configure ... --with-tooldir=/blabla

Best regards,

Thomas
Neal Frager Dec. 15, 2023, 12:08 p.m. UTC | #2
Hi Thomas,

> This patch is required to fix how the newlib headers are installed
> when using a sysroot install directory.
> 
> The cross compiler expects headers to be in
> .../host/usr/arm-none-eabi/sysroot/usr/include/newlib.h
> by default newlib installed the headers into
> .../host/usr/arm-none-eabi/sysroot/usr/arm-none-eabi/include/newlib.h
> 
> ${exec_prefix} provides the .../host/usr/arm-none-eabi/sysroot path
> ${target_noncanonical} provides an extra arm-none-eabi/ that must be removed.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> Co-developed-by: Chris Wardman <cjwfirmware@vxmdesign.com>
> ---
>  configure | 7 +++++++
>  1 file changed, 7 insertions(+)

> The configure script is generated from configure.ac, so you should not
> patch configure but configure.ac.

When building newlib, I am not seeing the configure script get
generated.  If I only patch configure.ac, the configure script stays
the same and my patch appears ignored.

Do you know what I could be missing?

> +  --with-install-sysroot  use sysroot install directory
>    --with-debug-prefix-map='A=B C=D ...'
>                            map A to B, C to D ... in debug information
>    --with-build-config='NAME NAME2...'
> @@ -7057,7 +7059,12 @@ esac
>  
>  # Some systems (e.g., one of the i386-aix systems the gas testers are
>  # using) don't handle "\$" correctly, so don't use it here.
> +# Check whether --with-install-sysroot was given.
> +if test "${with_install_sysroot+set}" = set; then :

> --with/--without options are usually not used for boolean things,
> instead --enable/--disable option are used for boolean things.

> However here, I think that what would make sense is a --with-tooldir
> option. When not passed, the value is
> '${exec_prefix}'/${target_noncanonical}, and when passed, the value is
> the one passed in the option:

> ./configure ... --with-tooldir=/blabla

I made this correction, and the --with-tooldir option only works
if I patch it directly into the configure script.

Could you help me figure out what is missing?

Best regards,
Neal Frager
AMD
Thomas Petazzoni Dec. 15, 2023, 5:07 p.m. UTC | #3
On Fri, 15 Dec 2023 12:08:37 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> > The configure script is generated from configure.ac, so you should not
> > patch configure but configure.ac.  
> 
> When building newlib, I am not seeing the configure script get
> generated.  If I only patch configure.ac, the configure script stays
> the same and my patch appears ignored.
> 
> Do you know what I could be missing?

Yes, you need to learn/read a bit about the autotools, and autoconf in
particular :-)

> I made this correction, and the --with-tooldir option only works
> if I patch it directly into the configure script.
> 
> Could you help me figure out what is missing?

See above :-)

Best regards,

Thomas
Jeff Johnston Jan. 3, 2024, 10:43 p.m. UTC | #4
Hi Neal,

It looks like this got missed during the holiday season.

When you patch configure.ac, you need to regenerate configure (not directly
edit it).  You can either run autoconf or autoreconf which will need
autoconf 2.69 installed on your system.  When you submit a patch, you don't
submit the generated files such as configure, but you will need to
regenerate it to test your changes locally.  Thus, your patch should have
the changes needed to configure.ac but not configure   The configure file
will get regenerated for the repo by whoever reviews, approves, and merges
the patch.

-- Jeff J.


On Fri, Dec 15, 2023 at 7:08 AM Frager, Neal <neal.frager@amd.com> wrote:

> Hi Thomas,
>
> > This patch is required to fix how the newlib headers are installed
> > when using a sysroot install directory.
> >
> > The cross compiler expects headers to be in
> > .../host/usr/arm-none-eabi/sysroot/usr/include/newlib.h
> > by default newlib installed the headers into
> > .../host/usr/arm-none-eabi/sysroot/usr/arm-none-eabi/include/newlib.h
> >
> > ${exec_prefix} provides the .../host/usr/arm-none-eabi/sysroot path
> > ${target_noncanonical} provides an extra arm-none-eabi/ that must be
> removed.
> >
> > Signed-off-by: Neal Frager <neal.frager@amd.com>
> > Co-developed-by: Chris Wardman <cjwfirmware@vxmdesign.com>
> > ---
> >  configure | 7 +++++++
> >  1 file changed, 7 insertions(+)
>
> > The configure script is generated from configure.ac, so you should not
> > patch configure but configure.ac.
>
> When building newlib, I am not seeing the configure script get
> generated.  If I only patch configure.ac, the configure script stays
> the same and my patch appears ignored.
>
> Do you know what I could be missing?
>
> > +  --with-install-sysroot  use sysroot install directory
> >    --with-debug-prefix-map='A=B C=D ...'
> >                            map A to B, C to D ... in debug information
> >    --with-build-config='NAME NAME2...'
> > @@ -7057,7 +7059,12 @@ esac
> >
> >  # Some systems (e.g., one of the i386-aix systems the gas testers are
> >  # using) don't handle "\$" correctly, so don't use it here.
> > +# Check whether --with-install-sysroot was given.
> > +if test "${with_install_sysroot+set}" = set; then :
>
> > --with/--without options are usually not used for boolean things,
> > instead --enable/--disable option are used for boolean things.
>
> > However here, I think that what would make sense is a --with-tooldir
> > option. When not passed, the value is
> > '${exec_prefix}'/${target_noncanonical}, and when passed, the value is
> > the one passed in the option:
>
> > ./configure ... --with-tooldir=/blabla
>
> I made this correction, and the --with-tooldir option only works
> if I patch it directly into the configure script.
>
> Could you help me figure out what is missing?
>
> Best regards,
> Neal Frager
> AMD
>
>
diff mbox series

Patch

diff --git a/configure b/configure
index eb0ba840b..29ab40cf6 100755
--- a/configure
+++ b/configure
@@ -821,6 +821,7 @@  enable_linker_plugin_flags
 enable_stage1_languages
 enable_objc_gc
 with_build_sysroot
+with_install_sysroot
 with_debug_prefix_map
 with_build_config
 enable_vtable_verify
@@ -1590,6 +1591,7 @@  Optional Packages:
   --with-isl-lib=PATH     Specify the directory for the installed isl library
   --with-build-sysroot=SYSROOT
                           use sysroot as the system root during the build
+  --with-install-sysroot  use sysroot install directory
   --with-debug-prefix-map='A=B C=D ...'
                           map A to B, C to D ... in debug information
   --with-build-config='NAME NAME2...'
@@ -7057,7 +7059,12 @@  esac
 
 # Some systems (e.g., one of the i386-aix systems the gas testers are
 # using) don't handle "\$" correctly, so don't use it here.
+# Check whether --with-install-sysroot was given.
+if test "${with_install_sysroot+set}" = set; then :
+tooldir='${exec_prefix}'
+else
 tooldir='${exec_prefix}'/${target_noncanonical}
+fi
 build_tooldir=${tooldir}
 
 # Create a .gdbinit file which runs the one in srcdir