Re: [Mingw-cross-env-list] [PATCH] Add OpenSceneGraph

[Volker Grabsch]

Hi Martin,

Your work looks very nice. Just two minor issues before I'll
include it:

Martin Lambers <address@hidden> schrieb:
> I forced cmake to use the correct pkg-config binary with
> '-DPKG_CONFIG_EXECUTABLE=$(PREFIX)/bin/$(TARGET)-pkg-config'.

1) Does this still work when you simplify this line to:

    -DPKG_CONFIG_EXECUTABLE=$(TARGET)-pkg-config

   ?

2) Please don't use <TAB> characters for alignment. I'll
   usually convert them to spaces before inclusion, but
   it would be great if the patches were tab-free in the
   first place.

   (The only exception are tabs in the main Makefile where
    the make syntax requires them ... which is BTW a real
    design mistake of make, but that's another story.)


> > 2) The command
> > 
> >         i686-pc-mingw32-pkg-config --cflags
> > 
> >    should output the "-DOSG_LIBRARY_STATIC" option.
> 
> I don't agree. Pkg-config has no way to report CFLAGS that are only
> required for static linking. On a system that has both dynamic and
> static OSG, pkg-config must not report -DOSG_LIBRARY_STATIC to avoid
> breaking dynamic linking.

That's true, and that's why I'm only requesting that for
static-only builds, i.e. builds where _shared_ libraries
are _disabled_.

Note that similar mechanisms are added in other mingw-cross-env
packages, too. The idea is that you shouldn't need to tell
every small library that you want to build it statically when
that's the only option anyway.

> We cannot hide the fact that static
> OSG is used anyway, so let's not make the behaviour on mingw-cross-env
> different from the behaviour on other platforms.

That's a valid point. So I agree that we don't need it for
OSG, but let's take libraries like OSG and Qt as exceptions,
not as the norm.

For libraries where we _can_ hide the fact that they are built
statically, we should do that. There is no need to make static
linking more inconvenient than necessary.

Most libraries work statically as well as dynamically without
any extra effort, and for those libraries we should add such
"XXX_STATIC" symbols either to pkg-config or put it directly
into their main header.

(Or, these libraries shouldn't need such symbols in the first
 place, which luckily is the case for the vast majority for
 libraries.)


Greets,

    Volker

-- 
Volker Grabsch
---<<(())>>---
Administrator
NotJustHosting GbR