diff mbox

[v2] Compile Poppler over Qt

Message ID 1396345399-28487-1-git-send-email-jeremie.scheer@armadeus.com
State Superseded
Headers show

Commit Message

jeremie.scheer@armadeus.com April 1, 2014, 9:43 a.m. UTC 
From: Jeremie Scheer <jeremie.scheer@armadeus.com>


Signed-off-by: Jeremie Scheer <jeremie.scheer@armadeus.com>
---
 Changes since v1:
  - Remove dependencies that doesn't depend on Qt from Qt package condition

 package/poppler/poppler.mk |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni April 6, 2014, 10:02 a.m. UTC | #1 
Hello,

The title of your commit should be:

	poppler: allow building Qt support

i.e we always want to have the commit titles to look like:

	<name of the package>: <what is going on>

On Tue,  1 Apr 2014 11:43:19 +0200, jeremie.scheer@armadeus.com wrote:

> diff --git a/package/poppler/poppler.mk b/package/poppler/poppler.mk
> index 040327f..386e7b8 100644
> --- a/package/poppler/poppler.mk
> +++ b/package/poppler/poppler.mk
> @@ -7,10 +7,18 @@
>  POPPLER_VERSION = 0.24.4
>  POPPLER_SOURCE = poppler-$(POPPLER_VERSION).tar.xz
>  POPPLER_SITE = http://poppler.freedesktop.org
> -POPPLER_DEPENDENCIES = fontconfig
>  POPPLER_LICENSE = GPLv2+
>  POPPLER_LICENSE_FILES = COPYING
>  POPPLER_CONF_OPT = --with-font-configuration=fontconfig
> +POPPLER_INSTALL_STAGING = YES

The commit log should indicate why this is added.

> +
> +POPPLER_DEPENDENCIES = fontconfig
> +
> +ifeq ($(BR2_PACKAGE_QT),y)
> +	POPPLER_DEPENDENCIES += qt
> +else
> +	POPPLER_CONF_OPT += --disable-poppler-qt4
> +endif

This doesn't really work: BR2_PACKAGE_QT is not sufficient, it also
needs QtGui and QtXml. From the Poppler configure.ac:

  PKG_CHECK_MODULES(POPPLER_QT4, 
                    QtCore >= 4.4.0 QtGui >= 4.4.0 QtXml >= 4.4.0)

So maybe I would suggest to instead add a Config.in option that looks
like this:

config BR2_PACKAGE_POPPLER_QT
	bool "qt support"
	depends on BR2_PACKAGE_QT
	select BR2_PACKAGE_QT_GUI
	select BR2_PACKAGE_QT_XML
	help
	  Some help text

and then in poppler.mk, do:

ifeq ($(BR2_PACKAGE_POPPLER_QT),y)
POPPLER_DEPENDENCIES += qt
POPPLER_CONF_OPT += --enable-poppler-qt4
else
POPPLER_CONF_OPT += --disable-poppler-qt4
endif

Thanks,

Thomas
Alan Ott Oct. 12, 2014, 7:56 p.m. UTC | #2 
These changes were proposed by Jeremie Scheer <jeremie.scheer@armadeus.com>
earlier this year.

Alan Ott (2):
  poppler: Add option for Qt support
  poppler: Install files into staging

 package/poppler/Config.in  | 9 +++++++++
 package/poppler/poppler.mk | 8 ++++++++
 2 files changed, 17 insertions(+)
Thomas Petazzoni Dec. 9, 2014, 11:08 p.m. UTC | #3 
Dear Alan Ott,

On Sun, 12 Oct 2014 15:56:51 -0400, Alan Ott wrote:
> These changes were proposed by Jeremie Scheer <jeremie.scheer@armadeus.com>
> earlier this year.
> 
> Alan Ott (2):
>   poppler: Add option for Qt support
>   poppler: Install files into staging
> 
>  package/poppler/Config.in  | 9 +++++++++
>  package/poppler/poppler.mk | 8 ++++++++
>  2 files changed, 17 insertions(+)

Thanks, both patches committed. I've done some minor changes on the
first patch in the Config.in file, see:

  http://git.buildroot.net/buildroot/commit/?id=fb79843453704ae68cf8d91e2a9cd5359d0b1027
  http://git.buildroot.net/buildroot/commit/?id=ed592d6e02275c4402f477693195d941a481e67d

Thanks,

Thomas
diff mbox

Patch

diff --git a/package/poppler/poppler.mk b/package/poppler/poppler.mk
index 040327f..386e7b8 100644
--- a/package/poppler/poppler.mk
+++ b/package/poppler/poppler.mk
@@ -7,10 +7,18 @@ 
 POPPLER_VERSION = 0.24.4
 POPPLER_SOURCE = poppler-$(POPPLER_VERSION).tar.xz
 POPPLER_SITE = http://poppler.freedesktop.org
-POPPLER_DEPENDENCIES = fontconfig
 POPPLER_LICENSE = GPLv2+
 POPPLER_LICENSE_FILES = COPYING
 POPPLER_CONF_OPT = --with-font-configuration=fontconfig
+POPPLER_INSTALL_STAGING = YES
+
+POPPLER_DEPENDENCIES = fontconfig
+
+ifeq ($(BR2_PACKAGE_QT),y)
+	POPPLER_DEPENDENCIES += qt
+else
+	POPPLER_CONF_OPT += --disable-poppler-qt4
+endif
 
 ifeq ($(BR2_PACKAGE_LCMS2),y)
 	POPPLER_CONF_OPT += --enable-cms=lcms2