MSYS2 compiling errors with Windows 10 and Version: 1.5.3-4-g7336027b9

[Daniel Catalina]

Hi Alberto,

I get some compiling issues with the commit mentioned in the title:

‌

[ 26%] Building CXX object rtengine/CMakeFiles/rtengine.dir/rawimagesource.cc.obj
M:/code/art/rtengine/procparams.cc: In member function 'int rtengine::procparams::ResizeParams::get_width() const':
M:/code/art/rtengine/procparams.cc:2373:12: error: expected primary-expression before ':' token
 2373 |     case IN: return std::round(ppi * width);
      |            ^
M:/code/art/rtengine/procparams.cc: In member function 'int rtengine::procparams::ResizeParams::get_height() const':
M:/code/art/rtengine/procparams.cc:2386:12: error: expected primary-expression before ':' token
 2386 |     case IN: return std::round(ppi * height);
      |            ^
M:/code/art/rtengine/procparams.cc: In member function 'int rtengine::procparams::ProcParams::save(rtengine::ProgressListener*, bool, rtengine::procparams::KeyFile&, const ParamsEdited*, const Glib::ustring&) const':
M:/code/art/rtengine/procparams.cc:3570:34: error: expected unqualified-id before ':' token
 3570 |             case ResizeParams::IN: u = "in"; break;
      |                                  ^
M:/code/art/rtengine/procparams.cc: In member function 'int rtengine::procparams::ProcParams::load(rtengine::ProgressListener*, bool, const rtengine::procparams::KeyFile&, const ParamsEdited*, bool, const Glib::ustring&)':
M:/code/art/rtengine/procparams.cc:4611:51: error: expected unqualified-id before ';' token
 4611 |                     resize.unit = ResizeParams::IN;
      |                                                   ^
make[2]: *** [rtengine/CMakeFiles/rtengine.dir/build.make:1032: rtengine/CMakeFiles/rtengine.dir/procparams.cc.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:187: rtengine/CMakeFiles/rtengine.dir/all] Error 2
make: *** [Makefile:150: all] Error 2

I am not sure if the problem here comes from MSYS2 or ART.

‌

Can you please have a look and comment?

[agriggio]

I think it comes from (recent versions of) exiv2. I’ll see if I can work around it

[Gaaned92]

Yes it comes from recent version of exiv2 see https://bitbucket.org/agriggio/art/issues/119/msys2-build-warnings-and-error-with-exiv2

Specifically, the exiv2 commit that breaks ART windows build seems to be v0.27.3-RC1-157-g7fefeb73

Checking out the previous commit is ok

[Daniel Catalina]

@Gaaned92 you are right, sorry I did not see that your report was also showing the errors.

@agriggio I gues we can close this issue, the problem reported is tha same as what was reported already.

[Gaaned92]

@agriggio The commit 7594450 corrected the above error, but now the build stops on following error:

In file included from D:/ART/ARTSOURCE/ART/rtengine/perspectivecorrection.cc:97:
D:/ART/ARTSOURCE/ART/rtengine/ashift_dt.c:313:3: error: declaration does not declare anything [-fpermissive]
  313 |   int near;
      |   ^~~
mingw32-make[2]: *** [rtengine/CMakeFiles/rtengine.dir/build.make:1480: rtengine/CMakeFiles/rtengine.dir/perspectivecorrection.cc.obj] Error 1

Thanks to have a look

Exiv2_0.27-maintenance_v0.27.3-19-gff0671cc

‌

Version: 1.5.3-30-g88bf9aa14
Branch: master
Commit: 88bf9aa14
Commit date: 2020-09-30
Compiler: gcc 10.2.0
Processor: Skylake
System: Windows
Bit depth: 64 bits
Gtkmm: V3.24.2
Lensfun: V0.3.95.0
Exiv2: V0.27.3
Build type: release
Build flags:  -m64 -mwin32 -msse2 -mfpmath=sse -mthreads -Wno-aggressive-loop-optimizations -Wno-parentheses -std=c++11 -march=native -Werror=unused-label -fno-math-errno -Wall -Wuninitialized -Wno-deprecated-declarations -Wno-unused-result -fopenmp -Werror=unknown-pragmas  -DNDEBUG  -O3 -ftree-vectorize
Link flags: -m64  -mthreads  -static-libgcc   -march=native  -s  -O3  -fno-use-linker-plugin
OpenMP support: ON
MMAP support: ON
Build OS: Windows
Build date: 2020-09-30T19:12:51Z

‌

[agriggio]

i tried to build with exiv2 0.27.3, and I gave up. I get too many errors that are due to some preprocessor macros shadowing variables used in the code. I think something went wrong with 0.27.3, a point release is not supposed to break the compilation in this way.

I suggest to dowgrade to 0.27.2, which works fine, or to wait for a new release and see if it fixes things…

[Gaaned92]

ok, I will come back to prvious commit that permits to build.

[Gaaned92]

I had a look at the offending commit v0.27.3-RC1-157-g7fefeb73.

It introduces a winsock2.h in the exiv2/config.h file. and suppress it from http.cpp. As I dont see the need to access the web from ART so I applied following DIFF to exiv2 last commit v0.27.3-119-g02b6990f and succeded to build ART:

‌

diff --git a/include/exiv2/config.h b/include/exiv2/config.h
index d81a2a00..0c15a270 100644
--- a/include/exiv2/config.h
+++ b/include/exiv2/config.h
@@ -93,10 +93,7 @@ typedef int pid_t;
 #endif
 //////////////////////////////////////

-#if defined(_MSC_VER) || defined(__CYGWIN__) || defined(__MINGW__)
-#define __USE_W32_SOCKETS
-#include <winsock2.h>
-#endif
+

 // https://softwareengineering.stackexchange.com/questions/291141/how-to-handle-design-changes-for-auto-ptr-deprecation-in-c11
 #if __cplusplus >= 201103L
diff --git a/src/http.cpp b/src/http.cpp
index afd79492..8c8bb715 100644
--- a/src/http.cpp
+++ b/src/http.cpp
@@ -20,6 +20,10 @@

 // included header files
 #include "config.h"
+#if defined(_MSC_VER) || defined(__CYGWIN__) || defined(__MINGW__)
+#define __USE_W32_SOCKETS
+#include <winsock2.h>
+#endif
 #include "datasets.hpp"
 #include "http.hpp"
 #include "futils.hpp"

As I don’t feel proficient in programming, perhaps you could contact @clanmills.

[Robin Mills]

Are you are saying that by including <winsock2.h> from exiv2/exiv2.hpp has broken the build of your code. Is that right?

I recall I had some issues involving the winsock on different flavours of Windows (msvc/cygwin/mingw) with/without libcurl. Promoting <winsock> solved that.

exiv2 v0.27.3 builds fine on MinGW/msys2 with GCC 7.4.0.

Can you confirm the compilation errors are in your code? Could I screen share with somebody on Zoom and together we’ll get this fixed without me having to build ART. [e-mail]

[Gaaned92]

@Robin Mills

1- All versions of 0.27.3-maintenance build on W10/MSYS2 and GCC 10.2.0-3; The only annoying thing is the huge amountt of warnings due to uni-ptr if I remember. If you are interested I can open a ticket on github exiv2.

2- What I observe: from the commit v0.27.3-RC1-157-g7fefeb73, it is impossible to build ART. If I apply the above patch to exiv2 (that partially reverts the 7fefeb73 commit) and build it, I can again build ART with 0.27.3-maintenance versions up to the last.

The commit just move four lines from config.h to http.cpp.

As I am not a dev, I regretfully cannot go further than the above observation, but if needed, I can make some tests.

Thanks to have a look.

‌

‌

[Robin Mills]

Please don’t open a ticket about the auto_ptr messages. I’ve documented the builds switches cmake .. -DCMAKE_CXX_STANDARD=11 -DCMAKE_CXX_FLAGS=-Wno-deprecated in README.md concerning this matter.

As you have a work around (demoting <winsock2.h> to http.cpp), you’re not stuck. I can’t change that in the codebase because I made the change for a solid reason: https://github.com/Exiv2/exiv2/pull/1222 to deal with C++11 which wasn’t supported by 0.27.2.

Here are three thoughts:

1 I add a flag EXIV2_BUILDING_LIBRARY and set it when the library is being built (but not when you’re using the library) and only include winsock when that is set. So you won’t “pull in” <winsock2.h>

2 You have an empty <winsock2.h> in your system header path that obscures the Microsoft version.

3 This is a collision between your code and the Microsoft <winsock2.h> and has nothing to do with Exiv2. However the collision has surfaced since I “promoted” winsock2.h into <exiv2/exiv2.hpp>

[agriggio]

thanks for the analysis. I will test whether we are in case 3 and report back with my findings

[Robin Mills]

The patch to implement (1) could be as simple as:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c8f9d088..fe323d78 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,6 +11,8 @@ project(exiv2          # use TWEAK to categorize the build
 )
 include(cmake/mainSetup.cmake  REQUIRED)

+add_compile_options(-DEXIV2_BUILDING_EXIV2)
+
 # options and their default values
 option( BUILD_SHARED_LIBS             "Build exiv2lib as a shared library"                    ON  )
 option( EXIV2_ENABLE_XMP              "Build with XMP metadata support"                       ON  )
diff --git a/include/exiv2/config.h b/include/exiv2/config.h
index d81a2a00..29d2ca51 100644
--- a/include/exiv2/config.h
+++ b/include/exiv2/config.h
@@ -95,8 +95,10 @@ typedef int pid_t;

 #if defined(_MSC_VER) || defined(__CYGWIN__) || defined(__MINGW__)
 #define __USE_W32_SOCKETS
+#ifdef DEXIV2_BUILDING_EXIV2
 #include <winsock2.h>
 #endif
+#endif

 // https://softwareengineering.stackexchange.com/questions/291141/how-to-handle-design-changes-for-auto-ptr-deprecation-in-c11
 #if __cplusplus >= 201103L

‌

[agriggio]

So, I tried to manually include winsock2 before exiv2, and this causes no harm with 0.27.2, but it fixes nothing with 0.27.3… I think there is some more complex interaction going on that breaks things, but I coulnd’t find out exactly what.

However, I also spent a bit more time reorganizing the include directives, and now (as of commit 19671034201ed3825544eeb6bc57bf74e72a202d) ART can be compiled successfully with exiv2 0.27.3. So, I think we can close this.

@Robin Mills it might still be a good idea to not include winsock2 unless it’s stricly necessary, but now we have a working solution, so this is not critical

[Robin Mills]

I’m glad you’ve found a way round. The reason for release candidates is to find these unknowable interactions. The changes between RC2 to GM are tiny (documentation). I didn’t get a single email concerning RC2.

I agree about <winsock.h> Poor citizenship to expose that from <exiv2/exiv2.hpp> I’ll either submit my patch above and <winsock.h> or reinvestigate only including it in src/http.cpp.

Anyway, good team-work here with a quick and happy result.

‌

[Gaaned92]

Building ART_master_1.5.3-32-g196710342 with last version of exiv2 0.27.3-maintenance is ok.

Thanks @agriggio and @Robin Mills

‌

[Robin Mills]

I’ve submitted an issue about this: https://github.com/Exiv2/exiv2/issues/1335
And a PR: https://github.com/Exiv2/exiv2/pull/1336

[agriggio]

great, thanks! I guess we can close this then

[Luis Díaz Más]

Hi!

I am also a maintainer of Exiv2 and I am trying to fix this issue in a different way than Robin Mills did. Would you mind to take a look to the following open PR to check if those changes would be fine for your case (I hope so ):

https://github.com/Exiv2/exiv2/pull/1349

Cheers,

Luis

[agriggio]

Hi,

I have since added a workaround for this, so testing the PR might require some effort on my side (also because I don’t regularly use windows). I see that your changes have been approved already, so I’m wondering whether my test is really needed. If so, I’ll try to do my best, but please let me know.

Alberto

[Luis Díaz Más]

I think it should be safe to merge the PR, I just wanted to let you know about the change in case you would have some time to check (just by inspecting the code or trying it out).

Anyways, in case it raises any problem I would be happy to work with you again to find the best approach.

[agriggio]

Ok, I’ll try to test when I have the chance then. Thanks!