common : add fs_write_atomic() (#29642)

- Check for buffered write errors when closing downloaded files.
- Use UTF-8 paths when writing ETag files on Windows.
- Write in binary mode on Windows.

Signed-off-by: Adrien Gallouët <angt@huggingface.co>

common/common.cpp

@@ -912,6 +912,29 @@ std::string fs_path_to_utf8(const std::filesystem::path & path) {
     return std::string(value.begin(), value.end());
 }
 
+void fs_write_atomic(const std::filesystem::path & path, const std::string & data) {
+    std::error_code ec;
+    std::filesystem::path path_tmp = path;
+    path_tmp += ".tmp";
+
+    if (path.has_parent_path()) {
+        std::filesystem::create_directories(path.parent_path(), ec);
+    }
+
+    std::ofstream file(path_tmp, std::ios::binary);
+    file << data;
+    file.close();
+
+    if (!file.fail()) {
+        std::filesystem::rename(path_tmp, path, ec);
+    }
+
+    if (file.fail() || ec) {
+        std::filesystem::remove(path_tmp, ec);
+        throw std::runtime_error("failed to write file: " + fs_path_to_utf8(path));
+    }
+}
+
 bool fs_is_directory(const std::string & path) {
     std::filesystem::path dir(path);
     return std::filesystem::exists(dir) && std::filesystem::is_directory(dir);

common/common.h

@@ -929,6 +929,8 @@ std::vector<common_file_info> fs_list(const std::string & path, bool include_dir
 // fs open, also handle UTF8 on Windows
 std::ifstream fs_open_ifstream(const std::string & fname, std::ios_base::openmode mode);
 
+void fs_write_atomic(const std::filesystem::path & path, const std::string & data);
+
 //
 // TTY utils
 //

common/download.cpp

@@ -46,39 +46,9 @@
 // downloader
 //
 
-// validate repo name format: owner/repo
-static void write_file(const std::string & fname, const std::string & content) {
-    const std::string fname_tmp = fname + ".tmp";
-    std::ofstream     file(fname_tmp);
-    if (!file) {
-        throw std::runtime_error(string_format("error: failed to open file '%s'\n", fname.c_str()));
-    }
-
-    try {
-        file << content;
-        file.close();
-
-        // Makes write atomic
-        if (rename(fname_tmp.c_str(), fname.c_str()) != 0) {
-            LOG_ERR("%s: unable to rename file: %s to %s\n", __func__, fname_tmp.c_str(), fname.c_str());
-            // If rename fails, try to delete the temporary file
-            if (remove(fname_tmp.c_str()) != 0) {
-                LOG_ERR("%s: unable to delete temporary file: %s\n", __func__, fname_tmp.c_str());
-            }
-        }
-    } catch (...) {
-        // If anything fails, try to delete the temporary file
-        if (remove(fname_tmp.c_str()) != 0) {
-            LOG_ERR("%s: unable to delete temporary file: %s\n", __func__, fname_tmp.c_str());
-        }
-
-        throw std::runtime_error(string_format("error: failed to write file '%s'\n", fname.c_str()));
-    }
-}
-
 static void write_etag(const std::string & path, const std::string & etag) {
     const std::string etag_path = path + ".etag";
-    write_file(etag_path, etag);
+    fs_write_atomic(std::filesystem::u8path(etag_path), etag);
     LOG_DBG("%s: file etag saved: %s\n", __func__, etag_path.c_str());
 }
 
@@ -274,6 +244,12 @@ static bool common_pull_file(httplib::Client & cli,
         return false;
     }
 
+    ofs.close();
+    if (!ofs) {
+        LOG_ERR("%s: error closing file: %s\n", __func__, path_tmp.c_str());
+        return false;
+    }
+
     return true;
 }
 

common/hf-cache.cpp

@@ -172,29 +172,6 @@ static bool is_valid_subpath(const fs::path & path, const fs::path & subpath) {
     return b_end == b.end();
 }
 
-static void safe_write_file(const fs::path & path, const std::string & data) {
-    fs::path path_tmp = path;
-    path_tmp += ".tmp";
-
-    if (path.has_parent_path()) {
-        fs::create_directories(path.parent_path());
-    }
-
-    std::ofstream file(path_tmp);
-    file << data;
-    file.close();
-
-    std::error_code ec;
-
-    if (!file.fail()) {
-        fs::rename(path_tmp, path, ec);
-    }
-    if (file.fail() || ec) {
-        fs::remove(path_tmp, ec);
-        throw std::runtime_error("failed to write file: " + fs_path_to_utf8(path));
-    }
-}
-
 static common_json api_get(const std::string & url,
                            const std::string & token) {
     auto [cli, parts] = common_http_client(url);
@@ -282,7 +259,7 @@ static std::string get_repo_commit(const std::string & repo_id,
             return {};
         }
 
-        safe_write_file(refs_path / name_path, commit);
+        fs_write_atomic(refs_path / name_path, commit);
         return commit;
 
     } catch (const common_json_error & e) {