| Message ID | 20200111115054.864819-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/poppler: really disable test programs | expand |
On Sat, 11 Jan 2020 12:50:54 +0100 Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > --- > ...1-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch | 32 +++++++++++++++++++ > 1 file changed, 32 insertions(+) > create mode 100644 package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > > diff --git a/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > new file mode 100644 > index 0000000000..476676199a > --- /dev/null > +++ b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > @@ -0,0 +1,32 @@ > +From ccc5f765c37b2d65e2e0b64d92326997f90acb3e Mon Sep 17 00:00:00 2001 > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> > +Date: Sat, 11 Jan 2020 12:37:07 +0100 > +Subject: [PATCH] CMakeLists.txt: fix BUILD_CPP_TESTS > + > +Do not build programs under the test directory if BUILD_CPP_TESTS is OFF > +as this directory contains multiple C++ test executables What is the problem with C++ executables, since poppler depends on C++ anyway ? Of course, it is good to disable building tests if possible, but I'm just curious to understand the motivation for the change: did you encounter a build failure? Or it is just that you realized the tests were being built while they should not? Thanks! Thomas
Dear Thomas, Le sam. 11 janv. 2020 à 15:35, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > On Sat, 11 Jan 2020 12:50:54 +0100 > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > > --- > > ...1-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch | 32 +++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > create mode 100644 package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > > > > diff --git a/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > > new file mode 100644 > > index 0000000000..476676199a > > --- /dev/null > > +++ b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch > > @@ -0,0 +1,32 @@ > > +From ccc5f765c37b2d65e2e0b64d92326997f90acb3e Mon Sep 17 00:00:00 2001 > > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> > > +Date: Sat, 11 Jan 2020 12:37:07 +0100 > > +Subject: [PATCH] CMakeLists.txt: fix BUILD_CPP_TESTS > > + > > +Do not build programs under the test directory if BUILD_CPP_TESTS is OFF > > +as this directory contains multiple C++ test executables > > What is the problem with C++ executables, since poppler depends on C++ > anyway ? > > Of course, it is good to disable building tests if possible, but I'm > just curious to understand the motivation for the change: did you > encounter a build failure? Or it is just that you realized the tests > were being built while they should not? Following your comment on the leveldb patch that was fixing the -latomic build failure, I'm going through the different -latomic workarounds used in the cmake packages. I'm trying to replace these workarounds by upstreamable patches. However, in some cases such as libraries (like poppler) these workarounds/patches are not strictly needed if we don't build any executable. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Best Regards, Fabrice
On Sat, 11 Jan 2020 15:44:44 +0100 Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > What is the problem with C++ executables, since poppler depends on C++ > > anyway ? > > > > Of course, it is good to disable building tests if possible, but I'm > > just curious to understand the motivation for the change: did you > > encounter a build failure? Or it is just that you realized the tests > > were being built while they should not? > Following your comment on the leveldb patch that was fixing the > -latomic build failure, I'm going through the different -latomic > workarounds used in the cmake packages. > I'm trying to replace these workarounds by upstreamable patches. > However, in some cases such as libraries (like poppler) these > workarounds/patches are not strictly needed if we don't build any > executable. It's not true: if libpoppler.so uses some symbols that are defined in libatomic.so, then it should link against libatomic.so (i.e libpoppler.so should have a DT_NEEDED entry for libatomic.so). Otherwise any application linking against libpoppler.so will have to know that it should also link against libatomic.so, which is not good. Thomas
Le sam. 11 janv. 2020 à 17:08, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > On Sat, 11 Jan 2020 15:44:44 +0100 > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > > > What is the problem with C++ executables, since poppler depends on C++ > > > anyway ? > > > > > > Of course, it is good to disable building tests if possible, but I'm > > > just curious to understand the motivation for the change: did you > > > encounter a build failure? Or it is just that you realized the tests > > > were being built while they should not? > > Following your comment on the leveldb patch that was fixing the > > -latomic build failure, I'm going through the different -latomic > > workarounds used in the cmake packages. > > I'm trying to replace these workarounds by upstreamable patches. > > However, in some cases such as libraries (like poppler) these > > workarounds/patches are not strictly needed if we don't build any > > executable. > > It's not true: if libpoppler.so uses some symbols that are defined in > libatomic.so, then it should link against libatomic.so (i.e > libpoppler.so should have a DT_NEEDED entry for libatomic.so). > Otherwise any application linking against libpoppler.so will have to > know that it should also link against libatomic.so, which is not good. OK, then I'll make a patch for poppler and sent it upstream. > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Best Regards, Fabrice
diff --git a/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch new file mode 100644 index 0000000000..476676199a --- /dev/null +++ b/package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch @@ -0,0 +1,32 @@ +From ccc5f765c37b2d65e2e0b64d92326997f90acb3e Mon Sep 17 00:00:00 2001 +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> +Date: Sat, 11 Jan 2020 12:37:07 +0100 +Subject: [PATCH] CMakeLists.txt: fix BUILD_CPP_TESTS + +Do not build programs under the test directory if BUILD_CPP_TESTS is OFF +as this directory contains multiple C++ test executables + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +[Upstream status: +https://gitlab.freedesktop.org/poppler/poppler/merge_requests/490] +--- + CMakeLists.txt | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index c182d146..d159582b 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -677,7 +677,9 @@ endif() + if(ENABLE_GLIB) + add_subdirectory(glib) + endif() ++if(BUILD_CPP_TESTS) + add_subdirectory(test) ++endif() + if(ENABLE_QT5) + add_subdirectory(qt5) + endif() +-- +2.24.1 +
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> --- ...1-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 package/poppler/0001-CMakeLists.txt-fix-BUILD_CPP_TESTS.patch