fix: fix logging and mergeOutput

- `fmt::println(stderr, message)` could crash if the message contained
  format specifiers like `{}`.
- fix the `mergeOutput` logic and extract it from the `Builder` class
  into a free function within the `detail` namespace. This refactoring
  allows for easier unit testing.
- Unit tests have been added for both `mergeOutput` function and the
  logging format fix.

Author: reddevillg

libs/linglong/src/linglong/builder/linglong_builder.cpp

@@ -142,6 +142,41 @@ utils::error::Result<void> pullDependency(const package::Reference &ref,
 
 } // namespace
 
+namespace detail {
+void mergeOutput(const std::vector<std::filesystem::path> &src,
+                 const std::filesystem::path &dest,
+                 const std::vector<std::string> &targets)
+{
+    for (const auto &dir : src) {
+        LogD("merge {} to {}", dir, dest);
+        auto matcher = [&dir, &targets](const std::filesystem::path &path) {
+            if (common::strings::starts_with(path.filename().string(), ".wh.")) {
+                return false;
+            }
+
+            struct stat st;
+            if (-1 == lstat((dir / path).c_str(), &st)) {
+                return false;
+            }
+            if (st.st_size == 0 && st.st_rdev == 0 && st.st_mode == S_IFCHR) {
+                return false;
+            }
+
+            for (const auto &target : targets) {
+                if (common::strings::starts_with(path.string(), target)) {
+                    return true;
+                }
+            }
+
+            return false;
+        };
+
+        utils::copyDirectory(dir, dest, matcher);
+    }
+}
+
+} // namespace detail
+
 // install module files by rules
 // files will be moved from buildOutput to moduleOutput
 utils::error::Result<std::vector<std::filesystem::path>>
@@ -904,7 +939,7 @@ utils::error::Result<void> Builder::buildStagePreCommit() noexcept
             src.push_back(runtimeOverlay->upperDirPath());
         }
     }
-    mergeOutput(src, buildOutput, { "bin/", "sbin/", "lib/" });
+    detail::mergeOutput(src, buildOutput, { "bin/", "sbin/", "lib/" });
 
     return LINGLONG_OK;
 }
@@ -2189,38 +2224,6 @@ void Builder::takeTerminalForeground()
     }
 }
 
-void Builder::mergeOutput(const std::vector<std::filesystem::path> &src,
-                          const std::filesystem::path &dest,
-                          const std::vector<std::string> &targets)
-{
-
-    for (const auto &dir : src) {
-        auto matcher = [&dir, &targets](const std::filesystem::path &path) {
-            if (path.filename().string().rfind(".wh.", 0) == 0) {
-                return false;
-            }
-
-            struct stat st;
-            if (-1 == lstat(path.c_str(), &st)) {
-                return false;
-            }
-            if (st.st_size == 0 && st.st_rdev == 0 && st.st_mode == S_IFCHR) {
-                return false;
-            }
-
-            for (const auto &target : targets) {
-                if (path.lexically_relative(dir).string().rfind(target, 0) == 0) {
-                    return true;
-                }
-            }
-
-            return false;
-        };
-
-        utils::copyDirectory(dir, dest, matcher);
-    }
-}
-
 void Builder::printBasicInfo()
 {
     printMessage("[Builder info]");

libs/linglong/src/linglong/builder/linglong_builder.h

@@ -53,6 +53,12 @@ utils::error::Result<void> cmdRemoveApp(repo::OSTreeRepo &repo,
                                         std::vector<std::string> refs,
                                         bool prune);
 
+namespace detail {
+void mergeOutput(const std::vector<std::filesystem::path> &src,
+                 const std::filesystem::path &dest,
+                 const std::vector<std::string> &targets);
+}
+
 class Builder
 {
 public:
@@ -128,9 +134,6 @@ class Builder
     auto generateBuildDependsScript() noexcept -> utils::error::Result<bool>;
     auto generateDependsScript() noexcept -> utils::error::Result<bool>;
     void takeTerminalForeground();
-    void mergeOutput(const std::vector<std::filesystem::path> &src,
-                     const std::filesystem::path &dest,
-                     const std::vector<std::string> &targets);
     void printBasicInfo();
     void printRepo();
     bool checkDeprecatedInstallFile();

libs/linglong/tests/ll-tests/src/linglong/builder/linglong_builder_test.cpp

@@ -138,6 +138,53 @@ TEST(LinglongBuilder, installModule)
     EXPECT_EQ(installedFiles, expectedFiles);
 }
 
+TEST(LinglongBuilder, mergeOutput)
+{
+    TempDir srcDir1;
+    ASSERT_TRUE(srcDir1.isValid());
+    TempDir srcDir2;
+    ASSERT_TRUE(srcDir2.isValid());
+    TempDir destDir;
+    ASSERT_TRUE(destDir.isValid());
+
+    // Create test files and directories
+    const auto &srcPath1 = srcDir1.path();
+    std::filesystem::create_directories(srcPath1 / "bin");
+    std::filesystem::create_directories(srcPath1 / "lib");
+    std::filesystem::create_directories(srcPath1 / "share");
+
+    std::ofstream(srcPath1 / "bin/tool1");
+    std::ofstream(srcPath1 / "lib/libfoo.so");
+    std::ofstream(srcPath1 / "share/data1");
+    std::ofstream(srcPath1 / ".wh.somefile");
+
+    const auto &srcPath2 = srcDir2.path();
+    std::filesystem::create_directories(srcPath2 / "bin");
+    std::filesystem::create_directories(srcPath2 / "lib");
+    std::filesystem::create_directories(srcPath2 / "share");
+
+    std::ofstream(srcPath2 / "bin/tool2");
+    std::ofstream(srcPath2 / "lib/libbar.so");
+    std::ofstream(srcPath2 / "share/data2");
+
+    // Define merge targets
+    std::vector<std::string> targets = { "bin/", "share/data1" };
+
+    linglong::builder::detail::mergeOutput({ srcPath1, srcPath2 }, destDir.path(), targets);
+
+    const auto &destPath = destDir.path();
+    // Check files are merged
+    EXPECT_TRUE(std::filesystem::exists(destPath / "bin/tool1"));
+    EXPECT_TRUE(std::filesystem::exists(destPath / "bin/tool2"));
+    EXPECT_TRUE(std::filesystem::exists(destPath / "share/data1"));
+
+    // Check files are NOT merged
+    EXPECT_FALSE(std::filesystem::exists(destPath / "lib/libfoo.so"));
+    EXPECT_FALSE(std::filesystem::exists(destPath / "lib/libbar.so"));
+    EXPECT_FALSE(std::filesystem::exists(destPath / "share/data2"));
+    EXPECT_FALSE(std::filesystem::exists(destPath / ".wh.somefile"));
+}
+
 TEST(LinglongBuilder, UabExportFilename)
 {
     linglong::builder::BuilderMock builder;
@@ -156,4 +203,4 @@ TEST(LinglongBuilder, LayerExportFilename)
     EXPECT_TRUE(ref.has_value()) << ref.error().message();
     auto filename = builder.layerExportFilename(*ref, "binary");
     EXPECT_EQ(filename, "org.deepin.demo_1.0.0.0_arm64_binary.layer");
-};
+};

libs/linglong/tests/ll-tests/src/linglong/utils/log.cpp

@@ -224,3 +224,20 @@ TEST_F(LoggerTest, MessageFormattingWorksCorrectly)
     ASSERT_TRUE(mock_journal_data.called);
     EXPECT_EQ(mock_journal_data.fields["MESSAGE"], "Hello, world! The number is 42.");
 }
+
+TEST_F(LoggerTest, LogFormat)
+{
+    logger_.setLogLevel(LogLevel::Debug);
+    logger_.setLogBackend(LogBackend::Console);
+
+    std::string captured_output;
+    {
+        StderrRedirector redirector;
+        EXPECT_TRUE(redirector.init());
+        std::string message = "message is {}";
+        logger_.log(context_, LogLevel::Debug, "Debug: {}", message);
+        captured_output = redirector.getOutput();
+    }
+
+    EXPECT_EQ("Debug: message is {}\n", captured_output);
+}

libs/utils/src/linglong/utils/log/log.h

@@ -97,7 +97,7 @@ class Logger
         }
 
         if ((logBackend & LogBackend::Console) != LogBackend::None) {
-            fmt::println(stderr, message);
+            fmt::println(stderr, "{}", message);
         }
 
         if ((logBackend & LogBackend::Journal) != LogBackend::None) {