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).

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()

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.
      */
+    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,13 +553,12 @@ 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 = build_dir / "gnc_data_home";
-        gnc_userdata_home = bfs::path(build_home);
-        g_free (build_home);
         try
         {
             gnc_validate_directory (gnc_userdata_home); // May throw
@@ -530,7 +574,6 @@ gnc_filepath_init (void)
         }
     }
 
-
     if (!have_valid_userdata_home)
     {
         /* If environment variable GNC_DATA_HOME is set, try whether

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)
+IF (NOT MAC_INTEGRATION AND NOT WIN32)
-#  ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c)
+  ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c)
-#ENDIF()
+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})

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;

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++)