Filesystem: Fix copy overwrite of symlink with file

... and improve log messages in case of failure.

5 files changed, 13 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76da1d49..66406237 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -56,6 +56,8 @@ A feature release on top of `1.2.0`, backwards compatible.
   resulting stage is well-formed, i.e., without tree conflicts.
 - Local execution and `just execute` now correctly create empty
   directories if they are part of the action's input.
+- Fixed overwrite of existing symlinks in the output directory
+  when using subcommands `install` and `install-cas`.
 
 ## Release `1.2.0` (2023-08-25)
 
diff --git a/src/buildtool/execution_api/git/git_api.hpp b/src/buildtool/execution_api/git/git_api.hpp
index 52c21627..dcab8335 100644
--- a/src/buildtool/execution_api/git/git_api.hpp
+++ b/src/buildtool/execution_api/git/git_api.hpp
@@ -83,6 +83,9 @@ class GitApi final : public IExecutionApi {
                     not FileSystemManager::WriteFileAs</*kSetEpochTime=*/true,
                                                        /*kSetWritable=*/true>(
                         *blob, output_paths[i], info.type)) {
+                    Logger::Log(LogLevel::Error,
+                                "staging to output path {} failed.",
+                                output_paths[i].string());
                     return false;
                 }
             }
diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp
index d46be8ed..a5596350 100644
--- a/src/buildtool/execution_api/local/local_api.hpp
+++ b/src/buildtool/execution_api/local/local_api.hpp
@@ -114,6 +114,9 @@ class LocalApi final : public IExecutionApi {
                              /*kSetEpochTime=*/true,
                              /*kSetWritable=*/true>(
                              *blob_path, output_paths[i], info.type)) {
+                    Logger::Log(LogLevel::Error,
+                                "staging to output path {} failed.",
+                                output_paths[i].string());
                     return false;
                 }
             }
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index 3d1cf40e..7539771e 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -263,6 +263,9 @@ auto BazelApi::CreateAction(
             if (not FileSystemManager::WriteFileAs</*kSetEpochTime=*/true,
                                                    /*kSetWritable=*/true>(
                     blobs[pos].data, output_paths[gpos], type)) {
+                Logger::Log(LogLevel::Error,
+                            "staging to output path {} failed.",
+                            output_paths[gpos].string());
                 return false;
             }
         }
diff --git a/src/buildtool/file_system/file_system_manager.hpp b/src/buildtool/file_system/file_system_manager.hpp
index 181cba27..71b319d1 100644
--- a/src/buildtool/file_system/file_system_manager.hpp
+++ b/src/buildtool/file_system/file_system_manager.hpp
@@ -1013,9 +1013,8 @@ class FileSystemManager {
             std::filesystem::copy_options::overwrite_existing) noexcept
         -> bool {
         try {
-            // src and dst should be actual files
-            if (std::filesystem::is_symlink(src) or
-                std::filesystem::is_symlink(dst)) {
+            // src should be an actual file
+            if (std::filesystem::is_symlink(src)) {
                 return false;
             }
             if (not RemoveFile(dst)) {