Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ message(STATUS " Modify using: -DENABLE_TESTING=ON/OFF")
########################################################################
option(ENABLE_PROFILING "Launch system profiler after build" OFF)
if(ENABLE_PROFILING)
option(ENABLE_XDG_CONFIG "Prefer XDG_CONFIG_HOME for config paths when enabled" OFF)
if(DEFINED VOLK_CONFIGPATH)
get_filename_component(VOLK_CONFIGPATH ${VOLK_CONFIGPATH} ABSOLUTE)
set(VOLK_CONFIGPATH "${VOLK_CONFIGPATH}/volk")
Expand All @@ -367,10 +368,22 @@ if(ENABLE_PROFILING)
STATUS "System profiling is enabled, using env path: $ENV{VOLK_CONFIGPATH}")
else()
message(STATUS "System profiling is enabled with default paths.")
if(DEFINED ENV{HOME})
set(VOLK_CONFIGPATH "$ENV{HOME}/.volk")
elseif(DEFINED ENV{APPDATA})
set(VOLK_CONFIGPATH "$ENV{APPDATA}/.volk")
if(ENABLE_XDG_CONFIG)
if(DEFINED ENV{XDG_CONFIG_HOME})
set(VOLK_CONFIGPATH "$ENV{XDG_CONFIG_HOME}/volk")
elseif(DEFINED ENV{HOME})
set(VOLK_CONFIGPATH "$ENV{HOME}/.config/volk")
elseif(DEFINED ENV{APPDATA})
# Windows APPDATA fallback
set(VOLK_CONFIGPATH "$ENV{APPDATA}/.volk")
endif()
else()
# Preserve previous behavior: prefer HOME/.volk (legacy) then APPDATA
if(DEFINED ENV{HOME})
set(VOLK_CONFIGPATH "$ENV{HOME}/.volk")
elseif(DEFINED ENV{APPDATA})
set(VOLK_CONFIGPATH "$ENV{APPDATA}/.volk")
endif()
endif()
endif()
else()
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ $ sudo ldconfig
$ volk_profile
```

## Configuration location

On Linux and other UNIX-like systems VOLK follows the XDG Base Directory
Specification for user configuration files where possible. User configuration
is sought in the following order: `VOLK_CONFIGPATH` (if set), `$XDG_CONFIG_HOME/volk`,
`$HOME/.config/volk`, and finally the legacy `$HOME/.volk`. For write operations
the library will create the XDG config directory when needed. Use the
`VOLK_CONFIGPATH` environment variable to override or force a custom location.

#### Missing submodule
We use [cpu_features](https://github.com/google/cpu_features) to detect CPU features, e.g. AVX.
Some platforms require a very recent version that is not available through the appropriate package manager.
Expand Down
114 changes: 85 additions & 29 deletions lib/volk_prefs.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
#include <stdlib.h>
#include <string.h>
#if defined(_MSC_VER)
#include <direct.h>
#include <io.h>
#define access _access
#define F_OK 0
#else
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#endif
#include <volk/volk_prefs.h>
Expand All @@ -24,52 +27,105 @@ void volk_get_config_path(char* path, bool read)
{
if (!path)
return;
const char* suffix = "/.volk/volk_config";
const char* suffix2 = "/volk/volk_config"; // non-hidden
char* home = NULL;
const char* legacy_suffix = "/.volk/volk_config";
const char* nonhidden_suffix = "/volk/volk_config"; // non-hidden
char tmp[512] = { 0 };

// allows config redirection via env variable
home = getenv("VOLK_CONFIGPATH");
if (home != NULL) {
strncpy(path, home, 512);
strcat(path, suffix2);
if (!read || access(path, F_OK) != -1) {
/* 1) explicit override via VOLK_CONFIGPATH */
const char* env_override = getenv("VOLK_CONFIGPATH");
if (env_override) {
snprintf(tmp, sizeof(tmp), "%s%s", env_override, nonhidden_suffix);
if (!read || access(tmp, F_OK) == 0) {
strncpy(path, tmp, 512);
return;
}
}

// check for user-local config file
home = getenv("HOME");
if (home != NULL) {
strncpy(path, home, 512);
strcat(path, suffix);
if (!read || (access(path, F_OK) != -1)) {
/* 2) XDG_CONFIG_HOME/volk if XDG set */
const char* xdg = getenv("XDG_CONFIG_HOME");
if (xdg) {
snprintf(tmp, sizeof(tmp), "%s/volk/volk_config", xdg);
if (!read) {
char parent[512];
char dir[512];
/* ensure XDG_CONFIG_HOME exists, then create XDG/volk */
snprintf(parent, sizeof(parent), "%s", xdg);
snprintf(dir, sizeof(dir), "%s/volk", xdg);
#if defined(_MSC_VER)
_mkdir(parent);
_mkdir(dir);
#else
struct stat st = { 0 };
if (stat(parent, &st) == -1) {
mkdir(parent, 0755);
}
if (stat(dir, &st) == -1) {
mkdir(dir, 0755);
}
#endif
Comment on lines +54 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old function behavior was to just check for possible folders and return the path. Why is it necessary to create folders now?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original function returned a path string, but didn't guarantee that path could actually be used for writing. If a user called volk_get_config_path(path, false) (write-mode) and tried to write the config file, it would fail with a file I/O error if the parent directories didn't exist.

I agree this might be outside the function's scope. Would you prefer:
Option A: Keep current behavior (create dirs in write-mode) — simpler for callers
Option B: Return path only, let caller handle directory creation — better separation of concerns

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go with option B. This function should just discover where to look for files =D

}
if (!read || access(tmp, F_OK) == 0) {
strncpy(path, tmp, 512);
return;
}
}

// check for config file in APPDATA (Windows)
home = getenv("APPDATA");
if (home != NULL) {
strncpy(path, home, 512);
strcat(path, suffix);
if (!read || (access(path, F_OK) != -1)) {
/* 3) $HOME/.config/volk */
const char* home = getenv("HOME");
if (home) {
snprintf(tmp, sizeof(tmp), "%s/.config/volk/volk_config", home);
if (!read) {
char parent[512];
char dir[512];
/* ensure HOME/.config exists, then create HOME/.config/volk */
snprintf(parent, sizeof(parent), "%s/.config", home);
snprintf(dir, sizeof(dir), "%s/.config/volk", home);
#if defined(_MSC_VER)
_mkdir(parent);
_mkdir(dir);
#else
struct stat st = { 0 };
if (stat(parent, &st) == -1) {
mkdir(parent, 0755);
}
if (stat(dir, &st) == -1) {
mkdir(dir, 0755);
}
#endif
}
if (!read || access(tmp, F_OK) == 0) {
strncpy(path, tmp, 512);
return;
}
}

// check for system-wide config file
if (access("/etc/volk/volk_config", F_OK) != -1) {
strncpy(path, "/etc", 512);
strcat(path, suffix2);
if (!read || (access(path, F_OK) != -1)) {
/* 4) legacy $HOME/.volk */
if (home) {
snprintf(tmp, sizeof(tmp), "%s%s", home, legacy_suffix);
if (!read || access(tmp, F_OK) == 0) {
strncpy(path, tmp, 512);
return;
}
}

// If still no path was found set path[0] to '0' and fall through
path[0] = 0;
return;
/* 5) Windows APPDATA fallback */
const char* appdata = getenv("APPDATA");
if (appdata) {
snprintf(tmp, sizeof(tmp), "%s%s", appdata, legacy_suffix);
if (!read || access(tmp, F_OK) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old behavior was access(tmp, F_OK) != -1, why does that need to change?

strncpy(path, tmp, 512);
return;
}
}

/* 6) System-wide */
if (access("/etc/volk/volk_config", F_OK) == 0) {
strncpy(path, "/etc/volk/volk_config", 512);
return;
}

/* Nothing found - return empty path */
path[0] = '\0';
}

size_t volk_load_preferences(volk_arch_pref_t** prefs_res)
Expand Down
27 changes: 21 additions & 6 deletions tests/CMakeLists.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does anything in this file need to change? The intended behavior before was to skip over these tests of the required dependencies are missing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I made the CMakeLists.txt fetch fmt / googletest only as a fallback when the system packages are not found (it still prefers find_package(...) first). This keeps CI reproducible by pinning a known-good version while not forcing all contributors to install those libs system-wide.
  • I also used FetchContent_Populate + add_subdirectory for googletest to work around a CMake/FetchContent issue we hit on Windows/WSL mounts.
  • If you'd rather keep the prior behavior (skip tests when deps are missing) I'm happy to revert to that — or I can gate the FetchContent behind an option so packagers can opt out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK to go with fetchContent after the check. One more thing though: We have similar stuff in other parts of our CMake structure. That'd need to be combined.

Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,28 @@ if(NOT ENABLE_TESTING)
return()
endif(NOT ENABLE_TESTING)

find_package(fmt)
find_package(GTest)
include(FetchContent)

if(NOT fmt_FOUND OR NOT GTest_FOUND)
message(warning "Missing fmtlib and/or googletest for this test suite")
return()
endif(NOT fmt_FOUND OR NOT GTest_FOUND)
# fmt is provided by the top-level CMakeLists.txt, no need to fetch here.

# Prefer system GTest if available, otherwise fetch googletest.
find_package(GTest QUIET)
if(NOT GTest_FOUND)
message(STATUS "GTest not found: fetching googletest via FetchContent")
set(BUILD_GMOCK OFF CACHE BOOL "Disable gmock" FORCE)
set(INSTALL_GTEST OFF CACHE BOOL "Disable gtest install" FORCE)
FetchContent_Declare(
googletest
GIT_REPOSITORY https://github.com/google/googletest.git
GIT_TAG v1.17.0
GIT_SHALLOW TRUE
)
FetchContent_GetProperties(googletest)
if(NOT googletest_POPULATED)
FetchContent_Populate(googletest)
add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()
endif()

file(GLOB volk_test_files "test_*.cc")

Expand Down
153 changes: 153 additions & 0 deletions tests/test_volk_paths.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// Tests for volk config path resolution (XDG preference, legacy fallback)
#include <gtest/gtest.h>
#include <volk/volk_prefs.h>

#include <filesystem>
#include <fstream>
#include <string>
#include <string_view>
#include <cstdlib>
#include <random>
#include <sstream>

#if defined(_WIN32)
static void set_env(std::string_view name, const char* value)
{
std::string n(name);
if (value)
_putenv_s(n.c_str(), value);
else
_putenv_s(n.c_str(), "");
}
#else
static void set_env(std::string_view name, const char* value)
{
std::string n(name);
if (value)
setenv(n.c_str(), value, 1);
else
unsetenv(n.c_str());
}
#endif

static std::string volk_config_path_read()
{
char buf[512] = {0};
volk_get_config_path(buf, true);
return std::string(buf);
}

static std::string volk_config_path_write()
{
char buf[512] = {0};
volk_get_config_path(buf, false);
return std::string(buf);
}

// Helper to generate unique temp directory names
static std::filesystem::path unique_temp_path(const char* prefix)
{
std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> distrib(100000, 999999);
std::ostringstream oss;
oss << prefix << "_" << distrib(gen);
return std::filesystem::temp_directory_path() / oss.str();
}

TEST(VolkPaths, OverrideEnv)
{
namespace fs = std::filesystem;
fs::path tmp = unique_temp_path("volk_test_override");
fs::create_directories(tmp / "volk");
std::ofstream(tmp / "volk" / "volk_config") << "test";

set_env("VOLK_CONFIGPATH", tmp.string().c_str());

std::string expect = (tmp / "volk" / "volk_config").generic_string();
std::string got = volk_config_path_read();
EXPECT_EQ(expect, got);

// cleanup
set_env("VOLK_CONFIGPATH", nullptr);
fs::remove_all(tmp);
}

TEST(VolkPaths, XDGPreference)
{
namespace fs = std::filesystem;
fs::path tmp = unique_temp_path("volk_test_xdg");
fs::create_directories(tmp / "volk");
std::ofstream(tmp / "volk" / "volk_config") << "test";

set_env("VOLK_CONFIGPATH", nullptr);
set_env("XDG_CONFIG_HOME", tmp.string().c_str());

std::string expect = (tmp / "volk" / "volk_config").generic_string();
std::string got = volk_config_path_read();
EXPECT_EQ(expect, got);

set_env("XDG_CONFIG_HOME", nullptr);
fs::remove_all(tmp);
}

TEST(VolkPaths, HomeDotConfigFallback)
{
namespace fs = std::filesystem;
fs::path tmp = unique_temp_path("volk_test_homecfg");
fs::create_directories(tmp / ".config" / "volk");
std::ofstream(tmp / ".config" / "volk" / "volk_config") << "test";

set_env("VOLK_CONFIGPATH", nullptr);
set_env("XDG_CONFIG_HOME", nullptr);
set_env("HOME", tmp.string().c_str());

std::string expect = (tmp / ".config" / "volk" / "volk_config").generic_string();
std::string got = volk_config_path_read();
EXPECT_EQ(expect, got);

set_env("HOME", nullptr);
fs::remove_all(tmp);
}

TEST(VolkPaths, LegacyFallback)
{
namespace fs = std::filesystem;
fs::path tmp = unique_temp_path("volk_test_legacy");
fs::create_directories(tmp / ".volk");
std::ofstream(tmp / ".volk" / "volk_config") << "test";

set_env("VOLK_CONFIGPATH", nullptr);
set_env("XDG_CONFIG_HOME", nullptr);
set_env("HOME", tmp.string().c_str());

// Ensure .config/volk does not exist to force legacy path selection
fs::remove_all(tmp / ".config");

std::string expect = (tmp / ".volk" / "volk_config").generic_string();
std::string got = volk_config_path_read();
EXPECT_EQ(expect, got);

set_env("HOME", nullptr);
fs::remove_all(tmp);
}

TEST(VolkPaths, WriteCreatesXDGDir)
{
namespace fs = std::filesystem;
fs::path tmp = unique_temp_path("volk_test_write");

set_env("VOLK_CONFIGPATH", nullptr);
set_env("XDG_CONFIG_HOME", tmp.string().c_str());

// Ensure directory does not exist yet
fs::remove_all(tmp);
EXPECT_FALSE(fs::exists(tmp / "volk"));

// Call write-mode; this should create the XDG/volk directory
volk_config_path_write();
EXPECT_TRUE(fs::exists(tmp / "volk"));

set_env("XDG_CONFIG_HOME", nullptr);
fs::remove_all(tmp);
}
Loading