From 6b66aa37f6e3769de6c3d4384ae133b725c0ebb5 Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Wed, 24 Apr 2024 11:27:30 +0800 Subject: [PATCH 1/4] Fix create dir ending with a slash --- src/common/file_system/local_file_system.cpp | 21 +++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/common/file_system/local_file_system.cpp b/src/common/file_system/local_file_system.cpp index c2a9521ebb..53aa3bf443 100644 --- a/src/common/file_system/local_file_system.cpp +++ b/src/common/file_system/local_file_system.cpp @@ -160,10 +160,25 @@ void LocalFileSystem::createDir(const std::string& dir) const { throw IOException(stringFormat("Directory {} already exists.", dir)); // LCOV_EXCL_STOP } - if (!std::filesystem::create_directories(dir)) { + auto directoryToCreate = dir; + if (directoryToCreate.ends_with('/')) { + // This is a known issue with std::filesystem::create_directories. (link: + // https://github.com/llvm/llvm-project/issues/60634). We have to manually remove the + // last '/' if the path ends with '/'. + directoryToCreate.substr(0, directoryToCreate.size() - 1); + } + std::error_code errCode; + if (!std::filesystem::create_directories(directoryToCreate, errCode)) { + // LCOV_EXCL_START + throw IOException( + stringFormat("Directory {} cannot be created. Check if it exists and remove it.", + directoryToCreate)); + // LCOV_EXCL_STOP + } + if (errCode) { // LCOV_EXCL_START - throw IOException(stringFormat( - "Directory {} cannot be created. Check if it exists and remove it.", dir)); + throw IOException(stringFormat("Failed to create directory: {}, error message: {}.", + dir, errCode.message())); // LCOV_EXCL_STOP } } catch (std::exception& e) { From bb5cb4c801952814e4ca05af6ab7c97bf37f0d6b Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Wed, 24 Apr 2024 11:47:25 +0800 Subject: [PATCH 2/4] update --- src/common/file_system/local_file_system.cpp | 2 +- test/c_api/database_test.cpp | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/common/file_system/local_file_system.cpp b/src/common/file_system/local_file_system.cpp index 53aa3bf443..8b513f8dbb 100644 --- a/src/common/file_system/local_file_system.cpp +++ b/src/common/file_system/local_file_system.cpp @@ -165,7 +165,7 @@ void LocalFileSystem::createDir(const std::string& dir) const { // This is a known issue with std::filesystem::create_directories. (link: // https://github.com/llvm/llvm-project/issues/60634). We have to manually remove the // last '/' if the path ends with '/'. - directoryToCreate.substr(0, directoryToCreate.size() - 1); + directoryToCreate = directoryToCreate.substr(0, directoryToCreate.size() - 1); } std::error_code errCode; if (!std::filesystem::create_directories(directoryToCreate, errCode)) { diff --git a/test/c_api/database_test.cpp b/test/c_api/database_test.cpp index 805a384b73..7a2efab743 100644 --- a/test/c_api/database_test.cpp +++ b/test/c_api/database_test.cpp @@ -60,8 +60,12 @@ TEST_F(CApiDatabaseTest, CreationInvalidPath) { } TEST_F(CApiDatabaseTest, CreationHomeDir) { - auto databasePathCStr = (char*)"~/ku_test.db"; + auto databasePathCStr = (char*)"~/ku_test.db/"; auto database = kuzu_database_init(databasePathCStr, kuzu_default_system_config()); + auto connection = kuzu_connection_init(database); + auto homePath = + getClientContext(*(Connection*)(connection->_connection))->getClientConfig()->homeDirectory; ASSERT_NE(database, nullptr); kuzu_database_destroy(database); + std::filesystem::remove_all(homePath + "/ku_test.db"); } From aa9c957d952eab445808178eb05565de088d95ae Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Wed, 24 Apr 2024 12:38:36 +0800 Subject: [PATCH 3/4] update --- tools/python_api/test/test_exception.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/python_api/test/test_exception.py b/tools/python_api/test/test_exception.py index 16fb9fdaea..a69d3da5d8 100644 --- a/tools/python_api/test/test_exception.py +++ b/tools/python_api/test/test_exception.py @@ -19,10 +19,8 @@ def test_exception(conn_db_readonly: ConnDB) -> None: def test_db_path_exception() -> None: path = "" - if sys.platform == "win32": - error_message = "Failed to create directory" - else: - error_message = "filesystem error" + error_message = ("IO exception: Failed to create directory due to: IO exception: Directory cannot be created. " + "Check if it exists and remove it.") with pytest.raises(RuntimeError, match=error_message): kuzu.Database(path) From 29a8cadf1bf07cc21d48c44b33c9425dd14f33ff Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Wed, 24 Apr 2024 13:01:13 +0800 Subject: [PATCH 4/4] fix --- test/c_api/database_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/c_api/database_test.cpp b/test/c_api/database_test.cpp index 7a2efab743..58b1ec07cb 100644 --- a/test/c_api/database_test.cpp +++ b/test/c_api/database_test.cpp @@ -66,6 +66,7 @@ TEST_F(CApiDatabaseTest, CreationHomeDir) { auto homePath = getClientContext(*(Connection*)(connection->_connection))->getClientConfig()->homeDirectory; ASSERT_NE(database, nullptr); + kuzu_connection_destroy(connection); kuzu_database_destroy(database); std::filesystem::remove_all(homePath + "/ku_test.db"); }