Improve ergonomics of using TheRock as a development driver for sub-projects. (ROCm#1242)

The motivation of this change is to enable the following developer
productivity features:

* Development in a sub-project `build` directory should cause the
super-project to see the sub-project as out of date and take appropriate
action. This is done by injecting an ALL `therock-touch` target into
each sub-project so that `ninja` will invalidate the super-project
stage-install state, causing a re-install and rebuild of dependents.
* One stop commands to build and distribute in a sub-project without
switching back and forth. Users can now just run `ninja therock-dist` in
any sub-project to build and distribute their local changes (i.e. to
`build/dist/rocm`, etc).
* More coverage of `expunge` targets, making sure that project state can
be reset more completely.

Key changes:

* Removes some `therock-` prefixes from top level convenience targets.
* Defines `artifacts`, `archives`, `dist`, and `expunge` top level
convenience targets explicitly.
* Adds `expunge` steps to all phases of build, artifact assembly and
dist population.
* Removes the formal sub-project `+dist` phase, collapsing the creation
of the local `dist` directory into the `+stage` (install) step. This
separation was done early on when creating the dist/ tree was slow but
it doesn't pay for itself anymore and was becoming hard to distinguish
the different "dist" concepts in the project.
* Artifacts can be declared as belonging to different distributions
(default "rocm"), resulting in population of different trees in
`build/dist/{distribution}`. This was a TODO from the beginning.
* `{subproject}+dist` now is a convenience target that builds all
artifacts the project is a part of and populates the
`build/dist/{distname}` directory.
* Distributions (i.e. `build/dist/rocm`) are now populated incrementally
as artifact dependencies are met instead of as part of one top level
target which requires the whole project to be (re)built.
* New targets `therock-touch` and `therock-dist` are injected into every
sub-project, allowing development to be done completely in a sub-project
build directory while keeping super-project level artifacts and
distributions populated.

This should be NFC for all automation since the new barename targets
(dist, archives) are aliased to their originals. We could simplify this
in followups.

Author: stellaraccident

CMakeLists.txt

@@ -16,6 +16,7 @@ cmake_policy(SET CMP0135 NEW)
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 include(CMakeDependentOption)
 include(ExternalProject)
+include(therock_default_targets)
 include(therock_amdgpu_targets)
 include(therock_artifacts)
 include(therock_compiler_config)
@@ -447,4 +448,3 @@ endif()
 # Finalization
 ################################################################################
 therock_subproject_merge_compile_commands()
-therock_create_dist()

build_tools/fileset_tool.py

@@ -269,8 +269,9 @@ def _do_artifact_flatten(args):
     flattener(*args.artifact)
     relpaths = list(flattener.relpaths)
     relpaths.sort()
-    for relpath in relpaths:
-        print(relpath)
+    if args.verbose:
+        for relpath in relpaths:
+            print(relpath)
 
 
 def _dup_list_or_str(v: list[str] | str) -> list[str]:

build_tools/tests/fileset_tool_test.py

@@ -173,6 +173,7 @@ def testSimpleArtifact(self):
                 FILESET_TOOL,
                 "artifact-flatten",
                 artifact_archive,
+                "--verbose",
                 "-o",
                 flat2_dir,
             ]

cmake/therock_artifacts.cmake

@@ -2,16 +2,30 @@
 # Facilities for bundling artifacts for bootstrapping and subsequent CI/CD
 # phases.
 
-# Property containing all artifact directories for all components created via
-# therock_provide_artifact(). This is used to populate the dist/rocm directory
-# by flattening them all. In the future, we may have multiple dist groups but
-# there is just this one for now.
-set_property(GLOBAL PROPERTY THEROCK_DIST_ARTIFACT_DIRS)
-
+# therock_provide_artifact
+# This populates directories under build/artifacts representing specific
+# subsets of the install tree. See docs/development/artifacts.md for further
+# design notes on the subsystem.
+#
+# While artifacts are the primary output of the build system, it is often
+# an aid to development to materialize them all locally into a `distribution`.
+# These are directories under build/dist/${ARG_DISTRIBUTION} (default "rocm").
+#
+# All artifact slices of a distribution should be non-overlapping, populating
+# some subset of the install directory tree.
+#
+# This will produce the following convenience targets:
+# - artifact-${slice_name} : Populate the build/artifacts/{qualified_name}
+#   directory. Added as a dependency of the `therock-artifacts` target.
+# - archive-${slice_name} : Populate the build/artifacts/{qualified_name}.tar.xz
+#   archive file. Added as a dependency of the `therock-archives` target.
+#
+# Convenience targets with a "+expunge" suffix are created to remove corresponding
+# files. Invoking the project level "expunge" will depend on all of them.
 function(therock_provide_artifact slice_name)
   cmake_parse_arguments(PARSE_ARGV 1 ARG
     "TARGET_NEUTRAL"
-    "DESCRIPTOR"
+    "DESCRIPTOR;DISTRIBUTION"
     "COMPONENTS;SUBPROJECT_DEPS"
   )
 
@@ -23,8 +37,8 @@ function(therock_provide_artifact slice_name)
   endif()
 
   # Normalize arguments.
-  set(_target_name "therock-artifact-${slice_name}")
-  set(_archive_target_name "therock-archive-${slice_name}")
+  set(_target_name "artifact-${slice_name}")
+  set(_archive_target_name "archive-${slice_name}")
   if(TARGET "${_target_name}")
     message(FATAL_ERROR "Artifact slice '${slice_name}' provided more than once")
   endif()
@@ -37,12 +51,24 @@ function(therock_provide_artifact slice_name)
   endif()
   cmake_path(ABSOLUTE_PATH ARG_DESCRIPTOR BASE_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
 
-  # We make all artifact slices available as therock-artifacts top-level target.
-  if(NOT TARGET therock-artifacts)
-    add_custom_target(therock-artifacts)
+  if(NOT DEFINED ARG_DISTRIBUTION)
+    set(ARG_DISTRIBUTION "rocm")
   endif()
-  if(NOT TARGET therock-archives)
-    add_custom_target(therock-archives)
+
+  if(ARG_DISTRIBUTION)
+    set(_dist_dir "${THEROCK_BINARY_DIR}/dist/${ARG_DISTRIBUTION}")
+    if(NOT TARGET "dist-${ARG_DISTRIBUTION}")
+      add_custom_target("dist-${ARG_DISTRIBUTION}" ALL)
+      add_dependencies(therock-dist "dist-${ARG_DISTRIBUTION}")
+
+      # expunge target for the dist
+      add_custom_target(
+        "dist-${ARG_DISTRIBUTION}+expunge"
+        COMMAND
+          "${CMAKE_COMMAND}" -E rm -rf "${_dist_dir}"
+      )
+      add_dependencies(therock-expunge "dist-${ARG_DISTRIBUTION}+expunge")
+    endif()
   endif()
 
   # Determine top-level name.
@@ -57,27 +83,34 @@ function(therock_provide_artifact slice_name)
   set(_stamp_file_deps)
   _therock_cmake_subproject_deps_to_stamp(_stamp_file_deps "stage.stamp" ${ARG_SUBPROJECT_DEPS})
 
-  # Assemble commands.
+  # Populate commands.
   set(_fileset_tool "${THEROCK_SOURCE_DIR}/build_tools/fileset_tool.py")
   set(_command_list)
   set(_manifest_files)
+  set(_component_dirs)
   foreach(_component ${ARG_COMPONENTS})
     set(_component_dir "${THEROCK_BINARY_DIR}/artifacts/${slice_name}_${_component}${_bundle_suffix}")
-    set_property(GLOBAL APPEND PROPERTY THEROCK_DIST_ARTIFACT_DIRS "${_component_dir}")
+    list(APPEND _component_dirs "${_component_dir}")
     set(_manifest_file "${_component_dir}/artifact_manifest.txt")
     list(APPEND _manifest_files "${_manifest_file}")
+    # Populate the artifact directory.
     list(APPEND _command_list
       COMMAND "${Python3_EXECUTABLE}" "${_fileset_tool}" artifact
         --output-dir "${_component_dir}"
         --root-dir "${THEROCK_BINARY_DIR}" --descriptor "${ARG_DESCRIPTOR}"
         --component "${_component}"
     )
+    # Populate the corresponding build/dist/DISTRIBUTION directory.
+    if(ARG_DISTRIBUTION)
+      list(APPEND _command_list
+        COMMAND "${Python3_EXECUTABLE}" "${_fileset_tool}" artifact-flatten
+          -o "${_dist_dir}" ${_component_dirs}
+      )
+    endif()
   endforeach()
-
-  # Set up command.
   add_custom_command(
     OUTPUT ${_manifest_files}
-    COMMENT "Merging artifact ${slice_name}"
+    COMMENT "Populate artifact ${slice_name}"
     ${_command_list}
     DEPENDS
       ${_stamp_file_deps}
@@ -89,15 +122,20 @@ function(therock_provide_artifact slice_name)
     DEPENDS ${_manifest_files}
   )
   add_dependencies(therock-artifacts "${_target_name}")
+  if(ARG_DISTRIBUTION)
+    add_dependencies("dist-${ARG_DISTRIBUTION}" "${_target_name}")
+  endif()
 
-  ### Generate artifact archive commands.
+  # Generate artifact archive commands.
   set(_archive_files)
+  set(_archive_sha_files)
   foreach(_component ${ARG_COMPONENTS})
     set(_component_dir "${THEROCK_BINARY_DIR}/artifacts/${slice_name}_${_component}${_bundle_suffix}")
     set(_manifest_file "${_component_dir}/artifact_manifest.txt")
     set(_archive_file "${THEROCK_BINARY_DIR}/artifacts/${slice_name}_${_component}${_bundle_suffix}${THEROCK_ARTIFACT_ARCHIVE_SUFFIX}.tar.xz")
     list(APPEND _archive_files "${_archive_file}")
     set(_archive_sha_file "${_archive_file}.sha256sum")
+    list(APPEND _archive_sha_files "${_archive_sha_file}")
     # TODO(#726): Lower compression levels are much faster for development and CI.
     #             Set back to 6+ for production builds?
     set(_archive_compression_level 2)
@@ -117,39 +155,37 @@ function(therock_provide_artifact slice_name)
         "${_fileset_tool}"
     )
   endforeach()
-
   add_custom_target("${_archive_target_name}" DEPENDS ${_archive_files})
   add_dependencies(therock-archives "${_archive_target_name}")
-endfunction()
-
 
-function(therock_create_dist)
-  # Currently there is only one dist se we hard-code. These could become
-  # settings later.
-  set(_dist_dir "${THEROCK_BINARY_DIR}/dist/rocm")
-  set(_stamp_file "${THEROCK_BINARY_DIR}/dist/.rocm.stamp")
-  set(_dist_name "rocm")
-  get_property(_artifact_dirs GLOBAL PROPERTY THEROCK_DIST_ARTIFACT_DIRS)
-
-  set(_fileset_tool "${THEROCK_SOURCE_DIR}/build_tools/fileset_tool.py")
-  list(TRANSFORM _artifact_dirs APPEND "/artifact_manifest.txt" OUTPUT_VARIABLE _manifest_files)
-
-  add_custom_command(
-    OUTPUT "${_stamp_file}"
-    COMMENT "Creating dist ${_dist_dir}"
-    COMMAND "${Python3_EXECUTABLE}" "${_fileset_tool}" artifact-flatten --verbose
-      -o "${_dist_dir}" ${_artifact_dirs}
+  # Archive expunge target.
+  add_custom_target(
+    "${_archive_target_name}+expunge"
     COMMAND
-      "${CMAKE_COMMAND}" -E touch "${_stamp_file}"
-    DEPENDS
-      "${_fileset_tool}"
-      ${_manifest_files}
+      "${CMAKE_COMMAND}" -E rm -f ${_archive_files} ${_archive_sha_files}
   )
+  add_dependencies(therock-expunge "${_archive_target_name}+expunge")
 
-  set(_dist_target_name "therock-dist-${_dist_name}")
-  add_custom_target("${_dist_target_name}" ALL DEPENDS "${_stamp_file}")
-  if(NOT TARGET therock-dist)
-    add_custom_target(therock-dist ALL)
+  # Generate expunge targets.
+  add_custom_target(
+    "${_target_name}+expunge"
+    COMMAND
+      "${CMAKE_COMMAND}" -E rm -rf ${_component_dirs}
+  )
+  add_dependencies(therock-expunge "${_target_name}+expunge")
+
+  # For each subproject dep, we add a dependency on its +dist target to also
+  # trigger overall artifact construction. In this way `ninja myfoo+dist`
+  # will always populate all related artifacts and distributions. Note that
+  # this only applies to the convenience +dist target, not the underlying
+  # stamp-file chain, which is what the core dependency mechanism uses.
+  if(ARG_DISTRIBUTION)
+    foreach(subproject_dep ${ARG_SUBPROJECT_DEPS})
+      set(_subproject_dist_target "${subproject_dep}+dist")
+      if(NOT TARGET "${_subproject_dist_target}")
+        message(FATAL_ERROR "Subproject convenience target ${_subproject_dist_target} does not exist")
+      endif()
+      add_dependencies("${_subproject_dist_target}" "${_target_name}")
+    endforeach()
   endif()
-  add_dependencies(therock-dist "${_dist_target_name}")
 endfunction()

cmake/therock_default_targets.cmake

@@ -0,0 +1,26 @@
+# Various top level targets that have dependencies added to them build system
+# wide. In general, the canonical name for all such targets is prefixed with
+# "therock-{name}", but also bare name targets are defined if not already.
+
+function(therock_add_convenience_target name)
+  add_custom_target("therock-${name}")
+  if(NOT TARGET "${name}")
+    add_custom_target("${name}" DEPENDS "therock-${name}")
+  endif()
+endfunction()
+
+function(therock_add_convenience_target_all name)
+  add_custom_target("therock-${name}" ALL)
+  if(NOT TARGET "${name}")
+    add_custom_target("${name}" DEPENDS "therock-${name}")
+  endif()
+endfunction()
+
+# Populates all artifacts and distributions (i.e. all artifact-foo targets).
+therock_add_convenience_target(artifacts)
+# Builds archives for all artifacts (i.e. all archive-foo targets).
+therock_add_convenience_target(archives)
+# Populates all distribution directories (i.e. all dist-foo targets).
+therock_add_convenience_target(dist)
+# Expunges configure/build byproducts and artifacts for all projects.
+therock_add_convenience_target(expunge)

cmake/therock_global_post_subproject.cmake

@@ -116,3 +116,33 @@ if(THEROCK_SPLIT_DEBUG_INFO AND CMAKE_SYSTEM_NAME STREQUAL "Linux")
     )
   endblock()
 endif()
+
+# Add convenience targets within the sub-project that interact with the super-project.
+# This allows developers to work entirely in the sub-project build directory and
+# perform most operations.
+
+# Removes the sub-project stage.stamp file. This will cause any subsequent action
+# on the super-project to see the sub-project as out of date with respect to
+# being stage installed and populated in distributions. We invoke it on ALL
+# so that most local build commands (i.e. `ninja` without args) triggers
+# invalidation. During normal super-project build, once the build phase is
+# done, the build.stamp will be touched, which will cause the stage.stamp to
+# show as out of date. So either way, the ordering is preserved.
+add_custom_target(therock-touch ALL
+  COMMAND
+    "${CMAKE_COMMAND}" -E rm -f "${THEROCK_STAGE_STAMP_FILE}"
+  VERBATIM
+)
+
+# Removes the sub-project build.stamp file, indicating that the project must be
+# rebuilt, then invokes the parent {target}+dist, causing all stage installation
+# and distributions to be populated upon successful build.
+add_custom_target(therock-dist
+  COMMAND
+    "${CMAKE_COMMAND}" -E rm -f "${THEROCK_BUILD_STAMP_FILE}"
+  COMMAND
+    "${CMAKE_COMMAND}" --build "${THEROCK_BINARY_DIR}" --target "${THEROCK_SUBPROJECT_TARGET}+dist"
+  COMMENT "Trigger super-project dist"
+  VERBATIM
+  USES_TERMINAL
+)