[Bf-blender-cvs] [939f20490d6] blender-projects-basics: Handle and test both Unix and Windows style slashes
Julian Eisel
noreply at git.blender.org
Thu Sep 29 00:18:14 CEST 2022
Commit: 939f20490d6fa2063171efa6c7ed529da0718f11
Author: Julian Eisel
Date: Thu Sep 29 00:09:33 2022 +0200
Branches: blender-projects-basics
https://developer.blender.org/rB939f20490d6fa2063171efa6c7ed529da0718f11
Handle and test both Unix and Windows style slashes
When unit testing on Unix with Windows style slashes, the project
directories would have the backslash in the name, rather than
recognizing it as nested directory. We could just expect native paths
only like most BLI functions, but it's not a big problem to just support
any format and just convert it internally. The most important part is
that the API defines well how it deals with the different formats, and
that this is unit tested. Ideally we'd have some path object type that
abstracts away the difference.
===================================================================
M source/blender/blenkernel/BKE_project_settings.hh
M source/blender/blenkernel/intern/project_settings.cc
M source/blender/blenkernel/intern/project_settings_test.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_project_settings.hh b/source/blender/blenkernel/BKE_project_settings.hh
index 536df0dd18e..646a38196bb 100644
--- a/source/blender/blenkernel/BKE_project_settings.hh
+++ b/source/blender/blenkernel/BKE_project_settings.hh
@@ -13,6 +13,7 @@
namespace blender::bke {
class ProjectSettings {
+ /* Path to the project root using slashes in the OS native format. */
std::string project_root_path_;
public:
@@ -21,6 +22,7 @@ class ProjectSettings {
/**
* Initializes a blender project by creating a .blender_project directory at the given \a
* project_root_path.
+ * Both Unix and Windows style slashes are allowed.
* \return True if the settings directory was created, or already existed. False on failure.
*/
static auto create_settings_directory(StringRef project_root_path) -> bool;
@@ -28,6 +30,7 @@ class ProjectSettings {
/**
* Read project settings from the given \a project_path, which may be either a project root
* directory or the .blender_project directory.
+ * Both Unix and Windows style slashes are allowed.
* \return The read project settings or null in case of failure.
*/
static auto load_from_disk [[nodiscard]] (StringRef project_path)
diff --git a/source/blender/blenkernel/intern/project_settings.cc b/source/blender/blenkernel/intern/project_settings.cc
index 8645f0e63d4..5e4f2d7f303 100644
--- a/source/blender/blenkernel/intern/project_settings.cc
+++ b/source/blender/blenkernel/intern/project_settings.cc
@@ -7,6 +7,7 @@
#include "BKE_project_settings.hh"
#include "BLI_fileops.h"
+#include "BLI_path_util.h"
namespace blender::bke {
@@ -17,24 +18,44 @@ ProjectSettings::ProjectSettings(StringRef project_root_path)
bool ProjectSettings::create_settings_directory(StringRef project_root_path)
{
- return BLI_dir_create_recursive(std::string(project_root_path + "/" + SETTINGS_DIRNAME).c_str());
+ std::string project_root_path_native = project_root_path;
+ BLI_path_slash_native(project_root_path_native.data());
+
+ return BLI_dir_create_recursive(
+ std::string(project_root_path_native + SEP + SETTINGS_DIRNAME).c_str());
+}
+
+static StringRef path_strip_trailing_native_slash(StringRef path)
+{
+ const int64_t pos_before_trailing_slash = path.find_last_not_of(SEP);
+ return (pos_before_trailing_slash == StringRef::not_found) ?
+ path :
+ path.substr(0, pos_before_trailing_slash + 1);
+}
+
+static bool path_contains_project_settings(StringRef path)
+{
+ return BLI_exists(std::string(path + SEP_STR + ProjectSettings::SETTINGS_DIRNAME).c_str());
}
std::unique_ptr<ProjectSettings> ProjectSettings::load_from_disk(StringRef project_path)
{
- if (!BLI_exists(std::string(project_path).c_str())) {
+ std::string project_path_native = project_path;
+ BLI_path_slash_native(project_path_native.data());
+
+ if (!BLI_exists(project_path_native.c_str())) {
return nullptr;
}
- StringRef project_root_path = project_path;
+ StringRef project_root_path = project_path_native;
- const int64_t pos_before_trailing_slash = project_path.find_last_not_of("\\/");
- const StringRef path_no_trailing_slashes = (pos_before_trailing_slash == StringRef::not_found) ?
- project_path :
- project_path.substr(
- 0, pos_before_trailing_slash + 1);
+ const StringRef path_no_trailing_slashes = path_strip_trailing_native_slash(project_path_native);
if (path_no_trailing_slashes.endswith(SETTINGS_DIRNAME)) {
- project_root_path = project_path.drop_suffix(SETTINGS_DIRNAME.size() + 1);
+ project_root_path = StringRef(project_path_native).drop_suffix(SETTINGS_DIRNAME.size() + 1);
+ }
+
+ if (!path_contains_project_settings(project_root_path)) {
+ return nullptr;
}
return std::make_unique<ProjectSettings>(project_root_path);
diff --git a/source/blender/blenkernel/intern/project_settings_test.cc b/source/blender/blenkernel/intern/project_settings_test.cc
index ce11cce4c4d..82bfd585789 100644
--- a/source/blender/blenkernel/intern/project_settings_test.cc
+++ b/source/blender/blenkernel/intern/project_settings_test.cc
@@ -6,72 +6,106 @@
#include "BLI_fileops.h"
#include "BLI_function_ref.hh"
+#include "BLI_path_util.h"
#include "testing/testing.h"
namespace blender::bke::tests {
class ProjectSettingsTest : public testing::Test {
+ /* RAII helper to delete the created directories reliably after testing or on errors. */
struct ProjectDirectoryRAIIWrapper {
std::string project_path_;
+ /* Path with OS preferred slashes ('/' on Unix, '\' on Windows). Important for some file
+ * operations. */
+ std::string native_project_path_;
+ std::string base_path_;
- ProjectDirectoryRAIIWrapper(StringRefNull project_path)
+ ProjectDirectoryRAIIWrapper(StringRefNull base_path, StringRefNull relative_project_path)
{
+ BLI_assert_msg(ELEM(base_path.back(), SEP, ALTSEP),
+ "Expected base_path to have trailing slash");
+ std::string project_path = base_path + relative_project_path;
+
+ native_project_path_ = project_path;
+ BLI_path_slash_native(native_project_path_.data());
+
/** Assert would be preferable but that would only run in debug builds, and #ASSERT_TRUE()
* doesn't support printing a message. */
- if (BLI_exists(project_path.c_str())) {
+ if (BLI_exists(native_project_path_.c_str())) {
throw std::runtime_error("Can't execute test, temporary path '" + project_path +
"' already exists");
}
- BLI_dir_create_recursive(project_path.c_str());
- if (!BLI_exists(project_path.c_str())) {
- throw std::runtime_error("Can't execute test, failed to create path '" + project_path +
- "'");
+ BLI_dir_create_recursive(native_project_path_.c_str());
+ if (!BLI_exists(native_project_path_.c_str())) {
+ throw std::runtime_error("Can't execute test, failed to create path '" +
+ native_project_path_ + "'");
}
+
+ base_path_ = base_path;
project_path_ = project_path;
+ BLI_assert(StringRef{&project_path_[base_path.size()]} == relative_project_path);
}
~ProjectDirectoryRAIIWrapper()
{
if (!project_path_.empty()) {
- BLI_delete(project_path_.c_str(), true, true);
+ /* Cut the path off at the first slash after the base path, so we delete the directory
+ * created for the test. */
+ const size_t first_slash_pos = native_project_path_.find_first_of(SEP, base_path_.size());
+ std::string path_to_delete = native_project_path_;
+ if (first_slash_pos != std::string::npos) {
+ path_to_delete.erase(first_slash_pos);
+ }
+ BLI_delete(path_to_delete.c_str(), true, true);
+ BLI_assert(!BLI_exists(native_project_path_.c_str()));
}
}
};
public:
/* Run the test on multiple paths or variations of the same path. Useful to test things like
- * unicode paths, with or without trailing slash, etc. */
- void test_foreach_project_path(FunctionRef<void(StringRefNull)> fn)
+ * unicode paths, with or without trailing slash, non-native slashes, etc. The callback gets both
+ * the unmodified path (possibly with non-native slashes), and the path converted to native
+ * slashes passed. Call functions under test with the former, and use the latter to check the
+ * results with BLI_fileops.h functions */
+ void test_foreach_project_path(FunctionRef<void(StringRefNull /* project_path */,
+ StringRefNull /* project_path_native */)> fn)
{
std::vector<StringRefNull> subpaths = {
"temporary-project-root",
- "test-temporary-unicode-dir-Ružena/temporary-project-root",
+ "test-temporary-unicode-dir-новый/temporary-project-root",
/* Same but with trailing slash. */
- "test-temporary-unicode-dir-Ružena/temporary-project-root/",
+ "test-temporary-unicode-dir-новый/temporary-project-root/",
+ /* Windows style slash. */
+ "test-temporary-unicode-dir-новый\\temporary-project-root",
+ /* Windows style trailing slash. */
+ "test-temporary-unicode-dir-новый\\temporary-project-root\\",
};
BKE_tempdir_init("");
const std::string tempdir = BKE_tempdir_session();
for (StringRefNull subpath : subpaths) {
- ProjectDirectoryRAIIWrapper temp_project_path(tempdir + subpath);
- fn(temp_project_path.project_path_);
+ ProjectDirectoryRAIIWrapper temp_project_path(tempdir, subpath);
+ fn(temp_project_path.project_path_, temp_project_path.native_project_path_);
}
}
};
TEST_F(ProjectSettingsTest, create)
{
- test_foreach_project_path([](StringRefNull project_path) {
+ test_foreach_project_path([](StringRefNull project_path, StringRefNull project_path_native) {
if (!ProjectSettings::create_settings_directory(project_path)) {
/* Not a regular test failure, this may fail if there is a permission issue for example. */
FAIL() << "Failed to create project directory in '" << project_path
<< "', check permissions";
}
- std::string project_settings_dir = project_path + "/" + ProjectSettings::SETTINGS_DIRNAME;
- EXPECT_TRUE(BLI_exists(project_settings_dir.c_str()))
+ std::string project_settings_dir = project_path_native + SEP_STR +
+ ProjectSettings::SETTINGS_DIRNAME;
+ EXPECT_TRUE(BLI_exists(project_settings_dir.c_str()) &&
+ BLI_is_dir(project_settings_dir.c_str()))
<< project_settings_dir + " was not created";
});
}
@@ -80,12 +114,12 @@ TEST_F(ProjectSettingsTest, create)
* directory). */
TEST_F(ProjectSettingsTest, load_from_project_root_path)
{
- test_foreach_project_path([](StringRefNull project_path) {
+ test_foreach_project_path([](StringRefNull project_path, StringRefNull project_path_native) {
ProjectSettings::creat
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list