Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DCMTK IO module does not compile on Windows #2817

Closed
jhlegarreta opened this issue Oct 15, 2021 · 20 comments
Closed

DCMTK IO module does not compile on Windows #2817

jhlegarreta opened this issue Oct 15, 2021 · 20 comments
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@jhlegarreta
Copy link
Member

jhlegarreta commented Oct 15, 2021

Description

The DCMTK IO module does not compile.

Steps to Reproduce

  1. Configure ITK with the Module_ITKIODCMTK:BOOL=ON; DCMTK_USE_ICU=OFF
  2. Build ITK
  3. ITK will fail to compile with the following message
/Modules/IO/DCMTK/include/itkDCMTKFileReader.h(27,10): fatal error C1083:
Cannot open include file: 'dcmtk/dcmdata/dcdict.h': No such file or directory

The file, however, does exist in the build tree:

/itk-head-Bin/Modules/ThirdParty/DCMTK/ITKDCMTK_ExtProject/dcmdata/include/dcmtk/dcmdata/dcdict.h

Expected behavior

ITK should build with no errors.

Actual behavior

It does not build.

Reproducibility

100%.

Versions

ITK master.

Environment

OS: Windows 10
CMake: 3.20.3
Compiler: MSVC 2019 19.29.30133.0
Build: static; debug

Additional Information

Reproduced/confirmed by @dzenanz in #2796 (comment).

The DCMTK module compiles fine as stand-alone project for the tag used in ITK, but linking to it as a system module (ITK_USE_SYSTEM_DCMTK) in CMake results also in another type of errors:

LINK : fatal error LNK1104: cannot open file 'ofstd.lib'

The file, however, does exist in the DCMTK build tree:

/dcmtk-Bin/lib/Debug

Related to PR #2796.

The warnings cast when building DCMTK were reported in #2798.

The module seems to compile fine on Linux:
https://open.cdash.org/build/7512875/notes
https://open.cdash.org/build/7512875

@jhlegarreta jhlegarreta added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Oct 15, 2021
@seanm
Copy link
Contributor

seanm commented Oct 15, 2021

I just tried on macOS and it built.

Did you try toggling DCMTK_USE_ICU?

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Oct 15, 2021

Did you try toggling DCMTK_USE_ICU?

Thanks for giving this a try @seanm. When setting the DCMTK_USE_ICU flag to ON CMake shows the following error:

"ICU is not built as part of ITK on Windows. Please compile it outside of ITK."

which comes from here.

I feel like if using a system ICU is required on Windows, the flag should be set to ON automatically when the (non-system) DCMTK is required so that the configuration fails early.

At the same time, is that really required? Why does DCMTK compile just fine as a stand-alone project then?

I'd say that the issue is not related to ICU.

@seanm
Copy link
Contributor

seanm commented Oct 15, 2021

Sorry if I was unclear, I didn't mean to suggest it would be related, I just noticed there was that toggle, and thought just maybe it would make a difference...

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the status:Use_Milestone_Backlog Use "Backlog" milestone instead of label for issues without a fixed deadline label Apr 16, 2022
@jhlegarreta
Copy link
Member Author

This is still relevant stale bot.

@stale stale bot removed the status:Use_Milestone_Backlog Use "Backlog" milestone instead of label for issues without a fixed deadline label Apr 16, 2022
@jhlegarreta
Copy link
Member Author

@tbirdso @dzenanz I'd be grateful if, at some point, you would be able to spend some time to look into this issue and hopefully fix it. If it were fixed, I could give #2796 a try, see if I can wrap that up. It could get us back to the 90% coverage level. Thanks both for your time and effort.

@tbirdso
Copy link
Contributor

tbirdso commented Jun 12, 2022

Hi @jhlegarreta, I won't be able to help this week but can take a look in the not-too-far future. I am not very familiar with the DCMTK CMake code in ITK. Given that the failure is only on Windows, maybe there is a path length issue somewhere?

@jhlegarreta
Copy link
Member Author

won't be able to help this week but can take a look in the not-too-far future.

No worries. It is not urgent, but nice to fix at some point.

I am not very familiar with the DCMTK CMake code in ITK. Given that the failure is only on Windows, maybe there is a path length issue somewhere?

Not sure about the underlying causes.

@dzenanz
Copy link
Member

dzenanz commented Jun 14, 2022

Also possibly related: #2872.

@tbirdso
Copy link
Contributor

tbirdso commented Jun 17, 2022

@jhlegarreta I was not able to immediately recreate this issue. The ITKIODCMTKTestDriver target appears to compile on first try on my Windows machine. I have double checked that Module_ITKIODCMTK=ON and DCMTK_USE_ICU=ON in my CMake configuration.

Could you please confirm whether you are still seeing this issue when you try building? If so, could you please try upgrading your CMake version and/or changing your build type to see if that helps?

Platform: Windows 10
CMake: 3.22.3
Compiler: MSVC 2022 14.31.31103
Build type: RelWithDebInfo

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Jun 20, 2022

Thanks for the effort Tom.

I submitted an experimental build to
https://open.cdash.org/viewBuildError.php?buildid=7989296

My toolchain is the following:
Platform: Windows 10
CMake 3.20.2
Compiler: MSVC 2019 (Microsoft Visual Studio Community 2019 Version 16.11.7)
Build type: Debug

(So "reasonably up-to-date" I'd dare to say)

Note that there are additional build errors due to some local, unfinished additions (sorry for the noise they add), but you can see that the IODCMTK module is not compiling, all errors pointing to compiler flags, e.g.:

dcmdata.lib(dcvrobow.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary':
value 'MTd_StaticDebug' doesn't match value 'MDd_DynamicDebug'
in ITKIODCMTKTestDriver.obj
[C:\SDKs\ITK\itk-head-Bin\Modules\IO\DCMTK\test\ITKIODCMTKTestDriver.vcxproj]

I did not check the DCTMK_USE_ICU CMake flag because the CMake configuration step failed when I checked it: contrary to what that is supposed to do (intuitively, and as its help says - Downloads and compile ICU for DCMTK), it resulted in the following CMake error:

CMake Error at Modules/ThirdParty/DCMTK/CMakeLists.txt:17 (message):
ICU is not built as part of ITK on Windows. Please compile it outside of
ITK.

And I was not willing to download and compile ICU in case it resulted in new issues.

In fact, I think that is a separate issue, though, as the build errors point rather to issue #2872

@tbirdso
Copy link
Contributor

tbirdso commented Jun 21, 2022

@jhlegarreta I agree that this is a separate issue. The original error message

LINK : fatal error LNK1104: cannot open file 'ofstd.lib'

does not appear to be present in the build you posted, and as you pointed out the new errors

error LNK2038: mismatch detected for 'RuntimeLibrary':

could indeed be explained by issue #2872.

It does not appear that we are able to reproduce this issue, I am in favor of closing it. @jhlegarreta @dzenanz do you agree?

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Jun 21, 2022

The error

LINK : fatal error LNK1104: cannot open file 'ofstd.lib

appears when using a system DCMTK.

Thus, there are multiple errors, from different nature, with the module, and thus I think the issue is still applicable and different from #2872. Note that the above error was reproduced by @dzenanz, as commented in the original issue report.

Note that this error appeared as I tried to use a system DCMTK because the remote module build was failing. I grant that the remote was originally failing with a different error (see the original issue report):

/Modules/IO/DCMTK/include/itkDCMTKFileReader.h(27,10): fatal error C1083:
Cannot open include file: 'dcmtk/dcmdata/dcdict.h': No such file or directory

but even if in the experimental build it does not seem to be present, I should double check with a regular build.

So I am not quite in favor of closing the issue.

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Jun 22, 2022

But even if in the experimental build it does not seem to be present, I should double check with a regular build.

Not present either in a regular build. So it somehow looks that that one is gone now (maybe the commit bump in PR #3119 solved it), and it's the compiler flag errors that persist (when using a non-system DCMTK).

@thewtex
Copy link
Member

thewtex commented Jul 1, 2022

We may want to see what Slicer is doing in their build for Windows.

@issakomi
Copy link
Member

issakomi commented Nov 10, 2022

I have compiled the module without issues just now (master, default configuration without testing plus DCMTK IO).

  • Windows 10
  • VS 2019
  • CMake 3.18

Both Debug and Release.
Does the issue still exist?

@issakomi
Copy link
Member

Didn't fail here too: https://open.cdash.org/build/8268393

@jhlegarreta
Copy link
Member Author

Thanks for the information @issakomi. I had tried compiling a few days ago without success, and the same error being reported. Now I've tried again this afternoon with a clean build and it looks like things are OK. I need to have a closer look. I will follow up with this as time permits.

@jcfr
Copy link
Contributor

jcfr commented Nov 29, 2022

We may want to see what Slicer is doing in their build for Windows.

Ditto. Thanks to @jamesobutler who started to work on updating DCMTK in Slicer, we will be working on this.

@jhlegarreta
Copy link
Member Author

I gave this another try with a new, clean target folder and reduced the scope of the error to the test, but did not find a solution.

In summary, DCMTK IO module DOES compile on Windows (Win 10, CMake 3.24.2, MSVC 2022; having enabled both ITKDCMTK and ITKIODCMTK modules enabled; with and without testing enabled). But the test in #2796 does not compile due to a missing header file.

So I'm closing this issue in favor of #3820, which has a more faithful report of the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

No branches or pull requests

7 participants