Opened 9 years ago

Closed 8 years ago

#47025 closed enhancement (fixed)

poppler: add Qt5 subport and optional test support.

Reported by: RJVB (René Bertin) Owned by: dbevans (David B. Evans)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: michaelld (Michael Dickens), MarcusCalhoun-Lopez (Marcus Calhoun-Lopez), mkae (Marko Käning), mojca (Mojca Miklavec)
Port: poppler

Description

This patch addresses 2 issues:

  • an implicit dependency on GTk for generating documentation. The patch adds a check if gtkdoc-rebase is available and if not, adds --disable-gtk-docs instead of --enable-gtk-docs .
  • missing poppler-qt5-mac .

Attachments (8)

build-without-gtk-doc.log (449.4 KB) - added by dbevans (David B. Evans) 9 years ago.
Build log showing success without gtk-doc gtk2 gtk3 active
build-with-gtk-doc.log (449.3 KB) - added by dbevans (David B. Evans) 9 years ago.
Build log showing success with gtk-doc active still no gtk2 gtk3
poppler-qt5-mac-main.log (353.8 KB) - added by dbevans (David B. Evans) 9 years ago.
Build log for poppler-qt5-mac showing failure
poppler-0.32-qt5-mac.diff (3.9 KB) - added by dbevans (David B. Evans) 9 years ago.
Proposed patch as revised by devans
poppler-qt5.diff (2.6 KB) - added by RJVB (René Bertin) 9 years ago.
poppler-0.32-qt5-mac.2.diff (802 bytes) - added by dbevans (David B. Evans) 9 years ago.
Proposed patch as revised by devans after commit of poppler 0.32.0 without poppler-qt5-mac
poppler-0.33-qt5-mac.diff (810 bytes) - added by dbevans (David B. Evans) 9 years ago.
Revised patch to add poppler-qt5-mac to poppler 0.33
poppler.diff (2.8 KB) - added by RJVB (René Bertin) 8 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 in reply to:  description ; Changed 9 years ago by larryv (Lawrence Velázquez)

Cc: devans@… removed
Owner: changed from macports-tickets@… to devans@…
Version: 2.3.3

Replying to rjvbertin@…:

  • an implicit dependency on GTk for generating documentation. The patch adds a check if gtkdoc-rebase is available and if not, adds --disable-gtk-docs instead of --enable-gtk-docs .

How exactly do the port’s contents change if gtkdoc-rebase is present?

Last edited 9 years ago by larryv (Lawrence Velázquez) (previous) (diff)

comment:2 Changed 9 years ago by dbevans (David B. Evans)

Status: newassigned

comment:3 in reply to:  description Changed 9 years ago by dbevans (David B. Evans)

Replying to rjvbertin@…:

This patch addresses 2 issues:

  • an implicit dependency on GTk for generating documentation. The patch adds a check if gtkdoc-rebase is available and if not, adds --disable-gtk-docs instead of --enable-gtk-docs .

I think this means that you would like to be able to generate documentation. You're approach yields differenct results based on what ports (gtk-doc) are installed. MacPorts policy is that the port should build the same regardless of what's installed. If one were to add an option to enable gtk-doc generation, the usual approach would be to use --disable-gtk-docs by default and then use a +docs variant that would add a build dependency on gtk-doc and switch the configuration item to --enable-gtk-docs.

As it stands, generation of documentation is intentionally disabled (the default for --enable-gtk-docs is no, see configure). This is as it should be since the documentation has already been generated upstream and is included (in devhelp compatible html format) in the tarball. No need to regenerate. No need to --disable/enable-gtk-docs, depend on gtk-doc, etc.

Also note that gtk-doc does not use gtk. It is only used by gtk developers to generate docs.

  • missing poppler-qt5-mac .

Thanks for this input. Have you tested that poppler-qt5-mac actually builds? The last time I tried it did not with our version of qt5.

comment:4 in reply to:  1 Changed 9 years ago by dbevans (David B. Evans)

Replying to larryv@…:

Replying to rjvbertin@…:

  • an implicit dependency on GTk for generating documentation. The patch adds a check if gtkdoc-rebase is available and if not, adds --disable-gtk-docs instead of --enable-gtk-docs .

How exactly do the port’s contents change if gtkdoc-rebase is present?

gtkdoc-rebase is one of the scripts provided by gtk-doc. So testing for gtk-doc-rebase is a way to see if gtk-doc is installed. As the port stands, there is no change in the port whether gtk-doc is installed or not.

comment:5 Changed 9 years ago by RJVB (René Bertin)

Re: the Qt5 subport: yes, I tested, it builds. And the tests succeed (as far as they concern the Qt5 bits ... which I haven't checked, but nothing is tested when building the main port with +test...)

As to the --enable-gtk-docs/--disable-gtk-docs issue: I stumbled across the need to disable gtk-docs when GTk is *not* installed. I realised later that it'd be easier to just pass --disable-gtk-docs unconditionally. I haven't investigated in detail because my focus was elsewhere (the GTk-less set-up is on Bradley's VM which is halfway across the globe from me) but I suspect that the claim that the feature is deactivated by default isn't perfectly valid. Maybe something goes awry during the autogen step?

comment:6 Changed 9 years ago by dbevans (David B. Evans)

Ok, I'll give this all a test and see if I can reproduce your results.

Questions:

  • Can you clarify that the issue occurs when port gtk2 is *not* installed or do you mean port gtk-doc?
  • What exactly is the offending behavior when it fails?
  • Can you please attach a logfile for the failure?
  • Does this occur when building the current poppler port or after your modifications?
  • Any other context information that might be relevant?

Thanks

comment:7 Changed 9 years ago by RJVB (René Bertin)

Well I'll be damned, I can no longer reproduce the gtkdoc-rebase issue myself. I can only recall and report that when poppler was being pulled in the 1st time I got an error about gtkdoc-rebase. I checked, confirmed it indeed wasn't installed, checked poppler's configure options, saw the --disable-gtk-docs option, and made a modified portfile copy that had the change included in the attached patch. The original portfile was the one dated 2015-02-06 from svn. As I said this was a dependency (of a dependency of a ...) of whatever I was installing at the time, so I didn't investigate ... and of course when my mod allowed the build to go through the logfile of the failure had already disappeared. I tried to approximate the environment I was in at the time, deactivating gtk-doc, gtk2, gtk3 etc, and still no error.

Makes you wonder if I experienced a bitflip at the time, but if you really want to be sure it was indeed a transitional glitch I fear you'll have to make a virgin MacPorts install and do a port install poppler in there to see what happens.

Changed 9 years ago by dbevans (David B. Evans)

Attachment: build-without-gtk-doc.log added

Build log showing success without gtk-doc gtk2 gtk3 active

Changed 9 years ago by dbevans (David B. Evans)

Attachment: build-with-gtk-doc.log added

Build log showing success with gtk-doc active still no gtk2 gtk3

comment:8 Changed 9 years ago by dbevans (David B. Evans)

Attached are logs from two clean test builds that I just did, one without gtk-doc gtk2 or gtk3 active and one with gtk-doc active but still no gtk2 gtk3. Both build successfully.

In the no gtk-doc case, I built using the following commands on my test system:

sudo port deactivate active
sudo port build poppler

the relevant lines of the log during configuration are

:info:configure checking for gtk-doc... no
:info:configure configure: WARNING:
:info:configure   You will not be able to create source packages with 'make dist'
:info:configure   because gtk-doc >= 1.14 is not found.
:info:configure checking for gtkdoc-check... no
:info:configure checking for gtkdoc-check... no
:info:configure checking for gtkdoc-rebase... no
:info:configure checking for gtkdoc-mkpdf... no
:info:configure checking whether to build gtk-doc documentation... no

As you can see, configure recognizes that gtk-doc is not installed and the build of gtk-doc documentation is disabled. The warning concerning 'make dist' is not significant because we are not trying to generate a distfile (tarball). That's the feature they use upstream to generate what we build from.

To build with gtk-doc active (still no gtk2 gtk3) I used the following additional commands:

sudo port install gtk-doc
sudo port clean poppler
sudo port build poppler

here the relevant lines from the log are

:info:configure checking for gtk-doc... yes
:info:configure checking for gtkdoc-check... gtkdoc-check.test
:info:configure checking for gtkdoc-check... /opt/local/bin/gtkdoc-check
:info:configure checking for gtkdoc-rebase... /opt/local/bin/gtkdoc-rebase
:info:configure checking for gtkdoc-mkpdf... /opt/local/bin/gtkdoc-mkpdf
:info:configure checking whether to build gtk-doc documentation... no

In this case, gtk-doc is recognized as installed but the gtk-doc documentation is still not built since it is disabled by default as claimed.

So at least this is not an issue for me. Is this what you are talking about or have a misunderstood your issue?

Last edited 9 years ago by dbevans (David B. Evans) (previous) (diff)

comment:9 Changed 9 years ago by RJVB (René Bertin)

No, I think you must have covered about all the states that the VM I was working on could have been in at the time. Let's say it was a false alarm.

sudo port deactivate active
sudo port build poppler

ports are reactivated as needed when building (or installing) another port, provided there is no ambiguity which version one to activate?

comment:10 in reply to:  9 Changed 9 years ago by dbevans (David B. Evans)

Replying to rjvbertin@…:

No, I think you must have covered about all the states that the VM I was working on could have been in at the time. Let's say it was a false alarm.

sudo port deactivate active
sudo port build poppler

ports are reactivated as needed when building (or installing) another port, provided there is no ambiguity which version one to activate?

Yes, this is inconvenient for a production system but guarantees that only the ports that are recursive dependencies of poppler are installed. The variants supplied with poppler are passed down to the dependents and used to select which version of each dependent to activate or build if missing. In this case, there are none so the default version of each dependency will be used over any other version/variants.

Last edited 9 years ago by dbevans (David B. Evans) (previous) (diff)

comment:11 Changed 9 years ago by dbevans (David B. Evans)

Concerning your comments concerning tests, these are intentionally disabled, both by using --disable-gtk-test during configure and by not enabling the test phase in the Portfile. In addition, as you have shown in your patch, to successfully test, a separate file of test data needs to be downloaded from the poppler git repository.

The reason for this is that enabling the tests DOES introduce a dependency on gtk3, that is, a gtk3 test program is used for testing. I didn't want this by default since poppler is used in many environments not just GTK+.

Having downloaded the data, the proper way to test is to enable and configure the test phase as necessary rather than using system as you have done. See the Macports Guide for details.

I'll plan to add the Qt5 and optional testing support in conjunction with the 0.32 release which should be coming out in the next week or so. They're on a once-a-month release schedule now.

Last edited 9 years ago by dbevans (David B. Evans) (previous) (diff)

comment:12 in reply to:  11 ; Changed 9 years ago by RJVB (René Bertin)

Replying to devans@…:

The reason for this is that enabling the tests DOES introduce a dependency on gtk3, that is, a gtk3 test program is used for testing. I didn't want this by default since poppler is used in many environments not just GTK+.

Are you sure that's still actual? FWIW, I just deactivated port:gtk3 and did a port destroot poppler-qt5-mac +test: I didn't see any reference to GTk3 and all tests completed just fine. BTW, make check only actually runs tests for the Qt subports; for poppler itself it does nothing. Maybe because GTk3 support is deactivated and thus no test executables are built?

comment:13 in reply to:  12 ; Changed 9 years ago by dbevans (David B. Evans)

Replying to rjvbertin@…:

Replying to devans@…:

The reason for this is that enabling the tests DOES introduce a dependency on gtk3, that is, a gtk3 test program is used for testing. I didn't want this by default since poppler is used in many environments not just GTK+.

Are you sure that's still actual? FWIW, I just deactivated port:gtk3 and did a port destroot poppler-qt5-mac +test: I didn't see any reference to GTk3 and all tests completed just fine. BTW, make check only actually runs tests for the Qt subports; for poppler itself it does nothing. Maybe because GTk3 support is deactivated and thus no test executables are built?

Yes, I was talking about the basic poppler build using --enable-gtk-tests. These definitely use/require gtk3 (see configure.ac for details).

Building poppler-qt*-mac is something different. Doesn't make much sense to test Qt wrappers with GTK+.

At any rate, I'll factor this all in with the next release.

Last edited 9 years ago by dbevans (David B. Evans) (previous) (diff)

comment:14 Changed 9 years ago by dbevans (David B. Evans)

Summary: poppler: build error correction and Qt5 subportpoppler: add Qt5 subport and optional test support.

comment:15 in reply to:  13 Changed 9 years ago by RJVB (René Bertin)

Replying to devans@…:

Yes, I was talking about the basic poppler build using --enable-gtk-tests. These definitely use/require gtk3 (see configure.ac for details).

I'd hope so :) And I didn't touch that option. I just thought it could be useful to do the testing, optionally.

comment:16 Changed 9 years ago by dbevans (David B. Evans)

poppler 0.32.0 was released this morning so I am planning on finishing up this ticket over the weekend.

Question about your patch:

What's the rationale behind these two lines:

configure.env-append    MOCQT4=${qt_bins_dir}/moc 
configure.env-append    MOCQT5=${qt_bins_dir}/moc 

The QT4 one doesn't seem to be necessary - haven't tested the other yet. Is this just pro forma?

Last edited 9 years ago by dbevans (David B. Evans) (previous) (diff)

comment:17 Changed 9 years ago by RJVB (René Bertin)

Whether the QT4 one is necessary will depend on how things are set up, i.e. if you have a moc in your path and what version that is. Rather than second guess or impose additional dependencies (that aren't yet incorporated in MacPorts) I preferred to use the easiest and most explicit way poppler provides to point the build to the correct moc.

Example: I have port:qtchooser installed, and configured such that ${prefix}/bin/{qmake,moc,etc} correspond to the Qt4 versions. So specifying MOCQT4 is redundant, but you get interesting results when the Qt5 build stumbles on and uses the Qt4 moc. (And that happens only quite far into the build.)

comment:18 Changed 9 years ago by dbevans (David B. Evans)

Cc: michaelld@… mcalhoun@… added

Attached is my revised version of the submitted patch but issues remain before it can be committed.

Poppler is updated to version 0.32.0.

Rather than introducing the +test variant, which seems cumbersome to use, I have implemented the test function using the MacPorts test phase unconditionally but only for the subports. I've verified that unit tests currently only exist for these subports and not for the base poppler. For example to run the unit tests for poppler-qt4-mac you can now use the command

sudo port -d test poppler-qt4-mac

Without the -d option, the test runs silently unless a test error is detected.

I have marked the two subports as conflicting since qt4-mac and qt5-mac still, unfortunately, conflict. I thought I saw a statement by michaelld that this situation had been fixed but apparently not. This is one of the reasons that I have not added this subport before now.

Additionally, the qt5 subport build continues to fail as I've seen it in the past. This appears to be due to the difference in the way qt4 and qt5 install the include file hierarchy. The poppler developers rely on the qt4 scheme for both qt4 and qt5.

libtool: compile:  /usr/bin/clang++ -DHAVE_CONFIG_H -I. -I../.. -I../../poppler -I../.. -I../../poppler -I/opt/local/Library/Frameworks/Qt5Xml/Headers -I/opt/local/Library/Frameworks/Qt5Widgets/Headers -I/opt/local/Library/Frameworks/Qt5Gui/Headers -I/opt/local/Library/Frameworks/Qt5Core/Headers -I/opt/local/include -I/opt/local/include -Dpoppler_qt5_EXPORTS -fPIC -fPIC -Wall -Woverloaded-virtual -Wnon-virtual-dtor -Wcast-align -fno-exceptions -fno-common -pipe -Os -arch x86_64 -stdlib=libc++ -MT libpoppler_qt5_la-poppler-fontinfo.lo -MD -MP -MF .deps/libpoppler_qt5_la-poppler-fontinfo.Tpo -c poppler-fontinfo.cc  -fno-common -DPIC -o .libs/libpoppler_qt5_la-poppler-fontinfo.o
In file included from poppler-fontinfo.cc:23:
In file included from ./poppler-qt5.h:37:
./poppler-annotation.h:31:In file included from poppler-document.ccIn file included from In file included from :poppler-embeddedfile.cc:21poppler-page.cc27:
In file included from :./poppler-qt5.h1034:
:
In file included from In file included from : fatal error: ./poppler-qt5.h'QtCore/QDateTime' file not found:
37:
./poppler-qt5.h:37:
./poppler-annotation.h:31:10:./poppler-annotation.h :fatal error31: :'QtCore/QDateTime' file not found10
: fatal error: 'QtCore/QDateTime' file not found
#include <QtCore/QDateTime>
         ^
#include <QtCore/QDateTime>
         ^
#include <QtCore/QDateTime>
         ^
:37:
./poppler-annotation.h:31:10: fatal error: 'QtCore/QDateTime' file not found
#include <QtCore/QDateTime>
         ^
1 error generated.
1 error generated.
make[4]: *** [libpoppler_qt5_la-poppler-fontinfo.lo] Error 1

Log file attached.

Changed 9 years ago by dbevans (David B. Evans)

Attachment: poppler-qt5-mac-main.log added

Build log for poppler-qt5-mac showing failure

Changed 9 years ago by dbevans (David B. Evans)

Attachment: poppler-0.32-qt5-mac.diff added

Proposed patch as revised by devans

comment:19 Changed 9 years ago by RJVB (René Bertin)

Agreed, the co-installable qt4 and qt5 ports have not yet been committed. I've been working on the universal variant of the Qt5 port (qt5-mac-devel) as we speak, which has kept me from addressing the poppler qt5 subport.

I can also confirm (from memory) that port:qt5-mac as it currently exists has a certain number of issues that I took care of when making it co-installable, before starting to work on port:qt5-mac-devel . In particular I set up the headers in same way for both Qt4 and Qt5, which should address the failure you've been seeing.

I'll see if I can get back to this over the next few days.

%> port installed "*poppler*"
The following ports are currently installed:
  poppler @0.29.0_0+universal
  poppler @0.31.0_0+universal (active)
  poppler-data @0.4.6_0
  poppler-data @0.4.7_0 (active)
  poppler-qt4-mac @0.29.0_0
  poppler-qt4-mac @0.31.0_0 (active)
  poppler-qt5-mac @0.31.0_0
  poppler-qt5-mac @0.31.0_0+test (active)

Changed 9 years ago by RJVB (René Bertin)

Attachment: poppler-qt5.diff added

comment:20 Changed 9 years ago by RJVB (René Bertin)

This should bring back my Qt5 subport.

Rick, could you indicated where the Qt5 headers are installed that aren't found? Maybe we can introduce some glue so that things build also with the current port:qt5-mac?

Changed 9 years ago by dbevans (David B. Evans)

Attachment: poppler-0.32-qt5-mac.2.diff added

Proposed patch as revised by devans after commit of poppler 0.32.0 without poppler-qt5-mac

comment:21 Changed 9 years ago by dbevans (David B. Evans)

Due to the ongoing Qt5 issues, I've gone ahead and updated poppler to version 0.32.0 without poppler-qt5-mac in r133751, r133752.

However, I did include the unit tests for poppler-qt4-mac and moved the Qt version-independent bits to a common section to simplify maintenance by reducing duplication when poppler-qt5-mac is ready for prime time.

This should give some time to work out the build issues with Qt5 and to make Qt4 and Qt5 (and therefore poppler-qt4-mac and poppler-qt5-mac) parallel installable without conflict.

An updated patch relative to the committed poppler 0.32.0 is attached.

comment:22 Changed 9 years ago by mkae (Marko Käning)

Cc: mk@… added

Cc Me!

comment:23 in reply to:  18 Changed 9 years ago by mkae (Marko Käning)

Replying to devans@…:

./poppler-annotation.h:31:10: fatal error: 'QtCore/QDateTime' file not found #include <QtCore/QDateTime>

1 error generated.

I've seen similar stuff on the Mountain Lion and Snow Leopard buildbots for my latest charm-qt5 commit.

comment:24 Changed 9 years ago by RJVB (René Bertin)

Those are due to issues with the way the current port:qt5-mac sets up the Qt header directories and/or inappropriate post-processing of the Qt5 cmake files.

I'll attach a refreshed Portfile for poppler with a poppler-mac-qt5 subport that has been tested on OS X 10.6 (Qt 5.3.2) and 10.9 (Qt 5.4.1) with the qt5-mac-devel port provided by https://trac.macports.org/attachment/ticket/46536/port%3Dqt5-mac-devel.tar.bz2 .

comment:25 Changed 9 years ago by dbevans (David B. Evans)

Attached is my current proposed upgrade to the current poppler 0.33 port. This assumes that qt4-mac and qt5-mac can now be installed in parallel (#48206).

Changed 9 years ago by dbevans (David B. Evans)

Attachment: poppler-0.33-qt5-mac.diff added

Revised patch to add poppler-qt5-mac to poppler 0.33

comment:26 Changed 8 years ago by mojca (Mojca Miklavec)

Cc: mojca@… added

Cc Me!

comment:27 Changed 8 years ago by mojca (Mojca Miklavec)

David, may I please ask you to commit this now that the Qt4/5 packaging issues have been resolved?

I wanted to build TeXworks (#16055, https://github.com/TeXworks/texworks) in order to be able to check some translation strings, but their CMake-based build system is failing to find poppler-qt4-mac for some reason and I'm not eager to debug yet another software right now.

Your patch poppler-0.33-qt5-mac.diff​ works for me and poppler-qt5-mac installs without any issues.

comment:28 Changed 8 years ago by RJVB (René Bertin)

Is poppler 0.41 ABI compatible with 0.40? If so, let me check and refresh my version of the port and patch to minimise the chance an upcoming commit will miss whatever changes I may have made in the last year (e.g. for KF5).

Changed 8 years ago by RJVB (René Bertin)

Attachment: poppler.diff added

comment:29 Changed 8 years ago by RJVB (René Bertin)

I'd actually forgotten that I've added an experimental variant to my own version of this port that preserves existing runtime shared libraries (libpoppler.53.dylib in my case) so that I don't have to rebuild a couple of *huge* dependents each time the ABI is broken. I'm keeping that bit to myself of course :)

So, I've refreshed my patch. The actual part for the Qt5 subport hasn't changed. I put the suppression of the main libs in a loop, in the Qt subport post-destroot as that makes upgrading just a bit more streamlined. More importantly, I've added a dependency on port:curl because the configure script will build support for libcurl-based HTTP when libcurl is detected. The feature can be turned off, but I think that curl is installed almost by definition so there is probably little benefit to disabling it.

comment:30 Changed 8 years ago by dbevans (David B. Evans)

Resolution: fixed
Status: assignedclosed

Yes, poppler 0.40 and 0.41 are ABI compatible. Also note that the curl dependency was already there so no need to add a new one.

Committed in r146564 with minor modifications. New subport is now poppler-qt5 to more closely match qt5 naming conventions. All poppler-qt5 tests pass and qt5 demo application poppler_qt5viewer works as expected.

Thanks for your updated patch.

Note: See TracTickets for help on using tickets.