gnucash unstable: Improve gnc_data_home verification and creation
Geert Janssens
gjanssens at code.gnucash.org
Fri Feb 2 06:18:58 EST 2018
Updated via https://github.com/Gnucash/gnucash/commit/0d4f6e05 (commit)
from https://github.com/Gnucash/gnucash/commit/0f5bb351 (commit)
commit 0d4f6e054d0b2cf66cd34aa4feb737ddeae66f85
Author: Geert Janssens <geert at kobaltwit.be>
Date: Fri Feb 2 10:24:04 2018 +0100
Improve gnc_data_home verification and creation
- Don't attempt to create a subdirectory of a non-existing home directory (use tmpdir as base directory in that case)
- Make sure all tests run in an environment with GNC_BUILDDIR and GNC_UNINSTALLED set. Otherwise
the one-shot old .gnucash to new GNC_DATA_HOME migration may already have run at build time,
preventing us from informing the user a run time.
- Re-enable the userdata-dir with invalid home test (linux only).
diff --git a/common/cmake_modules/GncAddTest.cmake b/common/cmake_modules/GncAddTest.cmake
index 6435f22..9c7863c 100644
--- a/common/cmake_modules/GncAddTest.cmake
+++ b/common/cmake_modules/GncAddTest.cmake
@@ -76,15 +76,15 @@ FUNCTION(GNC_ADD_TEST _TARGET _SOURCE_FILES TEST_INCLUDE_VAR_NAME TEST_LIBS_VAR_
IF (${HAVE_ENV_VARS})
SET(CMAKE_COMMAND_TMP "")
IF (${CMAKE_VERSION} VERSION_GREATER 3.1)
- SET(CMAKE_COMMAND_TMP ${CMAKE_COMMAND} -E env "GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}")
+ SET(CMAKE_COMMAND_TMP ${CMAKE_COMMAND} -E env "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}")
ENDIF()
ADD_TEST(${_TARGET} ${CMAKE_COMMAND_TMP}
${CMAKE_BINARY_DIR}/bin/${_TARGET}
)
- SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}")
+ SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}")
ELSE()
ADD_TEST(NAME ${_TARGET} COMMAND ${_TARGET})
- SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_BUILDDIR=${CMAKE_BINARY_DIR}")
+ SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR}")
ENDIF()
ADD_DEPENDENCIES(check ${_TARGET})
ENDFUNCTION()
diff --git a/libgnucash/core-utils/gnc-filepath-utils.cpp b/libgnucash/core-utils/gnc-filepath-utils.cpp
index fc57483..8b45780 100644
--- a/libgnucash/core-utils/gnc-filepath-utils.cpp
+++ b/libgnucash/core-utils/gnc-filepath-utils.cpp
@@ -318,6 +318,27 @@ gnc_path_find_localized_html_file (const gchar *file_name)
}
/* ====================================================================== */
+static auto gnc_userdata_home = bfs::path();
+static auto build_dir = bfs::path();
+
+static bool dir_is_descendant (const bfs::path& path, const bfs::path& base)
+{
+ auto test_path = path;
+ if (bfs::exists (path))
+ test_path = bfs::canonical (path);
+ auto test_base = base;
+ if (bfs::exists (base))
+ test_base = bfs::canonical (base);
+
+ auto is_descendant = (test_path.string() == test_base.string());
+ while (!test_path.empty() && !is_descendant)
+ {
+ test_path = test_path.parent_path();
+ is_descendant = (test_path.string() == test_base.string());
+ }
+ return is_descendant;
+}
+
/** @brief Check that the supplied directory path exists, is a directory, and
* that the user has adequate permissions to use it.
*
@@ -329,13 +350,39 @@ gnc_validate_directory (const bfs::path &dirname)
if (dirname.empty())
return false;
- /* Create directories if they don't exist yet
+ auto create_dirs = true;
+ if (build_dir.empty() || !dir_is_descendant (dirname, build_dir))
+ {
+ /* Gnucash won't create a home directory
+ * if it doesn't exist yet. So if the directory to create
+ * is a descendant of the homedir, we can't create it either.
+ * This check is conditioned on do_homedir_check because
+ * we need to overrule it during build (when guile interferes)
+ * and testing.
+ */
+ auto home_dir = bfs::path (g_get_home_dir ());
+ auto homedir_exists = bfs::exists(home_dir);
+ auto is_descendant = dir_is_descendant (dirname, home_dir);
+ if (!homedir_exists && is_descendant)
+ create_dirs = false;
+ }
+
+ /* Create directories if they don't exist yet and we can
+ *
* Note this will do nothing if the directory and its
* parents already exist, but will fail if the path
* points to a file or a softlink. So it serves as a test
* for that as well.
*/
- bfs::create_directories(dirname);
+ if (create_dirs)
+ bfs::create_directories(dirname);
+ else
+ throw (bfs::filesystem_error (
+ std::string (dirname.string() +
+ " is a descendant of a non-existing home directory. As " +
+ PACKAGE_NAME +
+ " will never create a home directory this path can't be used"),
+ dirname, bst::error_code(bst::errc::permission_denied, bst::generic_category())));
auto d = bfs::directory_entry (dirname);
auto perms = d.status().permissions();
@@ -357,8 +404,6 @@ gnc_validate_directory (const bfs::path &dirname)
return true;
}
-static auto gnc_userdata_home = bfs::path();
-
/* Will attempt to copy all files and directories from src to dest
* Returns true if successful or false if not */
static bool
@@ -508,16 +553,15 @@ gnc_filepath_init (void)
* in the base of the build directory. This is to deal with all kinds of
* issues when the build environment is not a complete environment (like
* it could be missing a valid home directory). */
- auto builddir = g_getenv ("GNC_BUILDDIR");
+ auto env_build_dir = g_getenv ("GNC_BUILDDIR");
+ build_dir = bfs::path(env_build_dir ? env_build_dir : "");
auto running_uninstalled = (g_getenv ("GNC_UNINSTALLED") != NULL);
- if (running_uninstalled && builddir)
+ if (running_uninstalled && !build_dir.empty())
{
- auto build_home = g_build_filename (builddir, "gnc_data_home", NULL);
- gnc_userdata_home = bfs::path(build_home);
- g_free (build_home);
+ gnc_userdata_home = build_dir / "gnc_data_home";
try
{
- gnc_validate_directory(gnc_userdata_home); // May throw
+ gnc_validate_directory (gnc_userdata_home); // May throw
have_valid_userdata_home = true;
gnc_userdata_home_exists = true; // To prevent possible migration further down
}
@@ -530,19 +574,18 @@ gnc_filepath_init (void)
}
}
-
if (!have_valid_userdata_home)
{
/* If environment variable GNC_DATA_HOME is set, try whether
* it points at a valid directory. */
- auto gnc_userdata_home_env = g_getenv("GNC_DATA_HOME");
+ auto gnc_userdata_home_env = g_getenv ("GNC_DATA_HOME");
if (gnc_userdata_home_env)
{
- gnc_userdata_home = bfs::path(gnc_userdata_home_env);
+ gnc_userdata_home = bfs::path (gnc_userdata_home_env);
try
{
- gnc_userdata_home_exists = bfs::exists(gnc_userdata_home);
- gnc_validate_directory(gnc_userdata_home); // May throw
+ gnc_userdata_home_exists = bfs::exists (gnc_userdata_home);
+ gnc_validate_directory (gnc_userdata_home); // May throw
have_valid_userdata_home = true;
}
catch (const bfs::filesystem_error& ex)
@@ -563,31 +606,31 @@ gnc_filepath_init (void)
gnc_userdata_home = userdata_home / PACKAGE;
try
{
- gnc_userdata_home_exists = bfs::exists(gnc_userdata_home);
- gnc_validate_directory(gnc_userdata_home);
+ gnc_userdata_home_exists = bfs::exists (gnc_userdata_home);
+ gnc_validate_directory (gnc_userdata_home);
}
catch (const bfs::filesystem_error& ex)
{
- g_warning("User data directory doesn't exist, yet could not be created. Proceed with caution.\n"
+ g_warning ("User data directory doesn't exist, yet could not be created. Proceed with caution.\n"
"(Error: %s)", ex.what());
}
}
auto migrated = FALSE;
if (!gnc_userdata_home_exists)
- migrated = copy_recursive(bfs::path (g_get_home_dir()) / ".gnucash",
- gnc_userdata_home);
+ migrated = copy_recursive (bfs::path (g_get_home_dir()) / ".gnucash",
+ gnc_userdata_home);
/* Try to create the standard subdirectories for gnucash' user data */
try
{
- gnc_validate_directory(gnc_userdata_home / "books");
- gnc_validate_directory(gnc_userdata_home / "checks");
- gnc_validate_directory(gnc_userdata_home / "translog");
+ gnc_validate_directory (gnc_userdata_home / "books");
+ gnc_validate_directory (gnc_userdata_home / "checks");
+ gnc_validate_directory (gnc_userdata_home / "translog");
}
catch (const bfs::filesystem_error& ex)
{
- g_warning("Default user data subdirectories don't exist, yet could not be created. Proceed with caution.\n"
+ g_warning ("Default user data subdirectories don't exist, yet could not be created. Proceed with caution.\n"
"(Error: %s)", ex.what());
}
diff --git a/libgnucash/core-utils/test/CMakeLists.txt b/libgnucash/core-utils/test/CMakeLists.txt
index 1baddd0..10d5352 100644
--- a/libgnucash/core-utils/test/CMakeLists.txt
+++ b/libgnucash/core-utils/test/CMakeLists.txt
@@ -19,9 +19,9 @@ ENDMACRO()
ADD_CORE_UTILS_TEST(test-gnc-glib-utils test-gnc-glib-utils.c)
ADD_CORE_UTILS_TEST(test-resolve-file-path test-resolve-file-path.c)
ADD_CORE_UTILS_TEST(test-userdata-dir test-userdata-dir.c)
-#IF (NOT MAC_INTEGRATION AND NOT WIN32)
-# ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c)
-#ENDIF()
+IF (NOT MAC_INTEGRATION AND NOT WIN32)
+ ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c)
+ENDIF()
IF (MAC_INTEGRATION)
TARGET_COMPILE_OPTIONS(test-userdata-dir PRIVATE ${OSX_EXTRA_COMPILE_FLAGS})
TARGET_COMPILE_DEFINITIONS(test-userdata-dir PRIVATE ${GTK_MAC_CFLAGS_OTHER})
diff --git a/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c b/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c
index 9595147..b28f3f3 100644
--- a/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c
+++ b/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c
@@ -70,6 +70,8 @@ main(G_GNUC_UNUSED int argc, G_GNUC_UNUSED char **argv)
#ifndef G_OS_WIN32
int i;
const char *tmp_dir = g_get_tmp_dir();
+ const char *builddir = g_getenv ("GNC_BUILDDIR");
+ char *homedir = g_build_filename (builddir, "notexist", NULL);
/* Assume we're not in a build environment to test
* the function's actual behaviour in a real world use case, using
@@ -80,7 +82,7 @@ main(G_GNUC_UNUSED int argc, G_GNUC_UNUSED char **argv)
/* Run usr conf dir tests with an invalid homedir
* The code should fall back to using the temporary
* directory in that case. */
- g_setenv("HOME", "/notexist", TRUE);
+ g_setenv("HOME", homedir, TRUE);
for (i = 0; strs2[i].funcname != NULL; i++)
{
char *daout;
diff --git a/libgnucash/core-utils/test/test-userdata-dir.c b/libgnucash/core-utils/test/test-userdata-dir.c
index d72d9b8..5a8cd5c 100644
--- a/libgnucash/core-utils/test/test-userdata-dir.c
+++ b/libgnucash/core-utils/test/test-userdata-dir.c
@@ -160,13 +160,14 @@ main(int argc, char **argv)
g_unsetenv("GNC_UNINSTALLED");
- /* Second run, with existing userdata_dir, but without the GnuCash subdir
+ /* Second run, with XDG_DATA_HOME set and with existing home_dir, but
+ * without the XDG_DATA_HOME subdirectories.
This test can not be run on OS X or Windows, as our code is not using
XDG_DATA_HOME on these platforms */
#ifndef MAC_INTEGRATION
#ifndef G_OS_WIN32
+ g_mkdir_with_parents(home_dir, 0750);
userdata_dir = g_build_filename(home_dir, ".local", "share", (gchar *)NULL);
- g_mkdir_with_parents(userdata_dir, 0750);
g_setenv("XDG_DATA_HOME", userdata_dir, TRUE);
gnc_filepath_init();
for (i = 0; strs2[i].funcname != NULL; i++)
Summary of changes:
common/cmake_modules/GncAddTest.cmake | 6 +-
libgnucash/core-utils/gnc-filepath-utils.cpp | 91 ++++++++++++++++------
libgnucash/core-utils/test/CMakeLists.txt | 6 +-
.../test/test-userdata-dir-invalid-home.c | 4 +-
libgnucash/core-utils/test/test-userdata-dir.c | 5 +-
5 files changed, 79 insertions(+), 33 deletions(-)
More information about the gnucash-changes
mailing list