diff mbox

[1/1] package/exfat-utils: fix compilation error with musl

Message ID 1437932111-411-1-git-send-email-brendanheading@gmail.com
State Superseded
Headers show

Commit Message

Brendan Heading July 26, 2015, 5:35 p.m. UTC
exfat-utils build would not build on Linux unless __GLIBC__ was defined.
Reworked header file to relax this requirement in favour of other libcs.
patch submitted to the exfat-utils mailing list for comment.

Also added missing dependency - exfat requires libfuse.

See http://autobuild.buildroot.net/results/c60/c60d0f9a93c90d41c3c86c78b0a0

Signed-off-by: Brendan Heading <brendanheading@gmail.com>
---
 .../0001-fix-compiling-with-non-glibc-libcs.patch  | 53 ++++++++++++++++++++++
 package/exfat-utils/Config.in                      |  2 +
 package/exfat-utils/exfat-utils.mk                 |  2 +-
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch

Comments

Baruch Siach July 26, 2015, 7:38 p.m. UTC | #1
Hi Brendan,

On Sun, Jul 26, 2015 at 06:35:11PM +0100, Brendan Heading wrote:
> exfat-utils build would not build on Linux unless __GLIBC__ was defined.
> Reworked header file to relax this requirement in favour of other libcs.
> patch submitted to the exfat-utils mailing list for comment.
> 
> Also added missing dependency - exfat requires libfuse.
> 
> See http://autobuild.buildroot.net/results/c60/c60d0f9a93c90d41c3c86c78b0a0
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
> ---

[snip]

> diff --git a/package/exfat-utils/Config.in b/package/exfat-utils/Config.in
> index aedff5f..ead5d47 100644
> --- a/package/exfat-utils/Config.in
> +++ b/package/exfat-utils/Config.in
> @@ -1,5 +1,6 @@
>  config BR2_PACKAGE_EXFAT_UTILS
>  	bool "exfat-utils"
> +	select BR2_PACKAGE_LIBFUSE
>  	depends on BR2_USE_WCHAR

Please propagate BR2_PACKAGE_LIBFUSE dependencies.

baruch
Brendan Heading July 26, 2015, 7:59 p.m. UTC | #2
>
> > --- a/package/exfat-utils/Config.in
> > +++ b/package/exfat-utils/Config.in
> > @@ -1,5 +1,6 @@
> >  config BR2_PACKAGE_EXFAT_UTILS
> >       bool "exfat-utils"
> > +     select BR2_PACKAGE_LIBFUSE
> >       depends on BR2_USE_WCHAR
>
> Please propagate BR2_PACKAGE_LIBFUSE dependencies.
>

Baruch,

Sorry .. can you define "propagate" ?

Brendan
Baruch Siach July 26, 2015, 8:04 p.m. UTC | #3
Hi Brendan,

On Sun, Jul 26, 2015 at 08:59:36PM +0100, Brendan Heading wrote:
> > > --- a/package/exfat-utils/Config.in
> > > +++ b/package/exfat-utils/Config.in
> > > @@ -1,5 +1,6 @@
> > >  config BR2_PACKAGE_EXFAT_UTILS
> > >       bool "exfat-utils"
> > > +     select BR2_PACKAGE_LIBFUSE
> > >       depends on BR2_USE_WCHAR
> >
> > Please propagate BR2_PACKAGE_LIBFUSE dependencies.
> 
> Baruch,
> 
> Sorry .. can you define "propagate" ?

Just copy the dependencies of BR2_PACKAGE_LIBFUSE to BR2_PACKAGE_EXFAT_UTILS. 
See the explanation at http://nightly.buildroot.org/manual.html#_config_files 
under "Choosing depends on or select".

baruch
Yann E. MORIN July 26, 2015, 8:07 p.m. UTC | #4
Brendan, All,

On 2015-07-26 18:35 +0100, Brendan Heading spake thusly:
> exfat-utils build would not build on Linux unless __GLIBC__ was defined.
> Reworked header file to relax this requirement in favour of other libcs.
> patch submitted to the exfat-utils mailing list for comment.
> 
> Also added missing dependency - exfat requires libfuse.

Your patch then does two different things;
  - add a missing dependency
  - fix a build failure with musl

It thus should be two different patches, one for each changes.

Regards,
Yann E. MORIN.
Brendan Heading July 26, 2015, 8:14 p.m. UTC | #5
>
> Just copy the dependencies of BR2_PACKAGE_LIBFUSE to
> BR2_PACKAGE_EXFAT_UTILS.
> See the explanation at
> http://nightly.buildroot.org/manual.html#_config_files
> under "Choosing depends on or select".
>

Baruch,

I think this has already been done ? package/exfat/Config.in :

config BR2_PACKAGE_EXFAT
bool "exFAT (FUSE)"
depends on BR2_TOOLCHAIN_HAS_THREADS # libfuse
depends on BR2_USE_MMU # libfuse
depends on BR2_USE_WCHAR
depends on !BR2_STATIC_LIBS # libfuse
select BR2_PACKAGE_LIBFUSE <-- added by me
help
  A full-featured exFAT file system implementation for GNU/Linux
  and other Unix-like systems as a FUSE module.

  http://code.google.com/p/exfat/

package/libfuse/Config.in :

config BR2_PACKAGE_LIBFUSE
bool "libfuse"
# Really doesn't like static, see fuse/lib/fuse.c
depends on !BR2_STATIC_LIBS
depends on BR2_TOOLCHAIN_HAS_THREADS
depends on BR2_USE_MMU # fork()
help
  FUSE (Filesystem in UserSpacE)

  http://fuse.sourceforge.net/
Brendan Heading July 26, 2015, 8:15 p.m. UTC | #6
Thanks Yann, I wasn't sure whether to split it or not and took a wrong
guess :) will submit a split update shortly.

Brendan

On 26 July 2015 at 21:07, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> Brendan, All,
>
> On 2015-07-26 18:35 +0100, Brendan Heading spake thusly:
> > exfat-utils build would not build on Linux unless __GLIBC__ was defined.
> > Reworked header file to relax this requirement in favour of other libcs.
> > patch submitted to the exfat-utils mailing list for comment.
> >
> > Also added missing dependency - exfat requires libfuse.
>
> Your patch then does two different things;
>   - add a missing dependency
>   - fix a build failure with musl
>
> It thus should be two different patches, one for each changes.
>
> Regards,
> Yann E. MORIN.
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There
> is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>
Baruch Siach July 26, 2015, 8:34 p.m. UTC | #7
Hi Brendan,

On Sun, Jul 26, 2015 at 09:14:29PM +0100, Brendan Heading wrote:
> > Just copy the dependencies of BR2_PACKAGE_LIBFUSE to
> > BR2_PACKAGE_EXFAT_UTILS.
> > See the explanation at
> > http://nightly.buildroot.org/manual.html#_config_files
> > under "Choosing depends on or select".
> 
> I think this has already been done ? package/exfat/Config.in :
> 
> config BR2_PACKAGE_EXFAT
> bool "exFAT (FUSE)"
> depends on BR2_TOOLCHAIN_HAS_THREADS # libfuse
> depends on BR2_USE_MMU # libfuse
> depends on BR2_USE_WCHAR
> depends on !BR2_STATIC_LIBS # libfuse
> select BR2_PACKAGE_LIBFUSE <-- added by me

As far as I can see your patch touches package/exfat-utils/Config.in, not 
package/exfat/Config.in.

baruch

> help
>   A full-featured exFAT file system implementation for GNU/Linux
>   and other Unix-like systems as a FUSE module.
> 
>   http://code.google.com/p/exfat/
> 
> package/libfuse/Config.in :
> 
> config BR2_PACKAGE_LIBFUSE
> bool "libfuse"
> # Really doesn't like static, see fuse/lib/fuse.c
> depends on !BR2_STATIC_LIBS
> depends on BR2_TOOLCHAIN_HAS_THREADS
> depends on BR2_USE_MMU # fork()
> help
>   FUSE (Filesystem in UserSpacE)
> 
>   http://fuse.sourceforge.net/
Brendan Heading July 26, 2015, 8:41 p.m. UTC | #8
> As far as I can see your patch touches package/exfat-utils/Config.in, not
> package/exfat/Config.in.
>

It's clearly not my day today Baruch! :) Fixing the patch now.

Brendan
Thomas Petazzoni July 26, 2015, 8:59 p.m. UTC | #9
Dear Brendan Heading,

On Sun, 26 Jul 2015 18:35:11 +0100, Brendan Heading wrote:
> exfat-utils build would not build on Linux unless __GLIBC__ was defined.
> Reworked header file to relax this requirement in favour of other libcs.
> patch submitted to the exfat-utils mailing list for comment.
> 
> Also added missing dependency - exfat requires libfuse.

The libfuse dependency seems to be unrelated to the musl problem, so it
should be a separate patch.

Also, I just did a build of exfat-utils which libfuse, and it built
just fine, so I think you're wrong with the fact that it needs libfuse.
Here is the defconfig I've used:

BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05-496-g85945aa.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_1=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_EXFAT_UTILS=y
# BR2_TARGET_ROOTFS_TAR is not set

And the list of steps that were done:

>>> skeleton undefined Extracting
>>> skeleton undefined Patching
>>> skeleton undefined Configuring
>>> skeleton undefined Building
>>> skeleton undefined Installing to target
>>> toolchain-external undefined Extracting
>>> toolchain-external undefined Patching
>>> toolchain-external undefined Configuring
>>> toolchain-external undefined Building
>>> toolchain-external undefined Installing to staging directory
>>> toolchain-external undefined Copying external toolchain sysroot to staging...
>>> toolchain-external undefined Building ext-toolchain wrapper
>>> toolchain-external undefined Fixing libtool files
>>> toolchain-external undefined Installing to target
>>> toolchain-external undefined Copying external toolchain libraries to target...
>>> toolchain virtual Extracting
>>> toolchain virtual Patching
>>> toolchain virtual Configuring
>>> toolchain virtual Building
>>> toolchain virtual Installing to target
>>> host-m4 1.4.17 Extracting
>>> host-m4 1.4.17 Patching
>>> host-m4 1.4.17 Updating config.sub and config.guess
>>> host-m4 1.4.17 Patching libtool
>>> host-m4 1.4.17 Configuring
>>> host-m4 1.4.17 Building
>>> host-m4 1.4.17 Installing to host directory
>>> host-libtool 2.4.6 Extracting
>>> host-libtool 2.4.6 Patching
>>> host-libtool 2.4.6 Updating config.sub and config.guess
>>> host-libtool 2.4.6 Configuring
>>> host-libtool 2.4.6 Building
>>> host-libtool 2.4.6 Installing to host directory
>>> host-autoconf 2.69 Extracting
>>> host-autoconf 2.69 Patching
>>> host-autoconf 2.69 Updating config.sub and config.guess
>>> host-autoconf 2.69 Patching libtool
>>> host-autoconf 2.69 Configuring
>>> host-autoconf 2.69 Building
>>> host-autoconf 2.69 Installing to host directory
>>> host-automake 1.15 Extracting
>>> host-automake 1.15 Patching
>>> host-automake 1.15 Updating config.sub and config.guess
>>> host-automake 1.15 Patching libtool
>>> host-automake 1.15 Configuring
>>> host-automake 1.15 Building
>>> host-automake 1.15 Installing to host directory
>>> host-pkgconf 0.8.9 Extracting
>>> host-pkgconf 0.8.9 Patching
>>> host-pkgconf 0.8.9 Updating config.sub and config.guess
>>> host-pkgconf 0.8.9 Patching libtool
>>> host-pkgconf 0.8.9 Configuring
>>> host-pkgconf 0.8.9 Building
>>> host-pkgconf 0.8.9 Installing to host directory
>>> host-expat 2.1.0 Extracting
>>> host-expat 2.1.0 Patching
>>> host-expat 2.1.0 Updating config.sub and config.guess
>>> host-expat 2.1.0 Patching libtool
>>> host-expat 2.1.0 Configuring
>>> host-expat 2.1.0 Building
>>> host-expat 2.1.0 Installing to host directory
>>> host-zlib 1.2.8 Extracting
>>> host-zlib 1.2.8 Patching
>>> host-zlib 1.2.8 Configuring
>>> host-zlib 1.2.8 Building
>>> host-zlib 1.2.8 Installing to host directory
>>> host-python 2.7.10 Extracting
>>> host-python 2.7.10 Patching
>>> host-python 2.7.10 Updating config.sub and config.guess
>>> host-python 2.7.10 Configuring
>>> host-python 2.7.10 Autoreconfiguring
>>> host-python 2.7.10 Building
>>> host-python 2.7.10 Installing to host directory
>>> host-scons 2.3.0 Extracting
>>> host-scons 2.3.0 Patching
>>> host-scons 2.3.0 Configuring
>>> host-scons 2.3.0 Building
>>> host-scons 2.3.0 Installing to host directory
>>> exfat-utils 1.1.1 Downloading
>>> exfat-utils 1.1.1 Extracting
>>> exfat-utils 1.1.1 Patching
>>> exfat-utils 1.1.1 Configuring
>>> exfat-utils 1.1.1 Building
>>> exfat-utils 1.1.1 Installing to target
>>>   Finalizing target directory

As you can see, no libfuse anywhere, and still it built fine. The
SConstruct indeed does:

program('fuse/*.c', 'fuse/mount.exfat-fuse', 'mount.exfat', [libs + [libfuse]])

but I don't see any fuse/ subdirectory in the exfat-utils sources. Am I
missing something?

> diff --git a/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch b/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch
> new file mode 100644
> index 0000000..90740fa
> --- /dev/null
> +++ b/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch
> @@ -0,0 +1,53 @@
> +[PATCH] fix compilation when libc does not define __GLIBC__
> +
> +libexfat would only compile on Linux with __GLIBC__ defined. Changed
> +the logic around to use standard defaults on Linux which will work
> +for glibc/uclibc/musl.
> +
> +Signed-off-by: Brendan Heading <brendanheading@gmail.com>
> +---
> + platform.h |   23 ++++++++++-------------
> + 1 file changed, 10 insertions(+), 13 deletions(-)
> +
> +Index: libexfat/libexfat/platform.h
> +===================================================================
> +--- libexfat/libexfat/platform.h	(revision 422)
> ++++ libexfat/libexfat/platform.h	(working copy)
> +@@ -24,19 +24,8 @@
> + #ifndef PLATFORM_H_INCLUDED
> + #define PLATFORM_H_INCLUDED
> + 
> +-#if defined(__GLIBC__)
> ++#if defined(__APPLE__)
> + 
> +-#include <endian.h>
> +-#include <byteswap.h>
> +-#define exfat_bswap16(x) bswap_16(x)
> +-#define exfat_bswap32(x) bswap_32(x)
> +-#define exfat_bswap64(x) bswap_64(x)
> +-#define EXFAT_BYTE_ORDER __BYTE_ORDER
> +-#define EXFAT_LITTLE_ENDIAN __LITTLE_ENDIAN
> +-#define EXFAT_BIG_ENDIAN __BIG_ENDIAN
> +-
> +-#elif defined(__APPLE__)
> +-
> + #include <machine/endian.h>
> + #include <libkern/OSByteOrder.h>
> + #define exfat_bswap16(x) OSSwapInt16(x)
> +@@ -57,7 +46,15 @@
> + #define EXFAT_BIG_ENDIAN _BIG_ENDIAN
> + 
> + #else 
> +-#error Unknown platform
> ++/* Assume Linux. glibc, uclibc(ng) and musl all provide bswap_X functions. */

Rather than doing this, just change:

#if defined(__GLIBC__)

by

#if defined(__linux__)

It will be a much simpler change (one-liner).

In the mean time, I'll mark your patch as Changes Requested.

Thanks!

Thomas
Brendan Heading July 26, 2015, 10:56 p.m. UTC | #10
> The libfuse dependency seems to be unrelated to the musl problem, so it
> should be a separate patch.
>
> Also, I just did a build of exfat-utils which libfuse, and it built
> just fine, so I think you're wrong with the fact that it needs libfuse.
> Here is the defconfig I've used:
>

Thomas, there must have been something in my coffee today.

I've resubmitted with your (superior) patch suggestion and with the
dependency stuff removed. Hopefully it will do the job :) If that looks
better I'll send the updated patch to the exfat mailing list.

regards

Brendan
diff mbox

Patch

diff --git a/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch b/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch
new file mode 100644
index 0000000..90740fa
--- /dev/null
+++ b/package/exfat-utils/0001-fix-compiling-with-non-glibc-libcs.patch
@@ -0,0 +1,53 @@ 
+[PATCH] fix compilation when libc does not define __GLIBC__
+
+libexfat would only compile on Linux with __GLIBC__ defined. Changed
+the logic around to use standard defaults on Linux which will work
+for glibc/uclibc/musl.
+
+Signed-off-by: Brendan Heading <brendanheading@gmail.com>
+---
+ platform.h |   23 ++++++++++-------------
+ 1 file changed, 10 insertions(+), 13 deletions(-)
+
+Index: libexfat/libexfat/platform.h
+===================================================================
+--- libexfat/libexfat/platform.h	(revision 422)
++++ libexfat/libexfat/platform.h	(working copy)
+@@ -24,19 +24,8 @@
+ #ifndef PLATFORM_H_INCLUDED
+ #define PLATFORM_H_INCLUDED
+ 
+-#if defined(__GLIBC__)
++#if defined(__APPLE__)
+ 
+-#include <endian.h>
+-#include <byteswap.h>
+-#define exfat_bswap16(x) bswap_16(x)
+-#define exfat_bswap32(x) bswap_32(x)
+-#define exfat_bswap64(x) bswap_64(x)
+-#define EXFAT_BYTE_ORDER __BYTE_ORDER
+-#define EXFAT_LITTLE_ENDIAN __LITTLE_ENDIAN
+-#define EXFAT_BIG_ENDIAN __BIG_ENDIAN
+-
+-#elif defined(__APPLE__)
+-
+ #include <machine/endian.h>
+ #include <libkern/OSByteOrder.h>
+ #define exfat_bswap16(x) OSSwapInt16(x)
+@@ -57,7 +46,15 @@
+ #define EXFAT_BIG_ENDIAN _BIG_ENDIAN
+ 
+ #else 
+-#error Unknown platform
++/* Assume Linux. glibc, uclibc(ng) and musl all provide bswap_X functions. */
++#include <endian.h>
++#include <byteswap.h>
++#define exfat_bswap16(x) bswap_16(x)
++#define exfat_bswap32(x) bswap_32(x)
++#define exfat_bswap64(x) bswap_64(x)
++#define EXFAT_BYTE_ORDER __BYTE_ORDER
++#define EXFAT_LITTLE_ENDIAN __LITTLE_ENDIAN
++#define EXFAT_BIG_ENDIAN __BIG_ENDIAN
+ #endif
+ 
+ #endif /* ifndef PLATFORM_H_INCLUDED */
diff --git a/package/exfat-utils/Config.in b/package/exfat-utils/Config.in
index aedff5f..ead5d47 100644
--- a/package/exfat-utils/Config.in
+++ b/package/exfat-utils/Config.in
@@ -1,5 +1,6 @@ 
 config BR2_PACKAGE_EXFAT_UTILS
 	bool "exfat-utils"
+	select BR2_PACKAGE_LIBFUSE
 	depends on BR2_USE_WCHAR
 	help
 	  exFAT filesystem utilities.
@@ -8,3 +9,4 @@  config BR2_PACKAGE_EXFAT_UTILS
 
 comment "exfat-utils needs a toolchain w/ wchar"
 	depends on !BR2_USE_WCHAR
+
diff --git a/package/exfat-utils/exfat-utils.mk b/package/exfat-utils/exfat-utils.mk
index 4f43233..37c9b30 100644
--- a/package/exfat-utils/exfat-utils.mk
+++ b/package/exfat-utils/exfat-utils.mk
@@ -6,7 +6,7 @@ 
 
 EXFAT_UTILS_VERSION = 1.1.1
 EXFAT_UTILS_SITE = http://distfiles.gentoo.org/distfiles
-EXFAT_UTILS_DEPENDENCIES = host-scons
+EXFAT_UTILS_DEPENDENCIES = host-scons libfuse
 EXFAT_UTILS_LICENSE = GPLv3+
 EXFAT_UTILS_LICENSE_FILES = COPYING