Created attachment 8476 [details] [review]
forementioned patch

How do you use these -uninstalled.pc files? Are there matching patches for evince?

When we build our packages, we build several modules and package them into a single package.  We build libspectre, poppler, poppler-data and evince.  We build them all without installing them and then package them.  Since evince depends on poppler, this only works if poppler has an uninstalled-pc file and when building evince we set PKG_CONFIG_PATH to the directory which contains any needed uninstalled-pc files it depends upon (such as the poppler one).

This is how pkg-config is supposed to work and a standard interface.  You can refer to the pkg-config man page for more details of how uninstalled.pc files and PKG_CONFIG_PATH is used by pkg-config.

No change in evince is needed.  If evince provides pc files, then it could also provide uninstalled-pc files to support people building evince and not installing it, and then building stuff that depends on evince.  However, whether or not evince supports uninstalled-pc files is really a separate issue.

It would be nice if poppler supported this feature, so can this trivial patch go upstraem.  Most GNOME libraries support this feature since building stuff without installing is not that uncommon.

poppler is not a GNOME library.

I really don't think we want to support that feature since it brings even more burden to maintaining the file system, not forgetting the patch misses integration with the cmake build system and misses support for the qt, qt4 and cpp frontends.

I was just using GNOME as an example.  The uninstalled-pc feature is a standard pkg-config interface and is used by all sorts of modules which use pkg-config, not just GNOME.

poppler-cairo.pc
poppler-cpp.pc
poppler-glib.pc
poppler-qt.pc
poppler-qt4.pc
poppler-splash.pc
poppler.pc

So, you are right, we probably should provide uninstalled.pc files for all of them if poppler is going to support this feature.  I would have no problem providing a patch with a full set of uninstalled pc files if that would be accepted upstream.

I don't think it is really that much of a burden.  It just adds a few extra files and a few lines to configure.ac to build them from the .in files.  There really isn't a need for you to worry about maintaining them.  If people use them, and there are any issues with using them, people will provide patches to fix those problems.

You forget also the poppler*.pc.cmake files

If you provide all of them we can see how to merge them in, but just for 0.16.x, not in the stable branch since i'd qualify that as a new feature

I don't see it as a feature, and it shouldn't affect to any poppler user.

Uninstalled pc files are not really a "feature" since it has no real affect on the built program.  It is really a build-time feature, making it possible for other modules to build with poppler even if poppler is not yet installed on the system.

After looking at the poppler*pc.cmake files, I do not think there is any additional work to "integrate" the uninstalled-pc feature with these cmake pc files.  To me, it looks like these poppler*pc.cmake are used to build the pkg-config files that get installed on the system when cmake is used.  For example, they seem to make sure that CMAKE_INSTALL_PREFIX gets set properly for prefix.

However, uninstalled-pc files are never installed to the system (they only live in the source directory), and never need to be aware of variables that relate to where the module will be installed.  Instead uninstalled-pc files just refer to the relative path of any .la files in the build tree and the location of header files in the build tree.  This way, if you build a module like evince using poppler*uninstalled.pc files, it knows how to set Cflags and Ldflags properly to build against the built code in the uninstalled module.  So, an uninstalled.pc file should work properly with any build system that calls pkg-config to set Cflags, Ldflags, etc.

The only variable that uninstalled.pc files typically need to have set is VERSION, so it is necessary to have uninstalled.pc.in files and have the actual uninstalled.pc file get generated by configure so that the VERSION gets set properly.  But VERSION is obviously hardcoded and does not depend on whether cmake is used or not.

So, I believe it is only necessary to provide the 7 uninstalled.pc.in files, the minor changes to configure to build the uninstalled.pc files from the uninstalled.pc.in files, and add the new uninstalled.pc.in files to EXTRA_DIST in Makefile.am.  That should be all the work that is needed.

It would be fine if they could be added to a future version like 0.16 if you do not think it is appropriate for 0.14.

I don't understand why you say we only need to care about the configure files, what about the people that use cmake, we don't give that uninstalled feature to them?

Forgive me, but I am new to cmake.  After looking at the poppler code a bit more, 
I think I understand what you are asking for.

Let me see if I understand.  I believe you are telling me that in order to 
support cmake, it is necessary to be able to build the uninstalled.pc files from 
the uninstalled.pc.in files by doing the following:

- Adding a macro to cmake/modules/PopplerMacros.cmake that builds the uninstalled
  pkg-config file but doesn't install it.  Perhaps something like

  macro(POPPLER_CREATE_UNINSTALLED_PKGCONFIG generated_file)
    if(NOT WIN32)
      configure_file(${generated_file}.cmake   
  ${CMAKE_CURRENT_BINARY_DIR}/${generated_file} @ONLY)
    endif(NOT WIN32)
  endmacro(POPPLER_CREATE_UNINSTALLED_PKGCONFIG)

- Then we'd need to add lines for each uninstalled pc file in the toplevel
  CMakeLists.txt file, something like:

  poppler_create_uninstalled_pkgconfig(poppler.pc lib${LIB_SUFFIX}/pkgconfig)

Is that what is needed in addition to providing the uninstalled.pc files and modifying configure.ac to build the uninstalled.pc files from the uninstalled.pc.in files for users who use configure?

Sorry for the delay, speaking with our CMake expert, he is not sure if this feature makes sense or not for the CMake buildsystem, so while he makes up his mind i think we can go with autotools only feature. Please post your patch once you have it ready.

Created attachment 36596 [details] [review]
updated patch


Here is an updated patch that adds an uninstalled.pc.in file for all the poppler pc files, updates configure.ac to build the uninstalled.pc files from the uninstalled.pc.in files, and updates Makefile.am to add the uninstalled.pc.in files to EXTRA_DIST.

Any reason for

-case $($PKG_CONFIG --version) in
+case `$PKG_CONFIG --version` in

?

It seems totally out of the scope of the patch

Created attachment 36987 [details] [review]
updated patch

Agreed.  Here is an updated patch without that unnecessary hunk.

Commited the patch, thanks.