Re: [Mingw-cross-env-list] Proposed new package: ftgl

[Volker Grabsch]

Stefan Löffler <address@hidden> schrieb:
> I'm new to this list, but I love mingw-cross-env and would like to
> contribute.

Thanks for you contribution, and sorry for the late reply.
Reviewing contributions is always somewhat time consuming,
but necessary to avoid trouble in the future.

To shorten the review time, you should try to follow our
guidelines: http://mingw-cross-env.nongnu.org/#creating-packages

>  * the patch bypasses the normal opengl checking routine. The reason for
> this is that "-lopengl32" must be specified after the object file (i.e.,
> "gcc test.c -lopengl32" instead of "gcc -lopengl32 test.c") in order to
> work and I didn't find a way to accomplish that with the standard
> autoconf macros. I'm just assuming that opengl32 and glu32 are
> automatically available in a standard location in all mingw-cross-env
> builds?

Yes, they are. Due to some faulty assumtions in the package's gl.m4
macros, those would have failed even if the libraries were named GL
and GLU.

If you find some time, it would be great if you could inform the
package author about this issue. You might also want to propose a
portabibily patch for those macros, i.e. to check for opengl32 and
glu32 additionally - instead of the current hack.

Feel free to ask on this list if you need some help for that. I'd
be glad to help, but I don't have the time to do it on my own.

>  * Because some of the m4 macros are altered by the patch, "configure"
> must be recreated. I added a call to "autoconf" in the build
> specifications. Is that correct? Or should something be added as a
> dependency in that case?

No, you don't need additional dependencies for autoconf etc.

However, the single call to autoconf is not sufficient for all
systems. Also, aclocal, libtoolize and automake need to be
called for some reasons.

Also, you should avoid long "cmd1 && cmd2 && cmd3" cascades,
as this somehow works against the features Make provides. So
instead of

    cd '$(1)' && autoconf && ... && ./configure

you should write:

    cd '$(1)' && autoconf
    ...
    cd '$(1)' && ./configure

>  * I've never used the update mechanism, so I can't tell if the
> "$(PKG)_UPDATE" definition is correct.

There was a small flaw regarding the tilde vs. minus issue
in the version number, i.e. "2.1.3~rc5" vs. "2.1.3-rc5".
Apart from that, your update macro was fine.

I separated my cleanups into multiple patches, so you can more
easily follow my changes:

    Your original contribution:
    http://hg.savannah.gnu.org/hgweb/mingw-cross-env/rev/8ba864b547c0

    Fix for the upgrade mechanism:
    http://hg.savannah.gnu.org/hgweb/mingw-cross-env/rev/e6112bf08ca7

    Minor cleanups:
    http://hg.savannah.gnu.org/hgweb/mingw-cross-env/rev/98fe5f8ca669

    Bugfix for a flaw I accidently introduced:
    http://hg.savannah.gnu.org/hgweb/mingw-cross-env/rev/0bb4d5b1eb48

    Running more auto* tools and improved Make arguments:
    http://hg.savannah.gnu.org/hgweb/mingw-cross-env/rev/aaf34c93e17d


Greets,
Volker

-- 
Volker Grabsch
---<<(())>>---