<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:Wingdings;
        panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
        {mso-style-priority:34;
        margin-top:0cm;
        margin-right:0cm;
        margin-bottom:0cm;
        margin-left:36.0pt;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri",sans-serif;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
/* List Definitions */
@list l0
        {mso-list-id:1272858853;
        mso-list-type:hybrid;
        mso-list-template-ids:-1965781944 -1 67567619 67567621 67567617 67567619 67567621 67567617 67567619 67567621;}
@list l0:level1
        {mso-level-start-at:4;
        mso-level-number-format:bullet;
        mso-level-text:\F0B7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Symbol;
        mso-fareast-font-family:"Times New Roman";
        mso-bidi-font-family:"Times New Roman";}
@list l0:level2
        {mso-level-number-format:bullet;
        mso-level-text:o;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:"Courier New";}
@list l0:level3
        {mso-level-number-format:bullet;
        mso-level-text:\F0A7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Wingdings;}
@list l0:level4
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Symbol;}
@list l0:level5
        {mso-level-number-format:bullet;
        mso-level-text:o;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:"Courier New";}
@list l0:level6
        {mso-level-number-format:bullet;
        mso-level-text:\F0A7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Wingdings;}
@list l0:level7
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Symbol;}
@list l0:level8
        {mso-level-number-format:bullet;
        mso-level-text:o;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:"Courier New";}
@list l0:level9
        {mso-level-number-format:bullet;
        mso-level-text:\F0A7;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        font-family:Wingdings;}
ol
        {margin-bottom:0cm;}
ul
        {margin-bottom:0cm;}
--></style>
</head>
<body lang="EN-GB" link="blue" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span lang="DE" style="font-size:10.0pt;font-family:"Courier New"">Hi Alexey and Roumen,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="DE" style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="DE" style="font-size:10.0pt;font-family:"Courier New"">thanks for the feedback, let me Reply on the questions/issues.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">In order to be able to see exactly what is happening during the build, please see two separate CI process log:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">The first build log is the git master, without the patch:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.64">https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.64</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">The second one is actually the build process using the proposed patch:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62">https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">You can see not just the 32 and 64 bit build process, but also the results of the test themselves, which shows what works properly  (all test are OK) and what is not working properly,
 starting approx. here:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62#L624">https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62#L624</a>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="DE" style="font-size:10.0pt;font-family:"Courier New"">></span><span style="font-size:10.0pt;font-family:"Courier New"">a) src/gcrypt/app.c<br>
>Not mingw* specific.<br>
>What is wrong with long cast for printf %ld flag?<br>
<br>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">Indeed this has nothing to do with mingw. However gcc 6.3.0 gives the following warning with the original code:<o:p></o:p></span></p>
<div>
<div id="L533">
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]</span><span style="font-size:10.0pt;font-family:"Courier New""><o:p></o:p></span></p>
</div>
<div id="L534">
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">"min_version=%ld", (long)GCRYPT_MIN_VERSION);</span><span style="font-size:10.0pt;font-family:"Courier New""><o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">See here:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.64#L532">https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.64#L532</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">>c)<br>
>- # To avoid problem with loading of a shared library (dlopen or<br>
>- # equivalent) at run time on some platforms we need to link<br>
>- # everything statically (it works without hack on 9x and under<br>
>- # emulation; on nt 5.x (w2k,xp) the error is 998: "Invalid<br>
>- # access to memory location").<br>
><br>
>I guess that above comment describes issue clearly.<br>
>If on OS version >= Win-7 issue does not exist then fine.  May be issue <br>
>was specific to 32-bit MS OS and does not exist for 64 bit .<br>
>Unfortunately today I cannot test xmlsec in native environment.<br>
><br>
>If I remember well Microsoft loader fail to load some shared <br>
>libraries(dll) dynamically due to limited functionality (failure in <br>
>initialization of some global variables(?)).<br>
<br>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">The shared library loading issue does not exists on Win10 (Windows 7 I have not tested), shared libraries are just working OK both on 32 and 64 bit.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">See test results:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62#L624">https://ci.appveyor.com/project/peterbud/mingw-packages/build/1.0.62#L624</a>
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">The only problem I have found is that dynamic loading of libraries are done with LoadLibrary and without the full path. That means that tests which are relying on LD_LIBRARY_PATH
 alone are not working unfortunately without exporting also the PATH (similar to LD_LIBRARY_PATH): after that LoadLibrary also find the dynamic libraries in the tests scripts. After make install shared libraries are also working properly, and the system finds
 them even without full path.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">So I agree that we need a more granular OS check here before forcing static libraries<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">>d) shared vs static build and definition of XMLSEC_STATIC<br>
>Define of  -DXMLSEC_STATIC=1 in configure is just plain wrong.<br>
><br>
>First I would like to point that xmlsec uses libtool for builds in unix <br>
>like environment<br>
>The build is controlled by libtool flags exported to configure<br>
>      --enable-shared[=PKGS]  build shared libraries [default=yes]<br>
>      --enable-static[=PKGS]  build static libraries [default=yes<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">I understand that so far on Windows static libraries were forced, but I believe it is possible to build both static and shared libraries as well. What I have found is that if you
 attempt to build xmlsec on windows with static libraries using MSYS2/MINGW, build fails as xmlsec1.exe as XMLSEC_STATIC has _<i>not</i>_ been defined anywhere. The only place where XMLSEC_STATIC is defined is the msvc makefile, which is obviously not used
 by gcc:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><a href="https://github.com/lsh123/xmlsec/blob/master/win32/Makefile.msvc#L615">https://github.com/lsh123/xmlsec/blob/master/win32/Makefile.msvc#L615</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">I’d suggest to define XMLSEC_STATIC in configure / configure.ac if static build is chosen, that would universally work, not just in case MSVC is used to build library. That means
 there is no need to define separately XMLSEC_STATIC in msvc makefile<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">Some final thoughts:<o:p></o:p></span></p>
<ul style="margin-top:0cm" type="disc">
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1"><span style="font-size:10.0pt;font-family:"Courier New"">In the current build I had to disable the gcrypt library, as the MINGW version of libgcrypt seems to have problems (which obviously
 results a few xmlsec test to fail). See issue reported here: <a href="https://github.com/Alexpux/MINGW-packages/issues/2460">
https://github.com/Alexpux/MINGW-packages/issues/2460</a> . As soon as that will be fixed, I’ll enable libgrcrypt and look into all gcrypt related test cases as well.<o:p></o:p></span></li><li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1"><span style="font-size:10.0pt;font-family:"Courier New"">The modifications in export.h (although have not been reviewed) I believe are also needed, would be also great to see if you
 are OK with that?<o:p></o:p></span></li><li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1"><span style="font-size:10.0pt;font-family:"Courier New"">For b) tests/testrun.sh related comments I’ll come back soon, just I need more time.<o:p></o:p></span></li></ul>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">Thank you once again for the thorough review, best regards,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New"">Peter<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt">----------------------------------------------------------------------<br>
<br>
Date: Tue, 16 May 2017 23:55:52 +0300<br>
From: Roumen Petrov <xmlsec@roumenpetrov.info><br>
To: "xmlsec@aleksey.com" <xmlsec@aleksey.com><br>
Subject: Re: [xmlsec] MinGW build pull request<br>
Message-ID: <591B6758.8010902@roumenpetrov.info><br>
Content-Type: text/plain; charset="utf-8"; Format="flowed"<br>
<br>
Hi Aleksey,<br>
<br>
Aleksey Sanin wrote:<br>
> Roumen (and anyone else who has experience with MinGW environment),<br>
><br>
> There is a pull request to make changes to MinGW build. Unfortunately,<br>
> I have no experience on this platform and would love to get your<br>
> feedback on whether this is the correct change or not:<br>
><br>
> <a href="https://github.com/lsh123/xmlsec/pull/115/files">https://github.com/lsh123/xmlsec/pull/115/files</a><br>
<br>
My recommendation is to reject the request for the reasons described below:<br>
<br>
<br>
a) src/gcrypt/app.c<br>
Not mingw* specific.<br>
What is wrong with long cast for printf %ld flag?<br>
<br>
<br>
b) tests/testrun.sh<br>
Wrong . Not mingw* specific.<br>
    mingw* produce native binaries that use OS functionality .<br>
     priv_key_suffix is related to OS (mis)functionality. Definitely is <br>
not related to mingw GNU  compiler. It is not compile time flag. It is <br>
runtime flag!<br>
<br>
If I remember well it is hack for MS crypto store for certain version of <br>
operation system.<br>
Perhaps logic could be improved to be forward compatible. I mean <br>
something like this "if OS version >= Win-XP then use methodA else methodB".<br>
<br>
For example runtime in above case mean build is on win2k and using the <br>
same build tree run tests on Win-XP, Win7 ... .<br>
<br>
<br>
c)<br>
- # To avoid problem with loading of a shared library (dlopen or<br>
- # equivalent) at run time on some platforms we need to link<br>
- # everything statically (it works without hack on 9x and under<br>
- # emulation; on nt 5.x (w2k,xp) the error is 998: "Invalid<br>
- # access to memory location").<br>
<br>
I guess that above comment describes issue clearly.<br>
If on OS version >= Win-7 issue does not exist then fine.  May be issue <br>
was specific to 32-bit MS OS and does not exist for 64 bit .<br>
Unfortunately today I cannot test xmlsec in native environment.<br>
<br>
If I remember well Microsoft loader fail to load some shared <br>
libraries(dll) dynamically due to limited functionality (failure in <br>
initialization of some global variables(?)).<br>
<br>
So removing  enable_static_linking="yes" and enable_crypto_dl="no" needs <br>
addition clarification (testing on native platform) and today I cannot help.<br>
=> Must be separate request.<br>
<br>
<br>
d) shared vs static build and definition of XMLSEC_STATIC<br>
Define of  -DXMLSEC_STATIC=1 in configure is just plain wrong.<br>
<br>
First I would like to point that xmlsec uses libtool for builds in unix <br>
like environment<br>
The build is controlled by libtool flags exported to configure<br>
      --enable-shared[=PKGS]  build shared libraries [default=yes]<br>
      --enable-static[=PKGS]  build static libraries [default=yes]<br>
<br>
During make phase libtool shows one command but actually performs two <br>
compilation - one for static and one for shared.<br>
<br>
<br>
Logic should be in common header something like following<br>
#ifdef PIE<br>
# defines for shared build<br>
#else<br>
# defines for static build<br>
#endif<br>
<br>
Remark : libtool pass -DPIE if compilation is for shared library<br>
<br>
Note I have no issue with current build system and I'm not sure why is <br>
necessary to change code.<br>
May be mingw* complier is old and issue is for mingw* gcc >= 6*.<br>
<br>
<br>
e) extra<br>
Out of scope but you may want to update libtool macros in configure script<br>
Please find attached file "0004-remove-obsolete-AC_PROG_LIBTOOL.patch"<br>
<br>
AC_PROG_LIBTOOL is defined for backward compatibility in libtool 2+ and <br>
LT_INIT([win32-dll]) is enough.<br>
<br>
<br>
Regards<br>
Roumen Petrov<br>
-------------- next part --------------<br>
A non-text attachment was scrubbed...<br>
Name: 0004-remove-obsolete-AC_PROG_LIBTOOL.patch<br>
Type: text/x-diff<br>
Size: 552 bytes<br>
Desc: not available<br>
URL: <<a href="http://www.aleksey.com/pipermail/xmlsec/attachments/20170516/8cb4b405/attachment-0001.patch">http://www.aleksey.com/pipermail/xmlsec/attachments/20170516/8cb4b405/attachment-0001.patch</a>><br>
<br>
<br>
</span></p>
</div>
</body>
</html>