Skip to content

Conversation

@thewtex
Copy link
Member

@thewtex thewtex commented Dec 19, 2020

  • BUG: Do not override ITK_DIR in CMake package configurations
  • COMP: Update JsonCpp API calls

Enable a different ITK to build built against when building the CLI,
e.g. when cross-compiling.
To address:

[3/5] Building CXX object CMakeFiles/CLPExample1Lib.dir/CLPExample1.cxx.o
In file included from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
/home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx: In function ‘int ModuleEntryPoint(int, char**)’:
./CLPExample1CLP.h:36:20: warning: ‘Reader’ is deprecated: Use CharReader and CharReaderBuilder instead. [-Wdeprecated-declarations]
   36 |       Json::Reader reader; \
      |                    ^~~~~~
./CLPExample1CLP.h:715:62: note: in expansion of macro ‘GENERATE_DESERIALIZATION’
  715 | #define PARSE_ARGS GENERATE_LOGO;GENERATE_XML;GENERATE_TCLAP;GENERATE_DESERIALIZATION;GENERATE_ECHOARGS;GENERATE_ProcessInformationAddressDecoding;GENERATE_SERIALIZATION;
      |                                                              ^~~~~~~~~~~~~~~~~~~~~~~~
/home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:5:3: note: in expansion of macro ‘PARSE_ARGS’
    5 |   PARSE_ARGS;
      |   ^~~~~~~~~~
In file included from /home/matt/src/jsoncpp/include/json/json.h:11,
                 from /home/matt/src/SlicerExecutionModel/ModuleDescriptionParser/JsonSerializationUtilities.h:18,
                 from ./CLPExample1CLP.h:15,
                 from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
/home/matt/src/jsoncpp/include/json/reader.h:37:63: note: declared here
   37 |     "Use CharReader and CharReaderBuilder instead.") JSON_API Reader {
      |                                                               ^~~~~~
In file included from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
./CLPExample1CLP.h:36:20: warning: ‘Json::Reader::Reader()’ is deprecated: Use CharReader and CharReaderBuilder instead [-Wdeprecated-declarations]
   36 |       Json::Reader reader; \
      |                    ^~~~~~
./CLPExample1CLP.h:715:62: note: in expansion of macro ‘GENERATE_DESERIALIZATION’
  715 | #define PARSE_ARGS GENERATE_LOGO;GENERATE_XML;GENERATE_TCLAP;GENERATE_DESERIALIZATION;GENERATE_ECHOARGS;GENERATE_ProcessInformationAddressDecoding;GENERATE_SERIALIZATION;
      |                                                              ^~~~~~~~~~~~~~~~~~~~~~~~
/home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:5:3: note: in expansion of macro ‘PARSE_ARGS’
    5 |   PARSE_ARGS;
      |   ^~~~~~~~~~
In file included from /home/matt/src/jsoncpp/include/json/json.h:11,
                 from /home/matt/src/SlicerExecutionModel/ModuleDescriptionParser/JsonSerializationUtilities.h:18,
                 from ./CLPExample1CLP.h:15,
                 from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
/home/matt/src/jsoncpp/include/json/reader.h:56:3: note: declared here
   56 |   Reader();
      |   ^~~~~~
In file included from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
./CLPExample1CLP.h:89:32: warning: ‘StyledStreamWriter’ is deprecated: Use StreamWriterBuilder instead [-Wdeprecated-declarations]
   89 |       Json::StyledStreamWriter writer; \
      |                                ^~~~~~
./CLPExample1CLP.h:715:148: note: in expansion of macro ‘GENERATE_SERIALIZATION’
  715 | #define PARSE_ARGS GENERATE_LOGO;GENERATE_XML;GENERATE_TCLAP;GENERATE_DESERIALIZATION;GENERATE_ECHOARGS;GENERATE_ProcessInformationAddressDecoding;GENERATE_SERIALIZATION;
      |                                                                                                                                                    ^~~~~~~~~~~~~~~~~~~~~~
/home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:5:3: note: in expansion of macro ‘PARSE_ARGS’
    5 |   PARSE_ARGS;
      |   ^~~~~~~~~~
In file included from /home/matt/src/jsoncpp/include/json/json.h:13,
                 from /home/matt/src/SlicerExecutionModel/ModuleDescriptionParser/JsonSerializationUtilities.h:18,
                 from ./CLPExample1CLP.h:15,
                 from /home/matt/src/itk-js/test/CLPExample1/CLPExample1.cxx:1:
/home/matt/src/jsoncpp/include/json/writer.h:298:5: note: declared here
  298 |     StyledStreamWriter {
      |     ^~~~~~~~~~~~~~~~~~
@thewtex thewtex changed the title itk dir avoid override ITK_DIR avoid override Dec 19, 2020
Copy link
Member

@jcfr jcfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ensuring the developer have the same ITK build tree is important is the general case, should we test for CMAKE_CROSSCOMPILING variable ?

@thewtex
Copy link
Member Author

thewtex commented Jan 11, 2021

This is not only useful for cross-compiling -- in general different ITK version or build configuration may be desired for the generated CLI.

@hjmjohnson
Copy link
Member

@jcfr Any other comments on this? Is it OK as is?

@jcfr
Copy link
Member

jcfr commented Aug 13, 2021

Thanks for the feedback. After further consideration, the proposed changes make sens.

@jcfr jcfr removed the request for review from johan-andruejol August 13, 2021 18:49
Copy link
Member

@jcfr jcfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's address #136 before merging

@hjmjohnson
Copy link
Member

@jcfr #136 is addressed, and this seems to have approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants