Re: [Buildroot] [PATCH] bcusdk: fix build without libxml

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Kurt,

On Wed, 12 Jun 2024 11:30:57 +0200
Kurt Van Dijck via buildroot <buildroot@buildroot.org> wrote:

> bcusdk is configured without libxml.
> This commit removes the autotools libxml dependency.
> 
> Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>

Thanks for your patch. However, could you clarify which build failure
it is fixing? Which Buildroot configuration is broken, which error
message?

I don't seem to see any build failure related to this in our
autobuilders... but there are other build failures related to bcusdk:

  http://autobuild.buildroot.net/?reason=bcusdk%

Some more comments below.

> ---
>  package/bcusdk/0003-remove-xml-autoconf.patch | 11 +++++++++++
>  package/bcusdk/bcusdk.mk                      |  2 ++
>  2 files changed, 13 insertions(+)
>  create mode 100644 package/bcusdk/0003-remove-xml-autoconf.patch
> 
> diff --git a/package/bcusdk/0003-remove-xml-autoconf.patch b/package/bcusdk/0003-remove-xml-autoconf.patch
> new file mode 100644
> index 0000000000..33ce45fe12
> --- /dev/null
> +++ b/package/bcusdk/0003-remove-xml-autoconf.patch

We need this patch to be generated with "git format-patch", and to have
a proper commit message + your Signed-off-by line. I encourage you to
look at the two other patches in package/bcusdk/ to see the format that
is expected.

Also, we expect the patch to be submitted upstream, and the patch to
carry an Upstream: tag that references the upstream pull request (or
patch posted to a mailing list).

> @@ -0,0 +1,11 @@
> +--- a/configure.in	2024-06-12 11:20:16.800828493 +0200
> ++++ b/configure.in	2024-06-12 11:18:19.275693151 +0200
> +@@ -81,7 +81,7 @@
> + 
> + if test x$onlyeibd = xfalse ; then
> + 
> +-AM_PATH_XML2(2.6.16,,[AC_MSG_ERROR([limxml2 not found])])
> ++#AM_PATH_XML2(2.6.16,,[AC_MSG_ERROR([limxml2 not found])])

Don't comment code, remove it entirely. But you need to clarify in the
commit message why it is not needed.

> + AC_PATH_PROG(TAS,[m68hc05-as],,[$PATH:$bindir:$prefix/bin])
> + AC_PATH_PROG(TLD,[m68hc05-ld],,[$PATH:$bindir:$prefix/bin])
> + AC_PATH_PROG(TAR,[m68hc05-ar],,[$PATH:$bindir:$prefix/bin])
> diff --git a/package/bcusdk/bcusdk.mk b/package/bcusdk/bcusdk.mk
> index f14b23c7cc..c572df725d 100644
> --- a/package/bcusdk/bcusdk.mk
> +++ b/package/bcusdk/bcusdk.mk
> @@ -23,6 +23,8 @@ BCUSDK_CONF_OPTS = \
>  
>  BCUSDK_DEPENDENCIES = libpthsem
>  
> +BCUSDK_AUTORECONF=YES

We need a comment above that explains why autoreconf is needed. Usually
just the patch file name is enough. And we need spaces around the =
sign.

Could you rework your patch according to those suggestions, and post a
new iteration?

And of course, ideally if you could fix the other build failure(s)
occurring with bcusdk, it would be awesome.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot