diff mbox

[1/1] packages/btrfs-progs: fix build issues under musl

Message ID CA+BsyQ5rwfHxZTShaEum3A456-zFekijGV_+EsUH9=tGenUTBA@mail.gmail.com
State Changes Requested
Headers show

Commit Message

Brendan Heading July 30, 2015, 2:16 p.m. UTC 
> While this is a quick hack that works, I think it's really nothing more
> than a hack. I've implemented what I believe is a better solution, can
> you look at:
>
>         https://github.com/tpetazzoni/btrfs-progs/commits/configure-improvements
[..]
> Can you check if that continues to work fine with glibc and musl, and
> if so, submit the patches upstream?

Thomas,

Unfortunately things moved a bit quickly - I'd submitted my patch to
btrfs-progs yesterday and it was accepted earlier this morning before
I had a chance to tell them about your more comprehensive approach.

Your inline patch had one small typo:

Also, the build still requires that limits.h is included, but that was
not in the scope of what you were doing.

Therefore I propose to add three patches to buildroot - your two (with
the #ifndef fixed), plus one for limits.h.

For the upstream, I will modify your patch slightly so that it reverts
the relevant three-line part of my previous patch. OK ?

regards

Brendan

Comments

Thomas Petazzoni July 30, 2015, 2:24 p.m. UTC | #1 
Dear Brendan Heading,

On Thu, 30 Jul 2015 15:16:04 +0100, Brendan Heading wrote:

> Unfortunately things moved a bit quickly - I'd submitted my patch to
> btrfs-progs yesterday and it was accepted earlier this morning before
> I had a chance to tell them about your more comprehensive approach.
> 
> Your inline patch had one small typo:
> =============================
> 
> --- a/kerncompat.h
> +++ b/kerncompat.h
> @@ -32,7 +32,7 @@
> 
>  #include <features.h>
> 
> -#ifdef HAVE_ALWAYS_INLINE
> +#ifndef HAVE_ALWAYS_INLINE
>  #define __always_inline __inline __attribute__ ((__always_inline__))

Humm, in fact my patch is wrong. So what they were doing is:

 #ifndef __GLIBC__
 #define __always_inline __inline __attribute__ ((__always_inline__))
 #endif

So in fact they are not testing that __always_inline__ is a valid gcc
function attribute.

They are defining __always_inline as a shortcut for marking a function
as always inline. This shortcut is defined by glibc, but not by other C
libraries, so they are doing it manually.

So, I think my patch should be adapted. You need to:

 * Remove the m4/ax_gcc_func_attribute.m4 file, revert my .gitignore
   change and remove the added AC_CONFIG_MACRO_DIR([m4]).

 * Add AC_CHECK_DECLS([__always_inline]) in configure.ac, and do
   #ifndef HAVE_DECL___ALWAYS_INLINE in kerncompat.h.

It will in fact make the patch a lot simpler, which is good.

Thanks!

Thomas
Brendan Heading July 30, 2015, 3:30 p.m. UTC | #2 
> So, I think my patch should be adapted. You need to:
>
>  * Remove the m4/ax_gcc_func_attribute.m4 file, revert my .gitignore
>    change and remove the added AC_CONFIG_MACRO_DIR([m4]).
>
>  * Add AC_CHECK_DECLS([__always_inline]) in configure.ac, and do
>    #ifndef HAVE_DECL___ALWAYS_INLINE in kerncompat.h.
>
> It will in fact make the patch a lot simpler, which is good.

Thomas,

Tested with both the autobuild config (musl) and Linaro 2014.9, builds
fine. Forwarding the buildroot patches now, then I'll look at
submitting them upstream.

regards

Brendan
diff mbox

Patch

=============================

--- a/kerncompat.h
+++ b/kerncompat.h
@@ -32,7 +32,7 @@ 

 #include <features.h>

-#ifdef HAVE_ALWAYS_INLINE
+#ifndef HAVE_ALWAYS_INLINE
 #define __always_inline __inline __attribute__ ((__always_inline__))
 #endif
=============================