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: make package_source creates tarballs with missing BuildStatsWrappers.cmake #11976

Closed
3 tasks done
ndellingwood opened this issue Jun 14, 2023 · 12 comments
Closed
3 tasks done
Labels
TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: bug The primary issue is a bug in Trilinos code or tests

Comments

@ndellingwood
Copy link
Contributor

ndellingwood commented Jun 14, 2023

Bug Report

@trilinos/tribits @bartlettroscoe

After using the make package_source option to generate packaged source code tarballs for a configuration of Trilinos, if I try to configure the packaged source with the same configuration I encounter errors due to the build_stats directory missing from the commonTools directory:

Configure error output (click to expand)
-- The Current CXX Standard is : 17

Configuring Trilinos build directory

-- PROJECT_SOURCE_DIR='/Users/ndellin/Research/trilinos/TestPackageSource/trilinos-14.1-Source'
-- PROJECT_BINARY_DIR='/Users/ndellin/Research/trilinos/TestPackageSource/Build'
-- Trilinos_TRIBITS_DIR='/Users/ndellin/Research/trilinos/TestPackageSource/trilinos-14.1-Source/cmake/tribits'
-- TriBITS_VERSION_STRING='0.9 (Dev)'
-- CMAKE_VERSION='3.24.4'
-- CMAKE_GENERATOR='Unix Makefiles'
-- CMAKE_HOST_SYSTEM_NAME='Darwin'
-- Trilinos_HOSTNAME='s1067110'
-- PYTHON_EXECUTABLE='/opt/local/bin/python'

Setting up major user options ...

CMake Error at cmake/CallbackSetupExtraOptions.cmake:91 (include):
  include could not find requested file:

    /Users/ndellin/Research/trilinos/TestPackageSource/trilinos-14.1-Source/commonTools/build_stats/BuildStatsWrappers.cmake
Call Stack (most recent call first):
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1167 (include)
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1292 (tribits_repository_setup_extra_options_runner)
  cmake/tribits/core/package_arch/TribitsProjectImpl.cmake:140 (tribits_process_extra_repos_options_files)
  cmake/tribits/core/package_arch/TribitsProject.cmake:92 (tribits_project_impl)
  CMakeLists.txt:126 (TRIBITS_PROJECT)


CMake Error at cmake/CallbackSetupExtraOptions.cmake:92 (generate_build_stats_wrappers):
  Unknown CMake command "generate_build_stats_wrappers".
Call Stack (most recent call first):
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1167 (include)
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1292 (tribits_repository_setup_extra_options_runner)
  cmake/tribits/core/package_arch/TribitsProjectImpl.cmake:140 (tribits_process_extra_repos_options_files)
  cmake/tribits/core/package_arch/TribitsProject.cmake:92 (tribits_project_impl)
  CMakeLists.txt:126 (TRIBITS_PROJECT)


-- Configuring incomplete, errors occurred!
See also "/Users/ndellin/Research/trilinos/TestPackageSource/Build/CMakeFiles/CMakeOutput.log".

Copying the missing commonTools/build_stats from the original Trilinos source to the packaged source directory allows for configuration and compilation

Steps to Reproduce

  1. SHA1: fba42ce
  2. Steps with just Teuchos enabled using gnu compilers
Reproduction details (click to expand)
# Part 1: Simple configuration of Trilinos and run make package_source to generate packaged source code tarballs
mkdir -p build
cd build

cmake -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_COMPILER=gcc -DCMAKE_INSTALL_PREFIX=$PWD/install -DTrilinos_ENABLE_Teuchos=ON -DTeuchos_ENABLE_TESTS=OFF $TRILINOS_DIR

make package_source
# This will create e.g. trilinos-14.1-Source.tar.gz


# Part 2: Attempt to configure from the packaged source

# Create a new build directory to test the packaged source
mv trilinos-14.1-Source.tar.gz ..
cd ..
tar -zxv trilinos-14.1-Source.tar.gz
mkdir -p build_package_source
cd build_package_source

cmake -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_COMPILER=gcc -DCMAKE_INSTALL_PREFIX=$PWD/install -DTrilinos_ENABLE_Teuchos=ON -DTeuchos_ENABLE_TESTS=OFF $PWD/../trilinos-14.1-Source
# At this point configuration errors occur

Tasks

  • Fix immediate problem in Trilinos (see PR Fix reduced tarball configuration (#11976) #11977)
  • Apply TriBITS fix to TriBITS repo and add missing TriBITS test case
  • Experiment with trying to keep commonTools/build_stats even if the TrilinosBuildStats package is not enabled (e.g. by removing the exclude from CPACK_SOURCE_IGNORE_FILES in the file cmake/CallbackDefineRepositoryPackaging.cmake)
  • [ ] Add new Trilinos tests to the TrilinosInstallTests package to test tarball creation and configure from expanded tarball source tree ... Deferred to Test and support creation of reduced Trilinos tarballs #12024
@ndellingwood ndellingwood added type: bug The primary issue is a bug in Trilinos code or tests TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework labels Jun 14, 2023
@ndellingwood
Copy link
Contributor Author

Adding @dmdunla , he initially reported this on project we work on

@bartlettroscoe
Copy link
Member

@ndellingwood and @dmdunla, I don't think anyone has created a source tarball for Trilinos in many years and there is no automated testing for this, so I am not surprised this failed. With that being the case, I can see what the problem is and put in a fix and set up an automated test as part of the Trilinos test suite for this if that is okay with @sebrowne and @ccober6. Otherwise, if no automated testing is going to be set up and run to protect these feature, then Trilinos should announce that creating source tarballs is not supported.

@sebrowne and @ccober6, what say you? Is Trilinos going to support creating source tarballs going forward?

@ndellingwood
Copy link
Contributor Author

Thanks @bartlettroscoe !
The driver for using the package_source feature and tarballs is that it results in a much reduced size for Trilinos, which can be useful (sometimes necessary) for distributing the code to systems unable to access github/internet

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Jun 15, 2023

Thanks @bartlettroscoe ! The driver for using the package_source feature and tarballs is that it results in a much reduced size for Trilinos, which can be useful (sometimes necessary) for distributing the code to systems unable to access github/internet

@ndellingwood, I am well aware of the advantages. The TriBITS automatic support for trimming down the source tallball based on what packages are enabled (or not enabled for the tarball excludes) was extensively used in CASL VERA project (for removing sensitive packages for some reduced releases as well as reducing source size).

The question is not if this feature is useful (it clearly is for some customers); the question is if Trilinos is willing to support it going forward. But supporting this is not that hard and I could write a basic set of tests to protect this that could run as part of Trilinos PR and nightly testing. (The test would create the source tarball for whatever set of Trilinos packages was enabled and would untar and try to configure the reduced Trilinos source as part of the test.)

@bartlettroscoe
Copy link
Member

The issue is that the directory commonTools/build_stats is also a TriBITS package called TrilinosBuildStats (so it can run a set of automated tests). When the package TrilinosBuildStats is not enabled, TriBITS automatically excludes the package directory for that package (see Creating a tarball of the source tree).

The fix is to guard this code based on the existence of the directory commonTools/build_stats. I will put in a PR against 'develop' that fixes this.

bartlettroscoe added a commit that referenced this issue Jun 15, 2023
This commit was just to streamline local testing to address #11976.
bartlettroscoe added a commit that referenced this issue Jun 15, 2023
This is also a TriBITS Trilinos package so if it is not enabled, then it gets
excluded.  I am not sure this is the right thing to do but for now, it fixes
the immediate problem reported in #11976.
bartlettroscoe added a commit that referenced this issue Jun 15, 2023
This is a temp fix the TriBITS that makes building from a reduced tarball
work.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 15, 2023
This is also a TriBITS Trilinos package so if it is not enabled, then it gets
excluded.  I am not sure this is the right thing to do but for now, it fixes
the immediate problem reported in trilinos#11976.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 15, 2023
This is a temp fix the TriBITS that makes building from a reduced tarball
work.
@bartlettroscoe
Copy link
Member

@ndellingwood and @dmdunla, this should be fixed in PR #11977. The local testing I did seems to suggest this should work. But if you would like to test this for your exact use case before this PR gets merged, that would be greatly appreciated :-)

@ccober6
Copy link
Contributor

ccober6 commented Jun 15, 2023

This sounds like it is a useful feature, and the support cost appears to be low. I would be in favor in setting up testing for this and see how it goes. If it starts becoming a burden, we should reevaluate. @sebrowne what are your thoughts?

@sebrowne
Copy link
Contributor

Seems fine to me

@bartlettroscoe
Copy link
Member

@ndellingwood, @sebrowne, @ccober6,

I added a Tasks section above for the remaining work to put some reasonable checks in place for the ability to create and configure/build/test/install from reduced tarballs.

But note that even though you can create a reduced tarball and configure from it, that does not guarantee that the enabled packages from the reduced source tree will build, pass the tests, and install successfully. The issue is that some Trilinos packages might have have hooks into other non-enabled packages (like grabbing tests files from another package) that may be missing from the untarred reduced source tree. The only way to know for sure would be to build, test, and install from the reduced tarball. (We did that on the CASL VERA project but it requires some more testing infrastructure to do that. It is actually not that hard to set up but that involves a lot more build and test time.)

@dmdunla
Copy link
Contributor

dmdunla commented Jun 15, 2023 via email

@bartlettroscoe
Copy link
Member

We configure first so that all of the dependencies are resolved. Then, when we call make package_source we (seem) to get everything that we need across the packages that we configured for. If there is a different approach we should use, please clarify. Thanks!

@dmdunla,

That is it which is consistent with:

Just make sure you follow that carefully (especially the part about running git clean -fd -x first to clean up everything in case you have other stuff in your source tree).

trilinos-autotester added a commit that referenced this issue Jun 16, 2023
…-fix

Automatically Merged using Trilinos Pull Request AutoTester
PR Title: Fix reduced tarball configuration (#11976)
PR Author: bartlettroscoe
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 16, 2023
…s:develop' (0f5e2d6).

* trilinos-develop:
  TriBITS: Temp fix for configuring from reduced tarball (trilinos#11976)
  Make work with missing commonTools/build_stats (trilinos#11976)
  Add a spec for all packages + complex<float>
  Update MueLu_FactoryManager_def.hpp
  KokkosKernels: Don't list include for non-existant 'batched' build dir (trilinos#11966)
  MueLu: Enabling better dumps for Maxwell1
  MueLu RefMaxwell: Accept "aggregation: phase3 avoid singletons"
  Tpetra: Carl's fixes
  Tpetra: Improving std::move()-ability of BlockMultiVector
  Tpetra: Improving std::move()-ability of BlockMultiVector
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 16, 2023
…s:develop' (0f5e2d6).

* trilinos-develop:
  TriBITS: Temp fix for configuring from reduced tarball (trilinos#11976)
  Make work with missing commonTools/build_stats (trilinos#11976)
  Add a spec for all packages + complex<float>
  Update MueLu_FactoryManager_def.hpp
  KokkosKernels: Don't list include for non-existant 'batched' build dir (trilinos#11966)
  MueLu: Enabling better dumps for Maxwell1
  MueLu RefMaxwell: Accept "aggregation: phase3 avoid singletons"
  Tpetra: Carl's fixes
  Tpetra: Improving std::move()-ability of BlockMultiVector
  Tpetra: Improving std::move()-ability of BlockMultiVector
@bartlettroscoe
Copy link
Member

@ndellingwood and @dmdunla,

With the merge of PR #11977 to 'develop', this should be working again. If it is not, please let me know.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jun 22, 2023
…ilinos#11976)

This failing test demonstrates a defect that got added with the recent TriBITS
refactorings.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jun 22, 2023
…ke file (trilinos/Trilinos#11976)

This fixes cases where a dirty/incomplete source directory for a package
remains but does not contain the Dependencies.cmake file.  This was a use case
I hit while experimenting with how to fix trilinos/Trilinos#11976.

This replaces a temp commit made to the Trilinos 'develop' branch.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jun 22, 2023
…ke file (trilinos/Trilinos#11976)

This fixes cases where a dirty/incomplete source directory for a package
remains but does not contain the Dependencies.cmake file.  This was a use case
I hit while experimenting with how to fix trilinos/Trilinos#11976.

This replaces a temp commit made to the Trilinos 'develop' branch.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jun 27, 2023
…ke file (trilinos/Trilinos#11976)

This fixes cases where a dirty/incomplete source directory for a package
remains but does not contain the Dependencies.cmake file.  This was a use case
I hit while experimenting with how to fix trilinos/Trilinos#11976.

This replaces a temp commit made to the Trilinos 'develop' branch.
bartlettroscoe added a commit that referenced this issue Jun 28, 2023
Origin repo remote tracking branch: 'github/master'
Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git'
Git describe: vera-release-3.5-start-1806-ge88b8af8

At commit:

commit 1a9b519a5cd6005d299be5d07c8823a6a87c974e
Author:  Roscoe A. Bartlett <[email protected]>
Date:    Tue Jun 27 17:28:15 2023 -0600
Summary: Only submit warning for existing ignored package dir in WARNING mode (#11976)
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
…nos#11976)"

This reverts commit 40e37d9.

There is a better fix in TriBITS 'master' that will be merged in from the
snapshot branch.  I needed to revert this or it would be a conflict.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
Brings in a better fix to the issue reported in trilinos#11976 on the TriBITS side
(not the Trilinos side).

Also brings in some other TriBITS updates.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
This reverts commit 7c630d7.

I am going to remove the exclude entry instead and put in excludes for the
wrapper TriBITS package CMakeLists.txt and cmake/Dependencies.cmake files
instead so that it will not show up as a TriBITS package.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
…s#11976)

This allows the TrilinosBuildStats test package to be removed but still keep
the build_stats files to provide that functionality to tarball customers.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
We removed autotools files from the Trilinos tarball 14 years ago because
those we kept those around but they were out of date and we did not want
customers using them.  But since they have been stripped out of every Trilinos
package (except Zoltan), there is no value in including this.  Also, the
latest TriBITS snapshot removed this macro.

The only package that still has old autotools files in Zoltan.  These have
been stripped out of every other Trilinos package.  For Zolan, we now leave
the autotools files in the tarball.

However, I did not remove the usage TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from
Kokkos or KokkosKernels so I could avoid the proces of having to get these
changes into the latest Kokkos 4.1 release.  I will post PRs to pull out the
usage of TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from the 'develop' branches of the
native repos so this can be cleaned up later.  For now, I will create an empty
macro TRIBITS_EXCLUDE_AUTOTOOLS_FILES() to allow these packages to be
unchanged.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 7, 2023
…nos#11976)"

This reverts commit 40e37d9.

There is a better fix in TriBITS 'master' that will be merged in from the
snapshot branch.  I needed to revert this or it would be a conflict.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 7, 2023
This reverts commit 7c630d7.

I am going to remove the exclude entry instead and put in excludes for the
wrapper TriBITS package CMakeLists.txt and cmake/Dependencies.cmake files
instead so that it will not show up as a TriBITS package.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 7, 2023
…s#11976)

This allows the TrilinosBuildStats test package to be removed but still keep
the build_stats files to provide that functionality to tarball customers.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 7, 2023
We removed autotools files from the Trilinos tarball 14 years ago because
those we kept those around but they were out of date and we did not want
customers using them.  But since they have been stripped out of every Trilinos
package (except Zoltan), there is no value in including this.  Also, the
latest TriBITS snapshot removed this macro.

The only package that still has old autotools files in Zoltan.  These have
been stripped out of every other Trilinos package.  For Zolan, we now leave
the autotools files in the tarball.

However, I did not remove the usage TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from
Kokkos or KokkosKernels so I could avoid the proces of having to get these
changes into the latest Kokkos 4.1 release.  I will post PRs to pull out the
usage of TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from the 'develop' branches of the
native repos so this can be cleaned up later.  For now, I will create an empty
macro TRIBITS_EXCLUDE_AUTOTOOLS_FILES() to allow these packages to be
unchanged.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 8, 2023
…rilinos#12024)

This sets up a test that would have caught the problem in trilinos#11976 and will
provide some protection for reduced tarball creation going forward.

NOTE: Currently, this test will only be added if configure settings are passed
through `-D${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE=<cache-file>.cmake`.
There is no way to pass that file (or thoses files) through `-C
<cache-file>.make`.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 9, 2023
…rilinos#12024)

This sets up a test that would have caught the problem in trilinos#11976 and will
provide some protection for reduced tarball creation going forward.

NOTE: Currently, this test will only be added if configure settings are passed
through `-D${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE=<cache-file>.cmake`.
There is no way to pass that file (or thoses files) through `-C
<cache-file>.make`.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 9, 2023
…rilinos#12024)

This sets up a test that would have caught the problem in trilinos#11976 and will
provide some protection for reduced tarball creation going forward.

NOTE: Currently, this test will only be added if configure settings are passed
through `-D${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE=<cache-file>.cmake`.
There is no way to pass that file (or thoses files) through `-C
<cache-file>.make`.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 9, 2023
bartlettroscoe added a commit that referenced this issue Jul 13, 2023
…ts-update

Base CMakeLists.txt cleanup, reduced tarball testing, CI enable tweak, TriBITS update (#11545, #11976)
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
…nos#11976)"

This reverts commit 40e37d9.

There is a better fix in TriBITS 'master' that will be merged in from the
snapshot branch.  I needed to revert this or it would be a conflict.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
This reverts commit 7c630d7.

I am going to remove the exclude entry instead and put in excludes for the
wrapper TriBITS package CMakeLists.txt and cmake/Dependencies.cmake files
instead so that it will not show up as a TriBITS package.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
…s#11976)

This allows the TrilinosBuildStats test package to be removed but still keep
the build_stats files to provide that functionality to tarball customers.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
We removed autotools files from the Trilinos tarball 14 years ago because
those we kept those around but they were out of date and we did not want
customers using them.  But since they have been stripped out of every Trilinos
package (except Zoltan), there is no value in including this.  Also, the
latest TriBITS snapshot removed this macro.

The only package that still has old autotools files in Zoltan.  These have
been stripped out of every other Trilinos package.  For Zolan, we now leave
the autotools files in the tarball.

However, I did not remove the usage TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from
Kokkos or KokkosKernels so I could avoid the proces of having to get these
changes into the latest Kokkos 4.1 release.  I will post PRs to pull out the
usage of TRIBITS_EXCLUDE_AUTOTOOLS_FILES() from the 'develop' branches of the
native repos so this can be cleaned up later.  For now, I will create an empty
macro TRIBITS_EXCLUDE_AUTOTOOLS_FILES() to allow these packages to be
unchanged.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
Origin repo remote tracking branch: 'github/master'
Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git'
Git describe: vera-release-3.5-start-1806-ge88b8af8

At commit:

commit 1a9b519a5cd6005d299be5d07c8823a6a87c974e
Author:  Roscoe A. Bartlett <[email protected]>
Date:    Tue Jun 27 17:28:15 2023 -0600
Summary: Only submit warning for existing ignored package dir in WARNING mode (trilinos#11976)
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
…rilinos#12024)

This sets up a test that would have caught the problem in trilinos#11976 and will
provide some protection for reduced tarball creation going forward.

NOTE: Currently, this test will only be added if configure settings are passed
through `-D${CMAKE_PROJECT_NAME}_CONFIGURE_OPTIONS_FILE=<cache-file>.cmake`.
There is no way to pass that file (or thoses files) through `-C
<cache-file>.make`.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Sep 23, 2023
…bits

This brings in the following TriBITS PRs:

* TriBITSPub/TriBITS#591: Allow 100% raw CMake to be used in a
  TriBITS-compliant package (TriBITSPub/TriBITS#582)

* TriBITSPub/TriBITS#588: gitdist: Pass in '-c color.status=always' when
  --dist-no-color is not added

* TriBITSPub/TriBITS#590: Fix: gitdist: dist-repo-status: Display tag or SHA1
  instead of 'HEAD'

* TriBITSPub/TriBITS#587: gitdist: dist-repo-status: Display tag or SHA1
  instead of 'HEAD'

* TriBITSPub/TriBITS#586: More cleanup of packaging support
  (trilinos#11976)

This goes back to TriBITS commits from June 2023.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: bug The primary issue is a bug in Trilinos code or tests
Projects
Development

No branches or pull requests

5 participants