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

Provide feedback on TriBITS package dependencies data-structures #377

Closed
2 tasks
bartlettroscoe opened this issue Jun 11, 2021 · 11 comments
Closed
2 tasks

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Jun 11, 2021

@fnrizzi, @MikolajZuzek, @marcinwrobel1986, @keitat

Parent Issue: #63

Description

This story will be to get feedback from NextGen Analytics staff on the basic data-structures and functions for the TriBITS packages dependencies system that is so critical to managing the complex internal and external dependencies of a large CMake project. It is critical to understand these basic data-structures and the code that creates and manipulates them in order to have anything that resembles TriBITS.

NOTE: This code has nothing to do with the internally called commands like tribits_project(), tribits_package(), tribits_add_library() or tribits_add_executable() . That is, this code is completely 100% orthogonal to how targets are created in the CMake project. It only determines the set of external and internal packages that will be enabled and the order and location of the CMakeLists.txt file that need to be included with add_subdirectory() commands.

The package dependency data-structures themselves (which are currently in a state of being refactored as per #63) are described in:

(NOTE: Some of the variables like ${PACKAGE_NAME}_PACKAGE_STATUS don't exist yet. This documentation is part of the planning for the refactoring to support #63.)

The code that reads in the *.cmake fragment files and creates the internal data-structures for the list of packages and their dependencies is rooted in the macro TRIBITS_READ_ALL_PROJECT_DEPS_FILES_CREATE_DEPS_GRAPH() and its function call graph is given in:

The <filename>:<line> location of each of these macros and functions are given in the extracted documentation blocks. The bulk of the code in this call graph are defined in just a few *.cmake files (as you can see in the documentation).

A CMake -P utility script that runs this code to create and export the dependency graph for any TriBITS project as an XML file is given in:

(See documentation at the top of that file. Also see the configure option -D <Project>_DEPS_XML_OUTPUT_FILE=<SOME_FILE_PATH>. And there the Python module TribitsDependencies.py makes it easy to read and process that XML file in Python.)

See some unit tests for TRIBITS_READ_ALL_PROJECT_DEPS_FILES_CREATE_DEPS_GRAPH() in:

The best way to see these data-structures is to configure as TriBITS-based project (like Trilinos or TribitsExampleProject) with the configure options -D<Project>_DUMP_PACKAGE_DEPENDENCIES=ON and -D<Project>_DUMP_FORWARD_PACKAGE_DEPENDENCIES=ON (and we can add another option to print all the package arch dependency data-structures).

After the package dependency data-structure is created by TRIBITS_READ_ALL_PROJECT_DEPS_FILES_CREATE_DEPS_GRAPH() and some initial set of enables/disables are set by the user, the enable/disable logic is applied by the macro TRIBITS_ADJUST_PACKAGE_ENABLES(). The behaviors of enable/disable logic are spelled out in TriBITS Dependency Handling Behaviors. The unit tests for this logic is given in:

Definition of Done

  • Have NGA staff read over all of this code and tests and provide comments on comprehensibility and refactoring suggestions to make more clear.
  • Consider factoring out files and code for this into separate subdirectory under tribits/core/ (as per Exposing parts of TriBITS as CMake modules #368).
  • Consider creating small TriBITS example project that uses this TriBITS package architecture code but uses 100% raw CMake targets (to prove it can be done).

Tasks

  • Set up meeting with NGA staff to walk through code and tests and discuss goals
  • ???
@bartlettroscoe
Copy link
Member Author

FYI: I will work to get #274 done so that this code is a little easier to read over.

@marcinwrobel1986
Copy link
Collaborator

As agreed on the meeting (18.06.2021) NGA will provide feedback on Function call tree for constructing package dependency graph

@marcinwrobel1986
Copy link
Collaborator

Hello Ross,

I have found something which needs to be completed:

Regarding complete feedback on 15 TriBITS System Maintainers Guide:

  • I was able to understand the flow
  • documentation in it's content is clear and precise
  • functions names are descriptive (there was just one typo in function name described in previous comment)
  • I have found some typos (described in previous comment)
  • I would strongly focus on how documentation looks (you mentioned plans for going with RTD and SPHINX)

Regarding documentation (user interface):

  • I find it difficult to read, mainly because of:
    • high contrast
    • font is not the best for readability
    • lack of different styling (helps find code faster)
  • lack of navigation menu (side menu would help a lot to jump over faster)
  • lack of source code built in (showing the source would help as well)

@bartlettroscoe
Copy link
Member Author

@marcinwrobel1986, thanks for the feedback. Can we discuss some at the meeting today or set up a meeting to go over this?

NOTE: I am working on partitioning the document as we discussed last week by working #381.

@bartlettroscoe
Copy link
Member Author

@marcinwrobel1986, could you please put in a PR with your suggested updates?

marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 15, 2021
@marcinwrobel1986
Copy link
Collaborator

marcinwrobel1986 commented Jul 15, 2021

@bartlettroscoe PR #388 is ready, I have just left the functions/variables names with typos. I could change them if you want Ross, but probably I would not be able to check how they work after changes.

marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 15, 2021
@bartlettroscoe
Copy link
Member Author

I have just left the functions/variables names with typos. I could change them if you want Ross, but probably I would not be able to check how they work after changes.

@marcinwrobel1986, as I said in #388 comment, please go ahead and make any name changes you want. And testing TriBITS locally should be as easy as any CMake project imaginable. Just follow instructions at:

Should take less than 5 mins to set up and run a serial build on any machine that has CMake 3.17+ and any C++ and C compiler.

@marcinwrobel1986
Copy link
Collaborator

Ok @bartlettroscoe , I will let you know when done

marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 15, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 15, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 15, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 16, 2021
@marcinwrobel1986
Copy link
Collaborator

marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 19, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 19, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 19, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 20, 2021
marcinwrobel1986 added a commit to marcinwrobel1986/TriBITS that referenced this issue Jul 20, 2021
- removed autogenerated index.rst
bartlettroscoe pushed a commit to marcinwrobel1986/TriBITS that referenced this issue Aug 12, 2021
bartlettroscoe pushed a commit to marcinwrobel1986/TriBITS that referenced this issue Aug 12, 2021
bartlettroscoe added a commit that referenced this issue Aug 12, 2021
…-TriBITS-package-dependencies-data-structures

#377: Fixing typos
@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Aug 12, 2021

With the merge of #388, I guess this is done for now. The real review is when code changes that need to be made by others happens.

TriBITS Refactor automation moved this from In Progress to Done Aug 12, 2021
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Sep 4, 2021
Brings in numerous refactorings to TriBITS over the last 3 months, but there
should be no breaks in backward compatibility.  Almost every file in TriBITS
is changed due to the lower-casing of command, macro and function names in PR
TriBITSPub/TriBITS#379.  But the main driver for this snapshot is to bring in
the change in PR TriBITSPub/TriBITS#413 that should make it so that Kokkos
INTERFACE_COMPILE_OPTIONS get propagated to downstream targets in TriBITS and
therefore to external customers through installed <Package>Config.cmake files
and IMPORTED targets.  I should have done several snapshots in the last few
months and not done a big snapshot like this (but I have been testing with
Trilinos locally along the way).

Overall, this merge brings in changes from a bunch of TriBITS PRs including
(from most recent):

* TriBITSPub/TriBITS#413: Change internal TriBITS target_link_libraries() to
  PUBLIC (TriBITSPub/TriBITS#299) component: core type: enhancement

* TriBITSPub/TriBITS#410: Upgrade from cmake 3.21.0 to 3.21.2
  (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394)

* TriBITSPub/TriBITS#394: DO NOT MERGE: Show TriBITS test failures with CMake
  3.21.0 that don't occur with CMake 3.17.5 (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#409: Add getTestDictStatusField() to handle empty
  'status' field (SESW-383) component: ci_support type: enhancement

* TriBITSPub/TriBITS#408: Deal with spaces in CDash url parts (SESW-383)
  component: ci_support type: enhancement

* TriBITSPub/TriBITS#403: Spelling fixes

* TriBITSPub/TriBITS#407: Move tribits_get_build_url_and_write_to_file() to
  stand-alone module (TriBITSPub/TriBITS#154) component: ctest_driver type:
  enhancement

* TriBITSPub/TriBITS#388: Fixing typos (TriBITSPub/TriBITS#377)

* TriBITSPub/TriBITS#406: Fix tribits_ctest_driver() package-by-package mode
  for CMake 3.19+ (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394) component:
  ctest_driver type: bug

* TriBITSPub/TriBITS#401: Improve GitHub Actions and CDash integration
  (TriBITSPub/TriBITS#154) type: enhancement

* TriBITSPub/TriBITS#366: CI: draft action yaml

* TriBITSPub/TriBITS#402: Revert some incorrect uppercase->lowercase changes

* TriBITSPub/TriBITS#387: Build and deploy TriBITS documentation with Sphinx
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#398: Deal with pr null in not preprending build name
  (TriBITSPub/TriBITS#363) type: bug

* TriBITSPub/TriBITS#396: Send PR results to 'Pull Request' CDash group and
  add PR ID to build names (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#397: Print the ninja path and version
  (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#393: GitHub Actions based testing for TriBITS
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#389: TriBITS CI testing with GitHub Actions
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#392: Fix broken tests for non-Fortran and CMake 3.21
  builds (TriBITSPub/TriBITS#363) component: core

* TriBITSPub/TriBITS#391: Fix up build_docs.sh for Sphinx doc generation
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#390: Add test for doc generation and fix usage of Python3
  component: documentation type: bug

* TriBITSPub/TriBITS#385: Replace last few references to
  TribitsDevelopersGuide.html (TriBITSPub/TriBITS#381) component:
  documentation type: enhancement

* TriBITSPub/TriBITS#384: Split TribitsDevelopersGuide.* into
  TribitsUsersGuide.* and TribitsMaintainersGuide.* (TriBITSPub/TriBITS#381)
  component: documentation type: enhancement

* TriBITSPub/TriBITS#383: Remove endfunction(<string>) and endmacro(<string>)
  (TriBITSPub/TriBITS#274, TriBITSPub/TriBITS#382) component: common_tpls
  type: bug

* TriBITSPub/TriBITS#380: More package-arch data-structure documentation
  updates (TriBITSPub/TriBITS#63) component: documentation type: enhancement

* TriBITSPub/TriBITS#379: Convert TriBITS to lower-case CMake command, macro,
  and function names (TriBITSPub/TriBITS#274)

The following files were conflicting where I went with what is on the Trilinos
'develop' branch:

* cmake/tribits/common_tpls/FindTPLBLAS.cmake
* cmake/tribits/common_tpls/FindTPLLAPACK.cmake
* cmake/tribits/common_tpls/FindTPLNetcdf.cmake

(It looks like the above changes never made it back into TriBITS proper.  The
conflicts were due to the case changes in cmake command calls in these files
due to TriBITSPub/TriBITS#379.)

There was also a conflict in the file:

* cmake/tribits/core/installation/TribitsProjectConfigTemplate.cmake.in

I looked at that one carefully and I think that may have been due to fixes on
both sides and then the case changes from TriBITSPub/TriBITS#379.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants