[ expat-Patches-458907 ] config.h appears to be unused

Patches item #458907, was opened at 2001-09-05 17:20
Message generated for change (Comment added) made by fdrake
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=310127&aid=458907&group_id=10127

Category: Build Control
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 5
Submitted By: Martin v. Löwis (loewis)
>Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: config.h appears to be unused

Initial Comment:
To compile expat as part of another package (e.g. 
PyXML), the expat configure might not have been run. 
For that kind of application, it is necessary to wrap 
each occurrence of config.h into HAVE_CONFIG_H; the 
attached patch does that.

While trying to figure out which of the defines are 
needed, it appears that none of them are (i.e. HAVE_ 
is never used). For stand-along compilation, I found 
that only VERSION, XML_NS, XML_DTD, XML_BYTE_ORDER, 
and XML_CONTEXT_BYTES must be defined. Is that 
impression correct?


----------------------------------------------------------------------

>Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2003-10-16 00:57

Message:
Logged In: YES 
user_id=3066

Ok, I've added a HAVE_EXPAT_CONFIG_H define that gets added
by Expat's Makefile; compiling Expat without expat_config.h
is possible, though the (static) winconfig.h and macconfig.h
are still needed.

The following file revisions commit the changes:

Makefile.in  1.43
lib/xmlparse.c  1.115
lib/xmlrole.c  1.15
lib/xmltok.c  1.29
tests/chardata.c  1.6
tests/runtests.c  1.54
xmlwf/xmlfile.c  1.13


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-07-14 03:41

Message:
Logged In: YES 
user_id=21627

Your conclusion is incorrect: that you need the #defines
doesn't mean that you need expat_config.h. Instead, the
configuration variables could also be passed on the compiler
command line.

To support configurations without a expat_config.h, you must
conditionally include the config.h, like

#ifdef HAVE_CONFIG_H
#include "expat_config.h"
#endif

Then, when a configure was run, you define HAVE_CONFIG_H in
the makefile; if configure was not run, you don't define it.
I could accept a negative test, like

#ifndef DONT_HAVE_CONFIG_H
#include "expat_config.h"
#endif

so that I would pass -DDONT_HAVE_CONFIG_H to my compiler
invocations.

Of all the HAVE_ tests in expat_config, only HAVE_BCOPY and
HAVE_MEMMOVE are used in the Expat library; everything else
is unneeded. If I can find out whether memmove is defined by
other means than running configure, I do not need to run
configure (platforms that don't have memmove are of no
interest to me, so I could just always define HAVE_MEMMOVE).

The HAVE_CONFIG_H feature is *not* to avoid clashes with
multiple config.h files; it historically predates the usage
of header files. configure would generate billions of -D
options into the makefile (and still does, if you tell it
that you don't use a config.h file). I want to use a
configuration without config.h, so I kindly request the
number of defines is reduced to those that are actually used.

With autoheader, you do have full control over the set of
symbols in config.h: namely those symbols that configure.in
checks for. For example, if you modify configure.in to not
mention fcntl.h, then autoheader will drop the unused
HAVE_FCNTL_H. 

Unfortunately, libtool.m4 pulls in a number of unneeded
defines (like HAVE_DLFCN_H); others (like getpagesize) are
pulled in through the test for mmap.

So please do consider making expat more friendly for
embedding into environments where configure is not run.

----------------------------------------------------------------------

Comment By: Greg Stein (gstein)
Date: 2002-07-13 16:59

Message:
Logged In: YES 
user_id=6501

I don't see how we could operate in the "no config" case. 
We *do* require some of those configuration variables, as 
you originally pointed out. Thus, you need expat_config.h 
from somewhere.

On the Windows platform, we hand-built the file and include 
it as winconfig.h. But we *do* have a config header in that 
case.
(and my preference would be to call it expat_config.hw and 
have the .dsp copy it over to the .h file; thus, we couldn't 
need to set or test COMPILED_FROM_DSP; just need to 
get to this at some point (it is now APR, APRUTIL, and SVN 
handle config on windows))

Further, I don't see how you can avoid running configure. 
We need to do that to create a Makefile (and 
expat_config.h, of course). And the precursor to running 
configure is to run buildconf.

When APR or APRUTIL are embedded into apps (Apache 
httpd or SVN), we run their buildconf (during release 
packaging) and we run their configure (at end-user config 
time; we run it from the app's configure). In my mind, if you 
are creating a library package to be embedded, then it is 
considered an opaque blob: you interact with it according 
to its defined mechanism. If you want to shortcut the 
system, then you're assuming too much knowledge about 
the library.

(I thought xmlwf used the MMAP stuff, but it doesn't; we 
just select a different .c file at config time; however, it does 
use HAVE_UNISTD_H and it should be using a lot more, 
such as HAVE_STRING_H or HAVE_STDDEF_H and 
whatnot; xmlparse.c and brethren should be, too, for that 
matter (e.g. some platforms have string.h and others 
strings.h))

Personally, I don't understand the HAVE_CONFIG_H thing. 
I'm presuming that is mostly because of past conflicts with 
everybody naming their file config.h. It meant you couldn't 
have several config.h files in use. But even then: how could 
an app get the defines it needs? Did the embedding app 
have to define a billion things on their CC command line? 
Man... talk about needing to *really* know about the 
internals of your embedded library. Our use of 
expat_config.h solves these historical conflicts.

And regarding the elimination of unneeded symbols: those 
are automatically generated by autoheader. We don't have 
control over the full set in there. In the past, it was possible 
to use acconfig.h to have fine-grained control over the 
contents, but much of that framework is deprecated. And it 
isn't really needed, in any case, since there isn't any real 
harm in letting autoheader just build one for us, with 
whatever it thinks is needed.

Let me turn the question around: why is it a problem for the 
parent application to run buildconf at packaging time, and 
to run Expat's configure at parent-configure time? It seems 
to work just fine for me, whenever I embed a library (APR
(UTIL), Expat, Neon, and Berkeley DB) into another app.

Changing status to Pending.

----------------------------------------------------------------------

Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2002-07-12 08:09

Message:
Logged In: YES 
user_id=3066

This has come up as still being a problem for a couple of 
projects on the Python XML-SIG, so this needs to be 
reconsidered.

Hopefully Martin will be interested in checking and updating 
his proposed patch against 1.95.4.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-06-02 05:15

Message:
Logged In: YES 
user_id=21627

Can you elaborate which defines of  expat_config.h is used
in xmlwf? I could not find any.

Why is it wrong to not run configure? autoconf packages,
historically, always supported "manual" configuration, by
editing config.h manually. Also,  your own build process
does not use expat_config.h if COMPILED_FROM_DSP is set. In
general, it is not possible to run configure if you don't
have /bin/sh on a system; this is a scenario that PyXML
needs to support.

Also, traditionally, autoconf supports both configuration
with and without a config.h; files including config.h would
always wrap this with a

#ifdef HAVE_CONFIG_H

(should be HAVE_EXPAT_CONFIG_H in your case).

All I'm asking is that the combination "manual
configuration" and "no config.h" is supported. If this is
not available, I have to fake-generate a config.h, which is
ugly.

It would also help if config.h was reduced to the set of
defines which are actually used, instead of autoconfiscating
every line of the source.

----------------------------------------------------------------------

Comment By: Greg Stein (gstein)
Date: 2002-06-01 19:44

Message:
Logged In: YES 
user_id=6501

We may not use most of those (autoconf creates them as part
of searching for particular functions and stuff), but xmlwf
does use some.

That said: if you're going to embed Expat into another
application, then you'll need to adjust things accordingly.
Note that we've switched to "expat_config.h", so it might be
possible to just include that into your own application
since a conflict on config.h won't occur any more.

Also, note that the premise of "not running configure" is
probably quite wrong. Expat is embedded into the ASF's
apr-util project, and we *do* run configure for that, and
build expat as a sub-library. (heck, we even run expat's
buildconf.sh at the appropriate time)

Given the change to expat_config.h, and how I believe
embedding should work, I'm going to close this patch as
rejected. However, even with that said, I'm quite happy to
be corrected and/or to make other changes to simplify
Expat's ability to be embedded.

----------------------------------------------------------------------

Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2002-05-31 00:10

Message:
Logged In: YES 
user_id=3066

Greg, can you please respond to this?

----------------------------------------------------------------------

Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-11-08 23:48

Message:
Logged In: YES 
user_id=3066

It certainly looks like it can be reduced and possibly
removed; I'll read up on a few of the autoconf things before
removing it completely.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=310127&aid=458907&group_id=10127