[angt]

Overview

The original function was broken on Windows for some unicode paths

Additional information

Paths without a trailing separator now create the last directory too, matching the function name.
All current callers already include a trailing separator, so this change does not affect them.

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES

[ngxson]

nice, I didn't know about u8path, maybe we should do another pass to use u8path everywhere?

[angt]

Yes, I have many iterations to come haha :)

[angt]

The only downside is that it will be deprecated in C++20, but it’s easy to replace with a custom helper (and easy to detect)

[ServeurpersoCom]

It should have worked; I'm checking.

D:\A\git>
D:\A\git>set "LLAMA_CACHE=D:\A\t2\sub\"

D:\A\git>llama.cpp\build\bin\Release\llama-cli.exe -mu https://huggingface.co/ggml-org/models/resolve/main/tinyllamas/stories260K.gguf
Download 'https://huggingface.co/ggml-org/models/resolve/main/tinyllamas/stories260K.gguf' failed with status code: -1

D:\A\git>dir /s /b D:\A\t2
D:\A\t2\sub

D:\A\git>set "LLAMA_CACHE=D:\A\tést\x\"

D:\A\git>llama.cpp\build\bin\Release\llama-cli.exe -mu https://huggingface.co/ggml-org/models/resolve/main/tinyllamas/stories260K.gguf
Download 'https://huggingface.co/ggml-org/models/resolve/main/tinyllamas/stories260K.gguf' failed with status code: -1

D:\A\git>dir /s /b D:\A\tést
Fichier introuvable

D:\A\git>

[ServeurpersoCom]

LGTM, tested on Windows MSVC, nested creation with a trailing separator works; side note, a non-ASCII LLAMA_CACHE or LOCALAPPDATA still lands in a U+FFFD directory because common_get_env uses std::getenv, which returns ANSI on Windows, but that predates this PR.

[ServeurpersoCom]

Here is a possible follow-up for it, reading the environment through _wgetenv and converting to UTF-8 so the cache and config directories keep their non-ASCII characters on Windows:

diff --git a/common/common.cpp b/common/common.cpp
index ae44f054ab..572067d69c 100644
--- a/common/common.cpp
+++ b/common/common.cpp
@@ -952,14 +952,20 @@ bool fs_is_directory(const std::string & path) {
     return std::filesystem::exists(dir) && std::filesystem::is_directory(dir);
 }

+// name and value are UTF-8 on every platform
 std::string common_get_env(const std::string & name) {
+#if defined(_WIN32)
+    const wchar_t * value = _wgetenv(utf8_to_wstring(name).c_str());
+    return value == nullptr ? "" : wstring_to_utf8(value);
+#else
     const char * value = std::getenv(name.c_str());
     return value == nullptr ? "" : value;
+#endif
 }

 void common_set_env(const std::string & name, const std::string & value) {
 #if defined(_WIN32)
-    _putenv_s(name.c_str(), value.c_str());
+    _wputenv_s(utf8_to_wstring(name).c_str(), utf8_to_wstring(value).c_str());
 #else
     if (value.empty()) {
         unsetenv(name.c_str());
diff --git a/tools/rpc/rpc-server.cpp b/tools/rpc/rpc-server.cpp
index 100df3c409..4bedb6a97d 100644
--- a/tools/rpc/rpc-server.cpp
+++ b/tools/rpc/rpc-server.cpp
@@ -33,25 +33,41 @@ static bool fs_create_directory_with_parents(const std::string & path) {
     return !ec;
 }

+// returns the variable as UTF-8 on every platform, empty if unset
+static std::string get_env(const char * name) {
+#ifdef _WIN32
+    const wchar_t * value = _wgetenv(std::filesystem::path(name).c_str());
+    if (value == nullptr) {
+        return "";
+    }
+    const auto utf8 = std::filesystem::path(value).u8string();
+    return std::string(utf8.begin(), utf8.end());
+#else
+    const char * value = std::getenv(name);
+    return value == nullptr ? "" : value;
+#endif
+}
+
 // NOTE: this is copied from common.cpp to avoid linking with libcommon
 static std::string fs_get_cache_directory() {
     std::string cache_directory = "";
     auto ensure_trailing_slash = [](std::string p) {
         // Make sure to add trailing slash
-        if (p.back() != DIRECTORY_SEPARATOR) {
+        if (p.empty() || p.back() != DIRECTORY_SEPARATOR) {
             p += DIRECTORY_SEPARATOR;
         }
         return p;
     };
-    if (getenv("LLAMA_CACHE")) {
-        cache_directory = std::getenv("LLAMA_CACHE");
-    } else {
+    cache_directory = get_env("LLAMA_CACHE");
+    if (cache_directory.empty()) {
 #if defined(__linux__) || defined(__FreeBSD__) || defined(_AIX) || \
     defined(__OpenBSD__) || defined(__NetBSD__)
-        if (std::getenv("XDG_CACHE_HOME")) {
-            cache_directory = std::getenv("XDG_CACHE_HOME");
-        } else if (std::getenv("HOME")) {
-            cache_directory = std::getenv("HOME") + std::string("/.cache/");
+        const std::string xdg_cache_home = get_env("XDG_CACHE_HOME");
+        const std::string home           = get_env("HOME");
+        if (!xdg_cache_home.empty()) {
+            cache_directory = xdg_cache_home;
+        } else if (!home.empty()) {
+            cache_directory = home + "/.cache/";
         } else {
 #if defined(__linux__)
             /* no $HOME is defined, fallback to getpwuid */
@@ -66,9 +82,9 @@ static std::string fs_get_cache_directory() {
 #endif /* defined(__linux__) */
         }
 #elif defined(__APPLE__)
-        cache_directory = std::getenv("HOME") + std::string("/Library/Caches/");
+        cache_directory = get_env("HOME") + "/Library/Caches/";
 #elif defined(_WIN32)
-        cache_directory = std::getenv("LOCALAPPDATA");
+        cache_directory = get_env("LOCALAPPDATA");
 #elif defined(__EMSCRIPTEN__)
         GGML_ABORT("not implemented on this platform");
 #else

[angt]

In fact, I initially started working on the _wgetenv function 😂