util/file: cleanup log initialization and some file-util functions (#2299)

Fixes both issues mentioned in #2297

common/log/log.cpp

@@ -111,22 +111,25 @@ void log_print(const char* message) {
 // how many extra log files for a single program should be kept?
 constexpr int LOG_ROTATE_MAX = 5;
 
-void set_file(const std::string& filename) {
+void set_file(const std::string& filename, const bool should_rotate) {
   ASSERT(!gLogger.fp);
   file_util::create_dir_if_needed_for_file(filename);
 
   // rotate files. log.txt is the current one, log.1.txt is the previous one, etc.
-  auto as_path = fs::path(filename);
-  auto stem = as_path.stem().string();
-  auto ext = as_path.extension().string();
-  auto dir = as_path.parent_path();
-  for (int i = LOG_ROTATE_MAX; i-- > 0;) {
-    auto src_name = i != 0 ? fmt::format("{}.{}{}", stem, i, ext) : fmt::format("{}{}", stem, ext);
-    auto src_path = dir / src_name;
-    if (file_util::file_exists(src_path.string())) {
-      auto dst_name = fmt::format("{}.{}{}", stem, i + 1, ext);
-      auto dst_path = dir / dst_name;
-      file_util::copy_file(src_path, dst_path);
+  if (should_rotate) {
+    auto as_path = fs::path(filename);
+    auto stem = as_path.stem().string();
+    auto ext = as_path.extension().string();
+    auto dir = as_path.parent_path();
+    for (int i = LOG_ROTATE_MAX; i-- > 0;) {
+      auto src_name =
+          i != 0 ? fmt::format("{}.{}{}", stem, i, ext) : fmt::format("{}{}", stem, ext);
+      auto src_path = dir / src_name;
+      if (file_util::file_exists(src_path.string())) {
+        auto dst_name = fmt::format("{}.{}{}", stem, i + 1, ext);
+        auto dst_path = dir / dst_name;
+        file_util::copy_file(src_path, dst_path);
+      }
     }
   }
 

common/log/log.h

@@ -40,7 +40,7 @@ void log_message(level log_level, LogTime& now, const char* message);
 void log_print(const char* message);
 }  // namespace internal
 
-void set_file(const std::string& filename);
+void set_file(const std::string& filename, const bool should_rotate = true);
 void set_flush_level(level log_level);
 void set_file_level(level log_level);
 void set_stdout_level(level log_level);

common/util/FileUtil.cpp

@@ -213,6 +213,12 @@ void write_binary_file(const fs::path& name, const void* data, size_t size) {
     throw std::runtime_error("couldn't open file " + name.string());
   }
 
+  if (size == 0) {
+    // nothing to write, just 'touch' the file
+    fclose(fp);
+    return;
+  }
+
   if (fwrite(data, size, 1, fp) != 1) {
     fclose(fp);
     throw std::runtime_error("couldn't write file " + name.string());
@@ -273,6 +279,10 @@ std::vector<uint8_t> read_binary_file(const fs::path& path) {
                              " cannot be opened: " + std::string(strerror(errno)));
   fseek(fp, 0, SEEK_END);
   auto len = ftell(fp);
+  if (len == 0) {
+    fclose(fp);
+    return {};
+  }
   rewind(fp);
 
   std::vector<uint8_t> data;
@@ -585,12 +595,16 @@ std::vector<fs::path> find_directories_in_dir(const fs::path& base_dir) {
 }
 
 void copy_file(const fs::path& src, const fs::path& dst) {
-  if (src == dst) {
-    lg::error("Failed to copy_file {}, source and destination are the same\n", src.string());
-    throw std::runtime_error("Failed to copy_file");
+  // Check that the src path exists
+  if (!fs::exists(src)) {
+    throw std::runtime_error(fmt::format("Cannot copy '{}', path does not exist", src.string()));
   }
-  auto data = read_binary_file(src);
-  write_binary_file(dst, data.data(), data.size());
+  // Ensure the directory can be copied into
+  if (!fs::exists(dst.parent_path()) && !create_dir_if_needed_for_file(dst)) {
+    throw std::runtime_error(fmt::format(
+        "Cannot copy '{}', couldn't make directory to copy into '{}'", src.string(), dst.string()));
+  }
+  fs::copy_file(src, dst, fs::copy_options::overwrite_existing);
 }
 
 }  // namespace file_util

common/util/FileUtil.h

@@ -63,6 +63,6 @@ std::vector<u8> decompress_dgo(const std::vector<u8>& data_in);
 FILE* open_file(const fs::path& path, const std::string& mode);
 std::vector<fs::path> find_files_recursively(const fs::path& base_dir, const std::regex& pattern);
 std::vector<fs::path> find_directories_in_dir(const fs::path& base_dir);
-// writes the contents of one file to another
+/// Will overwrite the destination if it exists
 void copy_file(const fs::path& src, const fs::path& dst);
 }  // namespace file_util

decompiler/extractor/main.cpp

@@ -290,8 +290,12 @@ int main(int argc, char** argv) {
     }
   }
 
-  auto log_path = file_util::get_jak_project_dir() / "extractor.log";
-  lg::set_file(log_path.string());
+  try {
+    lg::set_file(file_util::get_file_path({"log", "extractor.log"}));
+  } catch (const std::exception& e) {
+    lg::error("Failed to setup logging: {}", e.what());
+    return 1;
+  }
 
   fs::path iso_data_path;
 

decompiler/main.cpp

@@ -25,13 +25,20 @@ int main(int argc, char** argv) {
   ArgumentGuard u8_guard(argc, argv);
 
   if (!file_util::setup_project_path(std::nullopt)) {
+    lg::error("Unable to setup project path");
+    return 1;
+  }
+
+  try {
+    lg::set_file(file_util::get_file_path({"log", "decompiler.log"}));
+    lg::set_file_level(lg::level::info);
+    lg::set_stdout_level(lg::level::info);
+    lg::set_flush_level(lg::level::info);
+    lg::initialize();
+  } catch (const std::exception& e) {
+    lg::error("Failed to setup logging: {}", e.what());
     return 1;
   }
-  lg::set_file(file_util::get_file_path({"log/decompiler.txt"}));
-  lg::set_file_level(lg::level::info);
-  lg::set_stdout_level(lg::level::info);
-  lg::set_flush_level(lg::level::info);
-  lg::initialize();
 
   fs::path config_path;
   fs::path in_folder;

game/main.cpp

@@ -27,7 +27,7 @@ __declspec(dllexport) int AmdPowerXpressRequestHighPerformance = 1;
  * @param verbose : should we print debug-level messages to stdout?
  */
 void setup_logging(bool verbose) {
-  lg::set_file(file_util::get_file_path({"log/game.txt"}));
+  lg::set_file(file_util::get_file_path({"log", "game.log"}));
   if (verbose) {
     lg::set_file_level(lg::level::debug);
     lg::set_stdout_level(lg::level::debug);
@@ -101,7 +101,12 @@ int main(int argc, char** argv) {
     printf("AVX2 mode disabled\n");
   }
 
-  setup_logging(verbose);
+  try {
+    setup_logging(verbose);
+  } catch (const std::exception& e) {
+    lg::error("Failed to setup logging: {}", e.what());
+    return 1;
+  }
 
   bool force_debug_next_time = false;
   while (true) {

goalc/main.cpp

@@ -16,7 +16,7 @@
 #include "third-party/fmt/core.h"
 
 void setup_logging() {
-  lg::set_file(file_util::get_file_path({"log/compiler.txt"}));
+  lg::set_file(file_util::get_file_path({"log", "compiler.log"}));
   lg::set_file_level(lg::level::info);
   lg::set_stdout_level(lg::level::info);
   lg::set_flush_level(lg::level::info);
@@ -79,7 +79,13 @@ int main(int argc, char** argv) {
     return 1;
   }
 
-  setup_logging();
+  try {
+    setup_logging();
+  } catch (const std::exception& e) {
+    lg::error("Failed to setup logging: {}", e.what());
+    return 1;
+  }
+
   lg::info("OpenGOAL Compiler {}.{}", versions::GOAL_VERSION_MAJOR, versions::GOAL_VERSION_MINOR);
 
   // Figure out the username

lsp/main.cpp

@@ -35,7 +35,7 @@
 
 void setup_logging(bool verbose, std::string log_file) {
   if (!log_file.empty()) {
-    lg::set_file(log_file);
+    lg::set_file(log_file, false);
   }
   if (verbose) {
     lg::set_file_level(lg::level::debug);
@@ -68,7 +68,12 @@ int main(int argc, char** argv) {
   AppState appstate;
   LSPRouter lsp_router;
   appstate.verbose = verbose;
-  setup_logging(appstate.verbose, logfile);
+  try {
+    setup_logging(appstate.verbose, logfile);
+  } catch (const std::exception& e) {
+    lg::error("Failed to setup logging: {}", e.what());
+    return 1;
+  }
   lsp_router.init_routes();
 
   lg::info("OpenGOAL LSP Initialized, ready for requests");