[PATCH 0/3] Various build and doc fixes

[PATCH 0/3] Various build and doc fixes

[Daniel Kiper]

Hey,

As in subject... Please review...

Daniel

 INSTALL      | 51 +++++++++++++++++++++++++++------------------------
 autogen.sh   |  2 +-
 configure.ac | 21 +++++++++++++++------
 3 files changed, 43 insertions(+), 31 deletions(-)

Daniel Kiper (3):
      configure: Enforce gnu99 C language standard
      INSTALL/configure: Update install doc and configure comment
      autogen: Replace -iname with -ipath in find command

[PATCH 1/3] configure: Enforce gnu99 C language standard

[Daniel Kiper]

Commit d5a32255d (misc: Make grub_strtol() "end" pointers have safer
const qualifiers) introduced "restrict" keyword into some functions
definitions. This keyword was introduced in C99 standard. However, some
compilers by default may use C89 or something different. This behavior
leads to the breakage during builds when c89 or gnu89 is in force. So,
let's enforce gnu99 C language standard for all compilers. This way
a bit random build issue will be fixed and the GRUB source will be
build consistently regardless of type and version of the compiler.

It was decided to use gnu99 C language standard because it fixes the
issue mentioned above and also provides some useful extensions which are
used here and there in the GRUB source. Potentially we can use gnu11 too.
However, this may reduce pool of older compilers which can be used to
build the GRUB. So, let's live with gnu99 until we do not discover that
we strongly require a feature from newer C standard.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 configure.ac | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 88c0adbae..b7f40a1c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,9 +75,16 @@ grub_TRANSFORM([grub-sparc64-setup])
 grub_TRANSFORM([grub-render-label])
 grub_TRANSFORM([grub-file])
 
-# Optimization flag.  Allow user to override.
+if test "x$BUILD_CFLAGS" = x; then
+  BUILD_CFLAGS='-std=gnu99'
+fi
+
+if test "x$HOST_CFLAGS" = x; then
+  HOST_CFLAGS='-std=gnu99'
+fi
+
 if test "x$TARGET_CFLAGS" = x; then
-  TARGET_CFLAGS="$TARGET_CFLAGS -Os"
+  TARGET_CFLAGS='-Os -std=gnu99'
 fi
 
 # Default HOST_CPPFLAGS
-- 
2.11.0

Re: [PATCH 1/3] configure: Enforce gnu99 C language standard

[Leif Lindholm]

On Thu, Apr 02, 2020 at 17:07:48 +0200, Daniel Kiper wrote:
> Commit d5a32255d (misc: Make grub_strtol() "end" pointers have safer
> const qualifiers) introduced "restrict" keyword into some functions
> definitions. This keyword was introduced in C99 standard. However, some
> compilers by default may use C89 or something different. This behavior
> leads to the breakage during builds when c89 or gnu89 is in force. So,
> let's enforce gnu99 C language standard for all compilers. This way
> a bit random build issue will be fixed and the GRUB source will be
> build consistently regardless of type and version of the compiler.
> 
> It was decided to use gnu99 C language standard because it fixes the
> issue mentioned above and also provides some useful extensions which are
> used here and there in the GRUB source. Potentially we can use gnu11 too.
> However, this may reduce pool of older compilers which can be used to
> build the GRUB. So, let's live with gnu99 until we do not discover that
> we strongly require a feature from newer C standard.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
>  configure.ac | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 88c0adbae..b7f40a1c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -75,9 +75,16 @@ grub_TRANSFORM([grub-sparc64-setup])
>  grub_TRANSFORM([grub-render-label])
>  grub_TRANSFORM([grub-file])
>  
> -# Optimization flag.  Allow user to override.
> +if test "x$BUILD_CFLAGS" = x; then
> +  BUILD_CFLAGS='-std=gnu99'
> +fi
> +
> +if test "x$HOST_CFLAGS" = x; then
> +  HOST_CFLAGS='-std=gnu99'
> +fi
> +
>  if test "x$TARGET_CFLAGS" = x; then
> -  TARGET_CFLAGS="$TARGET_CFLAGS -Os"
> +  TARGET_CFLAGS='-Os -std=gnu99'
>  fi

Do these not add -std=gnu99 only if *_CFLAGS are not specified?
Would we not want to override these always?

For example, Debian/Ubuntu override HOST_CFLAGS when building grub.

/
    Leif

>  
>  # Default HOST_CPPFLAGS
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] configure: Enforce gnu99 C language standard

[Daniel Kiper]

On Thu, Apr 02, 2020 at 05:28:33PM +0100, Leif Lindholm wrote:
> On Thu, Apr 02, 2020 at 17:07:48 +0200, Daniel Kiper wrote:
> > Commit d5a32255d (misc: Make grub_strtol() "end" pointers have safer
> > const qualifiers) introduced "restrict" keyword into some functions
> > definitions. This keyword was introduced in C99 standard. However, some
> > compilers by default may use C89 or something different. This behavior
> > leads to the breakage during builds when c89 or gnu89 is in force. So,
> > let's enforce gnu99 C language standard for all compilers. This way
> > a bit random build issue will be fixed and the GRUB source will be
> > build consistently regardless of type and version of the compiler.
> >
> > It was decided to use gnu99 C language standard because it fixes the
> > issue mentioned above and also provides some useful extensions which are
> > used here and there in the GRUB source. Potentially we can use gnu11 too.
> > However, this may reduce pool of older compilers which can be used to
> > build the GRUB. So, let's live with gnu99 until we do not discover that
> > we strongly require a feature from newer C standard.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > ---
> >  configure.ac | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 88c0adbae..b7f40a1c3 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -75,9 +75,16 @@ grub_TRANSFORM([grub-sparc64-setup])
> >  grub_TRANSFORM([grub-render-label])
> >  grub_TRANSFORM([grub-file])
> >
> > -# Optimization flag.  Allow user to override.
> > +if test "x$BUILD_CFLAGS" = x; then
> > +  BUILD_CFLAGS='-std=gnu99'
> > +fi
> > +
> > +if test "x$HOST_CFLAGS" = x; then
> > +  HOST_CFLAGS='-std=gnu99'
> > +fi
> > +
> >  if test "x$TARGET_CFLAGS" = x; then
> > -  TARGET_CFLAGS="$TARGET_CFLAGS -Os"
> > +  TARGET_CFLAGS='-Os -std=gnu99'
> >  fi
>
> Do these not add -std=gnu99 only if *_CFLAGS are not specified?

Exactly...

> Would we not want to override these always?
>
> For example, Debian/Ubuntu override HOST_CFLAGS when building grub.

I was not sure about it. So, after some thinking I decided to give
a user a chance to override C language type using *_CFLAGS. However,
I am not so strongly tied to that. If you think we should add
"-std=gnu99" unconditionally I am OK with that.

Daniel

Re: [PATCH 1/3] configure: Enforce gnu99 C language standard

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

On 4/2/20 1:08 PM, Daniel Kiper wrote:
>> Would we not want to override these always?
>>
>> For example, Debian/Ubuntu override HOST_CFLAGS when building grub.
> 
> I was not sure about it. So, after some thinking I decided to give
> a user a chance to override C language type using *_CFLAGS. However,
> I am not so strongly tied to that. If you think we should add
> "-std=gnu99" unconditionally I am OK with that.
Under which situations would it be valid to avoid passing -std=gnu99? If
these features are used in the source, then compilers which don't
support gnu99 will fail to compile grub, wouldn't they?

-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1601 bytes --]

Re: [PATCH 1/3] configure: Enforce gnu99 C language standard

[Leif Lindholm]

On Thu, Apr 02, 2020 at 19:08:21 +0200, Daniel Kiper wrote:
> > > -# Optimization flag.  Allow user to override.
> > > +if test "x$BUILD_CFLAGS" = x; then
> > > +  BUILD_CFLAGS='-std=gnu99'
> > > +fi
> > > +
> > > +if test "x$HOST_CFLAGS" = x; then
> > > +  HOST_CFLAGS='-std=gnu99'
> > > +fi
> > > +
> > >  if test "x$TARGET_CFLAGS" = x; then
> > > -  TARGET_CFLAGS="$TARGET_CFLAGS -Os"
> > > +  TARGET_CFLAGS='-Os -std=gnu99'
> > >  fi
> >
> > Do these not add -std=gnu99 only if *_CFLAGS are not specified?
> 
> Exactly...
> 
> > Would we not want to override these always?
> >
> > For example, Debian/Ubuntu override HOST_CFLAGS when building grub.
> 
> I was not sure about it. So, after some thinking I decided to give
> a user a chance to override C language type using *_CFLAGS. However,
> I am not so strongly tied to that. If you think we should add
> "-std=gnu99" unconditionally I am OK with that.

I think we should. As long as we permit overriding *_CFLAGS, and put
the user-provided flags last, that would still permit someone to
override the --std option witn an alternative one. (Works just like
the -O flags.)

/
    Leif

Re: [PATCH 1/3] configure: Enforce gnu99 C language standard

[Mike Gilbert]

On Thu, Apr 2, 2020 at 1:38 PM Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Thu, Apr 02, 2020 at 19:08:21 +0200, Daniel Kiper wrote:
> > > > -# Optimization flag.  Allow user to override.
> > > > +if test "x$BUILD_CFLAGS" = x; then
> > > > +  BUILD_CFLAGS='-std=gnu99'
> > > > +fi
> > > > +
> > > > +if test "x$HOST_CFLAGS" = x; then
> > > > +  HOST_CFLAGS='-std=gnu99'
> > > > +fi
> > > > +
> > > >  if test "x$TARGET_CFLAGS" = x; then
> > > > -  TARGET_CFLAGS="$TARGET_CFLAGS -Os"
> > > > +  TARGET_CFLAGS='-Os -std=gnu99'
> > > >  fi
> > >
> > > Do these not add -std=gnu99 only if *_CFLAGS are not specified?
> >
> > Exactly...
> >
> > > Would we not want to override these always?
> > >
> > > For example, Debian/Ubuntu override HOST_CFLAGS when building grub.
> >
> > I was not sure about it. So, after some thinking I decided to give
> > a user a chance to override C language type using *_CFLAGS. However,
> > I am not so strongly tied to that. If you think we should add
> > "-std=gnu99" unconditionally I am OK with that.
>
> I think we should. As long as we permit overriding *_CFLAGS, and put
> the user-provided flags last, that would still permit someone to
> override the --std option witn an alternative one. (Works just like
> the -O flags.)

Agreed.

[PATCH 2/3] INSTALL/configure: Update install doc and configure comment

[Daniel Kiper]

..to reflect the GRUB build reality in them.

Additionally, fix ./configure command example formatting in INSTALL file.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 INSTALL      | 51 +++++++++++++++++++++++++++------------------------
 configure.ac | 10 ++++++----
 2 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/INSTALL b/INSTALL
index 8acb40902..d1b3bb60e 100644
--- a/INSTALL
+++ b/INSTALL
@@ -160,12 +160,12 @@ For this example the configure line might look like (more details below)
 (some options are optional and included here for completeness but some rarely
 used options are omitted):
 
-./configure BUILD_CC=gcc BUILD_PKG_CONFIG=pkg-config --host=amd64-linux-gnu
-CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" PKG_CONFIG=amd64-linux-gnu-pkg-config
---target=arm --with-platform=uboot TARGET_CC=arm-elf-gcc
-TARGET_CFLAGS="-Os -march=armv6" TARGET_CCASFLAGS="-march=armv6"
-TARGET_OBJCOPY="arm-elf-objcopy" TARGET_STRIP="arm-elf-strip"
-TARGET_NM=arm-elf-nm TARGET_RANLIB=arm-elf-ranlib LEX=gflex
+./configure BUILD_CC=gcc BUILD_PKG_CONFIG=pkg-config --host=amd64-linux-gnu \
+  CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" PKG_CONFIG=amd64-linux-gnu-pkg-config \
+  --target=arm --with-platform=uboot TARGET_CC=arm-elf-gcc \
+  TARGET_CFLAGS="-Os -march=armv6" TARGET_CCASFLAGS="-march=armv6" \
+  TARGET_OBJCOPY="arm-elf-objcopy" TARGET_STRIP="arm-elf-strip" \
+  TARGET_NM=arm-elf-nm TARGET_RANLIB=arm-elf-ranlib LEX=gflex
 
 You need to use following options to specify tools and platforms. For minimum
 version look at prerequisites. All tools not mentioned in this section under
@@ -182,28 +182,31 @@ corresponding platform are not needed for the platform in question.
 
   - For host
     1. --host= to autoconf name of host.
-    2. CC= for gcc able to compile for host
-    3. HOST_CFLAGS= for C options for host.
-    4. HOST_CPPFLAGS= for C preprocessor options for host.
-    5. HOST_LDFLAGS= for linker options for host.
-    6. PKG_CONFIG= for pkg-config for host (optional).
-    7. Libdevmapper if any must be in standard linker folders (-ldevmapper) (optional).
-    8. Libfuse if any must be in standard linker folders (-lfuse) (optional).
-    9. Libzfs if any must be in standard linker folders (-lzfs) (optional).
-    10. Liblzma if any must be in standard linker folders (-llzma) (optional).
+    2. CC= for gcc able to compile for host and target.
+    3. CFLAGS= for C options for host and target.
+    4. HOST_CFLAGS= for C options for host.
+    5. HOST_CPPFLAGS= for C preprocessor options for host.
+    6. HOST_LDFLAGS= for linker options for host.
+    7. PKG_CONFIG= for pkg-config for host (optional).
+    8. Libdevmapper if any must be in standard linker folders (-ldevmapper) (optional).
+    9. Libfuse if any must be in standard linker folders (-lfuse) (optional).
+    10. Libzfs if any must be in standard linker folders (-lzfs) (optional).
+    11. Liblzma if any must be in standard linker folders (-llzma) (optional).
 
   - For target
     1. --target= to autoconf cpu name of target.
     2. --with-platform to choose firmware.
-    3. TARGET_CC= for gcc able to compile for target
-    4. TARGET_CFLAGS= for C options for target.
-    5. TARGET_CPPFLAGS= for C preprocessor options for target.
-    6. TARGET_CCASFLAGS= for assembler options for target.
-    7. TARGET_LDFLAGS= for linker options for target.
-    8. TARGET_OBJCOPY= for objcopy for target.
-    9. TARGET_STRIP= for strip for target.
-    10. TARGET_NM= for nm for target.
-    11. TARGET_RANLIB= for ranlib for target.
+    3. CC= for gcc able to compile for host and target.
+    4. CFLAGS= for C options for host and target.
+    5. TARGET_CC= for gcc able to compile for target.
+    6. TARGET_CFLAGS= for C options for target.
+    7. TARGET_CPPFLAGS= for C preprocessor options for target.
+    8. TARGET_CCASFLAGS= for assembler options for target.
+    9. TARGET_LDFLAGS= for linker options for target.
+    10. TARGET_OBJCOPY= for objcopy for target.
+    11. TARGET_STRIP= for strip for target.
+    12. TARGET_NM= for nm for target.
+    13. TARGET_RANLIB= for ranlib for target.
 
   - Additionally for emu, for host and target.
     1. SDL is looked for in standard linker directories (-lSDL) (optional)
diff --git a/configure.ac b/configure.ac
index b7f40a1c3..42a9c35aa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -26,10 +26,12 @@ dnl This is necessary because the target type in autoconf does not
 dnl describe such a system very well.
 dnl
 dnl The current strategy is to use variables with no prefix (such as
-dnl CC, CFLAGS, etc.) for the host type, variables with prefix "BUILD_"
-dnl (such as BUILD_CC, BUILD_CFLAGS, etc.) for the build type and variables
-dnl with the prefix "TARGET_" (such as TARGET_CC, TARGET_CFLAGS, etc.) are
-dnl used for the target type. See INSTALL for full list of variables.
+dnl CC, CFLAGS, etc.) for the host and target type, variables with
+dnl prefix "BUILD_" (such as BUILD_CC, BUILD_CFLAGS, etc.) for the
+dnl build type, variables with prefix "HOST_" (such as HOST_CC,
+dnl HOST_CFLAGS, etc.) for the host type and variables with the prefix
+dnl "TARGET_" (such as TARGET_CC, TARGET_CFLAGS, etc.) are used for
+dnl the target type. See INSTALL for full list of variables.
 
 AC_INIT([GRUB],[2.05],[bug-grub@gnu.org])
 
-- 
2.11.0

[PATCH 3/3] autogen: Replace -iname with -ipath in find command

[Daniel Kiper]

..because -iname cannot be used to match paths.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 autogen.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index ef43270fc..31b0ced7e 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -13,7 +13,7 @@ fi
 export LC_COLLATE=C
 unset LC_ALL
 
-find . -iname '*.[ch]' ! -ipath './grub-core/lib/libgcrypt-grub/*' ! -ipath './build-aux/*' ! -ipath './grub-core/lib/libgcrypt/src/misc.c' ! -ipath './grub-core/lib/libgcrypt/src/global.c' ! -ipath './grub-core/lib/libgcrypt/src/secmem.c'  ! -ipath './util/grub-gen-widthspec.c' ! -ipath './util/grub-gen-asciih.c' ! -ipath './gnulib/*' ! -iname './grub-core/lib/gnulib/*' |sort > po/POTFILES.in
+find . -iname '*.[ch]' ! -ipath './grub-core/lib/libgcrypt-grub/*' ! -ipath './build-aux/*' ! -ipath './grub-core/lib/libgcrypt/src/misc.c' ! -ipath './grub-core/lib/libgcrypt/src/global.c' ! -ipath './grub-core/lib/libgcrypt/src/secmem.c'  ! -ipath './util/grub-gen-widthspec.c' ! -ipath './util/grub-gen-asciih.c' ! -ipath './gnulib/*' ! -ipath './grub-core/lib/gnulib/*' |sort > po/POTFILES.in
 find util -iname '*.in' ! -name Makefile.in  |sort > po/POTFILES-shell.in
 
 echo "Importing unicode..."
-- 
2.11.0