Patrice wrote:
> I think that the first one is not such an issue, as it is set quite
> early, the libunistring module is called afterwards.
I agree with this.
Bruno wrote:
> I first did it like this with libintl, ca. 25 years ago, and this approach
> to modify CPPFLAGS works fine, except there were problems with config.cache
> and early (old) versions of autoconf IIRC.
No chance of changing it then. We should be able to work with this.
> I removed the other perl flags because that were previously set
> at the point in the configure.ac file because I did not understand why
> they would be relevant here for the gnulib macros invokations, but I may
> have missed something.
One change at a time please. Any gnulib tests should be run
with the same compiler flags as used to compile the shared library.
As you can see in commit 1072afdfbdd8 (on 2021-01-01), the code was changed
following reports of incorrect tests being made due to flags not being
used consistently. This code has been tested over a long period and is
the way it is for a reason, so before changing it we should try to
understand the historical reasons for its current state. (You can see
what code introduced which line with "git blame".) It's tricky
to get right and we could easily get it wrong.
> I removed the setting of CFLAGS, CPPFLAGS and LDFLAGS late in
> configure.ac.
This is the crucial change. However, there is an issue, which may be
the reason that the code was here in the first place. PERL_EXT_CFLAGS
(and other variables) have the same function as CFLAGS in a straightforward
automake build system. They allow the user to override the compiler flags,
both when configure is run, and when "make" is run by passing the variable
on the command line.
If we append the compiler flags from the Perl configuration (e.g. the
output of "perl -V:ccflags") to PERL_EXT_CFLAGS and then use this as
CFLAGS, then we are potentially overriding the user's choice.
According to the autoconf manual:
Sometimes package developers are tempted to set user variables such
as ‘CFLAGS’ because it appears to make their job easier. However,
the package itself should never set a user variable, particularly
not to include switches that are required for proper compilation of
the package. Since these variables are documented as being for the
package builder, that person rightfully expects to be able to
override any of these variables at build time. If the package
developer needs to add switches without interfering with the user,
the proper way to do that is to introduce an additional variable.
Automake makes this easy by introducing ‘AM_CFLAGS’ (*note
(automake)Flag Variables Ordering::), but the concept is the same
even if Automake is not used.
That's consistent with propagating PERL_EXT_CFLAGS to CFLAGS without
adding anything at the end.
(Admittedly, with the current code, it is not hugely clear that it is in
fact the case that "any gnulib tests should be run with the same compiler
flags as used to compile the shared library" (as I said above), due to
the resetting of PERL_EXT_CFLAGS (and other variables). In configure.ac,
we append PERL_CONF_ccflags to CFLAGS, then run the configure tests,
but then reset CFLAGS not to include it. However, in Makefile.am,
we then append PERL_CONF_ccflags to AM_CFLAGS. Maybe we could encapsulate
this process with an extra autoconf output variable for flags to be
appended to AM_CFLAGS, which would ensure consistency between configure.ac
and Makefile.am.)
Now for the core of the isssue, which is the propagation of
PERL_EXT_CPPFLAGS to CPPFLAGS. If the proscription relating to CFLAGS in
the autoconf manual should also be taken as applying to CPPFLAGS, then
it's hard to see how gnulib's use of CPPFLAGS can be correct. If a user
specifies CPPFLAGS when they run "make", they will break compilation
of the program or library.
Perhaps we should keep on resetting CFLAGS and LDFLAGS (in line with
the autoconf manual), but leave CPPFLAGS alone due to gnulib's special
use of it.
Thank you for your patience with me trying to understand this issue.
Other points:
In Patrice's patch:
> -# Do not include Perl configuration values when outputting these variables
> -CFLAGS=$PERL_EXT_CFLAGS
> -CPPFLAGS=$PERL_EXT_CPPFLAGS
> -LDFLAGS=$PERL_EXT_LDFLAGS
> +# for per code, should use something along
> +#CFLAGS="$CFLAGS $PERL_CONF_ccflags $PERL_CONF_cccdlflags"
> +#LDFLAGS="$LDFLAGS $PERL_CONF_ccdlflags"
I don't know what "for per code" means.
Also:
Patrice wrote:
> defining those when $disable_xs = no seems actually confusing.
>
> CFLAGS=$PERL_EXT_CFLAGS
> CPPFLAGS=$PERL_EXT_CPPFLAGS
> LDFLAGS=$PERL_EXT_LDFLAGS
It shouldn't matter whether the XS modules are disabled or not, as
if they are not being built then no C code is compiled and the contents
of CFLAGS etc doesn't matter. This seems to be complicating the matter.
> I made another unrelated change, I added $(PERL_CONF_cccdlflags) to
> AM_CFLAGS in Makefile.am, as it seems to me that it would be missing
> there, if I understood the documentation correctly. My feeling is that
> it worked previously without that kind of flags because it is also added
> by libtool, but it seems to me to be cleaner to have it there.
Can we please stick to one change at a time; the original reported problem
is hard enough to understand and I'm still not sure how it can be fixed
in a satisfactory way, without adding extra complications.
