Re: MinGW build pull request

Hi Aleksey,

Aleksey Sanin wrote:
> Roumen (and anyone else who has experience with MinGW environment),
>
> There is a pull request to make changes to MinGW build. Unfortunately,
> I have no experience on this platform and would love to get your
> feedback on whether this is the correct change or not:
>
> https://github.com/lsh123/xmlsec/pull/115/files

My recommendation is to reject the request for the reasons described below:


a) src/gcrypt/app.c
Not mingw* specific.
What is wrong with long cast for printf %ld flag?


b) tests/testrun.sh
Wrong . Not mingw* specific.
    mingw* produce native binaries that use OS functionality .
     priv_key_suffix is related to OS (mis)functionality. Definitely is 
not related to mingw GNU  compiler. It is not compile time flag. It is 
runtime flag!

If I remember well it is hack for MS crypto store for certain version of 
operation system.
Perhaps logic could be improved to be forward compatible. I mean 
something like this "if OS version >= Win-XP then use methodA else methodB".

For example runtime in above case mean build is on win2k and using the 
same build tree run tests on Win-XP, Win7 ... .


c)
- # To avoid problem with loading of a shared library (dlopen or
- # equivalent) at run time on some platforms we need to link
- # everything statically (it works without hack on 9x and under
- # emulation; on nt 5.x (w2k,xp) the error is 998: "Invalid
- # access to memory location").

I guess that above comment describes issue clearly.
If on OS version >= Win-7 issue does not exist then fine.  May be issue 
was specific to 32-bit MS OS and does not exist for 64 bit .
Unfortunately today I cannot test xmlsec in native environment.

If I remember well Microsoft loader fail to load some shared 
libraries(dll) dynamically due to limited functionality (failure in 
initialization of some global variables(?)).

So removing  enable_static_linking="yes" and enable_crypto_dl="no" needs 
addition clarification (testing on native platform) and today I cannot help.
=> Must be separate request.


d) shared vs static build and definition of XMLSEC_STATIC
Define of  -DXMLSEC_STATIC=1 in configure is just plain wrong.

First I would like to point that xmlsec uses libtool for builds in unix 
like environment
The build is controlled by libtool flags exported to configure
      --enable-shared[=PKGS]  build shared libraries [default=yes]
      --enable-static[=PKGS]  build static libraries [default=yes]

During make phase libtool shows one command but actually performs two 
compilation - one for static and one for shared.


Logic should be in common header something like following
#ifdef PIE
# defines for shared build
#else
# defines for static build
#endif

Remark : libtool pass -DPIE if compilation is for shared library

Note I have no issue with current build system and I'm not sure why is 
necessary to change code.
May be mingw* complier is old and issue is for mingw* gcc >= 6*.


e) extra
Out of scope but you may want to update libtool macros in configure script
Please find attached file "0004-remove-obsolete-AC_PROG_LIBTOOL.patch"

AC_PROG_LIBTOOL is defined for backward compatibility in libtool 2+ and 
LT_INIT([win32-dll]) is enough.


Regards
Roumen Petrov

_______________________________________________
xmlsec mailing list
[email protected]
http://www.aleksey.com/mailman/listinfo/xmlsec

From cb937d6a441053562d876e686366102bbba394ec Mon Sep 17 00:00:00 2001
From: Roumen Petrov <[email protected]>
Date: Sun, 20 Jan 2013 02:50:51 +0200
Subject: [PATCH 4/8] remove obsolete AC_PROG_LIBTOOL

---
 configure.ac | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2a2cac4..894ae0b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,9 +36,8 @@ dnl
 AC_PROG_CC
 AC_PROG_INSTALL
 AC_HEADER_STDC
-AC_PROG_LIBTOOL
 
-LT_INIT
+LT_INIT([win32-dll])
 
 dnl
 dnl Find programs
-- 
2.9.0