diff mbox series

package/git: require wchar support

Message ID 20241018043437.28475-2-bagasdotme@gmail.com
State Changes Requested
Headers show
Series package/git: require wchar support | expand

Commit Message

Bagas Sanjaya Oct. 18, 2024, 4:34 a.m. UTC 
Git version 2.47.0 fails to build on systems without wide characters support
(e.g. uClibc-ng without BR2_TOOLCHAIN_BUILDROOT_WCHAR selected):

```
    CC t/unit-tests/unit-test.o
t/unit-tests/clar/clar.c: In function 'clar__assert_equal':
t/unit-tests/clar/clar.c:767:23: error: unknown type name 'wchar_t'
  767 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
      |                       ^~~~~~~
In file included from t/unit-tests/clar/clar.c:13:
t/unit-tests/clar/clar.c:767:58: error: unknown type name 'wchar_t'
  767 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
      |                                                          ^~~~~~~
t/unit-tests/clar/clar.c:768:23: error: unknown type name 'wchar_t'
  768 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
      |                       ^~~~~~~
t/unit-tests/clar/clar.c:768:58: error: unknown type name 'wchar_t'
  768 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
      |                                                          ^~~~~~~
t/unit-tests/clar/clar.c:769:65: warning: implicit declaration of function 'wcscmp' [-Wimplicit-function-declaration]
  769 |                 is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcscmp(wcs1, wcs2);
      |                                                                 ^~~~~~
t/unit-tests/clar/clar.c:784:23: error: unknown type name 'wchar_t'
  784 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
      |                       ^~~~~~~
t/unit-tests/clar/clar.c:784:58: error: unknown type name 'wchar_t'
  784 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
      |                                                          ^~~~~~~
t/unit-tests/clar/clar.c:785:23: error: unknown type name 'wchar_t'
  785 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
      |                       ^~~~~~~
t/unit-tests/clar/clar.c:785:58: error: unknown type name 'wchar_t'
  785 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
      |                                                          ^~~~~~~
t/unit-tests/clar/clar.c:787:65: warning: implicit declaration of function 'wcsncmp' [-Wimplicit-function-declaration]
  787 |                 is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcsncmp(wcs1, wcs2, len);
      |                                                                 ^~~~~~~
make[1]: *** [Makefile:2795: t/unit-tests/clar/clar.o] Error 1
```

This is because git now imports clar unit testing framework since
upstream commit 9b7caa2809cb (t: import the clar unit testing framework,
2024-09-04).

From upstream discussion [1], git requires toolchains with full ISO C99
support (including wchar). As such, depend the package on toolchain's
wchar support (BR2_USE_WCHAR).

[1]: https://lore.kernel.org/git/ZxCJqe4-rsRo1yHg@archie.me/t/#u

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 package/git/Config.in | 5 +++++
 1 file changed, 5 insertions(+)


base-commit: 0ad7035fce67cbe91db9dea7d81e6a4bc0691ad1

Comments

Brandon Maier Oct. 18, 2024, 12:33 p.m. UTC | #1 
Hi Bagas,

On Fri Oct 18, 2024 at 4:34 AM UTC, Bagas Sanjaya wrote:
> Git version 2.47.0 fails to build on systems without wide characters support
> (e.g. uClibc-ng without BR2_TOOLCHAIN_BUILDROOT_WCHAR selected):
>
> ```
>     CC t/unit-tests/unit-test.o
> t/unit-tests/clar/clar.c: In function 'clar__assert_equal':
> t/unit-tests/clar/clar.c:767:23: error: unknown type name 'wchar_t'
>   767 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
>       |                       ^~~~~~~
> In file included from t/unit-tests/clar/clar.c:13:
> t/unit-tests/clar/clar.c:767:58: error: unknown type name 'wchar_t'
>   767 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
>       |                                                          ^~~~~~~
> t/unit-tests/clar/clar.c:768:23: error: unknown type name 'wchar_t'
>   768 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
>       |                       ^~~~~~~
> t/unit-tests/clar/clar.c:768:58: error: unknown type name 'wchar_t'
>   768 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
>       |                                                          ^~~~~~~
> t/unit-tests/clar/clar.c:769:65: warning: implicit declaration of function 'wcscmp' [-Wimplicit-function-declaration]
>   769 |                 is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcscmp(wcs1, wcs2);
>       |                                                                 ^~~~~~
> t/unit-tests/clar/clar.c:784:23: error: unknown type name 'wchar_t'
>   784 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
>       |                       ^~~~~~~
> t/unit-tests/clar/clar.c:784:58: error: unknown type name 'wchar_t'
>   784 |                 const wchar_t *wcs1 = va_arg(args, const wchar_t *);
>       |                                                          ^~~~~~~
> t/unit-tests/clar/clar.c:785:23: error: unknown type name 'wchar_t'
>   785 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
>       |                       ^~~~~~~
> t/unit-tests/clar/clar.c:785:58: error: unknown type name 'wchar_t'
>   785 |                 const wchar_t *wcs2 = va_arg(args, const wchar_t *);
>       |                                                          ^~~~~~~
> t/unit-tests/clar/clar.c:787:65: warning: implicit declaration of function 'wcsncmp' [-Wimplicit-function-declaration]
>   787 |                 is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcsncmp(wcs1, wcs2, len);
>       |                                                                 ^~~~~~~
> make[1]: *** [Makefile:2795: t/unit-tests/clar/clar.o] Error 1
> ```
>
> This is because git now imports clar unit testing framework since
> upstream commit 9b7caa2809cb (t: import the clar unit testing framework,
> 2024-09-04).
>
> From upstream discussion [1], git requires toolchains with full ISO C99
> support (including wchar). As such, depend the package on toolchain's
> wchar support (BR2_USE_WCHAR).
>
> [1]: https://lore.kernel.org/git/ZxCJqe4-rsRo1yHg@archie.me/t/#u

I sent a patch last week that did this same change! Thomas Petazzoni
reviewed it and had some comments I haven't had time to address. If you
want to address those in your patch, I'd be happy to drop mine.

Thomas comments were:

  You need to look at reverse dependencies that "select BR2_PACKAGE_GIT"
  and propagate this wchar dependency. For example package/git-crypt/
  will need it. And also the reverse dependencies of the reverse
  dependencies, if any;

  Did you look at a way of disabling building the test suite? I very
  quickly glanced through the git Makefile, and it didn't seem like
  there was such a possibility. But if there was such a possibility, it
  would be an alternative solution to the addition of the wchar
  dependency.

My patch: https://patchwork.ozlabs.org/project/buildroot/patch/20241009-git-2-47-0-wchar-v1-1-23c261a5fdc7@gmail.com/

Thanks!
Brandon Maier

>
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>  package/git/Config.in | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/package/git/Config.in b/package/git/Config.in
> index d53921d7d5..a8ae5f59bf 100644
> --- a/package/git/Config.in
> +++ b/package/git/Config.in
> @@ -1,6 +1,7 @@
>  config BR2_PACKAGE_GIT
>  	bool "git"
>  	depends on BR2_USE_MMU # uses fork()
> +	depends on BR2_USE_WCHAR # clar unit test
>  	select BR2_PACKAGE_ZLIB
>  	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
>  	help
> @@ -9,3 +10,7 @@ config BR2_PACKAGE_GIT
>  	  projects.
>
>  	  http://git-scm.com
> +
> +comment "git needs a toolchain w/ wchar"
> +	depends on BR2_USE_MMU
> +	depends on !BR2_USE_WCHAR
>
> base-commit: 0ad7035fce67cbe91db9dea7d81e6a4bc0691ad1
Edgar Bonet Oct. 18, 2024, 12:59 p.m. UTC | #2 
Hello!

Bagas Sanjaya wrote:
> Git version 2.47.0 fails to build on systems without wide characters
> support (e.g. uClibc-ng without BR2_TOOLCHAIN_BUILDROOT_WCHAR
> selected): [...] From upstream discussion [1], git requires toolchains
> with full ISO C99 support (including wchar). As such, depend the
> package on toolchain's wchar support (BR2_USE_WCHAR).
>
> [1] https://lore.kernel.org/git/ZxCJqe4-rsRo1yHg@archie.me/t/#u

Brandon Maier replied:
> I sent a patch last week that did this same change! Thomas Petazzoni
> reviewed it and had some comments I haven't had time to address. [...]

I took a look at the issue, and here is what I found:

 1. There is no obvious way to disable building the git test suite.

 2. uClibc always installs wchar.h in sysroot, although this file is a
    stub if wchar support is disabled.

 3. Patrick Steinhardt proposed a patch in the git mailing list[1] that
    would make git compatible with wchar-less uClibc. This patch,
    however, won't work for us for a couple of reasons:

      * It doesn't apply on 2.47.0.

      * It assumes that, when using uClibc, __UCLIBC__ should be defined
        *before* the first #include, which is not the case.

Below is a version of Patrick Steinhardt's patch that seems to work on
Buildroot/master.

Regards,

Edgar.

[1] https://lore.kernel.org/git/ZxEXFI80i4Q_4NJT@pks.im/

------------------------------------------------------------------ >8 --
diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
index cef0f023c2..6de0b415b1 100644
--- a/t/unit-tests/clar/clar.c
+++ b/t/unit-tests/clar/clar.c
@@ -18,6 +18,13 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)
+   /* uClibc can be built without wchar support, in which case the
+      installed <wchar.h> is a stub that does not define wchar_t. */
+#else
+#  define HAVE_WCHAR
+#endif
+
 #ifdef _WIN32
 #	define WIN32_LEAN_AND_MEAN
 #	include <windows.h>
@@ -763,6 +770,7 @@ void clar__assert_equal(
 			}
 		}
 	}
+#ifdef HAVE_WCHAR
 	else if (!strcmp("%ls", fmt)) {
 		const wchar_t *wcs1 = va_arg(args, const wchar_t *);
 		const wchar_t *wcs2 = va_arg(args, const wchar_t *);
@@ -798,6 +806,7 @@ void clar__assert_equal(
 			}
 		}
 	}
+#endif // HAVE_WCHAR
 	else if (!strcmp("%"PRIuZ, fmt) || !strcmp("%"PRIxZ, fmt)) {
 		size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t);
 		is_equal = (sz1 == sz2);
Bagas Sanjaya Oct. 18, 2024, 1:35 p.m. UTC | #3 
On Fri, Oct 18, 2024 at 12:33:20PM +0000, Brandon Maier wrote:
> I sent a patch last week that did this same change! Thomas Petazzoni
> reviewed it and had some comments I haven't had time to address. If you
> want to address those in your patch, I'd be happy to drop mine.
> 
> Thomas comments were:
> 
>   You need to look at reverse dependencies that "select BR2_PACKAGE_GIT"
>   and propagate this wchar dependency. For example package/git-crypt/
>   will need it. And also the reverse dependencies of the reverse
>   dependencies, if any;
> 
>   Did you look at a way of disabling building the test suite? I very
>   quickly glanced through the git Makefile, and it didn't seem like
>   there was such a possibility. But if there was such a possibility, it
>   would be an alternative solution to the addition of the wchar
>   dependency.
> 
> My patch: https://patchwork.ozlabs.org/project/buildroot/patch/20241009-git-2-47-0-wchar-v1-1-23c261a5fdc7@gmail.com/

Oops! I didn't see that.

Thanks anyway.
diff mbox series

Patch

diff --git a/package/git/Config.in b/package/git/Config.in
index d53921d7d5..a8ae5f59bf 100644
--- a/package/git/Config.in
+++ b/package/git/Config.in
@@ -1,6 +1,7 @@ 
 config BR2_PACKAGE_GIT
 	bool "git"
 	depends on BR2_USE_MMU # uses fork()
+	depends on BR2_USE_WCHAR # clar unit test
 	select BR2_PACKAGE_ZLIB
 	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
 	help
@@ -9,3 +10,7 @@  config BR2_PACKAGE_GIT
 	  projects.
 
 	  http://git-scm.com
+
+comment "git needs a toolchain w/ wchar"
+	depends on BR2_USE_MMU
+	depends on !BR2_USE_WCHAR