[PATCH] Drop gnulib fix-base64.patch

[PATCH] Drop gnulib fix-base64.patch

[Robbie Harwood]

Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
fix-base64.patch handled two problems we have using gnulib, which are
exerciesd by the base64 module but not directly caused by it.

First, grub2 defines its own bool type, while gnulib expects the
equivalent of stdbool.h to be present.  Rather than patching gnulib,
instead use gnulib's stdbool module to provide a bool type if needed.
(Suggested by Simon Josefsson.)

Second, our config.h doesn't always inherit config-util.h, which is
where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
fix-base64.h worked around this by defining the attribute away, but this
workaround is better placed in config.h itself, not a gnulib patch.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
 bootstrap.conf                                |  3 ++-
 config.h.in                                   |  3 +++
 grub-core/lib/gnulib-patches/fix-base64.patch | 21 -------------------
 grub-core/lib/posix_wrap/sys/types.h          |  7 +++----
 grub-core/lib/xzembed/xz.h                    |  5 +----
 5 files changed, 9 insertions(+), 30 deletions(-)
 delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch

diff --git a/bootstrap.conf b/bootstrap.conf
index 0dd893c5c..21a8cf15d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -35,6 +35,7 @@ gnulib_modules="
   realloc-gnu
   regex
   save-cwd
+  stdbool
 "
 
 gnulib_tool_option_extras="\
@@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
 
 bootstrap_post_import_hook () {
   set -e
-  for patchname in fix-base64 fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
+  for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
       fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
     patch -d grub-core/lib/gnulib -p2 \
       < "grub-core/lib/gnulib-patches/$patchname.patch"
diff --git a/config.h.in b/config.h.in
index 9e8f9911b..2b65c86c4 100644
--- a/config.h.in
+++ b/config.h.in
@@ -64,4 +64,7 @@
 
 #define _GNU_SOURCE 1
 
+/* For gnulib's base64 code. */
+#define _GL_ATTRIBUTE_CONST /* empty */
+
 #endif
diff --git a/grub-core/lib/gnulib-patches/fix-base64.patch b/grub-core/lib/gnulib-patches/fix-base64.patch
deleted file mode 100644
index 985db1279..000000000
--- a/grub-core/lib/gnulib-patches/fix-base64.patch
+++ /dev/null
@@ -1,21 +0,0 @@
-diff --git a/lib/base64.h b/lib/base64.h
-index 9cd0183b8..185a2afa1 100644
---- a/lib/base64.h
-+++ b/lib/base64.h
-@@ -21,8 +21,14 @@
- /* Get size_t. */
- # include <stddef.h>
- 
--/* Get bool. */
--# include <stdbool.h>
-+#ifndef GRUB_POSIX_BOOL_DEFINED
-+typedef enum { false = 0, true = 1 } bool;
-+#define GRUB_POSIX_BOOL_DEFINED 1
-+#endif
-+
-+#ifndef _GL_ATTRIBUTE_CONST
-+# define _GL_ATTRIBUTE_CONST /* empty */
-+#endif
- 
- # ifdef __cplusplus
- extern "C" {
diff --git a/grub-core/lib/posix_wrap/sys/types.h b/grub-core/lib/posix_wrap/sys/types.h
index 854eb0122..eeda543c4 100644
--- a/grub-core/lib/posix_wrap/sys/types.h
+++ b/grub-core/lib/posix_wrap/sys/types.h
@@ -23,11 +23,10 @@
 
 #include <stddef.h>
 
+/* Provided by gnulib if not present. */
+#include <stdbool.h>
+
 typedef grub_ssize_t ssize_t;
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#define GRUB_POSIX_BOOL_DEFINED 1
-#endif
 
 typedef grub_uint8_t uint8_t;
 typedef grub_uint16_t uint16_t;
diff --git a/grub-core/lib/xzembed/xz.h b/grub-core/lib/xzembed/xz.h
index f7b32d800..d1417039a 100644
--- a/grub-core/lib/xzembed/xz.h
+++ b/grub-core/lib/xzembed/xz.h
@@ -29,10 +29,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <grub/misc.h>
-
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#endif
+#include <stdbool.h>
 
 /**
  * enum xz_ret - Return codes
-- 
2.33.0

Re: [PATCH] Drop gnulib fix-base64.patch

[Darren Kenny]

Hi Robbie,

Just tried it in a build, and it works for me.

On Thursday, 2021-10-28 at 15:22:27 -04, Robbie Harwood wrote:
> Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
> subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
> fix-base64.patch handled two problems we have using gnulib, which are
> exerciesd by the base64 module but not directly caused by it.
>
> First, grub2 defines its own bool type, while gnulib expects the
> equivalent of stdbool.h to be present.  Rather than patching gnulib,
> instead use gnulib's stdbool module to provide a bool type if needed.
> (Suggested by Simon Josefsson.)
>
> Second, our config.h doesn't always inherit config-util.h, which is
> where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
> fix-base64.h worked around this by defining the attribute away, but this
> workaround is better placed in config.h itself, not a gnulib patch.
>
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  bootstrap.conf                                |  3 ++-
>  config.h.in                                   |  3 +++
>  grub-core/lib/gnulib-patches/fix-base64.patch | 21 -------------------
>  grub-core/lib/posix_wrap/sys/types.h          |  7 +++----
>  grub-core/lib/xzembed/xz.h                    |  5 +----
>  5 files changed, 9 insertions(+), 30 deletions(-)
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
>
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 0dd893c5c..21a8cf15d 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -35,6 +35,7 @@ gnulib_modules="
>    realloc-gnu
>    regex
>    save-cwd
> +  stdbool
>  "
>  
>  gnulib_tool_option_extras="\
> @@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
>  
>  bootstrap_post_import_hook () {
>    set -e
> -  for patchname in fix-base64 fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
> +  for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
>        fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
>      patch -d grub-core/lib/gnulib -p2 \
>        < "grub-core/lib/gnulib-patches/$patchname.patch"
> diff --git a/config.h.in b/config.h.in
> index 9e8f9911b..2b65c86c4 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -64,4 +64,7 @@
>  
>  #define _GNU_SOURCE 1
>  
> +/* For gnulib's base64 code. */
> +#define _GL_ATTRIBUTE_CONST /* empty */
> +
>  #endif
> diff --git a/grub-core/lib/gnulib-patches/fix-base64.patch b/grub-core/lib/gnulib-patches/fix-base64.patch
> deleted file mode 100644
> index 985db1279..000000000
> --- a/grub-core/lib/gnulib-patches/fix-base64.patch
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -diff --git a/lib/base64.h b/lib/base64.h
> -index 9cd0183b8..185a2afa1 100644
> ---- a/lib/base64.h
> -+++ b/lib/base64.h
> -@@ -21,8 +21,14 @@
> - /* Get size_t. */
> - # include <stddef.h>
> - 
> --/* Get bool. */
> --# include <stdbool.h>
> -+#ifndef GRUB_POSIX_BOOL_DEFINED
> -+typedef enum { false = 0, true = 1 } bool;
> -+#define GRUB_POSIX_BOOL_DEFINED 1
> -+#endif
> -+
> -+#ifndef _GL_ATTRIBUTE_CONST
> -+# define _GL_ATTRIBUTE_CONST /* empty */
> -+#endif
> - 
> - # ifdef __cplusplus
> - extern "C" {
> diff --git a/grub-core/lib/posix_wrap/sys/types.h b/grub-core/lib/posix_wrap/sys/types.h
> index 854eb0122..eeda543c4 100644
> --- a/grub-core/lib/posix_wrap/sys/types.h
> +++ b/grub-core/lib/posix_wrap/sys/types.h
> @@ -23,11 +23,10 @@
>  
>  #include <stddef.h>
>  
> +/* Provided by gnulib if not present. */
> +#include <stdbool.h>
> +
>  typedef grub_ssize_t ssize_t;
> -#ifndef GRUB_POSIX_BOOL_DEFINED
> -typedef enum { false = 0, true = 1 } bool;
> -#define GRUB_POSIX_BOOL_DEFINED 1
> -#endif
>  
>  typedef grub_uint8_t uint8_t;
>  typedef grub_uint16_t uint16_t;
> diff --git a/grub-core/lib/xzembed/xz.h b/grub-core/lib/xzembed/xz.h
> index f7b32d800..d1417039a 100644
> --- a/grub-core/lib/xzembed/xz.h
> +++ b/grub-core/lib/xzembed/xz.h
> @@ -29,10 +29,7 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <grub/misc.h>
> -
> -#ifndef GRUB_POSIX_BOOL_DEFINED
> -typedef enum { false = 0, true = 1 } bool;
> -#endif
> +#include <stdbool.h>
>  
>  /**
>   * enum xz_ret - Return codes
> -- 
> 2.33.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Drop gnulib fix-base64.patch

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4597 bytes --]

On Thu, Oct 28, 2021 at 03:22:27PM -0400, Robbie Harwood wrote:
> Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
> subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
> fix-base64.patch handled two problems we have using gnulib, which are
> exerciesd by the base64 module but not directly caused by it.
> 
> First, grub2 defines its own bool type, while gnulib expects the
> equivalent of stdbool.h to be present.  Rather than patching gnulib,
> instead use gnulib's stdbool module to provide a bool type if needed.
> (Suggested by Simon Josefsson.)
> 
> Second, our config.h doesn't always inherit config-util.h, which is
> where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
> fix-base64.h worked around this by defining the attribute away, but this
> workaround is better placed in config.h itself, not a gnulib patch.
> 
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>

I agree this looks a lot cleaner than patching in support for booleans.
Thanks for the patch!

Reviewed-by: Patrick Steinhardt <ps@pks.im>

Patrick

> ---
>  bootstrap.conf                                |  3 ++-
>  config.h.in                                   |  3 +++
>  grub-core/lib/gnulib-patches/fix-base64.patch | 21 -------------------
>  grub-core/lib/posix_wrap/sys/types.h          |  7 +++----
>  grub-core/lib/xzembed/xz.h                    |  5 +----
>  5 files changed, 9 insertions(+), 30 deletions(-)
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
> 
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 0dd893c5c..21a8cf15d 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -35,6 +35,7 @@ gnulib_modules="
>    realloc-gnu
>    regex
>    save-cwd
> +  stdbool
>  "
>  
>  gnulib_tool_option_extras="\
> @@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
>  
>  bootstrap_post_import_hook () {
>    set -e
> -  for patchname in fix-base64 fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
> +  for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
>        fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
>      patch -d grub-core/lib/gnulib -p2 \
>        < "grub-core/lib/gnulib-patches/$patchname.patch"
> diff --git a/config.h.in b/config.h.in
> index 9e8f9911b..2b65c86c4 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -64,4 +64,7 @@
>  
>  #define _GNU_SOURCE 1
>  
> +/* For gnulib's base64 code. */
> +#define _GL_ATTRIBUTE_CONST /* empty */
> +
>  #endif
> diff --git a/grub-core/lib/gnulib-patches/fix-base64.patch b/grub-core/lib/gnulib-patches/fix-base64.patch
> deleted file mode 100644
> index 985db1279..000000000
> --- a/grub-core/lib/gnulib-patches/fix-base64.patch
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -diff --git a/lib/base64.h b/lib/base64.h
> -index 9cd0183b8..185a2afa1 100644
> ---- a/lib/base64.h
> -+++ b/lib/base64.h
> -@@ -21,8 +21,14 @@
> - /* Get size_t. */
> - # include <stddef.h>
> - 
> --/* Get bool. */
> --# include <stdbool.h>
> -+#ifndef GRUB_POSIX_BOOL_DEFINED
> -+typedef enum { false = 0, true = 1 } bool;
> -+#define GRUB_POSIX_BOOL_DEFINED 1
> -+#endif
> -+
> -+#ifndef _GL_ATTRIBUTE_CONST
> -+# define _GL_ATTRIBUTE_CONST /* empty */
> -+#endif
> - 
> - # ifdef __cplusplus
> - extern "C" {
> diff --git a/grub-core/lib/posix_wrap/sys/types.h b/grub-core/lib/posix_wrap/sys/types.h
> index 854eb0122..eeda543c4 100644
> --- a/grub-core/lib/posix_wrap/sys/types.h
> +++ b/grub-core/lib/posix_wrap/sys/types.h
> @@ -23,11 +23,10 @@
>  
>  #include <stddef.h>
>  
> +/* Provided by gnulib if not present. */
> +#include <stdbool.h>
> +
>  typedef grub_ssize_t ssize_t;
> -#ifndef GRUB_POSIX_BOOL_DEFINED
> -typedef enum { false = 0, true = 1 } bool;
> -#define GRUB_POSIX_BOOL_DEFINED 1
> -#endif
>  
>  typedef grub_uint8_t uint8_t;
>  typedef grub_uint16_t uint16_t;
> diff --git a/grub-core/lib/xzembed/xz.h b/grub-core/lib/xzembed/xz.h
> index f7b32d800..d1417039a 100644
> --- a/grub-core/lib/xzembed/xz.h
> +++ b/grub-core/lib/xzembed/xz.h
> @@ -29,10 +29,7 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <grub/misc.h>
> -
> -#ifndef GRUB_POSIX_BOOL_DEFINED
> -typedef enum { false = 0, true = 1 } bool;
> -#endif
> +#include <stdbool.h>
>  
>  /**
>   * enum xz_ret - Return codes
> -- 
> 2.33.0
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Drop gnulib fix-base64.patch

[Daniel Axtens]

Robbie Harwood <rharwood@redhat.com> writes:

> Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
> subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
> fix-base64.patch handled two problems we have using gnulib, which are
> exerciesd by the base64 module but not directly caused by it.
>
> First, grub2 defines its own bool type, while gnulib expects the
> equivalent of stdbool.h to be present.  Rather than patching gnulib,
> instead use gnulib's stdbool module to provide a bool type if needed.
> (Suggested by Simon Josefsson.)
>
> Second, our config.h doesn't always inherit config-util.h, which is
> where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
> fix-base64.h worked around this by defining the attribute away, but this
> workaround is better placed in config.h itself, not a gnulib patch.
>
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  bootstrap.conf                                |  3 ++-
>  config.h.in                                   |  3 +++
>  grub-core/lib/gnulib-patches/fix-base64.patch | 21 -------------------
>  grub-core/lib/posix_wrap/sys/types.h          |  7 +++----
>  grub-core/lib/xzembed/xz.h                    |  5 +----
>  5 files changed, 9 insertions(+), 30 deletions(-)
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
>
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 0dd893c5c..21a8cf15d 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -35,6 +35,7 @@ gnulib_modules="
>    realloc-gnu
>    regex
>    save-cwd
> +  stdbool
>  "
>  
>  gnulib_tool_option_extras="\
> @@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
>  
>  bootstrap_post_import_hook () {
>    set -e
> -  for patchname in fix-base64 fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
> +  for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
>        fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
>      patch -d grub-core/lib/gnulib -p2 \
>        < "grub-core/lib/gnulib-patches/$patchname.patch"
> diff --git a/config.h.in b/config.h.in
> index 9e8f9911b..2b65c86c4 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -64,4 +64,7 @@
>  
>  #define _GNU_SOURCE 1
>  
> +/* For gnulib's base64 code. */
> +#define _GL_ATTRIBUTE_CONST /* empty */

Do we support any compiler so old or configuration so weird that we
can't simply use 'const' here?

Kind regards,
Daniel

Re: [PATCH] Drop gnulib fix-base64.patch

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Daniel Axtens <dja@axtens.net> writes:

> Robbie Harwood <rharwood@redhat.com> writes:
>
>> +/* For gnulib's base64 code. */
>> +#define _GL_ATTRIBUTE_CONST /* empty */
>
> Do we support any compiler so old or configuration so weird that we
> can't simply use 'const' here?

Unfortunately it's not quite that simple.  _GL_ATTRIBUTE_CONST actually
gates turning on `__attribute ((__const__))`, which if memory serves is
a GCC extension.  I don't know what the support matrix on grub is, but
based on the original patch there was a need to support non-gcc-likes.
Would be fine with changing that, though.

(Personally, I think it should be obvious to a reasonable compiler that
isbase64() is purely arithmetic.  However, gnulib upstream did not like
my proposal to drop the attribute marker, so we're stuck with it.)

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Drop gnulib fix-base64.patch

[Daniel Kiper]

CC-ing Daren, Patrick and Vladimir...

On Tue, Nov 23, 2021 at 11:08:55AM -0500, Robbie Harwood wrote:
> Daniel Axtens <dja@axtens.net> writes:
>
> > Robbie Harwood <rharwood@redhat.com> writes:
> >
> >> +/* For gnulib's base64 code. */
> >> +#define _GL_ATTRIBUTE_CONST /* empty */
> >
> > Do we support any compiler so old or configuration so weird that we
> > can't simply use 'const' here?
>
> Unfortunately it's not quite that simple.  _GL_ATTRIBUTE_CONST actually
> gates turning on `__attribute ((__const__))`, which if memory serves is
> a GCC extension.  I don't know what the support matrix on grub is, but
> based on the original patch there was a need to support non-gcc-likes.
> Would be fine with changing that, though.
>
> (Personally, I think it should be obvious to a reasonable compiler that
> isbase64() is purely arithmetic.  However, gnulib upstream did not like
> my proposal to drop the attribute marker, so we're stuck with it.)

When I started looking at this issue I realized we have bigger problem
here than lack of _GL_ATTRIBUTE_CONST definition. In general all _GL_*
constants land in config-util.h.in and finally in config-util.h. It does
not make a lot of sense because config-util.h is not included when you
build GRUB core. Instead it is included when you build GRUB utils. Huh!
AFAICT this means the gnulib is deprived of all its specific constants.
I am not sure how it worked at all in so far. Anyway, I think we have
to fix it properly.

Robbie, may I ask you to talk to gnulib folks and ask them how we should
solve this problem? Please CC all folks CC-ed in this email.

Daniel

Re: [PATCH] Drop gnulib fix-base64.patch

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

Daniel Kiper <dkiper@net-space.pl> writes:

> CC-ing Daren, Patrick and Vladimir...
>
> When I started looking at this issue I realized we have bigger problem
> here than lack of _GL_ATTRIBUTE_CONST definition. In general all _GL_*
> constants land in config-util.h.in and finally in config-util.h. It
> does not make a lot of sense because config-util.h is not included
> when you build GRUB core. Instead it is included when you build GRUB
> utils. Huh!  AFAICT this means the gnulib is deprived of all its
> specific constants.  I am not sure how it worked at all in so
> far. Anyway, I think we have to fix it properly.
>
> Robbie, may I ask you to talk to gnulib folks and ask them how we
> should solve this problem? Please CC all folks CC-ed in this email.

I'm not opposed to getting gnulib's opinion, but don't we already know
what they suggest from
https://lists.gnu.org/archive/html/bug-gnulib/2021-11/msg00009.html
(i.e., run configure twice)?

(Though if other folks on CC want to provide more input on
https://lists.gnu.org/archive/html/bug-gnulib/2021-11/msg00028.html that
would be helpful.)

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Drop gnulib fix-base64.patch

[Daniel Kiper]

On Wed, Nov 24, 2021 at 09:36:14AM -0500, Robbie Harwood wrote:
> Daniel Kiper <dkiper@net-space.pl> writes:
>
> > CC-ing Daren, Patrick and Vladimir...
> >
> > When I started looking at this issue I realized we have bigger problem
> > here than lack of _GL_ATTRIBUTE_CONST definition. In general all _GL_*
> > constants land in config-util.h.in and finally in config-util.h. It
> > does not make a lot of sense because config-util.h is not included
> > when you build GRUB core. Instead it is included when you build GRUB
> > utils. Huh!  AFAICT this means the gnulib is deprived of all its
> > specific constants.  I am not sure how it worked at all in so
> > far. Anyway, I think we have to fix it properly.
> >
> > Robbie, may I ask you to talk to gnulib folks and ask them how we
> > should solve this problem? Please CC all folks CC-ed in this email.
>
> I'm not opposed to getting gnulib's opinion, but don't we already know
> what they suggest from
> https://lists.gnu.org/archive/html/bug-gnulib/2021-11/msg00009.html
> (i.e., run configure twice)?

Yeah, but I think it would require major overhaul. Does not it? If yes
then maybe we should consider move to the Kconfig or something like that.

> (Though if other folks on CC want to provide more input on
> https://lists.gnu.org/archive/html/bug-gnulib/2021-11/msg00028.html that
> would be helpful.)

Yeah, that would be perfect.

Daniel

Re: [PATCH] Drop gnulib fix-base64.patch

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Daniel Kiper <dkiper@net-space.pl> writes:

> Yeah, but I think it would require major overhaul. Does not it? If yes
> then maybe we should consider move to the Kconfig or something like
> that.

Perhaps, but please don't mistake me as volunteering for build system
hacking - I'm mostly just here to upstream patches :)

In the end, I submit that you as the upstream maintainer have should
something you can work with and solve problems in.  If what's there
isn't that, then something needs to change in order for things to get
better - and maybe a major overhaul would be worth it.

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Drop gnulib fix-base64.patch

[Daniel Kiper]

On Mon, Nov 29, 2021 at 06:21:46PM -0500, Robbie Harwood wrote:
> Daniel Kiper <dkiper@net-space.pl> writes:
>
> > Yeah, but I think it would require major overhaul. Does not it? If yes
> > then maybe we should consider move to the Kconfig or something like
> > that.
>
> Perhaps, but please don't mistake me as volunteering for build system

I do not... I am not sure how you inferred I do.

> hacking - I'm mostly just here to upstream patches :)

Cool! I have just asked you to investigate how to improve your patch.
Currently I am not happy with it. I still hope you will send me some
proposals (I do not expect patches immediately; if you prefer just tell
me how do you want to fix the issue in better way). If I get them then
we can continue discussion...

Daniel

Re: [PATCH] Drop gnulib fix-base64.patch

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Daniel Kiper <dkiper@net-space.pl> writes:

> On Mon, Nov 29, 2021 at 06:21:46PM -0500, Robbie Harwood wrote:
>> Daniel Kiper <dkiper@net-space.pl> writes:
>>
>> > Yeah, but I think it would require major overhaul. Does not it? If yes
>> > then maybe we should consider move to the Kconfig or something like
>> > that.
>>
>> Perhaps, but please don't mistake me as volunteering for build system
>
> I do not... I am not sure how you inferred I do.
>
>> hacking - I'm mostly just here to upstream patches :)
>
> Cool! I have just asked you to investigate how to improve your patch.
> Currently I am not happy with it. I still hope you will send me some
> proposals (I do not expect patches immediately; if you prefer just tell
> me how do you want to fix the issue in better way). If I get them then
> we can continue discussion...

Sorry, I think I missed this - what are you unhappy about with this
patch?

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Drop gnulib fix-base64.patch

[Daniel Kiper]

On Tue, Dec 07, 2021 at 03:34:29PM -0500, Robbie Harwood wrote:
> Daniel Kiper <dkiper@net-space.pl> writes:
> > On Mon, Nov 29, 2021 at 06:21:46PM -0500, Robbie Harwood wrote:
> >> Daniel Kiper <dkiper@net-space.pl> writes:
> >>
> >> > Yeah, but I think it would require major overhaul. Does not it? If yes
> >> > then maybe we should consider move to the Kconfig or something like
> >> > that.
> >>
> >> Perhaps, but please don't mistake me as volunteering for build system
> >
> > I do not... I am not sure how you inferred I do.
> >
> >> hacking - I'm mostly just here to upstream patches :)
> >
> > Cool! I have just asked you to investigate how to improve your patch.
> > Currently I am not happy with it. I still hope you will send me some
> > proposals (I do not expect patches immediately; if you prefer just tell
> > me how do you want to fix the issue in better way). If I get them then
> > we can continue discussion...
>
> Sorry, I think I missed this - what are you unhappy about with this
> patch?

It tries to fix one specific issue and do not address other problems
which we have with the gnulib _GL_* constants. I would prefer to fix
them at once now. I hope this should not be difficult. In the worst case
we can copy _GL_* constants to config.h.in from config-util.h.in. Though
I would prefer to avoid this manual work if we are able to find quite
simple solution doing this automatically.

Daniel