Skip to content

Commit

Permalink
Add option <Project>_CHECK_FOR_UNPARSED_ARGUMENTS with default WARNING (
Browse files Browse the repository at this point in the history
#200)

The new default will allow an existing TriBITS project to keep working but
just print out of warnings for unparsed otherwise ignored argumetns.  But this
var can be set to error out as well (see developers guide documentation).

This default had to be changed or it would be a nighmare to upgrade existing
projects.  For example, it took a while to upgrade Trilinos and we should
expect many mistakes like this in other TirBITS projects.
  • Loading branch information
bartlettroscoe committed Sep 6, 2017
1 parent 7e64c76 commit 822a64a
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 1 deletion.
1 change: 1 addition & 0 deletions test/core/ExamplesUnitTests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3227,6 +3227,7 @@ TRIBITS_ADD_ADVANCED_TEST( TribitsExampleProject_PkgWithUserErrors_UNPARSED_ARGS
ARGS
${TribitsExampleProject_COMMON_CONFIG_ARGS}
-DTribitsExProj_TRIBITS_DIR=${${PROJECT_NAME}_TRIBITS_DIR}
-DTribitsExProj_CHECK_FOR_UNPARSED_ARGUMENTS=FATAL_ERROR
-DTribitsExProj_ENABLE_Fortran=OFF
-DTribitsExProj_EXTRA_REPOSITORIES=PkgWithUserErrors
-DTribitsExProj_ENABLE_PkgWithUserErrors=ON
Expand Down
10 changes: 10 additions & 0 deletions tribits/ReleaseNotes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
Release Notes for TriBITS
----------------------------------------

2017/09/05:

(*) MINOR: Unparsed and otherwise ignored arguments are now flagged (see
developers guide documentation for
${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS). The default value is
'WARNING' which results in simply printing a warning but allow configure
to complete. This allows one to see the warnings but for the project to
continue to work as before. But this can be changed to 'SEND_ERROR' or
'FATAL_ERROR' that will fail the configure.

2017/06/24:

(*) MINOR: Add new all-at-once more for CTest -S driver scripts using
Expand Down
6 changes: 5 additions & 1 deletion tribits/core/package_arch/TribitsGeneralMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,11 @@ ENDFUNCTION()
MACRO(TRIBITS_CHECK_FOR_UNPARSED_ARGUMENTS)

IF( NOT "${PARSE_UNPARSED_ARGUMENTS}" STREQUAL "")
MESSAGE(FATAL_ERROR "Arguments are being passed in but not used. UNPARSED_ARGUMENTS = ${PARSE_UNPARSED_ARGUMENTS}")
MESSAGE(
${${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS}
"Arguments are being passed in but not used. UNPARSED_ARGUMENTS ="
" ${PARSE_UNPARSED_ARGUMENTS}"
)
ENDIF()

ENDMACRO()
22 changes: 22 additions & 0 deletions tribits/core/package_arch/TribitsGlobalMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,28 @@ MACRO(TRIBITS_DEFINE_GLOBAL_OPTIONS_AND_DEFINE_EXTRA_REPOS)
${${PROJECT_NAME}_ENABLE_CONFIGURE_DEBUG_DEFAULT} CACHE BOOL
"Enable debug checking of the process which finds errors in the project's CMake files (off by default unless ${PROJECT_NAME}_ENABLE_DEBUG=ON)." )

IF ("${${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS_DEFAULT}" STREQUAL "")
SET(${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS_DEFAULT "WARNING")
ENDIF()
ADVANCED_SET(${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS
${${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS_DEFAULT}
CACHE STRING
"Determins how unparsed arguments for TriBITS functions that use CMAKE_PARASE_ARUMENTS() internally are handled. Valid choices are 'WARNING', 'SEND_ERROR', and 'FATAL_ERROR'. The default is 'SEND_ERROR'."
)
IF (
(NOT ${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS STREQUAL "WARNING")
AND
(NOT ${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS STREQUAL "SEND_ERROR")
AND
(NOT ${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS STREQUAL "FATAL_ERROR")
)
MESSAGE(FATAL_ERROR "Error, the value of"
" ${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS ="
" '${${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS}' is invalid!"
" Valid valules include 'WANRING', 'SEND_ERROR', and 'FATAL_ERROR'"
)
ENDIF()

SET(${PROJECT_NAME}_ENABLE_TEUCHOS_TIME_MONITOR ON
CACHE BOOL
"Enable support for Teuchos Time Monitors in all Trilinos packages that support it."
Expand Down
69 changes: 69 additions & 0 deletions tribits/doc/developers_guide/TribitsDevelopersGuide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7724,6 +7724,7 @@ be documented in `TribitsBuildReference`_.
The global project-level TriBITS options for which defaults can be provided by
a given TriBITS project are:

* `${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS`_
* `${PROJECT_NAME}_CONFIGURE_OPTIONS_FILE_APPEND`_
* `${PROJECT_NAME}_CPACK_SOURCE_GENERATOR`_
* `${PROJECT_NAME}_CTEST_DO_ALL_AT_ONCE`_
Expand Down Expand Up @@ -7757,6 +7758,74 @@ a given TriBITS project are:

These options are described below.

.. _${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS:

**${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS**

The variable ``${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS`` determines how
unparsed and otherwise ignored arguments are handled in TriBITS functions
that are called by the client TriBITS projects. These are arguments that
are left over from parsing input options to functions and macros that take
both positional arguments and keyword arguments/options handled with the
``CMAKE_PARSE_ARGUMENTS()`` function. For example, for the a TriBITS
function declared like::

TRIBITS_COPY_FILES_TO_BINARY_DIR(
<targetName>
[SOURCE_FILES <file1> <file2> ...]
[SOURCE_DIR <sourceDir>]
...
)

the arguments ``SOURCE_FILES <file1> <file2> ...`` and those that follow are
parsed by the ``CMAKE_PARSE_ARGUMENTS()`` function while the argument
``<targetName>`` is a positional argument. The problem is that any
arguments passed between the first ``<targetName>`` argument and the
specified keyword arguments like ``SOURCE_FILES`` and ``SOURCE_DIR`` are
returned as unparsed arguments and are basically ignored (which is what
happened in earlier versions of TriBITS). For example, calling the function
as::

TRIBITS_COPY_FILES_TO_BINARY_DIR( FooTestCopyFiles
ThisArgumentIsNotParsedAndIsIgnored
SOURCE_FILES file1.cpp file2.cpp ...
...
)

would result in the unparsed argument
``ThisArgumentIsNotParsedAndIsIgnored``.

The value of ``${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS`` determines how
that ignored argument is handled. If the value is ``WARNING``, then it will
just result in a ``MESSAGE(WARNING ...)`` command that states the warning
but configure is allowed to be completed. This would be the right value to
allow an old TriBITS project to keep configuring until the warnings can be
cleaned up. If the value is ``SEND_ERROR``, then ``MESSAGE(SEND_ERROR
...)`` is called. This will result in the configure failing but will allow
configure to continue until the end (or a ``FATAL_ERROR`` is raised). This
would be the right value when trying to upgrade a TriBITS project where you
wanted to see all of the warnings when upgrading TriBITS (so you could fix
them all in one shot). Finally, the value of ``FATAL_ERROR`` will result in
``MESSAGE(FATAL_ERROR ...)`` being called which will halt configure right
away. This is the best value when developing on a TriBITS project that is
already clean but you want to catch new developer-inserted errors right
away.

The default value for ``${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS`` is
``WARNING``, so that it will be backward compatible for TriBITS projects
that might have previously undetected unparased and therefore ignored
argument . However, a project can change the default by setting, for
example::

SET(${PROJECT_NAME}_CHECK_FOR_UNPARSED_ARGUMENTS_DEFAULT FATAL_ERROR)

in the `<projectDir>/ProjectName.cmake`_ file.

The user of a TriBITS project should not be able to trigger this unparsed
arguments condition so this variable is not documented in the `TriBITS Build
Reference`_. But it is still a CMake cache var that is documented in the
CMakeCache.txt file and can be set by the user or developer if desired.

.. _${PROJECT_NAME}_CONFIGURE_OPTIONS_FILE_APPEND:

**${PROJECT_NAME}_CONFIGURE_OPTIONS_FILE_APPEND**
Expand Down

0 comments on commit 822a64a

Please sign in to comment.