[Mesa-dev] [PATCH 2/7] mapi: Inline shared-glapi/Makefile.

[Emil Velikov]

On 18/08/14 05:14, Matt Turner wrote:
> On Sun, Aug 17, 2014 at 1:06 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>> On Fri, Aug 15, 2014 at 10:47:06AM -0700, Matt Turner wrote:
>>> ---
>>>  configure.ac                      |  1 -
>>>  src/mapi/Makefile.am              | 44 ++++++++++++++++++++++++++++++++++++---
>>>  src/mapi/shared-glapi/Makefile.am | 34 ------------------------------
>>>  src/mesa/Makefile.sources         |  3 ---
>>>  4 files changed, 41 insertions(+), 41 deletions(-)
>>>  delete mode 100644 src/mapi/shared-glapi/Makefile.am
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index dc81c80..97d5394 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -2243,7 +2243,6 @@ AC_CONFIG_FILES([Makefile
>>>               src/mapi/glapi/Makefile
>>>               src/mapi/glapi/gen/Makefile
>>>               src/mapi/glapi/tests/Makefile
>>> -             src/mapi/shared-glapi/Makefile
>>>               src/mapi/shared-glapi/tests/Makefile
>>>               src/mapi/vgapi/Makefile
>>>               src/mapi/vgapi/vg.pc
>>> diff --git a/src/mapi/Makefile.am b/src/mapi/Makefile.am
>>> index ef53803..6b9444a 100644
>>> --- a/src/mapi/Makefile.am
>>> +++ b/src/mapi/Makefile.am
>>> @@ -1,4 +1,4 @@
>>> -# Copyright © 2013 Intel Corporation
>>> +# Copyright © 2013, 2014 Intel Corporation
>>>  #
>>>  # Permission is hereby granted, free of charge, to any person obtaining a
>>>  # copy of this software and associated documentation files (the "Software"),
>>> @@ -19,10 +19,46 @@
>>>  # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>>  # IN THE SOFTWARE.
>>>
>>> -SUBDIRS = glapi/gen
>>> +SUBDIRS = glapi/gen .
>>> +
>>> +TOP = $(top_srcdir)
>>> +
>>> +BUILT_SOURCES =
>>> +CLEANFILES = $(BUILT_SOURCES)
>>> +
>>> +lib_LTLIBRARIES =
>>> +
>>> +AM_CFLAGS = $(PTHREAD_CFLAGS)
>>> +AM_CPPFLAGS =                                                        \
>>> +     $(DEFINES)                                              \
>>> +     $(SELINUX_CFLAGS)                                       \
>>> +     -I$(top_srcdir)/include                                 \
>>> +     -I$(top_srcdir)/src/mapi                                \
>>> +     -I$(top_builddir)/src/mapi
>>> +
>>> +GLAPI = $(top_srcdir)/src/mapi/glapi
>>> +include Makefile.sources
>>> +include glapi/gen/glapi_gen.mk
>>>
>>>  if HAVE_SHARED_GLAPI
>>> -SUBDIRS += shared-glapi
>>> +SUBDIRS += shared-glapi/tests
>>> +
>>> +BUILT_SOURCES += shared-glapi/glapi_mapi_tmp.h
>>> +
>>> +lib_LTLIBRARIES += shared-glapi/libglapi.la
>>> +shared_glapi_libglapi_la_SOURCES = $(MAPI_GLAPI_FILES)
>>> +shared_glapi_libglapi_la_CPPFLAGS = \
>>> +     $(AM_CPPFLAGS) \
>>> +     -DMAPI_MODE_GLAPI \
>>> +     -DMAPI_ABI_HEADER=\"shared-glapi/glapi_mapi_tmp.h\"
>>> +shared_glapi_libglapi_la_LIBADD = $(SELINUX_LIBS)
>>> +shared_glapi_libglapi_la_LDFLAGS = \
>>> +     -no-undefined \
>>> +     $(GC_SECTIONS) \
>>> +     $(LD_NO_UNDEFINED)
>>> +
>>> +shared-glapi/glapi_mapi_tmp.h : $(GLAPI)/gen/gl_and_es_API.xml $(glapi_gen_mapi_deps)
>>> +     $(call glapi_gen_mapi,$<,shared-glapi)
>>>  endif
>>>
>>>  if HAVE_OPENGL
>>> @@ -40,3 +76,5 @@ endif
>>>  if HAVE_OPENVG
>>>  SUBDIRS += vgapi
>>>  endif
>>> +
>>> +include $(top_srcdir)/install-lib-links.mk
>>> diff --git a/src/mapi/shared-glapi/Makefile.am b/src/mapi/shared-glapi/Makefile.am
>>> deleted file mode 100644
>>> index 330719c..0000000
>>> --- a/src/mapi/shared-glapi/Makefile.am
>>> +++ /dev/null
>>> @@ -1,34 +0,0 @@
>>> -# Used by OpenGL ES or when --enable-shared-glapi is specified
>>> -
>>> -SUBDIRS = . tests
>>> -
>>> -TOP = $(top_srcdir)
>>> -GLAPI = $(top_srcdir)/src/mapi/glapi
>>> -include $(top_srcdir)/src/mapi/Makefile.sources
>>> -
>>> -lib_LTLIBRARIES = libglapi.la
>>> -libglapi_la_SOURCES = $(MAPI_GLAPI_FILES)
>>> -libglapi_la_LIBADD = $(PTHREAD_LIBS) $(SELINUX_LIBS)
>>
>> You didn't move $(PTHREAD_LIBS) up to shared_glpai_libglapi_la_LIBADD?
> 
> Right... Emil, do you remember whether PTHREAD_LIBS is needed?
> PTHREAD_CFLAGS seems sufficient for me, but I have a vague memory that
> FreeBSD or something needs PTHREAD_LIBS.
> 
This seems to be an interesting topic:

ldd states that our current pthreads linking is not needed. On the other hand
the libglapi.so.0.0 has at least one function(pthreads_once) coming from the
pthreads library. At the same time the function is _unused_ by whole of mesa.
Not to mention that *BSD people need the pthreads linking as their libc does
not provide any pthread* symbols.

So in summary, let's keep PTHREAD_LIBS in for now :)

-Emil

>>> -libglapi_la_LDFLAGS = \
>>> -     -no-undefined \
>>> -     $(GC_SECTIONS) \
>>> -     $(LD_NO_UNDEFINED)
>>> -
>>> -include $(GLAPI)/gen/glapi_gen.mk
>>> -glapi_mapi_tmp.h : $(GLAPI)/gen/gl_and_es_API.xml $(glapi_gen_mapi_deps)
>>> -     $(call glapi_gen_mapi,$<,shared-glapi)
>>> -
>>> -BUILT_SOURCES = glapi_mapi_tmp.h
>>> -CLEANFILES = $(BUILT_SOURCES)
>>> -
>>> -AM_CFLAGS = $(PTHREAD_CFLAGS)
>>> -AM_CPPFLAGS =                                                        \
>>> -     $(DEFINES)                                              \
>>> -     $(SELINUX_CFLAGS)                                       \
>>> -     -I$(top_srcdir)/include                                 \
>>> -     -I$(top_srcdir)/src/mapi                                \
>>> -     -I$(top_builddir)/src/mapi                              \
>>> -     -DMAPI_MODE_GLAPI                                       \
>>> -     -DMAPI_ABI_HEADER=\"shared-glapi/glapi_mapi_tmp.h\"
>>> -
>>> -include $(top_srcdir)/install-lib-links.mk
>>> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
>>> index 12336c0..2d2d244 100644
>>> --- a/src/mesa/Makefile.sources
>>> +++ b/src/mesa/Makefile.sources
>>> @@ -3,9 +3,6 @@
>>>  # This file is among different build systems. SRCDIR must be defined with
>>>  # a trailing slash because the Android build system leaves it undefined.
>>>
>>> -SRCDIR = $(top_srcdir)/src/mesa/
>>> -BUILDDIR = $(top_builddir)/src/mesa/
>>> -
>>
>> This looks like an unrelated change...
> 
> Whoops. Meant to make that a separate patch. I'll split it out.
>