kyz / libmspack

A library for some loosely related Microsoft compression formats, CAB, CHM, HLP, LIT, KWAJ and SZDD.
https://www.cabextract.org.uk/libmspack/
169 stars 45 forks source link

cabextract-1.8 fails to build with --with-external-libmspack against libmspack-0.8alpha #20

Closed Whissi closed 6 years ago

Whissi commented 6 years ago
src/cabinfo.c:37:10: fatal error: system.h: No such file or directory
 #include <system.h>
          ^~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:508: src/cabinfo.o] Error 1
kyz commented 6 years ago

Agreed, the distributed cabextract-1.8.tar.gz does not build with --with-external-libmspack. I noticed this myself, only after release. In future, my testing before release will include an additional distcheck with --with-external-libmspack set.

I'm waiting a few more days to see if any other issues are found, rather than immediately releasing a new release with only this fix.

The fix is already committed in 906b135 and 3d3dbd2 :

diff -ur cabextract-1.8/mspack/cabd.c cabextract-1.8-fixed/mspack/cabd.c
--- cabextract-1.8/mspack/cabd.c    2018-10-21 16:45:07.000000000 +0100
+++ cabextract-1.8-fixed/mspack/cabd.c    2018-10-24 08:52:49.984929988 +0100
@@ -23,6 +23,9 @@

 #include <system.h>
 #include <cab.h>
+#include <mszip.h>
+#include <qtm.h>
+#include <lzx.h>
 #include <assert.h>

 /* Notes on compliance with cabinet specification:
diff -ur cabextract-1.8/mspack/cab.h cabextract-1.8-fixed/mspack/cab.h
--- cabextract-1.8/mspack/cab.h    2018-10-21 16:44:47.000000000 +0100
+++ cabextract-1.8-fixed/mspack/cab.h    2018-10-24 08:52:39.572913382 +0100
@@ -10,10 +10,6 @@
 #ifndef MSPACK_CAB_H
 #define MSPACK_CAB_H 1

-#include <mszip.h>
-#include <qtm.h>
-#include <lzx.h>
-
 /* generic CAB definitions */

 /* structure offsets */
diff -ur cabextract-1.8/src/cabinfo.c cabextract-1.8-fixed/src/cabinfo.c
--- cabextract-1.8/src/cabinfo.c    2018-10-21 13:57:04.000000000 +0100
+++ cabextract-1.8-fixed/src/cabinfo.c    2018-10-24 08:52:22.320885881 +0100
@@ -34,9 +34,9 @@
 #endif

 /* include <system.h> from libmspack for LD and EndGetI?? macros */
-#include <system.h>
+#include <mspack/system.h>
 /* include <cab.h> from libmspack for cab structure offsets */
-#include <cab.h>
+#include <mspack/cab.h>

 #if HAVE_FSEEKO
 # define FSEEK fseeko 
Whissi commented 6 years ago

Thanks! But are you sure about the fix?

I patched my libmspack with

--- a/mspack/cab.h
+++ b/mspack/cab.h
@@ -10,10 +10,6 @@
 #ifndef MSPACK_CAB_H
 #define MSPACK_CAB_H 1

-#include <mszip.h>
-#include <qtm.h>
-#include <lzx.h>
-
 /* generic CAB definitions */

 /* structure offsets */
--- a/mspack/cabd.c
+++ b/mspack/cabd.c
@@ -23,7 +23,9 @@

 #include <system.h>
 #include <cab.h>
-#include <assert.h>
+#include <mszip.h>
+#include <lzx.h>
+#include <qtm.h>

 /* Notes on compliance with cabinet specification:
  *
@@ -1161,8 +1163,6 @@ static int cabd_init_decomp(struct mscab_decompressor_p *self, unsigned int ct)
 {
   struct mspack_file *fh = (struct mspack_file *) self;

-  assert(self && self->d);
-
   self->d->comp_type = ct;

   switch (ct & cffoldCOMPTYPE_MASK) {

and re-installed patched package.

When I now try to build cabextract-1.8 again which I also patched with

--- a/src/cabinfo.c
+++ b/src/cabinfo.c
@@ -34,9 +34,9 @@
 #endif

 /* include <system.h> from libmspack for LD and EndGetI?? macros */
-#include <system.h>
+#include <mspack/system.h>
 /* include <cab.h> from libmspack for cab structure offsets */
-#include <cab.h>
+#include <mspack/cab.h>

 #if HAVE_FSEEKO
 # define FSEEK fseeko

I'll get

In file included from src/cabinfo.c:39:
./mspack/cab.h:13:10: fatal error: mszip.h: No such file or directory
 #include <mszip.h>
          ^~~~~~~~~
compilation terminated.

Looks like it is still using some bundled headers which it shouldn't due to --with-external-libmspack. For example, when I delete mspack folder from cabextract source I get an error like

src/cabinfo.c:37:10: fatal error: mspack/system.h: No such file or directory
 #include <mspack/system.h>
          ^~~~~~~~~~~~~~~~~
compilation terminated.

which I wouldn't expect due to --with-external-libmspack.

Some additional notes: While testing, I tested previous version (libmspack-0.7alpha) and noticed another build failure:

src/cabextract.c: In function ‘main’:
src/cabextract.c:397:25: error: ‘MSCABD_PARAM_SALVAGE’ undeclared (first use in this function); did you mean ‘MSCABD_PARAM_SEARCHBUF’?
   cabd->set_param(cabd, MSCABD_PARAM_SALVAGE, args.fix);
                         ^~~~~~~~~~~~~~~~~~~~
                         MSCABD_PARAM_SEARCHBUF
src/cabextract.c:397:25: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:508: src/cabextract.o] Error 1

This makes me wonder if cabextract-1.8 should build against previous libmspack versions or should require libmspack-0.8alpha or newer. Maybe you can change cabextract's configure script to make use of pkconfig file provided by libmspack as well in a future update.

kyz commented 6 years ago

Yes, I'm sure about the fix, and it applies to cabextract only.

libmspack is unaffected and does not need any modification. The bundled cut-down copy of libmspack in the cabextract distribution needs modification.

The problem is that cabinfo, which is only included in the cabextract distribution (and does not link with libmspack at all), nonetheless uses identical macros and constants that are defined in some of the private libmspack headers, which happen to be bundled with cabextract, so for cabextract 1.8 I chose to include these bundled headers into cabinfo rather than repeat the macros/definitions. However I chose to use an include path that presumes the bundled cut-down libmspack will be built, which is untrue when using --with-external-libmspack

The fix is to make it explicitly include those private headers to get the macros and defines, independent of whether an external or bundled libmspack is in use.

cabextract 1.8 uses new functionality only available in libmspack 0.8alpha. You need at least libmspack 0.8alpha to build cabextract 1.8. Once built, cabextract would be binary compatible with earlier versions of libmspack, but cabextract 1.8's headline changes (improved salvaging) are entirely dependent on new code in libmspack 0.8, so it would be unreasonable to use cabextract 1.8 with earlier versions of libmspack.

I'll look into using the pkgconfig file.

kyz commented 6 years ago

Hi there, I've made a few changes, including making cabextract depending on pkg-config to get libmspack (which will also check the version is >= 0.8).

These changes haven't been released as packaged libmspack/cabextract yet, but if you're in a position to try building and testing what's in the repository, I'd appreciate it.

Whissi commented 6 years ago

I created a Gentoo Live Ebuild for cabextract and libmspack. It is picking up libmspack via pkg-config but I still get build failures:

>>> Unpacking source...
 * Repository id: kyz_libmspack.git
 * To override fetched repository properties, use:
 *   EGIT_OVERRIDE_REPO_KYZ_LIBMSPACK
 *   EGIT_OVERRIDE_BRANCH_KYZ_LIBMSPACK
 *   EGIT_OVERRIDE_COMMIT_KYZ_LIBMSPACK
 *   EGIT_OVERRIDE_COMMIT_DATE_KYZ_LIBMSPACK
 *
 * Fetching https://github.com/kyz/libmspack.git ...
git fetch https://github.com/kyz/libmspack.git +HEAD:refs/git-r3/HEAD
git symbolic-ref refs/git-r3/app-arch/cabextract/0/__main__ refs/git-r3/HEAD
 * Checking out https://github.com/kyz/libmspack.git to /var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999 ...
git checkout --quiet refs/git-r3/HEAD
GIT update -->
   repository:               https://github.com/kyz/libmspack.git
   at the commit:            ab392388a8d73fc0c1e314ab5ca5ef24928c548b
>>> Source unpacked in /var/tmp/portage/app-arch/cabextract-9999/work
>>> Preparing source in /var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999 ...
 * Running eautoreconf in '/var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999' ...
 * Running aclocal ...                                                                                           [ ok ]
 * Running autoconf --force ...                                                                                  [ ok ]
 * Running autoheader ...                                                                                        [ ok ]
 * Running automake --add-missing --copy --foreign --force-missing ...                                           [ ok ]
 * Running elibtoolize in: cabextract-9999/
>>> Source prepared.
>>> Configuring source in /var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999 ...
 * econf: updating cabextract-9999/config.guess with /usr/share/gnuconfig/config.guess
 * econf: updating cabextract-9999/config.sub with /usr/share/gnuconfig/config.sub
./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --disable-silent-rules --docdir=/usr/share/doc/cabextract-9999 --htmldir=/usr/share/doc/cabextract-9999/html --libdir=/usr/lib64 --with-external-libmspack=yes
configure: loading site script /usr/share/config.site
checking for a BSD-compatible install... /usr/lib/portage/python3.6/ebuild-helpers/xattr/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for x86_64-pc-linux-gnu-gcc... x86_64-pc-linux-gnu-gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether x86_64-pc-linux-gnu-gcc accepts -g... yes
checking for x86_64-pc-linux-gnu-gcc option to accept ISO C89... none needed
checking whether x86_64-pc-linux-gnu-gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of x86_64-pc-linux-gnu-gcc... none
checking for x86_64-pc-linux-gnu-ar... x86_64-pc-linux-gnu-ar
checking the archiver (x86_64-pc-linux-gnu-ar) interface... ar
checking for x86_64-pc-linux-gnu-ranlib... x86_64-pc-linux-gnu-ranlib
checking how to run the C preprocessor... x86_64-pc-linux-gnu-gcc -E
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking getopt.h usability... yes
checking getopt.h presence... yes
checking for getopt.h... yes
checking for inttypes.h... (cached) yes
checking for strings.h... (cached) yes
checking for an ANSI C-conforming const... yes
checking for inline... inline
checking for mode_t... yes
checking for off_t... yes
checking for size_t... yes
checking size of off_t... 8
checking for special C compiler options needed for large files... no
checking for _FILE_OFFSET_BITS value needed for large files... no
checking for working POSIX fnmatch... yes
checking for _LARGEFILE_SOURCE value needed for large files... no
checking for mkdir... yes
checking for _mkdir... no
checking whether mkdir takes one argument... no
checking for towlower... yes
checking for umask... yes
checking for utime... yes
checking for utimes... yes
checking for getopt_long... yes
checking for working alloca.h... yes
checking for alloca... yes
checking for mbstate_t... yes
checking for working POSIX fnmatch... (cached) yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking for ld used by x86_64-pc-linux-gnu-gcc... /usr/x86_64-pc-linux-gnu/bin/ld
checking if the linker (/usr/x86_64-pc-linux-gnu/bin/ld) is GNU ld... yes
checking for shared library run path origin... done
checking for iconv... yes
checking for working iconv... yes
checking for iconv declaration...
         extern size_t iconv (iconv_t cd, char * *inbuf, size_t *inbytesleft, char * *outbuf, size_t *outbytesleft);
checking for x86_64-pc-linux-gnu-pkg-config... /usr/bin/x86_64-pc-linux-gnu-pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for libmspack >= 0.8alpha... yes
checking mspack.h usability... yes
checking mspack.h presence... yes
checking for mspack.h... yes
checking for mspack_create_cab_decompressor in -lmspack... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating cabextract.spec
config.status: creating test/testcase
config.status: creating config.h
config.status: executing depfiles commands
>>> Source configured.
>>> Compiling source in /var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999 ...
make --jobs 6 AR=x86_64-pc-linux-gnu-ar
make  all-am
make[1]: Entering directory '/var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999'
x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I.     -O2 -pipe -march=ivybridge -mtune=ivybridge -mno-xsaveopt -Wno-error=missing-prototypes -Wno-error=enum-compare -Wno-error=unused-function -Wno-error=deprecated-declarations -frecord-gcc-switches -c -o md5.o md5.c
x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I.     -O2 -pipe -march=ivybridge -mtune=ivybridge -mno-xsaveopt -Wno-error=missing-prototypes -Wno-error=enum-compare -Wno-error=unused-function -Wno-error=deprecated-declarations -frecord-gcc-switches -c -o src/cabextract.o src/cabextract.c
x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I.     -O2 -pipe -march=ivybridge -mtune=ivybridge -mno-xsaveopt -Wno-error=missing-prototypes -Wno-error=enum-compare -Wno-error=unused-function -Wno-error=deprecated-declarations -frecord-gcc-switches -c -o src/cabinfo.o src/cabinfo.c
src/cabinfo.c:28:10: fatal error: mspack/system.h: No such file or directory
 #include <mspack/system.h>
          ^~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:692: src/cabinfo.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory '/var/tmp/portage/app-arch/cabextract-9999/work/cabextract-9999'
make: *** [Makefile:549: all] Error 2

Just for your information: In unpack phase we basically run git clone https://github.com/kyz/libmspack.git which will create S with /cabextract and /libmspack (=e.g. your current Git structure). In prepare phase for cabextract I'll remove "libmspack" folder and move cabextract folder one level up (I am doing the same for libmspack ebuild as well but here I remove "cabextract" folder and move "libmspack" folder one level up). So I will end with same structure like your release tarballs...

kyz commented 6 years ago

I'll remove "libmspack" folder and move cabextract folder one level up (I am doing the same for libmspack ebuild as well but here I remove "cabextract" folder and move "libmspack" folder one level up). So I will end with same structure like your release tarballs...

This is the problem. All files under cabextract/mspack/ are symlinks to the same file in libmspack/mspack/. Moving or deleting libmspack/ will break the symlinks and thus#include <mspack/system.h> will fail.

In my own copy of the repository, no directories are moved and make dist changes those symlinks into the linked-to files while making the tarball.

You should either leave the directories where they are, or replace the symlinks with the linked-to files.

Would you kindly try it again?

Whissi commented 6 years ago

Sorry, I missed the symlink. I'll update ebuild and make sure the "source" folder will have these files. Incoming status update.

Whissi commented 6 years ago

OK, now everything works. Thank you.

I must admit that I get nervous when I see that it uses headers from the bundled libmspack source but will link with the external libmspack version (which could in theory use different headers). Of course, when you make sure that libmspack requirement will be updated whenever there will be a header change breaking backward compatibility all should be good but I am just saying ;-)

kyz commented 6 years ago

I understand your nerves, but when --with-external-libmspack is set, cabextract will include only the installed mspack.h and links against only the installed mspack library.

cabinfo only includes private headers from the bundled mspack directory. It does not use either the installed or bundled mspack.h, and it does not link to the installed mspack library, it is standalone.

kyz commented 6 years ago

cabextract 1.9 has been released. Thanks for your report!