[PATCH v2] libsolv: don't pick up bundled db from host rpm

[PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

With rpm v4 in openembedded but on a host with existing /usr/include/rpm/db.h
the native build fails to compile.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---
 ...01-don-t-pick-up-bundled-db-from-host-rpm.patch | 43 ++++++++++++++++++++++
 meta/recipes-extended/libsolv/libsolv_0.6.27.bb    |  3 +-
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch

diff --git a/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch b/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch
new file mode 100644
index 0000000..41656fa
--- /dev/null
+++ b/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch
@@ -0,0 +1,43 @@
+From d5ff23ec64bc8b79d5335a2e0cd4b481b63fd95f Mon Sep 17 00:00:00 2001
+From: Max Krummenacher <max.krummenacher@toradex.com>
+Date: Sun, 7 May 2017 20:28:11 +0100
+Subject: [PATCH] don't pick up bundled db from host rpm
+
+For libsolv-native with rpm v4 in openembedded but on a host with
+existing /usr/include/rpm/db.h the build is configured to have
+HAVE_RPM_DB_H because of the existing header file from the host,
+but linking against the librpm.so in recipe-sysroot-native fails.
+
+Skip the check for rpm/db.h and assume that rpm is provided without
+a bundled berkley db.
+
+Fixes the following link errors:
+| ../ext/libsolvext.so.0: undefined reference to `db_create_rpmdb'
+| ../ext/libsolvext.so.0: undefined reference to `db_env_create_rpmdb'
+
+Observed on a openSUSE Leap 42.1 build host with rpm-devel installed.
+
+Upstream-Status: Inappropriate [oe build specific]
+
+Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
+---
+ CMakeLists.txt | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index bcdeee6..1ca7b41 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -220,8 +220,7 @@ IF (ENABLE_RPMDB)
+     ENDIF (RPMMISC_LIBRARY)
+   ENDIF (RPM5)
+ 
+-  # check if rpm contains a bundled berkeley db
+-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
++  set(HAVE_RPM_DB_H 0)
+   IF (NOT HAVE_RPM_DB_H)
+     FIND_LIBRARY (DB_LIBRARY NAMES db)
+     IF (DB_LIBRARY)
+-- 
+2.9.3
+
diff --git a/meta/recipes-extended/libsolv/libsolv_0.6.27.bb b/meta/recipes-extended/libsolv/libsolv_0.6.27.bb
index 7ddd533..0f988dc 100644
--- a/meta/recipes-extended/libsolv/libsolv_0.6.27.bb
+++ b/meta/recipes-extended/libsolv/libsolv_0.6.27.bb
@@ -8,7 +8,8 @@ LIC_FILES_CHKSUM = "file://LICENSE.BSD;md5=62272bd11c97396d4aaf1c41bc11f7d8"
 DEPENDS = "expat zlib rpm"
 
 SRC_URI = "git://github.com/openSUSE/libsolv.git \
-           "
+           file://0001-don-t-pick-up-bundled-db-from-host-rpm.patch \
+          "
 SRC_URI_append_libc-musl = " file://0001-Add-fallback-fopencookie-implementation.patch \
                              file://0002-Fixes-to-internal-fopencookie-implementation.patch \
                            "
-- 
2.9.3

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Alexander Kanavin]

On 05/26/2017 11:35 PM, Max Krummenacher wrote:
> +-  # check if rpm contains a bundled berkeley db
> +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
> ++  set(HAVE_RPM_DB_H 0)

Have you looked into what CHECK_INCLUDE_FILE does, and whether it can be 
fixed? I'd like to hear about your findings.

Alex

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

2017-05-29 12:50 GMT+02:00 Alexander Kanavin
<alexander.kanavin@linux.intel.com>:
> On 05/26/2017 11:35 PM, Max Krummenacher wrote:
>>
>> +-  # check if rpm contains a bundled berkeley db
>> +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
>> ++  set(HAVE_RPM_DB_H 0)
>
>
> Have you looked into what CHECK_INCLUDE_FILE does, and whether it can be
> fixed? I'd like to hear about your findings.

Yes, I did.

I creates a c file and tries to compile it with the CFLAGS specified
in CMaketext.

<<<<<<<<
#include "rpm/db.h"
int main () {
return 0;
}
>>>>>>>>

As we're talking about libsolv-native here it uses the build host's
gcc which has the standard include directory
/usr/include and thus finds a /usr/include/rpm/db.h

And again, I do not think that it is an error to find/use headers from
the build host's /usr/include for a *-native
build.

Max

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

2017-05-29 15:00 GMT+02:00 Max Krummenacher <max.oss.09@gmail.com>:
> 2017-05-29 12:50 GMT+02:00 Alexander Kanavin
> <alexander.kanavin@linux.intel.com>:
>> On 05/26/2017 11:35 PM, Max Krummenacher wrote:
>>>
>>> +-  # check if rpm contains a bundled berkeley db
>>> +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
>>> ++  set(HAVE_RPM_DB_H 0)
>>
>>
>> Have you looked into what CHECK_INCLUDE_FILE does, and whether it can be
>> fixed? I'd like to hear about your findings.
>
> Yes, I did.
>
> I creates a c file and tries to compile it with the CFLAGS specified
> in CMaketext.
>
> <<<<<<<<
> #include "rpm/db.h"
> int main () {
> return 0;
> }
>>>>>>>>>
>
> As we're talking about libsolv-native here it uses the build host's
> gcc which has the standard include directory
> /usr/include and thus finds a /usr/include/rpm/db.h
>
> And again, I do not think that it is an error to find/use headers from
> the build host's /usr/include for a *-native
> build.

Maybe to add to 'can it be fixed?'

As I said, IMHO it is not broken, but
yes, one could pass an additional parameter to the compiler '-nostdinc'.
CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H, "-nostdinc")
But this would break the intention of the test on the not '-native'
build as there
the rpm headers whould actually be found in the standard include directories.
So if OE ever would decide to change the rpm recipe to have an included db
this would not be picked up by the test in the not '-native' tests.

Max

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Alexander Kanavin]

On 05/29/2017 04:06 PM, Max Krummenacher wrote:
>>>> +-  # check if rpm contains a bundled berkeley db
>>>> +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
>>>> ++  set(HAVE_RPM_DB_H 0)
>>>
>>>
>>> Have you looked into what CHECK_INCLUDE_FILE does, and whether it can be
>>> fixed? I'd like to hear about your findings.
>>
>> Yes, I did.
>>
>> I creates a c file and tries to compile it with the CFLAGS specified
>> in CMaketext.
>>
>> <<<<<<<<
>> #include "rpm/db.h"
>> int main () {
>> return 0;
>> }
>>>>>>>>>>
>>
>> As we're talking about libsolv-native here it uses the build host's
>> gcc which has the standard include directory
>> /usr/include and thus finds a /usr/include/rpm/db.h
>>
>> And again, I do not think that it is an error to find/use headers from
>> the build host's /usr/include for a *-native
>> build.
>
> Maybe to add to 'can it be fixed?'
>
> As I said, IMHO it is not broken, but
> yes, one could pass an additional parameter to the compiler '-nostdinc'.
> CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H, "-nostdinc")
> But this would break the intention of the test on the not '-native'
> build as there
> the rpm headers whould actually be found in the standard include directories.
> So if OE ever would decide to change the rpm recipe to have an included db
> this would not be picked up by the test in the not '-native' tests.

I see. You are right here. I admit I don't fully understand if it's 
generally okay for -native recipes to look around in (and later use) 
host include  directories like that, as we're striving towards 
reproducible builds, and this behavior is not supportive of them. 
Richard, can we have your say please?

Alex

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Richard Purdie]

On Mon, 2017-05-29 at 18:54 +0300, Alexander Kanavin wrote:
> I see. You are right here. I admit I don't fully understand if it's 
> generally okay for -native recipes to look around in (and later use) 
> host include  directories like that, as we're striving towards 
> reproducible builds, and this behavior is not supportive of them. 
> Richard, can we have your say please?

Its unavoidable really, we have to have some host system we build on as
we need a compiler/headers from it. Even if you build a gcc-native,
what would you compile it with? :)

If you do want to avoid the risk, you'd choose your own custom base OS
(or container) and then always build on that. It would then be both
reproducible and deterministic. We don't want to mandate a specific
host configuration out the box though.

So this patch avoids an issue. We don't tend to run into too many
issues like this one thankfully.

Cheers,

Richard

[PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

With rpm v4 in openembedded but on a host with existing /usr/include/rpm/db.h
the native build fails to compile.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---
 ...01-don-t-pick-up-bundled-db-from-host-rpm.patch | 51 ++++++++++++++++++++++
 meta/recipes-extended/libsolv/libsolv_0.6.26.bb    |  3 +-
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch

changes in V2:

- instead of unconditionally assume db is not bundled
  evaluate the actually linked librpm.so
- fix commit text to include that libsolv-native is failing
  (but not libsolv and nativesdk-libsolv)

diff --git a/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch b/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch
new file mode 100644
index 0000000..f99cdca
--- /dev/null
+++ b/meta/recipes-extended/libsolv/libsolv/0001-don-t-pick-up-bundled-db-from-host-rpm.patch
@@ -0,0 +1,51 @@
+From f815c166f08f8a0a257630e91c0bcc20409f42cf Mon Sep 17 00:00:00 2001
+From: Max Krummenacher <max.krummenacher@toradex.com>
+Date: Sun, 7 May 2017 20:28:11 +0100
+Subject: [PATCH] don't pick up bundled db from host rpm
+
+For libsolv-native with rpm v4 in openembedded but on a host with
+existing /usr/include/rpm/db.h the build is configured to have
+HAVE_RPM_DB_H because of the existing header file from the host,
+but linking against the librpm.so in recipe-sysroot-native fails.
+
+Check for a bundled db by testing if librpm.so provides one of
+the db functions.
+
+Fixes the following link errors:
+| ../ext/libsolvext.so.0: undefined reference to `db_create_rpmdb'
+| ../ext/libsolvext.so.0: undefined reference to `db_env_create_rpmdb'
+
+Observed on a openSUSE Leap 42.1 build host with rpm-devel installed.
+
+Upstream-Status: Inappropriate [oe build specific]
+
+Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
+---
+ CMakeLists.txt | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 838f9d0..1ded881 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -185,7 +185,7 @@ IF (ENABLE_RPMDB)
+ SET (ENABLE_RPMPKG ON)
+ ENDIF (ENABLE_RPMDB)
+ 
+-INCLUDE (CheckIncludeFile)
++INCLUDE (CheckLibraryExists)
+ IF (ENABLE_RPMDB)
+   FIND_LIBRARY (RPMDB_LIBRARY NAMES rpmdb)
+ 
+@@ -206,7 +206,7 @@ IF (ENABLE_RPMDB)
+   ENDIF (RPM5)
+ 
+   # check if rpm contains a bundled berkeley db
+-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
++  CHECK_LIBRARY_EXISTS(rpm.so db_create_rpmdb "" HAVE_RPM_DB_H)
+   IF (NOT HAVE_RPM_DB_H)
+     FIND_LIBRARY (DB_LIBRARY NAMES db)
+     IF (DB_LIBRARY)
+-- 
+2.12.0
+
diff --git a/meta/recipes-extended/libsolv/libsolv_0.6.26.bb b/meta/recipes-extended/libsolv/libsolv_0.6.26.bb
index a33c251..ccd9cfa 100644
--- a/meta/recipes-extended/libsolv/libsolv_0.6.26.bb
+++ b/meta/recipes-extended/libsolv/libsolv_0.6.26.bb
@@ -8,7 +8,8 @@ LIC_FILES_CHKSUM = "file://LICENSE.BSD;md5=62272bd11c97396d4aaf1c41bc11f7d8"
 DEPENDS = "expat zlib rpm"
 
 SRC_URI = "git://github.com/openSUSE/libsolv.git \
-           "
+           file://0001-don-t-pick-up-bundled-db-from-host-rpm.patch \
+          "
 SRC_URI_append_libc-musl = " file://0001-Add-fallback-fopencookie-implementation.patch \
                              file://0002-Fixes-to-internal-fopencookie-implementation.patch \
                            "
-- 
2.9.3

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

On 19 May 2017 at 16:48, Max Krummenacher <max.oss.09@gmail.com> wrote:

> +Upstream-Status: Inappropriate [oe build specific]
>

Is this really inappropriate?  We're finding a bug in the build system but
that doesn't mean we're the cause of the breakage.

Ross

[-- Attachment #2: Type: text/html, Size: 690 bytes --]

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

Am Freitag, den 19.05.2017, 16:51 +0100 schrieb Burton, Ross:
> On 19 May 2017 at 16:48, Max Krummenacher <max.oss.09@gmail.com> wrote:
> 
> > +Upstream-Status: Inappropriate [oe build specific]
> > 
> 
> Is this really inappropriate?  We're finding a bug in the build system but
> that doesn't mean we're the cause of the breakage.

Yes, I think so.

In the OE setup we have our own sysroot with a variant of rpm-devel and the build host has one in
/usr/...

I think that upstream need not assume that it is configured on a build host where it sees two
differently configured rpm development packages.

For my approach to work the bundled Berkely db must provide the function db_create_rpmdb which is
more likely to change than the filename db.h.

Max

> 
> Ross

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Alexander Kanavin]

On 05/19/2017 06:48 PM, Max Krummenacher wrote:
> +   # check if rpm contains a bundled berkeley db
> +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
> ++  CHECK_LIBRARY_EXISTS(rpm.so db_create_rpmdb "" HAVE_RPM_DB_H)

Sorry Max, this is actually worse than the first version. HAVE_RPM_DB_H 
has to accurately reflect the presence or absence of the header, as that 
header is later included or not included depending on that. You cannot 
set it based on checking some library.

So please either fix CHECK_INCLUDE_FILE, or let's revert to the first 
version of the patch, as it at least doesn't introduce a new issue.

Alex

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Max Krummenacher]

Hi Alex

Am Montag, den 22.05.2017, 16:08 +0300 schrieb Alexander Kanavin:
> On 05/19/2017 06:48 PM, Max Krummenacher wrote:
> > +   # check if rpm contains a bundled berkeley db
> > +-  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H)
> > ++  CHECK_LIBRARY_EXISTS(rpm.so db_create_rpmdb "" HAVE_RPM_DB_H)
> 
> Sorry Max, this is actually worse than the first version. HAVE_RPM_DB_H 
> has to accurately reflect the presence or absence of the header, as that 
> header is later included or not included depending on that. You cannot 
> set it based on checking some library.
Well, that is somewhat true.
The test explicitly tests that the header file is available, but then
implicitely assumes that when the header file is available no standalone
needs be linked.

Turning that assumption to explicitly test that rpm.so provides the db
functionality and from that assuming that rpm/db.h is available is IMHO
equally right or equally incomplete testing.

> So please either fix CHECK_INCLUDE_FILE, 
The reason why I didn't fix it as follows is that this would break
the test for the non native (target and nativesdk) case.

+  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H "-nostdinc")

For the non native case the OE rpm headers are actually stored in
the compiler's standard include path, so searching for rpm/db.h
with -nostdinc wouldn't pick up the file even if it would exist.

> 
> or let's revert to the first 
> version of the patch, as it at least doesn't introduce a new issue.
v1 unconditionally assumes that rpm is provided without a bundled db.
v2 tries to find out from the to be linked librpm.so if it db is bundled.
Both versions work for libsolv, libsolv-native, and nativesdk-libsolv.

I can send a v3 with either variant plus the rebase on top of the now
updated libsolv recipe.

What do you think?

Max




> Alex

Re: [PATCH v2] libsolv: don't pick up bundled db from host rpm

[Alexander Kanavin]

On 05/23/2017 12:32 AM, Max Krummenacher wrote:
>> Sorry Max, this is actually worse than the first version. HAVE_RPM_DB_H
>> has to accurately reflect the presence or absence of the header, as that
>> header is later included or not included depending on that. You cannot
>> set it based on checking some library.
> Well, that is somewhat true.
> The test explicitly tests that the header file is available, but then
> implicitely assumes that when the header file is available no standalone
> needs be linked.
>
> Turning that assumption to explicitly test that rpm.so provides the db
> functionality and from that assuming that rpm/db.h is available is IMHO
> equally right or equally incomplete testing.

The first version of the patch assumes that rpm in oe-core is configured 
to use external db, and hardcodes that assumption.

The second version of the patch assumes that both the library and the 
header are available at the same time. This may be true right now when 
building with bitbake, it is certainly not true in other situations.

I would rather make assumptions about oe-core content, rather than 
bitbake behaviour.

>> So please either fix CHECK_INCLUDE_FILE,
> The reason why I didn't fix it as follows is that this would break
> the test for the non native (target and nativesdk) case.
>
> +  CHECK_INCLUDE_FILE(rpm/db.h HAVE_RPM_DB_H "-nostdinc")
>
> For the non native case the OE rpm headers are actually stored in
> the compiler's standard include path, so searching for rpm/db.h
> with -nostdinc wouldn't pick up the file even if it would exist.

This is not the fix I meant. You are fixing the *use* of 
CHECK_INCLUDE_FILE, but I wanted you to fix the internal content of 
CHECK_INCLUDE_FILE itself, so that it behaves correctly in 
bitbake-driven environment, and does not go out to check host machine 
include paths.

It's fine if the code to be fixed is too arcane or tricky and you cannot 
do it, then let's just revert to the first version of the patch.

Alex