Itk: Building ITK on Windows

[muschellij2]

Building ITK on Windows (through R)

I'm trying to build ITK on Windows, through R in the ITKR package (https://github.com/muschellij2/ITKR). I'm testing this through appveyor (https://ci.appveyor.com/project/muschellij2/itkr). I'm getting the following error:

[ 73%] Building C object Modules/ThirdParty/HDF5/src/itkhdf5/src/CMakeFiles/hdf5-static.dir/H5Defl.c.obj
In file included from C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5private.h:615:0,
                 from C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5Defl.c:29:
C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5Defl.c: In function 'H5D__efl_read':
C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5win32defs.h:57:66: error: expected expression before ')' token
 #define HDopen(S,F,...)       _open(S, F | _O_BINARY, __VA_ARGS__)
                                                                  ^
C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5Defl.c:291:18: note: in expansion of macro 'HDopen'
         if((fd = HDopen(full_name, O_RDONLY)) < 0)
                  ^
Modules\ThirdParty\HDF5\src\itkhdf5\src\CMakeFiles\hdf5-static.dir\build.make:770: recipe for target 'Modules/ThirdParty/HDF5/src/itkhdf5/src/CMakeFiles/hdf5-static.dir/H5Defl.c.obj' failed

This worked using commit 4f89edd8334da198099c686b993ec9f9f12dccdf, but not the latest or anything with respect to v5.0.

Steps to Reproduce

The steps can be seen here: https://ci.appveyor.com/project/muschellij2/itkr

1. The main configure script is https://github.com/muschellij2/ITKR/blob/master/configure.win
2. The make version is mingw32-make, and the Makevars necessary to reproduce are: https://github.com/muschellij2/ITKR/blob/master/src/Makevars.win
3. See the attached 00install.txt file below.

Expected behavior

Successful build such as https://ci.appveyor.com/project/muschellij2/itkr/build/1.0.278

Actual behavior

Build failure such as https://ci.appveyor.com/project/muschellij2/itkr/build/1.0.282

Reproducibility

100% of the builds with

Versions

ITK_VERSION_MAJOR = 5
ITK_VERSION_MINOR = 0
ITK_VERSION_PATCH = 0

Environment

Windows on Appveyor (I believe Windows Server 2012 R2 )

Additional Information

I've uploaded the CMakeLists and the installation output
CMakeLists.txt
00install.txt

[thewtex]

Hi @muschellij2, when I try building with the MinGW Makefiles CMake generator, I get an error that sh.exe cannot be in the PATH -- it looks like sh.exe might be in the PATH in this case?

Could you please try downloading ninja.exe, use the Ninja generator, and see what happens?

From this note, it sounds like Visual Studio is being falsely detected or other setup is not correct:

https://github.com/InsightSoftwareConsortium/ITK/blob/13bbb31cc5799f33db68bb632742c48469631ae6/Modules/ThirdParty/HDF5/src/itkhdf5/src/H5win32defs.h#L50-L57

[muschellij2]

I have been able to build before with MinGW. I need to use MinGW vs. ninja as this is the recommended and default way when building with R packages.

In https://github.com/muschellij2/ITKR/blob/master/configure.win#L58, The CMAKE_SH will remove the sh.exe error.

    -DCMAKE_SH:BOOL=OFF 

If you take sh.exe out of the PATH, then R will not be able to run anything. Also the configure.win needs git in the path, which also may have an sh.exe in PATH (but I believe it was the Rtools sh.exe if I remember correctly).

I need to check with @appveyor as it seems as though the configuration may have changed. I think the .Rproj file that's confusing appveyor: https://www.appveyor.com/docs/build-phase/. I'll try to remove and rerun.

[muschellij2]

Deleted the Rproj file in commit https://github.com/muschellij2/ITKR/commit/9ca09e1f3877b09f5d50e7e031ca54f9db42c2fd, but it still fails with the same error: https://ci.appveyor.com/project/muschellij2/itkr/build/job/1vngo4na0eryy4h0

As per https://help.appveyor.com/discussions/problems/12705-custom-build-script-script-mode, I'm going to try to add

build: off

https://github.com/muschellij2/ITKR/commit/7d0805167a6dfbafaf579ffeffa209b15bbbfdd8
to try to ensure build is off and Visual Studio isn't being detected

[muschellij2]

Still a fail with these modifications: https://ci.appveyor.com/project/muschellij2/itkr/build/1.0.289/job/vg0toj68d27fgams

[thewtex]

@muschellij2 I confirmed the build error locally with both mingw32-make and ninja.

I created a patch here:

http://review.source.kitware.com/#/c/23773/

I will submit this to upstream HDF5.

However, there were a lot of failing tests, which I think are unrelated.

So, I would recommend trying the tip of the release-4.13 branch.

[muschellij2]

Thanks for the update! Looks like it will fix the build. I will pull in when it's in the upstream branch (as I need v5 for ANTs building), but I appreciate the fix. @adigherman @stnava - this should help with Windows builds

[muschellij2]

Has this been incorporated? I don't see in the source.

[thewtex]

http://review.source.kitware.com/#/c/23773/

This caused issues with Visual Studio, so it was not incorporated.

We have been working with the HDF5 folks, who are in the process of opening up their issue tracker and source repository.

HDF5 1.10.4 came out recently -- I am working on updating our sources. Your help testing would be appreciated.

[thewtex]

@muschellij2 does this branch fix the build issue?

https://github.com/InsightSoftwareConsortium/ITK/pull/397

[dzenanz]

Is this issue still valid?

[muschellij2]

The fix in ITK currently doesn't seem to work on some Windows machines
without:
https://github.com/InsightSoftwareConsortium/ITK/compare/master...muschellij2:master?w=1

Even with the upstream HDF5 change: I tried this at
https://github.com/muschellij2/ITK/blob/ce07ea11e4e39d2dbcb07c162cb7b7eecb18f3bc/Modules/ThirdParty/HDF5/src/itkhdf5/src/H5win32defs.h#L53,
I could not get it to work as per
https://ci.appveyor.com/project/muschellij2/itkr/builds/23142316/job/3ul9xxdq3yjlmeo3,
specifically
https://ci.appveyor.com/project/muschellij2/itkr/builds/23142316/job/3ul9xxdq3yjlmeo3#L3864.
I'm not sure if the HDF5 will fix this overall, but I haven't seen that
upstream updated in ITK build, so I had this open still.

On Tue, Apr 9, 2019 at 4:43 PM Dženan Zukić [e-mail]
wrote:

Is this issue still valid?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/66#issuecomment-481427881,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABBnrmNHLfLbaHwe3uIGmPneWqHS3hAGks5vfPtpgaJpZM4W3P9C
.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[muschellij2]

Not sure if this is still relevant - but need to check and update.

[stevend12]

I can vouch that it's still relevant, downloaded the source (v5.0.1) online yesterday and had this same compile error with mingw-w64. Found this current thread though which helped, thanks!

[thewtex]

@stevend12 thanks for the report!

I contacted Dana Robinson from the HDF5 Group about this issue -- hopefully we will learn more.

[muschellij2]

I have a fix on https://github.com/muschellij2/ITK, but I'm not sure if it
will work with main ITK on all Windows build setups.

On Thu, Aug 29, 2019 at 4:46 PM Matt McCormick [e-mail]
wrote:

@stevend12 https://github.com/stevend12 thanks for the report!

I contacted Dana Robinson from the HDF5 Group about this issue --
hopefully we will learn more.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/66?email_source=notifications&email_token=AAIGPLUGEVMAGA2OSCIILBLQHAYRJA5CNFSM4FW475BKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5PYQGA#issuecomment-526354456,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIGPLUPUDBGGS523WXDJU3QHAYRJANCNFSM4FW475BA
.

[thewtex]

I have a fix on https://github.com/muschellij2/ITK, but I'm not sure if it
will work with main ITK on all Windows build setups.

@muschellij2 nice! Could you create a pull request? The CI testing will provide more information.

[muschellij2]

I had done this in the past but it was indicated that this problem is
fixed, so I'll try again. I may need to squash commits a bit. Are you
using mingw-64 or mingw32 in CI testing?

On Thu, Aug 29, 2019 at 5:04 PM Matt McCormick [e-mail]
wrote:

I have a fix on https://github.com/muschellij2/ITK, but I'm not sure if it
will work with main ITK on all Windows build setups.

@muschellij2 https://github.com/muschellij2 nice! Could you create a
pull request? The CI testing will provide more information.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/66?email_source=notifications&email_token=AAIGPLSME6OVWCWSZNNMTB3QHA2WNA5CNFSM4FW475BKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5P2DRA#issuecomment-526361028,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIGPLQ4LUGDWXPVUJIOWPDQHA2WNANCNFSM4FW475BA
.

[thewtex]

The CI testing service is using Visual Studio -- it will provide a check that the build still works with Visual Studio.

[muschellij2]

I guess this won't detect it because this happens with MinGW/MSYS2

[thewtex]

Ideally it will work with both. The Azure Pipelines Windows image include mingw64, so we could enable a build after it is working.

[muschellij2]

Finding Eigen3 issues now:
https://ci.appveyor.com/project/muschellij2/itkr/builds/30971743/job/kb7bu5w9u6549b5d#L760

[dzenanz]

CC: @phcerdan

[phcerdan]

Seems like the error is originated here:

CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage

Not sure, but I guess CMAKE_C/CXX_COMPILER are not populated by default in this setup with mingw?

We can explicitly pass those CMAKE_vars to the internal eigen configuration. Patch incoming.

In this sense, I would love to know what is the minimum required set of CMAKE_ variables related to compilers and toolchains that we need to pass to the Eigen configuration. Even though we are not compiling anything and those flags are not present in the Eigen3Config.cmake, CMake will choke if they are not properly set at configure time.

As I said, patch incoming, would be great to test it in this configuration @muschellij2

[muschellij2]

Looking further in https://ci.appveyor.com/project/muschellij2/itkr/builds/31248553/job/4y49buwbe4bc7twi#L614, it seems it's a CMAKE_SH issue, which will be fixed on CMake 3.17: https://github.com/Kitware/CMake/commit/82ddcf0db1d220564145122c3cce25d25ee0e254

[muschellij2]

So @thewtex @phcerdan I'm still seeing an errors on trying to build on Windows (with R, which uses MinGW):

.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5Defl.c:291:44: error: macro "HDopen" requires 3 arguments, but only 2 given
         if((fd = HDopen(full_name, O_RDONLY)) < 0)
                                            ^
C:\run\ITKR.Rcheck\00_pkg_src\ITKR\src\itks\Modules\ThirdParty\HDF5\src\itkhdf5\src\H5Defl.c:291:18: error: 'HDopen' undeclared (first use in this function)
         if((fd = HDopen(full_name, O_RDONLY)) < 0)
                  ^
C:\run\ITKR.Rcheck\00_pk

https://ci.appveyor.com/project/muschellij2/itkr/builds/32046208/job/elfoepg8sjavwrtu#L4132

[muschellij2]

I think the issue is that H5_HAVE_VISUAL_STUDIO will likely come up true because Appveyor (or some Windows builds on people's machines) will have Visual Studio, but it is not the toolchain being used. Thus, the if statement does not get picked up correctly.

[thewtex]

I think the issue is that H5_HAVE_VISUAL_STUDIO will likely come up true because Appveyor (or some Windows builds on people's machines) will have Visual Studio, but it is not the toolchain being used. Thus, the if statement does not get picked up correctly.

How about we use

#if defined(_MSC_VER)

instead of H5_HAVE_VISUAL_STUDIO?

[muschellij2]

Can it be deduced by Generator (MinGW Makefiles)?
Best,
John

On Mon, Apr 13, 2020 at 9:22 PM Matt McCormick [e-mail]
wrote:

I think the issue is that H5_HAVE_VISUAL_STUDIO will likely come up true
because Appveyor (or some Windows builds on people's machines) will have
Visual Studio, but it is not the toolchain being used. Thus, the if
statement does not get picked up correctly.

How about we use

if defined(_MSC_VER)

instead of H5_HAVE_VISUAL_STUDIO?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/66#issuecomment-613175123,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAIGPLWFAXYI36UHK3WXM4LRMO3E7ANCNFSM4FW475BA
.

[thewtex]

_MSC_VER is defined if the compiler is MSVC.

[Neptilo]

Looking further in https://ci.appveyor.com/project/muschellij2/itkr/builds/31248553/job/4y49buwbe4bc7twi#L614, it seems it's a CMAKE_SH issue, which will be fixed on CMake 3.17: Kitware/CMake@82ddcf0

I have CMake 3.17.1 but still have this issue. Note: I am not using Appveyor, but building directly using mingw32-make. And I have MSBuild installed on my system.

[thewtex]

@muschellij2 @Neptilo please test this patch: https://github.com/InsightSoftwareConsortium/ITK/pull/1773

[Neptilo]

@thewtex Yes, surrounding HDopen with #ifdef _MSC_VER fixed it. Thank you!