major cleanup / refactor of stage creation

modules/build/src/package_staging.cpp

@@ -12,29 +12,35 @@
 
 #include "package_staging.hpp"
 
-int build_package_stage(
+// generates a directory for the stage according to naming convention
+std::filesystem::path stage_determine_rootdir_path(
     const std::string& output_dir,
-    const std::string& contents_dir,
-    const std::string& hooks_dir,
     const std::string& package_name,
     const std::string& package_version,
     const std::string& architecture,
-    const std::string& os,
+    const std::string& os
-    bool force
 ) {
-    // Log start of package staging
-    dpm_log(LOG_INFO, "Starting package staging...");
-
-    // Create the basic package structure in the output directory
     std::string package_dirname = package_name + "-" + package_version + "." + os + "." + architecture;
     std::filesystem::path package_dir = std::filesystem::path(output_dir) / package_dirname;
 
+    return package_dir;
+}
+
+// check if a directory exists
+bool stage_directory_exists( const std::filesystem::path& package_dir )
+{
+    return std::filesystem::exists(package_dir);
+}
+
+bool stage_create_rootdir( const std::filesystem::path& package_dir, bool force )
+{
     // Check if the package directory already exists
-    if (std::filesystem::exists(package_dir)) {
+    if (stage_directory_exists(package_dir))
+    {
         if (!force) {
             dpm_log(LOG_ERROR, ("Package directory already exists: " + package_dir.string() +
-                      ". Use --force to overwrite.").c_str());
+                ". Use --force to overwrite.").c_str());
-            return 1;
+            return false;
         }
 
         // If force flag is set, try to remove the existing directory
@@ -43,124 +49,195 @@ int build_package_stage(
             std::filesystem::remove_all(package_dir);
         } catch (const std::filesystem::filesystem_error& e) {
             dpm_log(LOG_ERROR, ("Failed to remove existing directory: " + std::string(e.what())).c_str());
-            return 1;
+            return false;
         }
     }
 
-    // Create the package directory structure
+    // Create the directory
     try {
-        // Create main package directory
+        std::filesystem::create_directories(package_dir);
-        std::filesystem::create_directory(package_dir);
+        return true;
-
-        // Create subdirectories
-        std::filesystem::create_directory(package_dir / "contents");
-        std::filesystem::create_directory(package_dir / "hooks");
-        std::filesystem::create_directory(package_dir / "metadata");
-        std::filesystem::create_directory(package_dir / "signatures");
-
-        dpm_log(LOG_INFO, ("Created package directory structure at: " + package_dir.string()).c_str());
     } catch (const std::filesystem::filesystem_error& e) {
-        dpm_log(LOG_ERROR, ("Failed to create package directory structure: " + std::string(e.what())).c_str());
+        dpm_log(LOG_ERROR, ("Failed to create directory: " + std::string(e.what())).c_str());
-        return 1;
+        return false;
+    }
+}
+
+bool stage_create_subdir(const std::filesystem::path& package_dir, const std::string& subdir_name)
+{
+    try {
+        std::filesystem::create_directory(package_dir / subdir_name);
+        return true;
+    } catch (const std::filesystem::filesystem_error& e) {
+        dpm_log(LOG_ERROR, ("Failed to create subdirectory '" + subdir_name +
+                 "': " + std::string(e.what())).c_str());
+        return false;
+    }
+}
+
+static bool stage_build_stage_skeleton( const std::filesystem::path& package_dir, bool force )
+{
+    // Check if the package directory already exists and handle appropriately
+    if (!stage_create_rootdir(package_dir, force)) {
+        dpm_log( LOG_FATAL, ("Failed to create root directory: " + package_dir.string()).c_str());
+        return false;
     }
 
-    // Copy contents from source directory to package contents directory
+    // Create subdirectories
-    try {
+    if (!stage_create_subdir(package_dir, "contents"))     { return false; }
-        std::filesystem::path contents_source = std::filesystem::path(contents_dir);
+    if (!stage_create_subdir(package_dir, "hooks"))        { return false; }
-        std::filesystem::path contents_dest = package_dir / "contents";
+    if (!stage_create_subdir(package_dir, "metadata"))     { return false; }
+    if (!stage_create_subdir(package_dir, "signatures"))   { return false; }
 
-        dpm_log(LOG_INFO, ("Copying contents from: " + contents_source.string() +
+    // assume success if we made it to this point
-                 " to: " + contents_dest.string()).c_str());
+    dpm_log(LOG_INFO, ("Created package directory structure at: " + package_dir.string()).c_str());
+
+    return true;
+}
+
+
+static bool stage_copy_dir( const std::filesystem::path& source_path, const std::filesystem::path& dest_path )
+{
+    try {
+        dpm_log(LOG_INFO, ("Copying from: " + source_path.string() +
+                 " to: " + dest_path.string()).c_str());
+
+        // Check if source exists
+        if (!std::filesystem::exists(source_path)) {
+            dpm_log(LOG_ERROR, ("Source path does not exist: " + source_path.string()).c_str());
+            return false;
+        }
 
         // If the contents source is a directory, copy its contents
-        if (std::filesystem::is_directory(contents_source)) {
+        if (std::filesystem::is_directory(source_path)) {
-            for (const auto& entry : std::filesystem::directory_iterator(contents_source)) {
+            for (const auto& entry : std::filesystem::directory_iterator(source_path)) {
                 // Get the relative path from the source directory
-                std::filesystem::path relative = entry.path().lexically_relative(contents_source);
+                std::filesystem::path relative = entry.path().lexically_relative(source_path);
-                std::filesystem::path dest_path = contents_dest / relative;
+                std::filesystem::path dest_path_item = dest_path / relative;
 
                 if (entry.is_directory()) {
-                    std::filesystem::create_directories(dest_path);
+                    std::filesystem::create_directories(dest_path_item);
                     // Copy the directory contents recursively
                     for (const auto& subentry : std::filesystem::recursive_directory_iterator(entry)) {
-                        std::filesystem::path subrelative = subentry.path().lexically_relative(contents_source);
+                        std::filesystem::path subrelative = subentry.path().lexically_relative(source_path);
-                        std::filesystem::path subdest_path = contents_dest / subrelative;
+                        std::filesystem::path subdest_path = dest_path / subrelative;
 
                         if (subentry.is_directory()) {
                             std::filesystem::create_directories(subdest_path);
                         } else {
                             std::filesystem::copy_file(subentry.path(), subdest_path,
-                                                      std::filesystem::copy_options::overwrite_existing);
+                                                     std::filesystem::copy_options::overwrite_existing);
                         }
                     }
                 } else {
-                    std::filesystem::copy_file(entry.path(), dest_path,
+                    std::filesystem::copy_file(entry.path(), dest_path_item,
-                                              std::filesystem::copy_options::overwrite_existing);
+                                             std::filesystem::copy_options::overwrite_existing);
                 }
             }
+        } else {
+            dpm_log(LOG_ERROR, ("Source is not a directory: " + source_path.string()).c_str());
+            return false;
         }
-        // If the contents source is a file (like a tarball), just copy it
+
-        else if (std::filesystem::is_regular_file(contents_source)) {
+        return true;
-            std::filesystem::path dest_path = contents_dest / contents_source.filename();
-            std::filesystem::copy_file(contents_source, dest_path,
-                                      std::filesystem::copy_options::overwrite_existing);
-        }
     } catch (const std::filesystem::filesystem_error& e) {
-        dpm_log(LOG_ERROR, ("Failed to copy contents: " + std::string(e.what())).c_str());
+        dpm_log(LOG_ERROR, ("Failed to copy: " + std::string(e.what())).c_str());
-        return 1;
+        return false;
+    }
+}
+
+static bool stage_populate_contents(
+    const std::filesystem::path& package_dir,
+    const std::string& contents_dir
+) {
+    std::filesystem::path contents_source = std::filesystem::path(contents_dir);
+    std::filesystem::path contents_dest = package_dir / "contents";
+
+    if (!stage_copy_dir(contents_source, contents_dest))
+    {
+        dpm_log( LOG_FATAL, "Failed to copy the contents directory to the package stage.  Exiting." );
+        return false;
     }
 
+    return true;
+}
+
+static bool stage_populate_hooks(
+    const std::filesystem::path& package_dir,
+    const std::string& hooks_dir,
+    const std::string& package_name
+) {
+    // Define the required hook names
+    std::vector<std::string> hook_names = {
+        "PRE-INSTALL", "PRE-INSTALL_ROLLBACK",
+        "POST-INSTALL", "POST-INSTALL_ROLLBACK",
+        "PRE-UPDATE", "PRE-UPDATE_ROLLBACK",
+        "POST-UPDATE", "POST-UPDATE_ROLLBACK",
+        "PRE-REMOVE", "PRE-REMOVE_ROLLBACK",
+        "POST-REMOVE", "POST-REMOVE_ROLLBACK"
+    };
+
     // Copy hooks if provided
-    if (!hooks_dir.empty()) {
+    if (!hooks_dir.empty())
-        try {
+    {
-            std::filesystem::path hooks_source = std::filesystem::path(hooks_dir);
+        std::filesystem::path hooks_source = std::filesystem::path(hooks_dir);
-            std::filesystem::path hooks_dest = package_dir / "hooks";
+        std::filesystem::path hooks_dest = package_dir / "hooks";
 
-            dpm_log(LOG_INFO, ("Copying hooks from: " + hooks_source.string() +
+        // Validate the hooks directory before copying
-                     " to: " + hooks_dest.string()).c_str());
+        if (std::filesystem::exists(hooks_source) && std::filesystem::is_directory(hooks_source)) {
-
+            // Check if all required hooks are present
-            // If hooks source is a directory, copy its contents
+            for (const auto& hook_name : hook_names) {
-            if (std::filesystem::is_directory(hooks_source)) {
+                std::filesystem::path hook_path = hooks_source / hook_name;
-                for (const auto& entry : std::filesystem::directory_iterator(hooks_source)) {
+                if (!std::filesystem::exists(hook_path)) {
-                    // Get the relative path from the source directory
+                    dpm_log(LOG_ERROR, ("Missing required hook file: " + hook_name).c_str());
-                    std::filesystem::path relative = entry.path().lexically_relative(hooks_source);
+                    return false;
-                    std::filesystem::path dest_path = hooks_dest / relative;
-
-                    if (entry.is_directory()) {
-                        std::filesystem::create_directories(dest_path);
-                    } else {
-                        std::filesystem::copy_file(entry.path(), dest_path,
-                                                  std::filesystem::copy_options::overwrite_existing);
-
-                        // Make hook files executable
-                        chmod(dest_path.c_str(), 0755);
-                    }
                 }
             }
-            // If hooks source is a file, just copy it
-            else if (std::filesystem::is_regular_file(hooks_source)) {
-                std::filesystem::path dest_path = hooks_dest / hooks_source.filename();
-                std::filesystem::copy_file(hooks_source, dest_path,
-                                          std::filesystem::copy_options::overwrite_existing);
 
-                // Make hook file executable
+            // Check for unexpected files or directories
-                chmod(dest_path.c_str(), 0755);
+            for (const auto& entry : std::filesystem::directory_iterator(hooks_source)) {
+                std::string filename = entry.path().filename().string();
+
+                // Check if entry is a directory and reject it
+                if (std::filesystem::is_directory(entry)) {
+                    dpm_log(LOG_ERROR, ("Unexpected directory in hooks directory: " + filename).c_str());
+                    return false;
+                }
+
+                // Check if the file is a valid hook
+                bool is_valid_hook = false;
+                for (const auto& hook_name : hook_names) {
+                    if (filename == hook_name) {
+                        is_valid_hook = true;
+                        break;
+                    }
+                }
+
+                if (!is_valid_hook) {
+                    dpm_log(LOG_ERROR, ("Unexpected file in hooks directory: " + filename).c_str());
+                    return false;
+                }
+            }
+        } else {
+            dpm_log(LOG_ERROR, ("Hooks directory does not exist or is not a directory: " + hooks_source.string()).c_str());
+            return false;
+        }
+
+        // Now copy the directory after validation
+        if (!stage_copy_dir(hooks_source, hooks_dest))
+        {
+            dpm_log(LOG_FATAL, "Failed to copy the hooks directory to the package stage. Exiting.");
+            return false;
+        }
+
+        // Make hook files executable
+        for (const auto& entry : std::filesystem::recursive_directory_iterator(hooks_dest)) {
+            if (!entry.is_directory()) {
+                chmod(entry.path().c_str(), 0755);
             }
-        } catch (const std::filesystem::filesystem_error& e) {
-            dpm_log(LOG_ERROR, ("Failed to copy hooks: " + std::string(e.what())).c_str());
-            return 1;
         }
     } else {
         // If no hooks directory provided, create empty hook script templates
-        std::vector<std::string> hook_names = {
-            "PRE-INSTALL", "PRE-INSTALL_ROLLBACK",
-            "POST-INSTALL", "POST-INSTALL_ROLLBACK",
-            "PRE-UPDATE", "PRE-UPDATE_ROLLBACK",
-            "POST-UPDATE", "POST-UPDATE_ROLLBACK",
-            "PRE-REMOVE", "PRE-REMOVE_ROLLBACK",
-            "POST-REMOVE", "POST-REMOVE_ROLLBACK"
-        };
-
         dpm_log(LOG_INFO, "Creating empty hook templates");
 
         for (const auto& hook_name : hook_names) {
@@ -179,11 +256,20 @@ int build_package_stage(
                 chmod(hook_path.c_str(), 0755);
             } else {
                 dpm_log(LOG_ERROR, ("Failed to create hook file: " + hook_path.string()).c_str());
+                return false;
             }
         }
     }
 
-    // Create basic metadata files
+    return true;
+}
+
+static bool stage_populate_basic_metadata(
+    const std::filesystem::path& package_dir,
+    const std::string& package_name,
+    const std::string& package_version,
+    const std::string& architecture
+) {
     try {
         std::filesystem::path metadata_dir = package_dir / "metadata";
 
@@ -216,9 +302,18 @@ int build_package_stage(
 
         // Create empty placeholder files for other metadata
         std::vector<std::string> metadata_files = {
-            "AUTHOR", "MAINTAINER", "DEPENDENCIES", "DESCRIPTION",
+            "AUTHOR",
-            "CONTENTS_MANIFEST_DIGEST", "LICENSE", "PACKAGE_DIGEST",
+            "MAINTAINER",
-            "HOOKS_DIGEST", "PROVIDES", "REPLACES", "SOURCE", "CHANGELOG"
+            "DEPENDENCIES",
+            "DESCRIPTION",
+            "CONTENTS_MANIFEST_DIGEST",
+            "LICENSE",
+            "PACKAGE_DIGEST",
+            "HOOKS_DIGEST",
+            "PROVIDES",
+            "REPLACES",
+            "SOURCE",
+            "CHANGELOG"
         };
 
         for (const auto& file_name : metadata_files) {
@@ -227,8 +322,52 @@ int build_package_stage(
         }
 
         dpm_log(LOG_INFO, "Created metadata files");
+        return true;
     } catch (const std::exception& e) {
         dpm_log(LOG_ERROR, ("Failed to create metadata files: " + std::string(e.what())).c_str());
+        return false;
+    }
+}
+
+int build_package_stage(
+    const std::string& output_dir,
+    const std::string& contents_dir,
+    const std::string& hooks_dir,
+    const std::string& package_name,
+    const std::string& package_version,
+    const std::string& architecture,
+    const std::string& os,
+    bool force
+) {
+    // Log start of package staging
+    dpm_log(LOG_INFO, "Starting package staging...");
+
+    // Generate package directory path
+    std::filesystem::path package_dir = stage_determine_rootdir_path(
+        output_dir, package_name, package_version, architecture, os
+    );
+
+    // Build the package skeleton
+    if (!stage_build_stage_skeleton(package_dir, force))
+    {
+        return 1;
+    }
+
+    // copy the contents dir to the contents part of the package stage
+    if (!stage_populate_contents(package_dir, contents_dir))
+    {
+        return 1;
+    }
+
+    // copy the supplied hooks or create a new blank hooks dir in the stage
+    if (!stage_populate_hooks(package_dir, hooks_dir, package_name))
+    {
+        return 1;
+    }
+
+    // Populate metadata files
+    if (!stage_populate_basic_metadata(package_dir, package_name, package_version, architecture))
+    {
         return 1;
     }