[1/1] package/brltty: fix build with expat
diff mbox series

Message ID 20190929084004.21350-1-fontaine.fabrice@gmail.com
State Superseded
Headers show
Series
  • [1/1] package/brltty: fix build with expat
Related show

Commit Message

Fabrice Fontaine Sept. 29, 2019, 8:40 a.m. UTC
tbl2hex is a host command line that is built with:

TBL2HEX_OBJECTS_FOR_BUILD = tbl2hex.$(O_FOR_BUILD) $(PROGRAM_OBJECTS_FOR_BUILD) dataarea.$(O_FOR_BUILD) ttb_compile.$(O_FOR_BUILD) ttb_native.$(O_FOR_BUILD) atb_compile.$(O_FOR_BUILD) ctb_compile.$(O_FOR_BUILD) cldr.$(O_FOR_BUILD)
TBL2HEX_OBJECTS = $(TBL2HEX_OBJECTS_FOR_BUILD:.$(O_FOR_BUILD)=.$B)

tbl2hex$(X_FOR_BUILD): $(TBL2HEX_OBJECTS)
$(CC_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ $(TBL2HEX_OBJECTS) $(EXPAT_LIBS_FOR_BUILD) $(LDLIBS_FOR_BUILD)

So build fails if expat is enabled on target but not found on host:

gcc -DFOR_BUILD -I. -I. -I./../Programs -I../Programs -I../Headers -I./.. -I.. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE -DHAVE_CONFIG_H -g -O2 -std=gnu99 -Wall -Werror=format-security -o cldr.build.o -c cldr.c
cldr.c:31:10: fatal error: expat.h: No such file or directory
 #include <expat.h>
          ^~~~~~~~~

To fix this issue, build host-expat if needed

Fixes:
 - http://autobuild.buildroot.org/results/362cfb57e4a91a066493269d8078d931529ddf69

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/brltty/brltty.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni Sept. 30, 2019, 9:46 p.m. UTC | #1
Hello Fabrice,

On Sun, 29 Sep 2019 10:40:04 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> tbl2hex is a host command line that is built with:
> 
> TBL2HEX_OBJECTS_FOR_BUILD = tbl2hex.$(O_FOR_BUILD) $(PROGRAM_OBJECTS_FOR_BUILD) dataarea.$(O_FOR_BUILD) ttb_compile.$(O_FOR_BUILD) ttb_native.$(O_FOR_BUILD) atb_compile.$(O_FOR_BUILD) ctb_compile.$(O_FOR_BUILD) cldr.$(O_FOR_BUILD)
> TBL2HEX_OBJECTS = $(TBL2HEX_OBJECTS_FOR_BUILD:.$(O_FOR_BUILD)=.$B)
> 
> tbl2hex$(X_FOR_BUILD): $(TBL2HEX_OBJECTS)
> $(CC_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ $(TBL2HEX_OBJECTS) $(EXPAT_LIBS_FOR_BUILD) $(LDLIBS_FOR_BUILD)
> 
> So build fails if expat is enabled on target but not found on host:
> 
> gcc -DFOR_BUILD -I. -I. -I./../Programs -I../Programs -I../Headers -I./.. -I.. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE -DHAVE_CONFIG_H -g -O2 -std=gnu99 -Wall -Werror=format-security -o cldr.build.o -c cldr.c
> cldr.c:31:10: fatal error: expat.h: No such file or directory
>  #include <expat.h>
>           ^~~~~~~~~
> 
> To fix this issue, build host-expat if needed
> 
> Fixes:
>  - http://autobuild.buildroot.org/results/362cfb57e4a91a066493269d8078d931529ddf69
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

Maybe it's because it's late and I'm no longer thinking properly, but
do you understand how disabling expat prevents the cldr program from
being built, and therefore the tbl2hex utility tool from being built ?
They seem to be unconditionally built by Programs/Makefile.in.

Thanks,

Thomas
Fabrice Fontaine Oct. 1, 2019, 6:42 a.m. UTC | #2
Dear Thomas,

Le lun. 30 sept. 2019 à 23:46, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Sun, 29 Sep 2019 10:40:04 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > tbl2hex is a host command line that is built with:
> >
> > TBL2HEX_OBJECTS_FOR_BUILD = tbl2hex.$(O_FOR_BUILD) $(PROGRAM_OBJECTS_FOR_BUILD) dataarea.$(O_FOR_BUILD) ttb_compile.$(O_FOR_BUILD) ttb_native.$(O_FOR_BUILD) atb_compile.$(O_FOR_BUILD) ctb_compile.$(O_FOR_BUILD) cldr.$(O_FOR_BUILD)
> > TBL2HEX_OBJECTS = $(TBL2HEX_OBJECTS_FOR_BUILD:.$(O_FOR_BUILD)=.$B)
> >
> > tbl2hex$(X_FOR_BUILD): $(TBL2HEX_OBJECTS)
> > $(CC_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ $(TBL2HEX_OBJECTS) $(EXPAT_LIBS_FOR_BUILD) $(LDLIBS_FOR_BUILD)
> >
> > So build fails if expat is enabled on target but not found on host:
> >
> > gcc -DFOR_BUILD -I. -I. -I./../Programs -I../Programs -I../Headers -I./.. -I.. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE -DHAVE_CONFIG_H -g -O2 -std=gnu99 -Wall -Werror=format-security -o cldr.build.o -c cldr.c
> > cldr.c:31:10: fatal error: expat.h: No such file or directory
> >  #include <expat.h>
> >           ^~~~~~~~~
> >
> > To fix this issue, build host-expat if needed
> >
> > Fixes:
> >  - http://autobuild.buildroot.org/results/362cfb57e4a91a066493269d8078d931529ddf69
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>
> Maybe it's because it's late and I'm no longer thinking properly, but
> do you understand how disabling expat prevents the cldr program from
> being built, and therefore the tbl2hex utility tool from being built ?
> They seem to be unconditionally built by Programs/Makefile.in.
cldr.c is mostly an empty shell without expat as most of the code is
protected by ifdef HAVE_EXPAT blocks, see
https://github.com/brltty/brltty/blob/aba3d8cc2dc765a0933aabb609928e568e085d39/Programs/cldr.c.
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Thomas Petazzoni Oct. 5, 2019, 8:37 p.m. UTC | #3
On Tue, 1 Oct 2019 08:42:24 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > Maybe it's because it's late and I'm no longer thinking properly, but
> > do you understand how disabling expat prevents the cldr program from
> > being built, and therefore the tbl2hex utility tool from being built ?
> > They seem to be unconditionally built by Programs/Makefile.in.  
> cldr.c is mostly an empty shell without expat as most of the code is
> protected by ifdef HAVE_EXPAT blocks, see
> https://github.com/brltty/brltty/blob/aba3d8cc2dc765a0933aabb609928e568e085d39/Programs/cldr.c.

OK, but where is the logic that links with -lexpat when it's available,
and doesn't link with -lexpat when not available ? I'm a bit confused
by the build logic of this package I must say.

Thomas
Fabrice Fontaine Oct. 9, 2019, 5:16 p.m. UTC | #4
Dear Thomas,

Le sam. 5 oct. 2019 à 22:37, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> On Tue, 1 Oct 2019 08:42:24 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > > Maybe it's because it's late and I'm no longer thinking properly, but
> > > do you understand how disabling expat prevents the cldr program from
> > > being built, and therefore the tbl2hex utility tool from being built ?
> > > They seem to be unconditionally built by Programs/Makefile.in.
> > cldr.c is mostly an empty shell without expat as most of the code is
> > protected by ifdef HAVE_EXPAT blocks, see
> > https://github.com/brltty/brltty/blob/aba3d8cc2dc765a0933aabb609928e568e085d39/Programs/cldr.c.
>
> OK, but where is the logic that links with -lexpat when it's available,
> and doesn't link with -lexpat when not available ? I'm a bit confused
> by the build logic of this package I must say.
Linking with expat is done through EXPAT_LIBS in Programs/Makefile.in.
EXPAT_LIBS is set to @expat_libs@ in config.mk.in.
expat_libs is set in configure.ac depending on BRLTTY_HAVE_PACKAGE
result (which is basically a pkg-config wrapper defined in
m4/brltty.m4).
The main issue with this package is that it assumes that if expat is
available on target then it is also available on host.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Thomas Petazzoni Oct. 12, 2019, 7:58 p.m. UTC | #5
Hello Fabrice,

On Sun, 29 Sep 2019 10:40:04 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> tbl2hex is a host command line that is built with:
> 
> TBL2HEX_OBJECTS_FOR_BUILD = tbl2hex.$(O_FOR_BUILD) $(PROGRAM_OBJECTS_FOR_BUILD) dataarea.$(O_FOR_BUILD) ttb_compile.$(O_FOR_BUILD) ttb_native.$(O_FOR_BUILD) atb_compile.$(O_FOR_BUILD) ctb_compile.$(O_FOR_BUILD) cldr.$(O_FOR_BUILD)
> TBL2HEX_OBJECTS = $(TBL2HEX_OBJECTS_FOR_BUILD:.$(O_FOR_BUILD)=.$B)
> 
> tbl2hex$(X_FOR_BUILD): $(TBL2HEX_OBJECTS)
> $(CC_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ $(TBL2HEX_OBJECTS) $(EXPAT_LIBS_FOR_BUILD) $(LDLIBS_FOR_BUILD)
> 
> So build fails if expat is enabled on target but not found on host:
> 
> gcc -DFOR_BUILD -I. -I. -I./../Programs -I../Programs -I../Headers -I./.. -I.. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE -DHAVE_CONFIG_H -g -O2 -std=gnu99 -Wall -Werror=format-security -o cldr.build.o -c cldr.c
> cldr.c:31:10: fatal error: expat.h: No such file or directory
>  #include <expat.h>
>           ^~~~~~~~~

Did you really test this patch on a system where expat was not
installed system-wide ? Because I did, and building host-expat before
building brltty doesn't solve the build issue, I still get:

cldr.c:31:19: fatal error: expat.h: No such file or directory
compilation terminated.

And this is quite expected if you look at the build command line:

gcc -DFOR_BUILD -I. -I. -I./../Programs -I../Programs -I../Headers -I./.. -I.. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE  -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_XOPEN_SOURCE_EXTENDED -D_GNU_SOURCE -DHAVE_CONFIG_H -g -O2 -std=gnu99 -Wall -Werror=format-security -o cldr.build.o -c cldr.c

There's nowhere a -I flag that points to $(HOST_DIR)/include. I guess
it at least needs passing some CFLAGS_FOR_BUILD or something like that.

Could you have a look ?

Thanks,

Thomas

Patch
diff mbox series

diff --git a/package/brltty/brltty.mk b/package/brltty/brltty.mk
index b9d608b9f5..3ae8b899b8 100644
--- a/package/brltty/brltty.mk
+++ b/package/brltty/brltty.mk
@@ -51,7 +51,8 @@  BRLTTY_CONF_OPTS += --without-espeak
 endif
 
 ifeq ($(BR2_PACKAGE_EXPAT),y)
-BRLTTY_DEPENDENCIES += expat
+# host-expat is needed by tbl2hex's host program
+BRLTTY_DEPENDENCIES += host-expat expat
 BRLTTY_CONF_OPTS += --enable-expat
 else
 BRLTTY_CONF_OPTS += --disable-expat