All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
@ 2018-08-31 20:22 Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 1/3] leveldb: install memenv static library and headers Gaël PORTAY
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Gaël PORTAY @ 2018-08-31 20:22 UTC (permalink / raw)
  To: buildroot

Hi all,

This patchset fixes the build issue reported by autobuilder [0].

The issue happens when the package leveldb is enabled.

QtWebKit builds its own copy of leveldb [1] if it is not provided by the system
(i.e. buildroot). It builds it differently and this is the origin of that
issue. Instead of using the Makefile provided by leveldb [2], QtWebKit uses
qmake to build that library [3].

        /home/naourr/work/instance-2/output/build/qt5webkit-5.9.1/Source/WebCore//.obj/platform/leveldb/LevelDBDatabase.o: In function `WebCore::LevelDBDatabase::openInMemory(WebCore::LevelDBComparator const*)':
LevelDBDatabase.cpp.text._ZN7WebCore15LevelDBDatabase12openInMemoryEPKNS_17LevelDBComparatorE+0x34): undefined reference to `leveldb::NewMemEnv(leveldb::Env*)'
        collect2: error: ld returned 1 exit status
        make[3]: *** [Makefile.api:97: ../lib/libQt5WebKit.so.5.9.1] Error 1

The missing symbol issue happens because the symbol leveldb::NewMemEnv is
shipped in a static library named libmemenv.a (aside libleveldb.so) and that
static library is *NOT* installed in the staging directory. Unfortunatly, that
symbol is required latter by WebCore [4].

Note that the copy built by QtWebKit is an all-in-one library including both
libleveldb and libmemenv *AND* thus QtWebKit links against libleveldb only.
Also, note that the linker finds the buildroot's copy first; not its internal
copy. That explains why it is complaining about a missing symbol.

Fortunatly, QtWebKit provides a facility to link against the system leveldb
package. The qmake flag WEBKIT_CONFIG+=use_system_leveldb tells Qt5WebKit to
link against libleveldb *AND* libmemenv [5].

To fix that issue, this patchset builds the leveldb package and tells QtWebKit
to use it instead of building its own copy.

The first two patches concern the leveldb package.

The first one adds installation of headers and the missing static library in
the staging directory.

The second one patches leveldb to generate position independant code for the
static library libmemenv.a; in order let it linkable by the Qt5WebKit module.
Without that patch, the build raises the error below:

	/home/gportay/src/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/x86_64-amd-linux-gnu/6.2.0/../../../../x86_64-amd-linux-gnu/bin/ld: /home/gportay/src/buildroot/output/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib/libmemenv.a(memenv.o): relocation R_X86_64_32S against `.rodata' can not be used when making a shared object; recompile with -fPIC
	/home/gportay/src/buildroot/output/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib/libmemenv.a: error adding symbols: Bad value
	collect2: error: ld returned 1 exit status

It it not relevant to upstream that patch since the project has already moved
to cmake [6] for the (upcoming?) version 1.21. Backporting the upstreamed
patches was way too complicated.

The last patch selects the leveldb package in qt5webkit package and sets the
appropriate qmake flag to tell it to use the package provided by buildroot.

[0]: http://autobuild.buildroot.net/results/8afc8771741f62da3b2117d2124d1c6f2d941903/
[1]: https://github.com/qt/qtwebkit/tree/5.9/Source/ThirdParty/leveldb
[2]: https://github.com/qt/qtwebkit/blob/5.9/Source/ThirdParty/leveldb/Makefile#L167-L169
[3]: https://github.com/qt/qtwebkit/blob/5.9/Source/ThirdParty/leveldb/Target.pri#L80
[4]: https://github.com/qt/qtwebkit/blob/5.9/Source/WebCore/platform/leveldb/LevelDBDatabase.cpp#L185
[5]: https://github.com/qt/qtwebkit/blob/5.9/Source/WebCore/WebCore.pri#L254
[6]: https://github.com/google/leveldb/commit/739c25100e46576cdcdfff2d6f43f9f7008103c7

Regards,
Ga?l PORTAY (3):
  leveldb: install memenv static library and headers
  leveldb: generate pic for static libraries
  qt5webkit: select leveldb package

 ...sition-independant-code-for-static-librar.patch | 52 ++++++++++++++++++++++
 package/leveldb/leveldb.mk                         |  2 +
 package/qt5/qt5webkit/Config.in                    |  3 ++
 package/qt5/qt5webkit/qt5webkit.mk                 |  4 +-
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 package/leveldb/0001-Generate-position-independant-code-for-static-librar.patch

-- 
2.11.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 1/3] leveldb: install memenv static library and headers
  2018-08-31 20:22 [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Gaël PORTAY
@ 2018-08-31 20:22 ` Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 2/3] leveldb: generate pic for static libraries Gaël PORTAY
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Gaël PORTAY @ 2018-08-31 20:22 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
---
 package/leveldb/leveldb.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/leveldb/leveldb.mk b/package/leveldb/leveldb.mk
index 54942a0f27..efbd61d16a 100644
--- a/package/leveldb/leveldb.mk
+++ b/package/leveldb/leveldb.mk
@@ -25,6 +25,8 @@ define LEVELDB_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) \
 		INSTALL_ROOT=$(STAGING_DIR) INSTALL_PREFIX=/usr \
 		$(LEVELDB_MAKE_ARGS) -C $(@D) install
+	$(INSTALL) -m 0644 $(@D)/out-static/libmemenv.a $(STAGING_DIR)/usr/lib
+	cp -dpfr $(@D)/helpers $(STAGING_DIR)/usr/include
 endef
 
 define LEVELDB_INSTALL_TARGET_CMDS
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 2/3] leveldb: generate pic for static libraries
  2018-08-31 20:22 [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 1/3] leveldb: install memenv static library and headers Gaël PORTAY
@ 2018-08-31 20:22 ` Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package Gaël PORTAY
  2018-08-31 21:19 ` [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Thomas Petazzoni
  3 siblings, 0 replies; 11+ messages in thread
From: Gaël PORTAY @ 2018-08-31 20:22 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
---
 ...sition-independant-code-for-static-librar.patch | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 package/leveldb/0001-Generate-position-independant-code-for-static-librar.patch

diff --git a/package/leveldb/0001-Generate-position-independant-code-for-static-librar.patch b/package/leveldb/0001-Generate-position-independant-code-for-static-librar.patch
new file mode 100644
index 0000000000..dce06ec725
--- /dev/null
+++ b/package/leveldb/0001-Generate-position-independant-code-for-static-librar.patch
@@ -0,0 +1,52 @@
+From 6ed1b57ef6bcee0d497c181730710b2b0fafbfb3 Mon Sep 17 00:00:00 2001
+From: =?utf-8?q?Ga=C3=ABl=20PORTAY?= <gael.portay@savoirfairelinux.com>
+Date: Fri, 31 Aug 2018 12:23:46 -0400
+Subject: [PATCH] Generate position independant code for static library
+MIME-Version: 1.0
+Content-Type: text/plain; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+Currently, only shared libraries are using the PIC flag.
+
+Generalize this flag for static libraries in order to let them linkable
+by dynamic libraries.
+
+Fixes:
+
+	/home/gportay/src/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/x86_64-amd-linux-gnu/6.2.0/../../../../x86_64-amd-linux-gnu/bin/ld: /home/gportay/src/buildroot/output/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib/libmemenv.a(memenv.o): relocation R_X86_64_32S against `.rodata' can not be used when making a shared object; recompile with -fPIC
+	/home/gportay/src/buildroot/output/host/x86_64-buildroot-linux-gnu/sysroot/usr/lib/libmemenv.a: error adding symbols: Bad value
+	collect2: error: ld returned 1 exit status
+
+Upstream-Status: Inappropriate [upstream has migrated to cmake]
+Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
+---
+ build_detect_platform | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/build_detect_platform b/build_detect_platform
+index d2a20ce..4839444 100755
+--- a/build_detect_platform
++++ b/build_detect_platform
+@@ -55,8 +55,8 @@ fi
+ 
+ COMMON_FLAGS=
+ CROSS_COMPILE=
+-PLATFORM_CCFLAGS=
+-PLATFORM_CXXFLAGS=
++PLATFORM_CCFLAGS="-fPIC"
++PLATFORM_CXXFLAGS="-fPIC"
+ PLATFORM_LDFLAGS=
+ PLATFORM_LIBS=
+ PLATFORM_SHARED_EXT="so"
+@@ -197,7 +197,7 @@ else
+ EOF
+     if [ "$?" = 0 ]; then
+         COMMON_FLAGS="$COMMON_FLAGS -DLEVELDB_PLATFORM_POSIX -DLEVELDB_ATOMIC_PRESENT"
+-        PLATFORM_CXXFLAGS="-std=c++0x"
++        PLATFORM_CXXFLAGS="$PLATFORM_CXXFLAGS -std=c++0x"
+     else
+         COMMON_FLAGS="$COMMON_FLAGS -DLEVELDB_PLATFORM_POSIX"
+     fi
+-- 
+2.18.0
+
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package
  2018-08-31 20:22 [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 1/3] leveldb: install memenv static library and headers Gaël PORTAY
  2018-08-31 20:22 ` [Buildroot] [PATCH 2/3] leveldb: generate pic for static libraries Gaël PORTAY
@ 2018-08-31 20:22 ` Gaël PORTAY
  2018-09-04 21:10   ` Arnout Vandecappelle
  2018-08-31 21:19 ` [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Thomas Petazzoni
  3 siblings, 1 reply; 11+ messages in thread
From: Gaël PORTAY @ 2018-08-31 20:22 UTC (permalink / raw)
  To: buildroot

QtWebKit may build its own copy of leveldb.

Select leveldb package and build the buildroot's copy instead.

Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
---
 package/qt5/qt5webkit/Config.in    | 3 +++
 package/qt5/qt5webkit/qt5webkit.mk | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/package/qt5/qt5webkit/Config.in b/package/qt5/qt5webkit/Config.in
index 72c641b9f7..e8008f7bab 100644
--- a/package/qt5/qt5webkit/Config.in
+++ b/package/qt5/qt5webkit/Config.in
@@ -3,10 +3,13 @@ config BR2_PACKAGE_QT5WEBKIT
 	depends on !BR2_STATIC_LIBS
 	depends on BR2_PACKAGE_QT5_JSCORE_AVAILABLE
 	depends on BR2_HOST_GCC_AT_LEAST_4_8 # icu
+	depends on BR2_INSTALL_LIBSTDCPP # leveldb
 	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # icu
+	depends on BR2_TOOLCHAIN_HAS_THREADS # leveldb
 	depends on !BR2_BINFMT_FLAT # icu
 	# assumes a FPU is available on MIPS
 	depends on !BR2_MIPS_SOFT_FLOAT
+	select BR2_PACKAGE_LEVELDB
 	select BR2_PACKAGE_QT5BASE
 	select BR2_PACKAGE_QT5BASE_ICU
 	select BR2_PACKAGE_QT5BASE_GUI
diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk
index e8d368fb01..a0a0998d12 100644
--- a/package/qt5/qt5webkit/qt5webkit.mk
+++ b/package/qt5/qt5webkit/qt5webkit.mk
@@ -16,7 +16,7 @@ endif
 QT5WEBKIT_SOURCE = qtwebkit-opensource-src-$(QT5WEBKIT_VERSION).tar.xz
 QT5WEBKIT_DEPENDENCIES = \
 	host-bison host-flex host-gperf host-python host-ruby \
-	qt5base sqlite
+	leveldb qt5base sqlite
 QT5WEBKIT_INSTALL_STAGING = YES
 
 QT5WEBKIT_LICENSE_FILES = Source/WebCore/LICENSE-LGPL-2 Source/WebCore/LICENSE-LGPL-2.1
@@ -45,7 +45,7 @@ endef
 QT5WEBKIT_PRE_CONFIGURE_HOOKS += QT5WEBKIT_PYTHON2_SYMLINK
 
 define QT5WEBKIT_CONFIGURE_CMDS
-	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5WEBKIT_ENV) $(HOST_DIR)/bin/qmake)
+	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5WEBKIT_ENV) $(HOST_DIR)/bin/qmake WEBKIT_CONFIG+=use_system_leveldb)
 endef
 
 define QT5WEBKIT_BUILD_CMDS
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
  2018-08-31 20:22 [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Gaël PORTAY
                   ` (2 preceding siblings ...)
  2018-08-31 20:22 ` [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package Gaël PORTAY
@ 2018-08-31 21:19 ` Thomas Petazzoni
  2018-09-02 20:21   ` Gaël PORTAY
  3 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2018-08-31 21:19 UTC (permalink / raw)
  To: buildroot

Hello,

First of all: there are a *lot* of explanations in the cover letter,
but your commit logs are almost empty. The cover letter is lost in the
mailing list archives, while the commit logs are nicely preserved in
the Git history. Therefore, we want more explanations in the commit
logs!

On Fri, 31 Aug 2018 16:22:01 -0400, Ga?l PORTAY wrote:

> QtWebKit builds its own copy of leveldb [1] if it is not provided by the system
> (i.e. buildroot).

And indeed, using the leveldb library built by the leveldb Buildroot
package is better.

> It builds it differently and this is the origin of that
> issue. Instead of using the Makefile provided by leveldb [2], QtWebKit uses
> qmake to build that library [3].
> 
>         /home/naourr/work/instance-2/output/build/qt5webkit-5.9.1/Source/WebCore//.obj/platform/leveldb/LevelDBDatabase.o: In function `WebCore::LevelDBDatabase::openInMemory(WebCore::LevelDBComparator const*)':
> LevelDBDatabase.cpp.text._ZN7WebCore15LevelDBDatabase12openInMemoryEPKNS_17LevelDBComparatorE+0x34): undefined reference to `leveldb::NewMemEnv(leveldb::Env*)'
>         collect2: error: ld returned 1 exit status
>         make[3]: *** [Makefile.api:97: ../lib/libQt5WebKit.so.5.9.1] Error 1
> 
> The missing symbol issue happens because the symbol leveldb::NewMemEnv is
> shipped in a static library named libmemenv.a (aside libleveldb.so) and that
> static library is *NOT* installed in the staging directory. Unfortunatly, that
> symbol is required latter by WebCore [4].

latter -> later.

Is it libleveldb.so itself that uses this symbol, or a separate part of
WebCore that directly uses some leveldb internals ?

> Note that the copy built by QtWebKit is an all-in-one library including both
> libleveldb and libmemenv *AND* thus QtWebKit links against libleveldb only.
> Also, note that the linker finds the buildroot's copy first; not its internal
> copy. That explains why it is complaining about a missing symbol.
> 
> Fortunatly, QtWebKit provides a facility to link against the system leveldb
> package. The qmake flag WEBKIT_CONFIG+=use_system_leveldb tells Qt5WebKit to
> link against libleveldb *AND* libmemenv [5].
> 
> To fix that issue, this patchset builds the leveldb package and tells QtWebKit
> to use it instead of building its own copy.
> 
> The first two patches concern the leveldb package.
> 
> The first one adds installation of headers and the missing static library in
> the staging directory.

I am a bit uneasy about this one, it seems to install "internal" stuff
of leveldb that are normally not meant to be installed. Do you/we
understand what this internal stuff, beyond just "it fixes qt5webkit" ?

> The second one patches leveldb to generate position independant code for the
> static library libmemenv.a; in order let it linkable by the Qt5WebKit module.

OK, I guess this is reasonable. We don't really build twice the object
files so that the .a object files are non-PIC and the .so object files
are PIC.

All in all, I'm just worried by the installation of what looks like
some internal library/headers of leveldb. Could you comment on this ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
  2018-08-31 21:19 ` [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Thomas Petazzoni
@ 2018-09-02 20:21   ` Gaël PORTAY
  2018-09-03 22:53     ` Arnout Vandecappelle
  0 siblings, 1 reply; 11+ messages in thread
From: Gaël PORTAY @ 2018-09-02 20:21 UTC (permalink / raw)
  To: buildroot

Thomas,

On Fri, Aug 31, 2018 at 11:19:30PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> First of all: there are a *lot* of explanations in the cover letter,
> but your commit logs are almost empty. The cover letter is lost in the
> mailing list archives, while the commit logs are nicely preserved in
> the Git history. Therefore, we want more explanations in the commit
> logs!
>

I knew you would says something about that. I sent the patch serie as
soon as possible for a review. I think you may want it for the upcoming
release.

The thing is, I did not know how to properly arrange my commits :/ I did
not want to mention something about qt5webkit in patches related to
leveldb.

Maybe, I can add the content of the coverletter in the commit message of
the last patch and reword a bit.

> On Fri, 31 Aug 2018 16:22:01 -0400, Ga?l PORTAY wrote:
>
> (...)
>
> Is it libleveldb.so itself that uses this symbol, or a separate part of
> WebCore that directly uses some leveldb internals ?
> 

It is WebCore that use directly that symbol; libleveldb does not.

> > Note that the copy built by QtWebKit is an all-in-one library including both
> > libleveldb and libmemenv *AND* thus QtWebKit links against libleveldb only.
> > Also, note that the linker finds the buildroot's copy first; not its internal
> > copy. That explains why it is complaining about a missing symbol.
> > 
> > Fortunatly, QtWebKit provides a facility to link against the system leveldb
> > package. The qmake flag WEBKIT_CONFIG+=use_system_leveldb tells Qt5WebKit to
> > link against libleveldb *AND* libmemenv [5].
> > 
> > To fix that issue, this patchset builds the leveldb package and tells QtWebKit
> > to use it instead of building its own copy.
> > 
> > The first two patches concern the leveldb package.
> > 
> > The first one adds installation of headers and the missing static library in
> > the staging directory.
> 
> I am a bit uneasy about this one, it seems to install "internal" stuff
> of leveldb that are normally not meant to be installed. Do you/we
> understand what this internal stuff, beyond just "it fixes qt5webkit" ?
> 

This static library consists of this *VERY SINGLE* symbol which is like
an extra that is not shipped at installation.

Apparently WebKit see value in that function and has decided to use it
[1].

As QtWebKit bundled the leveldb third-party to build WebKit, it rewrote
the way how to build it (i.e. not using Makefile) in order to use qmake
build system.

As that "Makefile" has been rewritten, QtWebKit has included this symbol
directly to libleveldb.so to link against this library *ONLY* (which
looks an error to me).

Later, QtWebKit has provided a configure option to use the libleveldb.so
provided by the system (i.e. to disable the included third-party). This
option links Qt5WebKit.so with both libleveldb.so and libmemenv.a
libraries.

Is it more clear?

Note: Also, since leveldb has moved to cmake, this symbol is now a part
of libleveldb.so and there is not more libmemenv.a [2] :/

> (...) 
> 
> All in all, I'm just worried by the installation of what looks like
> some internal library/headers of leveldb. Could you comment on this ?
>

We can consider memenv as an option of leveldb which installs the memenv
development files (static library and header).

Then, QtWebKit will select that option because it requires memenv to be
installed to successfully build itself.

Which probably looks like:

	if BR2_PACKAGE_LEVELDB
	
	config BR2_PACKAGE_LEVELDB_MEMENV
		bool "memenv"	
		default n
		help
		  Installs memenv static library and header to create in-memory LevelDB
		  database.
	
	endif

> Thanks!
> 
> Thomas

[1]: https://github.com/qt/qtwebkit/blob/5.6/Source/WebCore/platform/leveldb/LevelDBDatabase.cpp#185
[2]: https://github.com/google/leveldb/blob/739c25100e46576cdcdfff2d6f43f9f7008103c7/CMakeLists.txt#L181-L186

Regards,
Ga?l

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
  2018-09-02 20:21   ` Gaël PORTAY
@ 2018-09-03 22:53     ` Arnout Vandecappelle
  2018-09-04 15:11       ` Gaël PORTAY
  0 siblings, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2018-09-03 22:53 UTC (permalink / raw)
  To: buildroot



On 02/09/2018 22:21, Ga?l PORTAY wrote:
> Thomas,
> 
> On Fri, Aug 31, 2018 at 11:19:30PM +0200, Thomas Petazzoni wrote:
[snip]
>> I am a bit uneasy about this one, it seems to install "internal" stuff
>> of leveldb that are normally not meant to be installed. Do you/we
>> understand what this internal stuff, beyond just "it fixes qt5webkit" ?
>>
> 
> This static library consists of this *VERY SINGLE* symbol which is like
> an extra that is not shipped at installation.
> 
> Apparently WebKit see value in that function and has decided to use it
> [1].
> 
> As QtWebKit bundled the leveldb third-party to build WebKit, it rewrote
> the way how to build it (i.e. not using Makefile) in order to use qmake
> build system.
> 
> As that "Makefile" has been rewritten, QtWebKit has included this symbol
> directly to libleveldb.so to link against this library *ONLY* (which
> looks an error to me).
> 
> Later, QtWebKit has provided a configure option to use the libleveldb.so
> provided by the system (i.e. to disable the included third-party). This
> option links Qt5WebKit.so with both libleveldb.so and libmemenv.a
> libraries.
> 
> Is it more clear?
> 
> Note: Also, since leveldb has moved to cmake, this symbol is now a part
> of libleveldb.so and there is not more libmemenv.a [2] :/

 So, wouldn't a bump of leveldb which moves to cmake solve the issue entirely?


> 
>> (...) 
>>
>> All in all, I'm just worried by the installation of what looks like
>> some internal library/headers of leveldb. Could you comment on this ?
>>
> 
> We can consider memenv as an option of leveldb which installs the memenv
> development files (static library and header).

 Since it's only a static library and a header, it only gets installed to
staging, not target, so there is no reason to make it optional.

 Regards,
 Arnout


[snip]

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
  2018-09-03 22:53     ` Arnout Vandecappelle
@ 2018-09-04 15:11       ` Gaël PORTAY
  2018-09-04 21:09         ` Arnout Vandecappelle
  0 siblings, 1 reply; 11+ messages in thread
From: Gaël PORTAY @ 2018-09-04 15:11 UTC (permalink / raw)
  To: buildroot

Arnout,

On Tue, Sep 04, 2018 at 12:53:17AM +0200, Arnout Vandecappelle wrote:
> (...)
> 
> > Note: Also, since leveldb has moved to cmake, this symbol is now a part
> > of libleveldb.so and there is not more libmemenv.a [2] :/
> 
>  So, wouldn't a bump of leveldb which moves to cmake solve the issue entirely?
> 

First of all, the version is not yet out (1.21).

I think, the bump will partially solve that issue (not tested, but I can
give it a try).

With the bump, QtWebKit would compile it as a third-party (fair but not
wanted) and would link against libleveldb.so which will be provided by
buildroot (1.21 with the missing symbol). In that situation, the
third-party is compiled but not used and the build will succeed.

If the configure flag WEBKIT_CONFIG+=use_system_leveldb is added,
QtWebKit would not compile leveldb as a third-party (wanted) but it
would try to link against libraries liblevel.db (okay) *AND* libmemenv.a
from buildroot's staging. But the last one will not exist and the
compilation would fail.

Even with the bump, (and if the configure flag is added) QtWebKit needs
to be patched to remove the -lmemenv from the compilation line; because
the libmemenv will disappear.

A good fix consists in to force QtWebKit to use its third-party library
(giving the full path?) so it will not link against the copy from
buildroot's staging.

> >> (...) 
> >>
> >> All in all, I'm just worried by the installation of what looks like
> >> some internal library/headers of leveldb. Could you comment on this ?
> >>
> > 
> > We can consider memenv as an option of leveldb which installs the memenv
> > development files (static library and header).
> 
>  Since it's only a static library and a header, it only gets installed to
> staging, not target, so there is no reason to make it optional.
>

Okay, I will drop the option and respin a new version.

>  Regards,
>  Arnout
> 
> 
> [snip]
> 
> -- 
> 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Regards,
Ga?l

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb
  2018-09-04 15:11       ` Gaël PORTAY
@ 2018-09-04 21:09         ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2018-09-04 21:09 UTC (permalink / raw)
  To: buildroot



On 04/09/2018 17:11, Ga?l PORTAY wrote:
> Arnout,
> 
> On Tue, Sep 04, 2018 at 12:53:17AM +0200, Arnout Vandecappelle wrote:
>> (...)
>>
>>> Note: Also, since leveldb has moved to cmake, this symbol is now a part
>>> of libleveldb.so and there is not more libmemenv.a [2] :/
>>
>>  So, wouldn't a bump of leveldb which moves to cmake solve the issue entirely?
>>
> 
> First of all, the version is not yet out (1.21).

 OK, so we will not bump as a way to solve this issue. But the patch may be
reverted (or changed) after a leveldb bump.

> 
> I think, the bump will partially solve that issue (not tested, but I can
> give it a try).
> 
> With the bump, QtWebKit would compile it as a third-party (fair but not
> wanted) and would link against libleveldb.so which will be provided by
> buildroot (1.21 with the missing symbol). In that situation, the
> third-party is compiled but not used and the build will succeed.
> 
> If the configure flag WEBKIT_CONFIG+=use_system_leveldb is added,
> QtWebKit would not compile leveldb as a third-party (wanted) but it
> would try to link against libraries liblevel.db (okay) *AND* libmemenv.a
> from buildroot's staging. But the last one will not exist and the
> compilation would fail.

 In other words, after the leveldb bump, we'll still need patch 3/3 to avoid
building the bundled leveldb, *and* we need to add a patch to QtWebKit to avoid
linking with libmemenv. Gee...


> Even with the bump, (and if the configure flag is added) QtWebKit needs
> to be patched to remove the -lmemenv from the compilation line; because
> the libmemenv will disappear.
> 
> A good fix consists in to force QtWebKit to use its third-party library
> (giving the full path?) so it will not link against the copy from
> buildroot's staging.

 Ugh no we want to avoid bundled libraries.

> 
>>>> (...) 
>>>>
>>>> All in all, I'm just worried by the installation of what looks like
>>>> some internal library/headers of leveldb. Could you comment on this ?
>>>>
>>>
>>> We can consider memenv as an option of leveldb which installs the memenv
>>> development files (static library and header).
>>
>>  Since it's only a static library and a header, it only gets installed to
>> staging, not target, so there is no reason to make it optional.
>>
> 
> Okay, I will drop the option and respin a new version.

 Er, your current iteration doesn't have it as an option...

 Regards,
 Arnout

> 
>>  Regards,
>>  Arnout
>>
>>
>> [snip]
>>
>> -- 
>> 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
> 
> Regards,
> Ga?l
> 

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package
  2018-08-31 20:22 ` [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package Gaël PORTAY
@ 2018-09-04 21:10   ` Arnout Vandecappelle
  2018-09-05  6:53     ` Thomas Petazzoni
  0 siblings, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2018-09-04 21:10 UTC (permalink / raw)
  To: buildroot



On 31/08/2018 22:22, Ga?l PORTAY wrote:
> +	depends on BR2_INSTALL_LIBSTDCPP # leveldb

 I don't think we need to propagate this particular dependency - qt5webkit
already depends on libstdcpp indirectly through Qt5.

 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package
  2018-09-04 21:10   ` Arnout Vandecappelle
@ 2018-09-05  6:53     ` Thomas Petazzoni
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Petazzoni @ 2018-09-05  6:53 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 4 Sep 2018 23:10:46 +0200, Arnout Vandecappelle wrote:
> On 31/08/2018 22:22, Ga?l PORTAY wrote:
> > +	depends on BR2_INSTALL_LIBSTDCPP # leveldb  
> 
>  I don't think we need to propagate this particular dependency - qt5webkit
> already depends on libstdcpp indirectly through Qt5.

I think we need at some point to define a real policy for this. When a
dependency is redundant, should we:

 (1) Avoid duplicating it, to simplify things, but with the risk that
     if we remove the dependency at a higher-level, we forget to re-add
     back to the sub-options.

 (2) Always duplicate them, even if they are useless right now, so
     that we don't forget them in the future if the higher-level option
     drops this dependency. Drawback of this solution is that such
     dependencies don't get "exercised" by autobuilder testing so they
     are often wrong.

I don't have a strong opinion, but we're doing (1) or (2) depending on
the package developer and/or situation, and this isn't really nice.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-09-05  6:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-31 20:22 [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Gaël PORTAY
2018-08-31 20:22 ` [Buildroot] [PATCH 1/3] leveldb: install memenv static library and headers Gaël PORTAY
2018-08-31 20:22 ` [Buildroot] [PATCH 2/3] leveldb: generate pic for static libraries Gaël PORTAY
2018-08-31 20:22 ` [Buildroot] [PATCH 3/3] qt5webkit: select leveldb package Gaël PORTAY
2018-09-04 21:10   ` Arnout Vandecappelle
2018-09-05  6:53     ` Thomas Petazzoni
2018-08-31 21:19 ` [Buildroot] [PATCH 0/3] qt5webkit: fix build issue using system leveldb Thomas Petazzoni
2018-09-02 20:21   ` Gaël PORTAY
2018-09-03 22:53     ` Arnout Vandecappelle
2018-09-04 15:11       ` Gaël PORTAY
2018-09-04 21:09         ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.