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

COMP: Move ITK_DISALLOW_COPY_AND_ASSIGN calls to public section. #54

Conversation

jhlegarreta
Copy link
Member

Move ITK_DISALLOW_COPY_AND_ASSIGN calls to public section following
the discussion in
https://discourse.itk.org/t/noncopyable

If legacy (pre-macro) copy and assing methods existed, subsitute them
for the ITK_DISALLOW_COPY_AND_ASSIGN macro.

Move `ITK_DISALLOW_COPY_AND_ASSIGN` calls to public section following
the discussion in
https://discourse.itk.org/t/noncopyable

If legacy (pre-macro) copy and assing methods existed, subsitute them
for the `ITK_DISALLOW_COPY_AND_ASSIGN` macro.
@jhlegarreta
Copy link
Member Author

PR #53 and this are waiting to be merged. Anyone to review @blowekamp 's PR and this one?

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Apr 20, 2018

@jbvimort @blowekamp @thewtex Can you please review and comment please? Thanks !

  1. Travis reports an error during CMake configuration
CMake Error at CMakeLists.txt:2 (cmake_minimum_required):
  CMake 3.9.5 or higher is required.  You are running version 3.9.4
  1. Circle CI reports a compilation problem: am not familiar with it and I am not getting it in my local build (Win 7, MSVC 2015)
[9/19] Generating ../../../../../ITK-cp27-cp27m-manylinux1_x64/Wrapping/itkRunLengthTextureFeaturesImageFilter.xml
FAILED: ../../ITK-cp27-cp27m-manylinux1_x64/Wrapping/itkRunLengthTextureFeaturesImageFilter.xml 
cd /work/_skbuild/cmake-build/Wrapping/Modules/TextureFeatures && /work/_skbuild/cmake-build/Wrapping/Generators/CastXML/castxml/bin/castxml -o /work/ITK-cp27-cp27m-manylinux1_x64/Wrapping/itkRunLengthTextureFeaturesImageFilter.xml --castxml-gccxml --target=x86_64-linux-gnu --castxml-start _wrapping_ --castxml-cc-gnu "(" /opt/rh/devtoolset-2/root/usr/bin/g++ -Wno-array-bounds -Wno-array-bounds ")" -w -c @/work/ITK-cp27-cp27m-manylinux1_x64/Wrapping/TextureFeatures.castxml.inc /work/ITK-cp27-cp27m-manylinux1_x64/Wrapping/itkRunLengthTextureFeaturesImageFilter.cxx
In file included from /work/ITK-cp27-cp27m-manylinux1_x64/Wrapping/itkRunLengthTextureFeaturesImageFilter.cxx:20:
/work/include/itkRunLengthTextureFeaturesImageFilter.h:150:10: error: unknown type name 'constexpr'
  static constexpr unsigned int DefaultBinsPerAxis = 256;
         ^
/work/include/itkRunLengthTextureFeaturesImageFilter.h:150:20: error: expected member name or ';' after declaration specifiers
  static constexpr unsigned int DefaultBinsPerAxis = 256;

OTOH, what I do get is the KWStyle error, and as it happens lately with other remotes, a failure that I'd dare to say is related to some CMakeLists.txt:

Error    C1083    Cannot open source file:
'C:\Binaries\ITKTextureFeatures\test\TextureFeaturesHeaderTest1.cxx':
No such file or directory

As for the KWStyle error, and as a general note for ITK and its remotes, can we throw a CMake error if the ITK_USE_KWSTYLE is ON (which is by default as it appears) and the path for the KW Style checker (KWSTYLE_EXECUTABLE) is not provided?

IMHO, it is annoying to get the related build time error.

@thewtex
Copy link
Member

thewtex commented Apr 20, 2018

CMake 3.9.5 or higher is required. You are running version 3.9.4

This should be fixed by ITKPythonPackage improvements -- restarting the build.

[9/19] Generating ../../../../../ITK-cp27-cp27m-manylinux1_x64
static constexpr unsigned int DefaultBinsPerAxis = 256;

Restarting the build should fix this issue since the CI now uses ITKv5, which enables C++11 support, required for constexpr.

OTOH, what I do get is the KWStyle error,

Do you get this with all Remote Modules? If they are built as a Remote Module or External Module?

@thewtex thewtex merged commit 6bf2398 into InsightSoftwareConsortium:master Apr 20, 2018
@jhlegarreta
Copy link
Member Author

@thewtex it also happened when building SkullStrip as an external project.

It looks like the ITK_USE_KWSTYLE is ON by default in ITK, and gets inherited by the remotes. Providing the appropriate KWSTYLE_EXECUTABLE path solves the issue.

IMHO, I'd dare to say that rather than getting a build-time error, even if related to KWStyle, I think it would be best to cast a CMake error if the executable is not provided and if the flag should be ON by default.

The same happened here:
http://review.source.kitware.com/#/c/23345/

OTOH, the No such file or directory error was also present in SkullStrip:

Error	C1083	Cannot open source file: 'C:\Binaries\SkullStrip\test\SkullStripHeaderTest1.cxx': No such file or directory

I honestly ignore how and why that happens. A hypothesis is that it has to do with the KWStyle issue: providing the exe path to the checker makes both projects build without errors; then removing the path yields now (even cleaning):

Error	MSB6006	"cmd.exe" exited with code 1.	TextureFeaturesHeaderTestClean	C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.CppCommon.targets	171	

Providing again the exe path makes both compile without errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants