Bug 9139

Created attachment 7882 [details] [review]
Patch to compile under MSYS

See previous comment for info about this patch (bug 9139)

*** Bug 9137 has been marked as a duplicate of this bug. ***

I don't like much removing the AM_CONFIG_HEADER, does this work for you?

-AM_CONFIG_HEADER(config.h)
-AM_CONFIG_HEADER(poppler/poppler-config.h)
+AC_CONFIG_HEADERS([config.h:config.h.in])
+AC_CONFIG_HEADERS([poppler/poppler-config.h:poppler/poppler-config.h.in])

Okay, I have to say that there is currently a problem with the config.h file.

You define the file named poppler-config.h.in in the poppler directory. That
file has different configuration information that you say come from the
$(topdir)/config.h file. This is not correct. What you need to do is include the
config.h file. If you need to install it (which is a problem I ran into!) then
you need to use the AC_CONFIG_HEADERS () and rename (and possibly move!) that
header file.

The poppler-config.h.in, as it stands, is NOT an AC_CONFIG_HEADERS(). Actually,
at some point, I deleted it thinking that it was. It is not since it has
additional information that the autoreconf tool suite cannot regenerate. So it
is a header you created. Clearly!

What I'd suggest is something like that:

poppler/poppler-config.h
+#include <config.h>
-<all these #ifndef <blah> which are in config.h>

configure.ac
-AC_CONFIG_HEADERS([config.h:config.h.in])
+AC_CONFIG_HEADERS([poppler/config.h:poppler/config.h.in])
-AC_CONFIG_HEADERS([poppler/poppler-config.h:poppler/poppler-config.h.in])

You may want to talk about this to other autoconf savvy people...

If you need to install the poppler-config.h, this is not going to be the best
way because the the name 'config.h' is way to common. So in that case, you may
want to rename both files (possibly the config.h as poppler-config.h and maybe
move the poppler-config.h into the poppler.h header?)

If you read this about AC_CONFIG_HEADERS()

http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_node/autoconf_27.html

they show an example with two lines, but in the text they clearly talk about the
config file in singular: i.e. just one file. Though you can auto-generate
duplicates, people generally don't do that.

Also, if you never include config.h anywhere, then you clearly don't make use of
these #define! 8-) [it has been only a little over 1 year I really started to
learn about the configure tools... so I do not know everything, but this just
makes sense]

Again, before you turn down this specific entry, try to talk with some other
people about this. Look at other projects to see whether they have similar
setups. I'd be surprised if any would offer a config.h.in which wasn't generated
by autoreconf.

Thank you for looking into those!
Alexis

<tsdgeos> also have a look at https://bugs.freedesktop.org/show_bug.cgi?id=9139
please
<krh> there's is nothing wrong with generating multiple config files
<krh> and the poppler-config.h.in is indeed hand crafted to only expose a subset
of the config options
<krh> so that it is installable
<tsdgeos> that's what i thought
<tsdgeos> can you please answer him?
<krh> the problem when I first did this was that all public headerfiles would
include config.h and there was no way we could just install config.h
<krh> yeah, I'll just paste this
* genstef (n=stefan@gentoo/developer/genstef) has joined #poppler
<genstef> hi!
<tsdgeos> hi
<krh> also, config.h exposes a lot of symbols that we don't want as public API,
thus, the hand-crafted poppler-config.h.in

Is this still needed?

Frankly, I have no clue. I have not been using anything like Poppler in a long while and now we've switched to Qt... plus, I mainly do work under Linux where everything works just fine. 8-)

Thank you for checking.
Alexis

Closing as wontfix then until someone actually cares enough to post a new patch (if needed)