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

Dropping support for Makefile.export.* files with TriBITS/Trilinos refactoring to support modern CMake features #8498

Closed
8 tasks done
bartlettroscoe opened this issue Dec 17, 2020 · 43 comments
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. impacting: documentation The issue is primarily about documentation vs. a problem with the build or tests MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. PA: Framework Issues that fall under the Trilinos Framework Product Area TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: enhancement Issue is an enhancement, not a bug

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Dec 17, 2020

CC: @trilinos/framework, @trilinos/trilinos-product-owners

Blocked By: #8401

As part of the imminent TriBITS refactoring described in TriBITSPub/TriBITS#299 to use modern CMake target-based features will that will be enabled by the upgrade of the minimum required CMake (see #8401) be that the current implementation for generating export makefiles (i.e. Makefile.export.* files) will be broken and it may be difficult to bring it back.

We discussed this topic and the topic of dropping support for Makefile.export.* files at the Trilinos Product Leaders Meeting on 12/16/2020. The action items that came out of that were:

@bartlettroscoe bartlettroscoe added type: enhancement Issue is an enhancement, not a bug impacting: documentation The issue is primarily about documentation vs. a problem with the build or tests PA: Framework Issues that fall under the Trilinos Framework Product Area labels Dec 17, 2020
@bartlettroscoe bartlettroscoe added this to Backlog in Trilinos Framework via automation Dec 17, 2020
@mathstuf
Copy link

Create a plan for customers to transition to no Makefile.export.Trilinos files. (I.e., either switch to CMake or manually get the list of libraries you need such as with CMAKE_EXPORT_COMPILE_COMMANDS).

Making this way more general could help: https://gitlab.kitware.com/paraview/paraview/-/blob/master/Clients/CommandLineExecutables/paraview-config.in. That is heavily ParaView-specific, but is an outline of what would be needed for something more generally.

@csiefer2
Copy link
Member

csiefer2 commented Jan 6, 2021

Aw, man. Those things are useful.

@bartlettroscoe
Copy link
Member Author

Aw, man. Those things are useful.

@csiefer2, the goal would be to move customers to use CMake for their projects using Trilinos.

@csiefer2
Copy link
Member

csiefer2 commented Jan 7, 2021

Guess that just means I need to learn how to write cmake from scratch for little toy codes that use Trilinos...

Do we have any documentation for that?

@rppawlo
Copy link
Contributor

rppawlo commented Jan 7, 2021

Do we have any documentation for that?

Before these changes are pushed into Trilinos, we will have instructions and an example in Trilinos (probably in the demos directory). It's only a few lines of code to pull trilinos into a new cmake app.

@bartlettroscoe
Copy link
Member Author

Do we have any documentation for that?

@csiefer2, we will. See above.

@csiefer2
Copy link
Member

@csiefer2, we will. See above.

Missed that! Cool! Thanks!

@bartlettroscoe bartlettroscoe added the TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework label Feb 11, 2021
@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Mar 8, 2021

Related to:

  • SEPW-213
  • SEPW-214

@bartlettroscoe
Copy link
Member Author

Looks like there is some urgency for at least and example using find_pakage(Trilinos) as per #8857. I can't believe there is no documentation or even an hint in Trilinos about how to do this. (In my defense, I did not write any of the CMake code in TriBITS or Trilinos to support TrilinosConfig.cmake. But I will have to rewrite this as part of TriBITSPub/TriBITS#299.)

@bartlettroscoe
Copy link
Member Author

CC: @rppawlo, @jwillenbring

Turns out an example of how to build against Trilinos using CMake already exists under demos/buildAgainstTrilinos. I have no idea if that still works or not. I did not even know that this example even existed!

I can test and update this example to make sure it works but then we need to protect this in PR testing.

@trilinos/framework, could you consider setting up the PR builds to do an install of Trilinos and then build and test this example project? We need to ensure that example always works and it will also do some level of installation testing.

bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
This now builds without errors with Clang 10.0.  (There was a build error
before this update.)

* Require CMake 3.17.1 which is the same minimum now required by Trilinos

* Updated the documentation to use Markdown and use raw CMake and no do-cmake
  script

* Deleted unneccasary do-cmake script

* Deleted usage of a 'Trilinos_PREFIX' var and just use built-in CMake var
  CMAKE_PREFIX_PATH directly as documented in CMake documentation for
  'find_package()'

* Turn off warning from Kokkos about not suupporting C++ extensions

* Fix build error with source code
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
…8498)

If one is going to demonstrate how to use CTest one should also show good
software engineering practicies like minimizing duplication.

I also set the PROCESSORS property so that ctest -j<N>.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
This now builds without errors with Clang 10.0.

* Require CMake 3.17.1 which is the same minimum now required by Trilinos

* Updated the documentation to use Markdown and use raw CMake and no do-cmake
  script

* Deleted unneccasary do-cmake script

* Deleted usage of a 'Trilinos_PREFIX' var and just use built-in CMake var
  CMAKE_PREFIX_PATH directly as documented in CMake documentation for
  'find_package()'

* Turned off warning from Kokkos about not suupporting C++ extensions

* Fixed build error with the source code usage of Teuchos
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
…8498)

If one is going to demonstrate how to use CTest one should also show good
software engineering practicies like minimizing duplication.

I also set the PROCESSORS property so that ctest -j<N>.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
…8498)

I removed the author and just gave the framework team email address for a
contact.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Mar 10, 2021
@bartlettroscoe
Copy link
Member Author

Looks good! Nice catch on the documentation!

I merged trilinos/trilinos.github.io#49 and now the page:

looks to be updated including the link there to the updated text file:

Last step is to merge an updated snapshot of TriBITS that removes support for . Makefile.export.* files on Thursday, June 10 :-)

bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 14, 2021
Disabling support for Makefile.export.* files in Trilinos while TriBITS is
refactored in TriBITSPub/TriBITS#299 that will remove support for this
permanently.
@bartlettroscoe
Copy link
Member Author

PR #9284 removes support for Makefile.export.* files from Trilinos. Ripping out support for Makefile.export.* in TriBITS will actually break a lot of tests because of the dependence of the TribitsExampleProject WrapExternal package that uses a generated Makefile.export.* file to glue things together. I will need to refactor that exact to use either CMake or just a raw link line before I rip out support for Makefile.export.*. By removing support from Trilinos we can deal with any blow back that that may be coming. (who were not on the email lists or were on the emails lists and just did not read the emails).

trilinos-autotester added a commit that referenced this issue Jun 18, 2021
…kefile-export-files

Automatically Merged using Trilinos Pull Request AutoTester
PR Title: Disable support for Makefile.export.* files (#8498)
PR Author: bartlettroscoe
@bartlettroscoe
Copy link
Member Author

With the merge of PR #9284 just now, Trilinos 'develop' no longer supports the installation of Makefile.export.* files. I will send out one last set of emails to the various email lists announcing this.

jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 19, 2021
…s:develop' (16c177b).

* trilinos-develop: (71 commits)
  Tpetra: Remove some output from the Bug7758 test
  MueLu Stratimikos adapter: Enable half precision for factory-style PLs
  Tpetra: remove some deprecated usage
  ROL: implement the apply function for Thyra Vector
  Piro: changes to ROL adapters comply with ROL changes
  Piro: bug-fix in Piro::NOX_Solver
  Ifpack2: disabling tests causing build errors with extended scalar types (see issue trilinos#9280).
  Ifpack2: cleaning up unused variables in tests.
  Ctest: Adding Amesos2/Belos tests
  Ctest: Stuff failing on ride that worked on ascicgpu
  Ctest: Enabling non-UVM Ifpack2 tests
  Ifpack2: changing GO to the one in Tpetra_Details_DefaultTypes.hpp.
  Disable support for Makefile.export.* files (trilinos#8498)
  Tpetra: remove unused variable (copied too many times when breaking up a function)
  ats2: Comment out listing of long-broken XL builds (trilinos#9270, trilinos#7376)
  Ifpack2: adding missing logic for new tests.
  Belos: writing tests for 'long double' and 'float128' ScalarType.
  STK: Snapshot 06-11-21 17:50
  Tpetra: remove comments that don't apply to HIP
  Tpetra: Use HIPSpace for HIPWrapperNode
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 19, 2021
…s:develop' (16c177b).

* trilinos-develop: (71 commits)
  Tpetra: Remove some output from the Bug7758 test
  MueLu Stratimikos adapter: Enable half precision for factory-style PLs
  Tpetra: remove some deprecated usage
  ROL: implement the apply function for Thyra Vector
  Piro: changes to ROL adapters comply with ROL changes
  Piro: bug-fix in Piro::NOX_Solver
  Ifpack2: disabling tests causing build errors with extended scalar types (see issue trilinos#9280).
  Ifpack2: cleaning up unused variables in tests.
  Ctest: Adding Amesos2/Belos tests
  Ctest: Stuff failing on ride that worked on ascicgpu
  Ctest: Enabling non-UVM Ifpack2 tests
  Ifpack2: changing GO to the one in Tpetra_Details_DefaultTypes.hpp.
  Disable support for Makefile.export.* files (trilinos#8498)
  Tpetra: remove unused variable (copied too many times when breaking up a function)
  ats2: Comment out listing of long-broken XL builds (trilinos#9270, trilinos#7376)
  Ifpack2: adding missing logic for new tests.
  Belos: writing tests for 'long double' and 'float128' ScalarType.
  STK: Snapshot 06-11-21 17:50
  Tpetra: remove comments that don't apply to HIP
  Tpetra: Use HIPSpace for HIPWrapperNode
  ...
@bartlettroscoe
Copy link
Member Author

I just sent out the final set of emails on this topic to the Trilinos developers and users list including the one shown below.


From: Bartlett, Roscoe A
Sent: Monday, June 21, 2021 5:33 PM
To: trilinos-announce@trilinos.org; trilinos-users@trilinos.org
Subject: RE: Trilinos dropping support for Makefile.export.* files

Hello Trilinos users,

Again, if you are not using the installed Makefile.export.Trilinos or the package-level Makefile.export. files, then please ignore.

After some delays, the commit that removed support for the installed Makefile.export.* files was merged to Trilinos ‘develop’ last Friday, June 18. It has yet to be merged to the Trilinos ‘master’ branch. (Looks the ‘develop’ branch has not been merged to the ‘master’ branch in about a month.)

If you have any questions, please let me know, or better yet, please comment in:

https://github.com/trilinos/Trilinos/issues/8498

Otherwise, no more emails will be sent out on this topic.

Thanks,

-Ross

@bartlettroscoe
Copy link
Member Author

My meeting with Scott Collis got pushed back from today 6/22/2021 to 7/9/2021. Therefore, I am not going to hold up this Issue anymore. I will go ahead and put this issue in review and mark it off #367. I fully expect we will get some pushback but we will deal with that later.

@bartlettroscoe bartlettroscoe moved this from In Progress to In Review in Trilinos TriBITS Refactor Jun 22, 2021
seamill pushed a commit to seamill/Trilinos that referenced this issue Jul 28, 2021
…develop' (51336fe).

* potential-trilinos-develop: (57 commits)
  Geminga: Another attempt to get valgrind suppressions working
  STK: Snapshot 03-12-21 10:28
  MiniEM: Enable reuse, test it
  Remove older and more complex buildAgainstTrilinos example (trilinos#8498)
  Update to point to demos/simpleBuildAgainstTrilinos (trilinos#8498)
  Add explicit COMPONENTS (trilinos#8498)
  Simpler one-directory build example using Trilinos (trilinos#8498)
  Intrepid2: UVM-free MonolothicExecutable Tests (trilinos#8852)
  Link to local INSTALL.rst on same branch (trilinos#8489)
  Update INSTALL.rst with link to demos/buildAgainstTrilinos (trilinos#8498)
  Eliminate duplication in tests and set PROCESSORS property (trilinos#8498)
  Modernize for newer CMake and fix compile error (trilinos#8498)
  Piro: add 'Optimizer Iteration Number' to Piro_ThyraProductME
  Tpetra:  remove UVM dependence from test (trilinos#8830)
  tpetra:  trilinos#8794 trilinos#8804  test to pick threshold timed out in PR testing. Lowering the upper bound of testing to fit run into PR testing time. Threshold is close to 400, so lowering upper bound of testing should be OK.
  Correct gcc module loaded for tlcc2 builds
  MueLu: Add phase2a agg factor to param list for hybrid aggregation
  Piro: move the Enable Explicit Matrix Transpose option to the app parameter list
  Piro: print response even for iteration 0
  std: -> std::
  ...
seamill pushed a commit to seamill/Trilinos that referenced this issue Jul 28, 2021
…develop' (4d10f3a).

* trilinos/develop:
  tpetra:  adding syncs of import buffer before unpacking trilinos#9116
  Make enable of Fortran optional and note about only needing C++ (trilinos#8498)
  Eliminate warnings when using BUILD_SHARED_LIBS=ON (trilinos#8498)
  Add missing -D to cmake line (trilinos#8498)
  add missing copyright statements
  Added missing headers to CMakeList.txt. Added summarize function to Reduced_Objective_SimOpt. Removed white space in Reduced_Constraint_SimOpt.
  Replaced 'and' with && and 'or' with || for MSVC compatibility.
  Set test only to build when ROL_ParameterList=simple. Imported streaming operator for displaying vector
  Added <numeric> header.
  Disabled use of fenv.h and feenableexcept() in tests and examples.
  Moved new native ROL::ParameterList to src/compatibility/simple/parameterlist and added CMake config options. Added get<T> and isSublist methods to class. Added ROL::getArrayFromParameterString free function to follow Teuchos design. Updated parameterlist to use new features. test/algorithm/TypeB/test_01.cpp can now be compiled and runs successfully with the new native list
  Missed one UpdateType.
  A few typo fixes.
  Added minimal ROL-native implementation of parameter list
  Update Version-2.0.md
  Fixed typo in Version 1.0 solver.
seamill pushed a commit to seamill/Trilinos that referenced this issue Jul 28, 2021
…develop' (7591b32).

* trilinos/develop: (77 commits)
  zoltan2:  fix memory leak when sizeof(SCOTCH_Num) == sizeof(lno_t) trilinos#9312
  Tpetra: Remove some output from the Bug7758 test
  MueLu Stratimikos adapter: Enable half precision for factory-style PLs
  Tpetra: remove some deprecated usage
  Fixed some deprecated code
  MueLu Thyra adapter: Allow construction of half precision operator
  ROL: implement the apply function for Thyra Vector
  Piro: changes to ROL adapters comply with ROL changes
  Piro: bug-fix in Piro::NOX_Solver
  MueLu: Print Scalar in MG Summary for high and extreme verbosity
  Ifpack2: disabling tests causing build errors with extended scalar types (see issue trilinos#9280).
  Ifpack2: cleaning up unused variables in tests.
  Ctest: Adding Amesos2/Belos tests
  Ctest: Stuff failing on ride that worked on ascicgpu
  Ctest: Enabling non-UVM Ifpack2 tests
  Ifpack2: changing GO to the one in Tpetra_Details_DefaultTypes.hpp.
  Disable support for Makefile.export.* files (trilinos#8498)
  Tpetra: remove unused variable (copied too many times when breaking up a function)
  ats2: Comment out listing of long-broken XL builds (trilinos#9270, trilinos#7376)
  Ifpack2: adding missing logic for new tests.
  ...
@jwillenbring jwillenbring removed this from Backlog in Trilinos Framework Aug 20, 2021
@bartlettroscoe bartlettroscoe moved this from In Review to Done in Trilinos TriBITS Refactor Sep 9, 2021
@github-actions
Copy link

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 Jun 25, 2022
@bartlettroscoe bartlettroscoe removed the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Jun 27, 2022
@bartlettroscoe
Copy link
Member Author

https://github.com/orgs/trilinos/teams/framework, could you consider setting up the PR builds to do an install of Trilinos and then build and test this example project? We need to ensure that example always works and it will also do some level of installation testing.

FYI: The upgraded example demos/simpleBuildAgainstTrilinos has been under automated PR testing for some time now as of the merge of #10813.

bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jun 29, 2023
…BITSPub/TriBITS#299)

Support for Makefile.export.* files was removed from TriBITS and snaphsotted
into Trilinos a long time ago.  This support was removed in the Trilinos 14.0
release.  There is no reason to keep this cache variable anymore.  It just
clutters up the base CMakeLists.txt file.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 7, 2023
…BITSPub/TriBITS#299)

Support for Makefile.export.* files was removed from TriBITS and snaphsotted
into Trilinos a long time ago.  This support was removed in the Trilinos 14.0
release.  There is no reason to keep this cache variable anymore.  It just
clutters up the base CMakeLists.txt file.
JacobDomagala pushed a commit to NexGenAnalytics/Trilinos that referenced this issue Aug 4, 2023
…BITSPub/TriBITS#299)

Support for Makefile.export.* files was removed from TriBITS and snaphsotted
into Trilinos a long time ago.  This support was removed in the Trilinos 14.0
release.  There is no reason to keep this cache variable anymore.  It just
clutters up the base CMakeLists.txt file.
@github-actions
Copy link

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 Sep 30, 2023
Copy link

github-actions bot commented Nov 1, 2023

This issue was closed due to inactivity for 395 days.

@github-actions github-actions bot added the CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. label Nov 1, 2023
@github-actions github-actions bot closed this as completed Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. impacting: documentation The issue is primarily about documentation vs. a problem with the build or tests MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. PA: Framework Issues that fall under the Trilinos Framework Product Area TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: enhancement Issue is an enhancement, not a bug
Projects
Development

No branches or pull requests

5 participants