[Pixman] [PATCH] build: Ensure config.h does not undefine `sqrtf`

[Siarhei Siamashka]

On Thu, 6 Oct 2016 11:32:00 +0100
Peter TB Brett <peter at peter-b.co.uk> wrote:

> On 06/10/2016 05:25, Siarhei Siamashka wrote:
> > On Mon, 26 Sep 2016 10:46:50 +0100
> > Peter TB Brett <peter at peter-b.co.uk> wrote:
> >  
> >> When compiling for Windows using MSVC, `sqrtf` is a macro.
> >> Previously, `configure` was detecting `sqrtf`'s availability, but then
> >> undefining it in `config.h`, resulting in link failures.
> >>
> >> This patch modifies `configure` to use `HAVE_SQRTF` as a feature macro
> >> for the `sqrtf()` function, and adds an additional stanza to
> >> `config.h` which defines a suitable alternative when `HAVE_SQRTF` is
> >> not defined.
>   >>
>   >> [snip]  
> >
> > Thanks for posting the patch here.
> >
> > As I asked in https://bugs.freedesktop.org/show_bug.cgi?id=97898
> > earlier, a bit more details would be very much welcome. Such as a
> > complete configure error report (then a relevant part of it may be
> > also included in the commit message).  
> 
> Hi Sarhei,
> 
> There's nothing to see in the output of the configure script -- it succeeds.
> 
> > There is just one thing that I don't quite understand. I'm not a
> > Windows person, but I believe that the MSVC compiler is not exactly
> > compatible with GCC command line options. And autotools generally
> > expect a GCC compatible compiler. So I'm quite curious about how
> > you managed to use autotools with the MSVC compiler for compiling
> > pixman.  
> 
> We build pixman for a large number of platform/architecture 
> combinations, and need to have as similar a behaviour as possible on 
> each.  In order to achieve this, we:
> 
> 1. Run autoconf on a suitable reference host [1] with our desired 
> configuration options.  It is impossible to run ./configure for several 
> of our targets.
> 
> 2. Make (very few) modifications to the config.h file, and check this 
> into our configuration management system. [2]
> 
> 3. Ignore pixman's generated Makefiles and use other tools to actually 
> compile.  On Windows we use Visual Studio. [3]
> 
> On a recent pixman update we did not initially make _any_ modifications 
> to the generated config.h (this is ideal!)  Unfortunately, the Windows 
> build link failed in Visual Studio because "sqrtf" was an unresolved 
> symbol.  In the Windows x86 library, "sqrtf" is a macro, not a function.

OK, basically the configure script runs on one system and then the
generated config.h is used on another system. I'm not sure if I
approve this particular approach, but the downstream maintainers
and users are surely free to experiment and use whatever works for
them. Thanks for sharing this information anyway.

And there is always more than one way to skin a cat. For example,
the others have also tried CMake:

    https://lists.freedesktop.org/archives/pixman/2016-May/004610.html

> The only reason this occurred because AC_DEFINE generates "#undef 
> <macro>" in config.h by default (in this case, on AC_SEARCH_LIBS 
> success).  In this case, the clear intent of the person who originally 
> wrote the test was that if AC_SEARCH_LIBS succeeds, "sqrtf" is left 
> _unchanged_.

Yes, this is some wacky code in the configure.ac and the function
availability detection is normally done via defining a HAVE_SQRTF
macro, just like you did.

Somebody apparently tried to be NIH-savvy with the sqrtf function
detection, and this backfired :-)

> The patch I provided makes it considerably easier for other users in our 
> position to get pixman to build correctly, by making it clearer what's 
> going on when reading "config.h" in order to make appropriate edits.

Just a side note. We still can't make any guarantees about using
the pixman build system in a non-intended way.

> Do you have any specific objections to merging it?

I'm fine with fixing this. But would prefer to get rid of the
AH_VERBATIM stuff and just directly add the

    #ifndef HAVE_SQRTF
    #define sqrtf sqrt
    #endif

chunk into the pixman-compiler.h header file. If this adjustment is
done, then the patch will be IMHO good enough to be pushed into
pixman git (unless other people have objections).

You can wait maybe a day (so that more people have a chance to
comment) and then send a v2 version of the patch. Thanks!

> 
>                                         Peter
> 
> [1] I believe the config.h we're currently using was generated on an OS 
> X 10.9 machine (but that's not really relevant).
> 
> [2] 
> https://github.com/livecode/livecode-thirdparty/commits/develop/libcairo/src/pixman-config.h
>
> 
> [3] We generate all build control files with gyp.
> 


-- 
Best regards,
Siarhei Siamashka