Add back deprecated non-namespaced library targets (#299, #433)

[bartlettroscoe]

Description

This PR adds back the non-namespaced library targets described in #299 (comment) using the approach described in #299 (comment).

I added a new example/test project TribitsOldSimpleExampleApp that uses the old TriBITS interface pull in info and should work with old and new TriBITS.

As shown here, the new test TriBITS_TribitsOldSimpleExampleApp_STATIC_USE_DEPRECATED_TARGETS generates the deprecated warning message:

-- Configuring done
CMake Warning (dev) at CMakeLists.txt:33 (target_link_libraries):
  The library that is being linked to, pws_b, is marked as being deprecated
  by the owner.  The message provided by the developer is:

  WARNING: The non-namespaced target 'pws_b' is deprecated! If always using
  newer versions of the project 'TribitsExProj', then use the new namespaced
  target 'WithSubpackagesB::pws_b', or better yet,
  'WithSubpackagesB::all_libs' to be less sensitive to changes in the
  definition of targets in the package 'WithSubpackagesB'.  Or, to maintain
  compatibility with older or newer versions the project 'TribitsExProj',
  instead link against the libraries specified by the variable
  'WithSubpackagesB_LIBRARIES'.

This warning is for project developers.  Use -Wno-dev to suppress it.

-- Generating done

Hopefully that will be enough to guide users for how to refactor their build systems based on their needs.

See the commits for more details.

How was this tested?

I ran the TriBITS tests locally of course and the GitHub Actions tests will/have run.

I also ran the new TribitsOldSimpleExampleApp tests against and older version of TribitsExampleProject and TriBITS and it passed the build and ran the executables as shown below.

Details of TribitsOldSimpleExampleApp tests with older version of TribitsExampleProject and TriBITS with non-namespaced targts (Click to expand)

.

Using the older TriBITS version for TribitsExampleProject:

$ cd ${HOME}/TriBITS.base/TriBITS/

$ git log-short --name-status -1
881ad33 "Merge pull request #423 from bartlettroscoe/snl-kitware-183-test-measurement"
Author: Roscoe A. Bartlett <rabartl@sandia.gov>
Date:   Wed Sep 29 18:26:07 2021 -0400 (4 months ago)

Using the files:

load-env.sh:

# Env for testing TriBITS on crf450 using Python 2.7.5 (the default on RHEL-7)

module purge

export PATH=${PATH_ORIG}

# From ~/load_dev_env.sh

source ~/load_vera_dev_env.gcc-4.8.3.crf450.sh
module load sems-env
module load sems-git/2.10.1
module load sems-cmake/3.17.1
module load sems-ninja_fortran/1.10.0

# Extra stuff for TriBITS

#export PATH=/home/vera_env/common_tools/cmake-3.17.0/bin:${PATH}
export PATH=/home/vera_env/common_tools/ccache-3.7.9/bin:${PATH}

# Make default install permissions 700 so that we can test that TriBITS will
# use recursive chmod to open up permissions.
umask g-rwx,o-rwx

export TribitsExMetaProj_GIT_URL_REPO_BASE=git@github.com:tribits/

do-configure:

#!/bin/bash

if [ -d CMakeFiles ] ; then
  rm -r CMakeFiles
fi
if [ -e CMakeCache.txt ] ; then
  rm CMakeCache.txt
fi

if [ "$TRIBITS_BASE_DIR" == "" ] ; then
  _ABS_FILE_PATH=`readlink -f $0`
  _SCRIPT_DIR=`dirname $_ABS_FILE_PATH`
  TRIBITS_BASE_DIR=$_SCRIPT_DIR/../../..
fi

${TRIBITS_BASE_DIR}/dev_testing/generic/do-configure-mpi-debug \
-DDART_TESTING_TIMEOUT=85 \
-DCTEST_PARALLEL_LEVEL=16 \
-DTriBITS_CTEST_DRIVER_COVERAGE_TESTS=TRUE \
-DTriBITS_CTEST_DRIVER_MEMORY_TESTS=TRUE \
-DTriBITS_ENABLE_CONFIGURE_TIMING=ON \
-DTriBITS_ENABLE_PACKAGE_CONFIGURE_TIMING=ON \
-DTribitsExProj_INSTALL_BASE_DIR=/tmp/tribits_install_tests \
-DTribitsExProj_INSTALL_OWNING_GROUP=wg-sems-users-son \
-DTriBITS_ENABLE_REAL_GIT_CLONE_TESTS=ON \
-DTriBITS_SHOW_TEST_START_END_DATE_TIME=ON \
"$@"

I ran:

$ cd ${HOME}/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG/

$  . load-env.sh

$ ./do-configure \
  -DTribitsOldSimpleExampleApp_TribitsExampleProject_TRIBITS_DIR=${HOME}/TriBITS/tribits

$ make && ctest -j16 -R TribitsOldSimpleExampleApp

...

0% tests passed, 2 tests failed out of 2

Subproject Time Summary:
TriBITS    =  57.36 sec*proc (2 tests)

Total Test time (real) =  28.87 sec

The following tests FAILED:
        247 - TriBITS_TribitsOldSimpleExampleApp_STATIC_USE_DEPRECATED_TARGETS (Failed)
        248 - TriBITS_TribitsOldSimpleExampleApp_SHARED_USE_NEW_TARGETS (Failed)

but only the regex checks at the end failed, the build of the APP passed as shown by:

$ grep "\[FAILED\]" Testing/Temporary/LastTest.log 
TEST_4: Pass criteria = Match REGEX {Full Deps: WithSubpackages:B A SimpleCxx simpletpl headeronlytpl SimpleCxx simpletpl headeronlytpl A SimpleCxx simpletpl headeronlytpl[;] MixedLang:Mixed Language[;] SimpleCxx:simpletpl headeronlytpl} [FAILED]
TEST_4: Pass criteria = Match REGEX {util_test [.]+   Passed} [FAILED]
TEST_4: Pass criteria = Match REGEX {app_test [.]+   Passed} [FAILED]
TEST_4: Pass criteria = Match REGEX {100% tests passed, 0 tests failed out of 2} [FAILED]
TEST_4: Pass criteria = ALWAYS_FAIL_ON_NONZERO_RETURN [FAILED]

So this shows that new TriBITS satisfies the old TriBITS interface using the variables and even the non-namespaced targets and and downstream clients can work with both old and new versions by using the variables.

I also tested against the older version of Albany before the merge of sandialabs/Albany#778 as described in internal tribitsrefactoring#1. And it showed the deprecation warning:

CMake Warning (dev) at src/CMakeLists.txt:199 (target_link_libraries):
  The library that is being linked to, teuchosparameterlist, is marked as
  being deprecated by the owner.  The message provided by the developer is:

  WARNING: The non-namespaced target 'teuchosparameterlist' is deprecated! If
  always using newer versions of the project 'Trilinos', then use the new
  namespaced target 'TeuchosParameterList::teuchosparameterlist', or better
  yet, 'TeuchosParameterList::all_libs' to be less sensitive to changes in
  the definition of targets in the package 'TeuchosParameterList'.  Or, to
  maintain compatibility with older or newer versions the project 'Trilinos',
  instead link against the libraries specified by the variable
  'TeuchosParameterList_LIBRARIES'.

This warning is for project developers.  Use -Wno-dev to suppress it.

[bartlettroscoe]

@mperrinel and @marcinwrobel1986, can you do a review of this PR? I need to do some acceptance testing against Albany before I merge so it will be several hours before I can merge (likely tomorrow morning). In case I am ready to merge this to 'master' so that I can updated the Trilinos branch in trilinos/Trilinos#9978 before you can finish your review, you can always do a detailed post-merge PR review and I can address issues in a follow-up PR.

[marcinwrobel1986]

Hello Ross @bartlettroscoe I read the code and wanted to build TribitsExampleProject as below:

# I was in:
<tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject/build 
# On branch:
433-nonnamespaced-targets 

cmake -DTribitsExProj_TRIBITS_DIR=<tribits-dir>/TriBITS/tribits -DTribitsExProj_ENABLE_TESTS=ON -DTribitsExProj_ENABLE_ALL_PACKAGES=ON -DCMAKE_CXX_COMPILER=g++ -DTPL_ENABLE_HeaderOnlyTpl=OFF ..

Configuring TribitsExProj build directory

-- PROJECT_SOURCE_DIR='<tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject'
-- PROJECT_BINARY_DIR='<tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject/build'
-- TribitsExProj_TRIBITS_DIR='<tribits-dir>/TriBITS/tribits'
-- TriBITS_VERSION_STRING='0.9 (Dev)'
-- CMAKE_VERSION='3.20.2'
-- CMAKE_GENERATOR='Unix Makefiles'
-- CMAKE_HOST_SYSTEM_NAME='Linux'
-- TribitsExProj_HOSTNAME='pop-os'
-- PYTHON_EXECUTABLE='/usr/bin/python'

Setting up major user options ...

-- Setting TribitsExProj_ENABLE_EXAMPLES=ON because TribitsExProj_ENABLE_TESTS=ON

***
*** NOTE: Setting TribitsExProj_ENABLE_WrapExternal=OFF because TribitsExProj_ENABLE_INSTALL_CMAKE_CONFIG_FILES='ON'!
***

-- TribitsExProj_GENERATE_REPO_VERSION_FILE='OFF'

Reading list of native TPLs from <tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject/TPLsList.cmake

-- TribitsExProj_NUM_TPLS='3'

Reading list of native packages from <tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject/PackagesList.cmake

-- TribitsExProj_NUM_PACKAGES='4'

Processing Project, Repository, and Package dependency files and building internal dependencies graph ...

-- TribitsExProj_NUM_SE_PACKAGES='7'

Explicitly enabled packages on input (by user):  0

Explicitly enabled SE packages on input (by user):  0

Explicitly disabled packages on input (by user or by default):  WrapExternal 1

Explicitly disabled SE packages on input (by user or by default):  WrapExternal 1

Explicitly enabled TPLs on input (by user):  0

Explicitly disabled TPLs on input (by user or by default):  HeaderOnlyTpl 1

Disabling all packages that have a required dependency on disabled TPLs and optional package TPL support based on TPL_ENABLE_<TPL>=OFF ...

-- Setting TribitsExProj_ENABLE_SimpleCxx=OFF because SimpleCxx has a required library dependence on disabled TPL HeaderOnlyTpl

Disabling subpackages for hard disables of parent packages due to TribitsExProj_ENABLE_<PARENT_PACKAGE>=OFF ...


Disabling forward required SE packages and optional intra-package support that have a dependency on disabled SE packages TribitsExProj_ENABLE_<TRIBITS_PACKAGE>=OFF ...

-- Setting TribitsExProj_ENABLE_WithSubpackagesA=OFF because WithSubpackagesA has a required library dependence on disabled package SimpleCxx
-- Setting TribitsExProj_ENABLE_WithSubpackagesB=OFF because WithSubpackagesB has a required library dependence on disabled package SimpleCxx
-- Setting TribitsExProj_ENABLE_WithSubpackagesC=OFF because WithSubpackagesC has a required library dependence on disabled package WithSubpackagesA
-- Setting TribitsExProj_ENABLE_WithSubpackages=OFF because WithSubpackages has a required library dependence on disabled package WithSubpackagesA

Enabling subpackages for hard enables of parent packages due to TribitsExProj_ENABLE_<PARENT_PACKAGE>=ON ...


Enabling all SE packages that are not currently disabled because of TribitsExProj_ENABLE_ALL_PACKAGES=ON (TribitsExProj_ENABLE_SECONDARY_TESTED_CODE=OFF) ...

-- Setting TribitsExProj_ENABLE_MixedLang=ON

Enabling all tests and/or examples that have not been explicitly disabled because TribitsExProj_ENABLE_[TESTS,EXAMPLES]=ON ...

-- Setting MixedLang_ENABLE_TESTS=ON
-- Setting MixedLang_ENABLE_EXAMPLES=ON

Enabling all required (and optional since TribitsExProj_ENABLE_ALL_OPTIONAL_PACKAGES=ON) upstream SE packages for current set of enabled packages (TribitsExProj_ENABLE_SECONDARY_TESTED_CODE=OFF) ...


Enabling all optional intra-package enables <TRIBITS_PACKAGE>_ENABLE_<DEPPACKAGE> that are not currently disabled if both sets of packages are enabled ...


Enabling all remaining required TPLs for current set of enabled packages ...


Enabling all optional package TPL support <TRIBITS_PACKAGE>_ENABLE_<DEPTPL> not currently disabled for enabled TPLs ...


Enabling TPLs based on <TRIBITS_PACKAGE>_ENABLE_<TPL>=ON if TPL is not explicitly disabled ...


Set cache entries for optional packages/TPLs and tests/examples for packages actually enabled ...


Enabling the shell of non-enabled parent packages (mostly for show) that have at least one subpackage enabled ...


Final set of enabled packages:  MixedLang 1

Final set of enabled SE packages:  MixedLang 1

Final set of non-enabled packages:  SimpleCxx WithSubpackages WrapExternal 3

Final set of non-enabled SE packages:  SimpleCxx WithSubpackagesA WithSubpackagesB WithSubpackagesC WithSubpackages WrapExternal 6

Final set of enabled TPLs:  0

Final set of non-enabled TPLs:  MPI HeaderOnlyTpl SimpleTpl 3

Setting up export dependencies for all enabled SE packages ...


Probing the environment ...

-- USE_XSDK_DEFAULTS='FALSE'
-- BUILD_SHARED_LIBS='FALSE'
-- CMAKE_BUILD_TYPE='RELEASE'
-- CMAKE_C_COMPILER_ID='GNU'
-- CMAKE_C_COMPILER_VERSION='9.3.0'
-- CMAKE_CXX_COMPILER_ID='GNU'
-- CMAKE_CXX_COMPILER_VERSION='9.3.0'
-- TribitsExProj_SET_INSTALL_RPATH='TRUE'
-- CMAKE_INSTALL_RPATH_USE_LINK_PATH='TRUE'
-- Setting default for CMAKE_INSTALL_RPATH pointing to TribitsExProj_INSTALL_LIB_DIR
-- CMAKE_INSTALL_RPATH='/usr/local/lib'

Getting information for all enabled TPLs ...


Setting up testing support ...

-- CTEST_DROP_METHOD='http'
-- CTEST_DROP_SITE='my.cdash.org'
-- CTEST_PROJECT_NAME='TribitsExampleProject'
-- CTEST_DROP_LOCATION='/submit.php?project=TribitsExampleProject'
-- CTEST_TRIGGER_SITE=''
-- CTEST_DROP_SITE_CDASH='TRUE'

Configuring individual enabled TribitsExProj packages ...

Processing enabled package: MixedLang (Libs, Tests, Examples)

No ETI support requested by packages.


Skipping setup for distribution because TribitsExProj_ENABLE_CPACK_PACKAGING=OFF


Finished configuring TribitsExProj!

-- Configuring done
-- Generating done
-- Build files have been written to: <tribits-dir>/TriBITS/tribits/examples/TribitsExampleProject/build

But I haven't found the deprecation note there. Is there any specific project I need to build in order to check if the depreciation note appears?

[bartlettroscoe]

But I haven't found the deprecation note there. Is there any specific project I need to build in order to check if the depreciation note appears?

@marcinwrobel1986, the deprecation warnings are only seen in downstream customer CMake projects. See above for how these deprecation warnings are manifested.

[marcinwrobel1986]

Ok, @bartlettroscoe so are we able to check if the deprecation warning actually appears in some easy manner?

[bartlettroscoe]

Ok, @bartlettroscoe so are we able to check if the deprecation warning actually appears in some easy manner?

@marcinwrobel1986, yes, just run the test TriBITS_TribitsOldSimpleExampleApp_STATIC_USE_DEPRECATED_TARGETS and look at the output. I am not worried about if the deprecation warning is being generated. (The automated test ensures that with about 100% confidence. I can go over why I can make a statement like that. Also, I confirmed that myself with Trilinos and Albany as described above.)

What I am looking is a review of the code changes and the wording of the deprecation message itself. You can't automated that.

[bartlettroscoe]

I will go ahead and merge this and any review can occur post-merge.

[bartlettroscoe]

@marcinwrobel1986 and @mperrinel, can you both please post a post-merge review of this, even if you don't suggest any changes? To do that, go to the "Files changed" tab and in the upper right click on the green "Review Changes" button. But before you post the review, you can, of course, add comments to the code by clicking "Start a review".

[mperrinel]

Seems good for me. I put some few comments.

[marcinwrobel1986]

@marcinwrobel1986 and @mperrinel, can you both please post a post-merge review of this, even if you don't suggest any changes? To do that, go to the "Files changed" tab and in the upper right click on the green "Review Changes" button. But before you post the review, you can, of course, add comments to the code by clicking "Start a review".

Found some typos in the files which were edited in this PR

[marcinwrobel1986]

I just found some typos in two files which were changed HERE is the pull request.