Cleanup for `library_plan`, and put executables in subdirs. (Required moving dds.main.cpp)

src/dds/build/plan/library.cpp

@@ -4,78 +4,101 @@
 
 #include <range/v3/view/concat.hpp>
 #include <range/v3/view/filter.hpp>
+#include <range/v3/view/transform.hpp>
+
+#include <cassert>
 
 using namespace dds;
 
 library_plan library_plan::create(const library&               lib,
                                   const library_build_params&  params,
                                   const usage_requirement_map& ureqs) {
-    std::vector<compile_file_plan>     compile_files;
-    std::vector<link_executable_plan>  link_executables;
-    std::optional<create_archive_plan> create_archive;
-
+    // Source files are kept in three groups:
     std::vector<source_file> app_sources;
     std::vector<source_file> test_sources;
     std::vector<source_file> lib_sources;
 
+    // Collect the source for this library. This will look for any compilable sources in the `src/`
+    // subdirectory of the library.
     auto src_dir = lib.src_dir();
     if (src_dir.exists()) {
+        // Sort each source file between the three source arrays, depending on
+        // the kind of source that we are looking at.
         auto all_sources = src_dir.sources();
-        auto to_compile  = all_sources | ranges::views::filter([&](const source_file& sf) {
-                              return (sf.kind == source_kind::source
-                                      || (sf.kind == source_kind::app && params.build_apps)
-                                      || (sf.kind == source_kind::test && params.build_tests));
-                          });
-
-        for (const auto& sfile : to_compile) {
+        for (const auto& sfile : all_sources) {
             if (sfile.kind == source_kind::test) {
                 test_sources.push_back(sfile);
             } else if (sfile.kind == source_kind::app) {
                 app_sources.push_back(sfile);
-            } else {
+            } else if (sfile.kind == source_kind::source) {
                 lib_sources.push_back(sfile);
+            } else {
+                assert(sfile.kind == source_kind::header);
             }
         }
     }
 
+    // Load up the compile rules
     auto compile_rules              = lib.base_compile_rules();
     compile_rules.enable_warnings() = params.enable_warnings;
+
+    // Apply our transitive usage requirements. This gives us the search directories for our
+    // dependencies.
     for (const auto& use : lib.manifest().uses) {
         ureqs.apply(compile_rules, use.namespace_, use.name);
     }
 
-    for (const auto& sf : lib_sources) {
-        compile_files.emplace_back(compile_rules,
-                                   sf,
-                                   lib.manifest().name,
-                                   params.out_subdir / "obj");
-    }
+    // Convert the library sources into their respective file compilation plans.
+    auto lib_compile_files =  //
+        lib_sources           //
+        | ranges::views::transform([&](const source_file& sf) {
+              return compile_file_plan(compile_rules,
+                                       sf,
+                                       lib.manifest().name,
+                                       params.out_subdir / "obj");
+          })
+        | ranges::to_vector;
 
-    if (!lib_sources.empty()) {
-        create_archive.emplace(lib.manifest().name, params.out_subdir, std::move(compile_files));
+    // If we have any compiled library files, generate a static library archive
+    // for this library
+    std::optional<create_archive_plan> create_archive;
+    if (!lib_compile_files.empty()) {
+        create_archive.emplace(lib.manifest().name,
+                               params.out_subdir,
+                               std::move(lib_compile_files));
     }
 
-    std::vector<fs::path> in_libs;
+    // Collect the paths to linker inputs that should be used when generating executables for this
+    // library.
+    std::vector<fs::path> link_libs;
     for (auto& use : lib.manifest().uses) {
-        extend(in_libs, ureqs.link_paths(use.namespace_, use.name));
+        extend(link_libs, ureqs.link_paths(use.namespace_, use.name));
     }
     for (auto& link : lib.manifest().links) {
-        extend(in_libs, ureqs.link_paths(link.namespace_, link.name));
+        extend(link_libs, ureqs.link_paths(link.namespace_, link.name));
     }
 
-    auto test_in_libs = in_libs;
-    extend(test_in_libs, params.test_link_files);
+    // Linker inputs for tests may contain additional code for test execution
+    auto test_link_libs = link_libs;
+    extend(test_link_libs, params.test_link_files);
 
+    // There may also be additional #include paths for test source files
     auto test_rules = compile_rules.clone();
     extend(test_rules.include_dirs(), params.test_include_dirs);
+
+    // Generate the plans to link any executables for this library
+    std::vector<link_executable_plan> link_executables;
     for (const source_file& source : ranges::views::concat(app_sources, test_sources)) {
+        const bool is_test = source.kind == source_kind::test;
         // Pick a subdir based on app/test
-        auto subdir
-            = source.kind == source_kind::test ? params.out_subdir / "test" : params.out_subdir;
+        const auto subdir_base = is_test ? params.out_subdir / "test" : params.out_subdir;
+        // Put test/app executables in a further subdirectory based on the source file path
+        const auto subdir
+            = subdir_base / fs::relative(source.path.parent_path(), lib.src_dir().path);
         // Pick compile rules based on app/test
-        auto rules = source.kind == source_kind::test ? test_rules : compile_rules;
+        auto rules = is_test ? test_rules : compile_rules;
         // Pick input libs based on app/test
-        auto& exe_link_libs = source.kind == source_kind::test ? test_in_libs : in_libs;
+        auto& exe_link_libs = is_test ? test_link_libs : link_libs;
         // TODO: Apps/tests should only see the _public_ include dir, not both
         link_executables.emplace_back(exe_link_libs,
                                       compile_file_plan(rules,
@@ -86,6 +109,7 @@ library_plan library_plan::create(const library&               lib,
                                       source.path.stem().stem().string());
     }
 
+    // Done!
     return library_plan{lib.manifest().name,
                         lib.path(),
                         std::move(create_archive),

src/dds/build/plan/library.hpp

@@ -12,26 +12,64 @@
 
 namespace dds {
 
+/**
+ * The parameters that tweak the behavior of building a library
+ */
 struct library_build_params {
+    /// The subdirectory of the build root in which this library should place its files.
     fs::path out_subdir;
-    bool     build_tests     = false;
-    bool     build_apps      = false;
-    bool     enable_warnings = false;
+    /// Whether tests should be compiled and linked for this library
+    bool build_tests = false;
+    /// Whether applications should be compiled and linked for this library
+    bool build_apps = false;
+    /// Whether compiler warnings should be enabled for building the source files in this library.
+    bool enable_warnings = false;
 
-    // Extras for compiling tests:
+    /// Directories that should be on the #include search path when compiling tests
     std::vector<fs::path> test_include_dirs;
+    /// Files that should be added as inputs when linking test executables
     std::vector<fs::path> test_link_files;
 };
 
+/**
+ * A `library_plan` is a composite object that keeps track of the parameters for building a library,
+ * including:
+ *
+ * - If the library has compilable library source files, a `create_archive_plan` that details the
+ *   compilation of those source files and their collection into a static library archive.
+ * - The executables that need to be linked when this library is built. This includes any tests and
+ *   apps that are part of this library. These can be enabled/disabled by setting the appropriate
+ *   values in `library_build_params`.
+ * - The libraries that this library *uses*.
+ * - The libraries that this library *links*.
+ *
+ * While there is a public constructor, it is best to use the `create` named constructor, which will
+ * initialize all of the constructor parameters correctly.
+ */
 class library_plan {
-    std::string                        _name;
-    fs::path                           _source_root;
+    /// The name of the library
+    std::string _name;
+    /// The directory at the root of this library
+    fs::path _source_root;
+    /// The `create_archive_plan` for this library, if applicable
     std::optional<create_archive_plan> _create_archive;
-    std::vector<link_executable_plan>  _link_exes;
-    std::vector<lm::usage>             _uses;
-    std::vector<lm::usage>             _links;
+    /// The executables that should be linked as part of this library's build
+    std::vector<link_executable_plan> _link_exes;
+    /// The libraries that we use
+    std::vector<lm::usage> _uses;
+    /// The libraries that we link
+    std::vector<lm::usage> _links;
 
 public:
+    /**
+     * Construct a new `library_plan`
+     * @param name The name of the library
+     * @param source_root The directory that contains this library
+     * @param ar The `create_archive_plan`, or `nullopt` for this library.
+     * @param exes The `link_executable_plan` objects for this library.
+     * @param uses The identities of the libraries that are used by this library
+     * @param links The identities of the libraries that are linked by this library
+     */
     library_plan(std::string_view                   name,
                  path_ref                           source_root,
                  std::optional<create_archive_plan> ar,
@@ -45,15 +83,46 @@ public:
         , _uses(std::move(uses))
         , _links(std::move(links)) {}
 
+    /**
+     * Get the name of the library
+     */
+    auto& name() const noexcept { return _name; }
+    /**
+     * The directory that defines the source root of the library.
+     */
     path_ref source_root() const noexcept { return _source_root; }
-    auto&    name() const noexcept { return _name; }
-    auto&    create_archive() const noexcept { return _create_archive; }
-    auto&    executables() const noexcept { return _link_exes; }
-    auto&    uses() const noexcept { return _uses; }
-    auto&    links() const noexcept { return _links; }
-
-    static library_plan
-    create(const library&, const library_build_params&, const usage_requirement_map&);
+    /**
+     * A `create_archive_plan` object, or `nullopt`, depending on if this library has compiled
+     * components
+     */
+    auto& create_archive() const noexcept { return _create_archive; }
+    /**
+     * The executables that should be created by this library
+     */
+    auto& executables() const noexcept { return _link_exes; }
+    /**
+     * The library identifiers that are used by this library
+     */
+    auto& uses() const noexcept { return _uses; }
+    /**
+     * The library identifiers that are linked by this library
+     */
+    auto& links() const noexcept { return _links; }
+
+    /**
+     * Named constructor: Create a new `library_plan` automatically from some build-time parameters.
+     *
+     * @param lib The `library` object from which we will inherit several properties.
+     * @param params Parameters controlling the build of the library. i.e. if we create tests,
+     * enable warnings, etc.
+     * @param ureqs The usage requirements map. This should be populated as appropriate.
+     *
+     * The `lib` parameter defines the usage requirements of this library, and they are looked up in
+     * the `ureqs` map. If there are any missing requirements, an exception will be thrown.
+     */
+    static library_plan create(const library&               lib,
+                               const library_build_params&  params,
+                               const usage_requirement_map& ureqs);
 };
 
 }  // namespace dds

tests/test_drivers/catch/test_catch.py

@@ -8,6 +8,6 @@ from dds_ci import proc
 )
 def test_catch_testdriver(dds: DDS):
     dds.build(tests=True)
-    test_exe = dds.build_dir / f'test/calc{dds.exe_suffix}'
+    test_exe = dds.build_dir / f'test/testlib/calc{dds.exe_suffix}'
     assert test_exe.exists()
     assert proc.run([test_exe]).returncode == 0