[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Jérôme Pouiller]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Fixes:

  http://autobuild.buildroot.org/results/7ee57d01917ea72d1811469e482513dda2ceb1ea/build-end.log

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
v2:
   - Fix boolean expression in patch
   - Renumber patch
   - Add my Signed-off

 ...bffi-006-Make-thread-support-conditionnal.patch | 74 ++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 package/libffi/libffi-006-Make-thread-support-conditionnal.patch

diff --git a/package/libffi/libffi-006-Make-thread-support-conditionnal.patch b/package/libffi/libffi-006-Make-thread-support-conditionnal.patch
new file mode 100644
index 0000000..72e7e5d
--- /dev/null
+++ b/package/libffi/libffi-006-Make-thread-support-conditionnal.patch
@@ -0,0 +1,74 @@
+From a7f6342120060564a829704cceb843e53e0b34a9 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+Date: Sat, 17 Nov 2012 18:44:16 +0100
+Subject: [PATCH 3/3] Make thread support conditionnal
+
+When libffi is linked against a C library that does not have thread
+support, it is not necessary to use a mutex to protect global
+variables, since the application calling libffi cannot be
+multi-threaded.
+
+Therefore, make the libffi thread support conditionnal: when we're
+building against uClibc with no thread support, don't use the
+pthread_mutex.
+
+Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
+---
+ src/closures.c |   12 ++++++++++++
+ 1 file changed, 12 insertions(+)
+
+diff --git a/src/closures.c b/src/closures.c
+index 1b37827..3d151f6 100644
+--- a/src/closures.c
++++ b/src/closures.c
+@@ -70,7 +70,10 @@
+ 
+ # elif FFI_MMAP_EXEC_WRIT /* !FFI_EXEC_TRAMPOLINE_TABLE */
+ 
++#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
+ #define USE_LOCKS 1
++#endif
++
+ #define USE_DL_PREFIX 1
+ #ifdef __GNUC__
+ #ifndef USE_BUILTIN_FFS
+@@ -116,7 +119,10 @@
+ #include <mntent.h>
+ #endif /* HAVE_MNTENT */
+ #include <sys/param.h>
++
++#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
+ #include <pthread.h>
++#endif
+ 
+ /* We don't want sys/mman.h to be included after we redefine mmap and
+    dlmunmap.  */
+@@ -214,8 +220,10 @@ static int dlmunmap(void *, size_t);
+ 
+ #if !(defined(X86_WIN32) || defined(X86_WIN64) || defined(__OS2__)) || defined (__CYGWIN__) || defined(__INTERIX)
+ 
++#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
+ /* A mutex used to synchronize access to *exec* variables in this file.  */
+ static pthread_mutex_t open_temp_exec_file_mutex = PTHREAD_MUTEX_INITIALIZER;
++#endif
+ 
+ /* A file descriptor of a temporary file from which we'll map
+    executable pages.  */
+@@ -473,9 +481,13 @@ dlmmap (void *start, size_t length, int prot,
+ 
+   if (execsize == 0 || execfd == -1)
+     {
++#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
+       pthread_mutex_lock (&open_temp_exec_file_mutex);
++#endif
+       ptr = dlmmap_locked (start, length, prot, flags, offset);
++#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
+       pthread_mutex_unlock (&open_temp_exec_file_mutex);
++#endif
+ 
+       return ptr;
+     }
+-- 
+1.7.9.5
+
-- 
1.9.1

[Buildroot] [PATCH v2 2/2] Revert "libffi and python: need threads support"

[Jérôme Pouiller]

Buildroot now add a patch to libffi to fix support of threads. Thus, this
patch, is no more needed.

This reverts commit 77cf5b5a71aa54039f4dbc509a1c7cb40aa04934.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/libffi/Config.in  | 4 ----
 package/python/Config.in  | 5 ++---
 package/python3/Config.in | 5 ++---
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/package/libffi/Config.in b/package/libffi/Config.in
index 6edf570..7211f8f 100644
--- a/package/libffi/Config.in
+++ b/package/libffi/Config.in
@@ -1,6 +1,5 @@
 config BR2_PACKAGE_LIBFFI
 	bool "libffi"
-	depends on BR2_TOOLCHAIN_HAS_THREADS
 	help
 	  The libffi library provides a portable, high level
 	  programming interface to various calling conventions. This
@@ -8,6 +7,3 @@ config BR2_PACKAGE_LIBFFI
 	  interface description at run-time.
 
 	  http://sourceware.org/libffi/
-
-comment "libffi needs a toolchain w/ threads"
-	depends on !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/python/Config.in b/package/python/Config.in
index 8b27630..a0c78c6 100644
--- a/package/python/Config.in
+++ b/package/python/Config.in
@@ -3,16 +3,15 @@ config BR2_PACKAGE_PYTHON
 	depends on BR2_USE_WCHAR
 	# uses fork()
 	depends on BR2_USE_MMU
-	depends on BR2_TOOLCHAIN_HAS_THREADS # libffi
 	select BR2_PACKAGE_LIBFFI
 	help
 	  The python language interpreter.
 
 	  http://www.python.org/
 
-comment "python needs a toolchain w/ wchar, threads"
+comment "python needs a toolchain w/ wchar"
 	depends on BR2_USE_MMU
-	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_USE_WCHAR
 
 if BR2_PACKAGE_PYTHON
 
diff --git a/package/python3/Config.in b/package/python3/Config.in
index 375390f..3e193c8 100644
--- a/package/python3/Config.in
+++ b/package/python3/Config.in
@@ -4,16 +4,15 @@ config BR2_PACKAGE_PYTHON3
 	depends on BR2_USE_WCHAR
 	# uses fork()
 	depends on BR2_USE_MMU
-	depends on BR2_TOOLCHAIN_HAS_THREADS # libffi
 	select BR2_PACKAGE_LIBFFI
 	help
 	  The python language interpreter.
 
 	  http://www.python.org/
 
-comment "python3 needs a toolchain w/ wchar, threads"
+comment "python3 needs a toolchain w/ wchar"
 	depends on BR2_USE_MMU
-	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_USE_WCHAR
 
 if BR2_PACKAGE_PYTHON3
 
-- 
1.9.1

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Thomas Petazzoni]

Dear J?r?me Pouiller,

On Mon, 15 Sep 2014 14:26:41 +0200, J?r?me Pouiller wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> Fixes:
> 
>   http://autobuild.buildroot.org/results/7ee57d01917ea72d1811469e482513dda2ceb1ea/build-end.log
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>

To be honest, I am wondering if we really should bother merging a patch
to make libffi usable on configurations that have thread support
disabled. Nowadays, not having thread support is also impossible due to
the large number of libraries/applications that rely on threads. libffi
is generally used in "big" things like Python or glib, so not having
thread support in systems using such big things is a bit unlikely.

In addition, I'm not sure the patch could be upstreamed as is, which
means we would have to carry it in Buildroot forever.

Peter, Arnout, Gustavo, what is your point of view on this?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Gustavo Zacarias]

On 10/29/2014 06:24 PM, Thomas Petazzoni wrote:

> To be honest, I am wondering if we really should bother merging a patch
> to make libffi usable on configurations that have thread support
> disabled. Nowadays, not having thread support is also impossible due to
> the large number of libraries/applications that rely on threads. libffi
> is generally used in "big" things like Python or glib, so not having
> thread support in systems using such big things is a bit unlikely.
> 
> In addition, I'm not sure the patch could be upstreamed as is, which
> means we would have to carry it in Buildroot forever.
> 
> Peter, Arnout, Gustavo, what is your point of view on this?

+1, ditching threads (uClibc) is for really small targets which these
days would mostly mean noMMU.
Regards.

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Arnout Vandecappelle]

On 29/10/14 22:24, Thomas Petazzoni wrote:
> Dear J?r?me Pouiller,
> 
> On Mon, 15 Sep 2014 14:26:41 +0200, J?r?me Pouiller wrote:
>> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>
>> Fixes:
>>
>>   http://autobuild.buildroot.org/results/7ee57d01917ea72d1811469e482513dda2ceb1ea/build-end.log
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> 
> To be honest, I am wondering if we really should bother merging a patch
> to make libffi usable on configurations that have thread support
> disabled. Nowadays, not having thread support is also impossible due to
> the large number of libraries/applications that rely on threads. libffi
> is generally used in "big" things like Python or glib, so not having
> thread support in systems using such big things is a bit unlikely.
> 
> In addition, I'm not sure the patch could be upstreamed as is, which
> means we would have to carry it in Buildroot forever.
> 
> Peter, Arnout, Gustavo, what is your point of view on this?

 I agree. Removing toolchain requirements from a package is really a feature
patch, so we shouldn't do it unless it's upstreamable or when it's more of a
build issue than a code issue. There may be an exception when a package that has
a lot of dependants suddenly grows a toolchain dependency, but that's not the
case here.

 On the other hand, why shouldn't this patch be upstreamable?


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Thu, 30 Oct 2014 09:09:35 +0100, Arnout Vandecappelle wrote:

>  I agree. Removing toolchain requirements from a package is really a feature
> patch, so we shouldn't do it unless it's upstreamable or when it's more of a
> build issue than a code issue. There may be an exception when a package that has
> a lot of dependants suddenly grows a toolchain dependency, but that's not the
> case here.

Agreed.

>  On the other hand, why shouldn't this patch be upstreamable?

Because it shouldn't hardcode:

+#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)

But instead detect in configure.ac if threads are available, and then
use that.

So, what I would propose is that if J?r?me is really interested in
having libffi available in non-threaded configurations, he works on a
patch that is upstreamable, submit it upstream, and once it's upstream,
we backport it in Buildroot, until upstream does a new release.

What do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2 1/2] libffi: make thread support optional

[Jérôme Pouiller]

On Thursday 30 October 2014 11:31:34 Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
> 
> On Thu, 30 Oct 2014 09:09:35 +0100, Arnout Vandecappelle wrote:
> >  I agree. Removing toolchain requirements from a package is really a
> >  feature
> > 
> > patch, so we shouldn't do it unless it's upstreamable or when it's more of
> > a build issue than a code issue. There may be an exception when a package
> > that has a lot of dependants suddenly grows a toolchain dependency, but
> > that's not the case here.
> 
> Agreed.
> 
> >  On the other hand, why shouldn't this patch be upstreamable?
> 
> Because it shouldn't hardcode:
> 
> +#if !defined(__UCLIBC__) || !defined(__HAS_NO_THREADS__)
> 
> But instead detect in configure.ac if threads are available, and then
> use that.
> 
> So, what I would propose is that if J?r?me is really interested in
> having libffi available in non-threaded configurations, he works on a
> patch that is upstreamable, submit it upstream, and once it's upstream,
> we backport it in Buildroot, until upstream does a new release.
My only objective was to fix properly an autobuilder failure :). 
We can keep current implementation and mark this patch as rejected.


I have noticed that dependency to BR2_TOOLCHAIN_HAS_THREADS is not properly 
propagated to all packages that depend on python. I will check that (and send 
a patch before 2014.11rc1 release).

-- 
J?r?me Pouiller, Sysmic