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

Tribits enabling more packages than necessary. #2011

Open
bartgol opened this issue Nov 21, 2017 · 6 comments
Open

Tribits enabling more packages than necessary. #2011

bartgol opened this issue Nov 21, 2017 · 6 comments
Labels
DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot.

Comments

@bartgol
Copy link
Contributor

bartgol commented Nov 21, 2017

@trilinos/tribits

Expectations

I think the packages listed in the TEST_REQUIRED_DEP_PACKAGES list inside the package's Dependencies.cmake file should only be enabled if the testing for that package is enabled.

Current Behavior

It appears that the packages in the list TEST_REQUIRED_DEP_PACKAGES are currently enabled, regarldess of whether the testing is on for the package.

Motivation and Context

I am trying to build a version of trilinos without the Epetra stack, but some packages list Epetra in their TEST_REQUIRED_DEP_PACKAGES variable, in the Dependencies.cmake file. This variables is apparently treated the same as LIB_REQUIRED_DEP_PACKAGES when it comes to enabling upstream packages, regardless of the build configuration (in particular, regardless of whether tests are enabled).

Definition of Done

Anything that does not cause test-related dependencies to be built when tests are not enabled.

Possible Solution

Inside the TRIBITS_ENABLE_UPSTREAM_SE_PACKAGES macro, guard the lines

    FOREACH(DEP_PKG ${${PACKAGE_NAME}_TEST_REQUIRED_DEP_PACKAGES})
      TRIBITS_PRIVATE_ENABLE_DEP_PACKAGE(${PACKAGE_NAME} ${DEP_PKG} REQUIRED)
    ENDFOREACH()

    FOREACH(DEP_PKG ${${PACKAGE_NAME}_TEST_OPTIONAL_DEP_PACKAGES})
      TRIBITS_PRIVATE_ENABLE_DEP_PACKAGE(${PACKAGE_NAME} ${DEP_PKG} OPTIONAL)
    ENDFOREACH()

with something like IF(${PACKAGE_NAME}_ENABLE_TESTS} OR ${PROJECT_NAME}_ENABLE_TESTS}), or something like that.

Steps to Reproduce

Configure script:

rm -rf CMakeFiles
rm -f  CMakeCache.txt

cmake -Wno-dev                                        \
  -D CMAKE_BUILD_TYPE:STRING=RELEASE                  \
  -D CMAKE_INSTALL_PREFIX:PATH=${INSTALL_DIR}         \
  -D CMAKE_CXX_COMPILER:STIRNG=mpicxx                 \
  -D CMAKE_C_COMPILER:STIRNG=mpicc                    \
  -D CMAKE_Fortran_COMPILER:STRING=mpifort            \
  \
  -D Trilinos_VERBOSE_CONFIGURE:BOOL=OFF              \
  -D Trilinos_ENABLE_ALL_PACKAGES:BOOL=OFF            \
  -D Trilinos_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=OFF   \
  -D Trilinos_ENABLE_SECONDARY_TESTED_CODE:BOOL=OFF   \
  -D Trilinos_ENABLE_TESTS:BOOL=OFF                   \
  -D Trilinos_ENABLE_EXAMPLES:BOOL=OFF                \
  -D Trilinos_ENABLE_OpenMP:BOOL=ON                   \
  \
  -D Trilinos_ENABLE_Amesos2:BOOL=ON                  \
  -D Trilinos_ENABLE_Belos:BOOL=ON                    \
  -D Trilinos_ENABLE_Ifpack2:BOOL=ON                  \
  -D Trilinos_ENABLE_Kokkos:BOOL=ON                   \
  -D Trilinos_ENABLE_MueLu:BOOL=ON                    \
  -D Trilinos_ENABLE_NOX:BOOL=ON                      \
  -D Trilinos_ENABLE_Piro:BOOL=ON                     \
  -D Trilinos_ENABLE_Sacado:BOOL=OFF                  \
  -D Trilinos_ENABLE_Stratimikos:BOOL=ON              \
  -D Trilinos_ENABLE_Tempus:BOOL=ON                   \
  -D Trilinos_ENABLE_Teuchos:BOOL=ON                  \
  -D Trilinos_ENABLE_Thyra:BOOL=ON                    \
  -D Trilinos_ENABLE_Tpetra:BOOL=ON                   \
  \
  -D Piro_ENABLE_Epetra:BOOL=OFF                      \
  \
  -D TPL_ENABLE_MPI:BOOL=ON                           \
  \
  ${SOURCE_DIR}

(replacing SOURCE_DIR and INSTALL_DIR with proper directories).

Your Environment

  • Relevant repo SHA1s: Current master: 1a24197
  • Relevant configure flags or configure script: see above
  • Operating system and version: RHEL7 (7.4)
  • Compiler and TPL versions: gcc 4.8.5

Additional Information

I am not enabling Epetra, but Tempus causes Epetra to be enabled, even if Epetra is only required for the testing. I can confirm the cause of the problem are required test packages, since I see the log message

-- Setting Trilinos_ENABLE_Epetra=ON because Tempus has a required dependence on Epetra
and Tempus depends on Epetra only for the tests.

@mhoemmen
Copy link
Contributor

@bartgol Is this properly just a TriBITS issue? If so, I'll close this issue and direct further discussion to TriBITSPub/TriBITS#243 . Thanks!

@bartgol
Copy link
Contributor Author

bartgol commented Nov 27, 2017

Maybe? I've never dived deep into tribits, so I'm not 100% sure. More likely than not though.

@mhoemmen
Copy link
Contributor

@bartgol On second thought, it might be better to leave this Trilinos issue open, because Trilinos won't see the fix until TriBITS fixes it and snapshots into Trilinos. Please do continue discussion on the TriBITS issue, though. Thanks!

@bartlettroscoe
Copy link
Member

As I mentioned in TriBITSPub/TriBITS#243, this unfortunately is a long known defect (see TriBITSPub/TriBITS#56). As mentioned in TriBITSPub/TriBITS#56, it could be quite difficult to update all of the TriBITS projects for this change.

@github-actions
Copy link

github-actions bot commented May 1, 2021

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label May 1, 2021
@bartlettroscoe bartlettroscoe added DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot. and removed MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. labels May 3, 2021
@bartlettroscoe
Copy link
Member

FYI: This will actually get fixed finally in the next couple of mounts as part of TriBITSPub/TriBITS#367

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot.
Projects
None yet
Development

No branches or pull requests

3 participants