From 52e3c2a265c5a0e76ed6dc1092c6ae630238242d Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 10 Feb 2022 09:50:49 -0600 Subject: [PATCH 1/5] Add some tests for parentPath/basename Signed-off-by: Michael Carroll --- src/Filesystem_TEST.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index ebe39d698..64f629825 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -413,38 +413,47 @@ TEST_F(FilesystemTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(cwd_error)) } ///////////////////////////////////////////////// -TEST_F(FilesystemTest, basename) +TEST_F(FilesystemTest, decomposition) { std::string absolute = joinPaths("", "home", "bob", "foo"); EXPECT_EQ(basename(absolute), "foo"); + EXPECT_EQ(parentPath(absolute), "/home/bob"); std::string relative = joinPaths("baz", "foobar"); EXPECT_EQ(basename(relative), "foobar"); + EXPECT_EQ(parentPath(relative), "baz"); std::string bname = "bzzz"; EXPECT_EQ(basename(bname), "bzzz"); + EXPECT_EQ(parentPath(bname), "bzzz"); std::string nobase = joinPaths("baz", ""); EXPECT_EQ(basename(nobase), "baz"); + EXPECT_EQ(parentPath(nobase), "baz/"); std::string multiple_slash = separator("baz") + separator("") + separator("") + separator(""); EXPECT_EQ(basename(multiple_slash), "baz"); + EXPECT_EQ(parentPath(multiple_slash), "baz//"); std::string multiple_slash_middle = separator("") + separator("home") + separator("") + separator("") + separator("bob") + separator("foo"); EXPECT_EQ(basename(multiple_slash_middle), "foo"); + EXPECT_EQ(parentPath(multiple_slash_middle), "/home///bob"); std::string multiple_slash_start = separator("") + separator("") + separator("") + separator("home") + separator("bob") + separator("foo"); EXPECT_EQ(basename(multiple_slash_start), "foo"); + EXPECT_EQ(parentPath(multiple_slash_start), "///home/bob"); std::string slash_only = separator("") + separator(""); EXPECT_EQ(basename(slash_only), separator("")); + EXPECT_EQ(parentPath(slash_only), ""); std::string multiple_slash_only = separator("") + separator("") + separator("") + separator(""); EXPECT_EQ(basename(multiple_slash_only), separator("")); + EXPECT_EQ(parentPath(multiple_slash_only), "//"); } ///////////////////////////////////////////////// From 3e101f870245df4bf887d37dc3787a767954ea2c Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 9 Feb 2022 08:25:07 -0600 Subject: [PATCH 2/5] Match common5 separator/basename/parentPath * Separator with an emptry string argument should return the preferred separator for that platform * Basename should return basename in the case with multiple trailing slashes Signed-off-by: Michael Carroll --- src/Filesystem.cc | 35 +++++++++++++++++++---------------- src/Filesystem_TEST.cc | 10 ++++++++++ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/Filesystem.cc b/src/Filesystem.cc index 99cc9094b..ba89779d3 100644 --- a/src/Filesystem.cc +++ b/src/Filesystem.cc @@ -99,8 +99,7 @@ bool ignition::common::createDirectories(const std::string &_path) ///////////////////////////////////////////////// std::string const ignition::common::separator(std::string const &_s) { - fs::path path(_s); - return (_s / fs::path("")).string(); + return _s + std::string{fs::path::preferred_separator}; } ///////////////////////////////////////////////// @@ -144,6 +143,11 @@ std::string ignition::common::joinPaths( fs::path p1{_path1}; fs::path p2{_path2}; + if (p1.empty()) + { + p1 = ignition::common::separator(""); + } + bool is_url = false; if (_path1.find("://") == std::string::npos) @@ -201,25 +205,24 @@ bool ignition::common::chdir(const std::string &_dir) ///////////////////////////////////////////////// std::string ignition::common::basename(const std::string &_path) { - fs::path p(_path); - // Maintain compatibility with ign-common - if (*_path.rbegin() == fs::path::preferred_separator) - p = fs::path(_path.substr(0, _path.size()-1)); - return p.filename().string(); + fs::path p = fs::path(_path); + p /= "FOO.TXT"; + p = p.lexically_normal(); + p = p.parent_path(); + return p.filename().string().empty() ? + std::string{fs::path::preferred_separator} : p.filename().string(); } ///////////////////////////////////////////////// std::string ignition::common::parentPath(const std::string &_path) { - fs::path p(_path); - // Maintain compatibility with ign-common - if (*_path.rbegin() == fs::path::preferred_separator) - p = fs::path(_path.substr(0, _path.size()-1)); - - if (!p.has_parent_path()) - return _path; - - return p.parent_path().string(); + std::string result; + size_t last_sep = _path.find_last_of(separator("")); + // If slash is the last character, find its parent directory + if (last_sep == _path.length() - 1) + last_sep = _path.substr(0, last_sep).find_last_of(separator("")); + result = _path.substr(0, last_sep); + return result; } ///////////////////////////////////////////////// diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index 64f629825..271a69bc9 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -620,6 +620,16 @@ TEST_F(FilesystemTest, uniquePaths) EXPECT_TRUE(removeDirectory(dirExistingRt)) << dirExistingRt; } +///////////////////////////////////////////////// +TEST_F(FilesystemTest, separator) +{ +#ifndef _WIN32 + EXPECT_EQ("/", ignition::common::separator("")); +#else + EXPECT_EQ("\\", ignition::common::separator("")); +#endif +} + ///////////////////////////////////////////////// /// Main int main(int argc, char **argv) From d453ab664033441b004c2a0ac66523c2e52cdda8 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Thu, 3 Mar 2022 09:29:55 -0800 Subject: [PATCH 3/5] Address PR feedback Signed-off-by: Louise Poubel --- src/Filesystem.cc | 4 ++-- src/Filesystem_TEST.cc | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Filesystem.cc b/src/Filesystem.cc index ba89779d3..2b1b26f68 100644 --- a/src/Filesystem.cc +++ b/src/Filesystem.cc @@ -205,11 +205,11 @@ bool ignition::common::chdir(const std::string &_dir) ///////////////////////////////////////////////// std::string ignition::common::basename(const std::string &_path) { - fs::path p = fs::path(_path); + fs::path p(_path); p /= "FOO.TXT"; p = p.lexically_normal(); p = p.parent_path(); - return p.filename().string().empty() ? + return p.filename().string().empty() ? std::string{fs::path::preferred_separator} : p.filename().string(); } diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index 271a69bc9..89cb1f3ee 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -417,7 +417,11 @@ TEST_F(FilesystemTest, decomposition) { std::string absolute = joinPaths("", "home", "bob", "foo"); EXPECT_EQ(basename(absolute), "foo"); +#ifndef _WIN32 EXPECT_EQ(parentPath(absolute), "/home/bob"); +#else + EXPECT_EQ(parentPath(absolute), "\\home\\bob"); +#endif std::string relative = joinPaths("baz", "foobar"); EXPECT_EQ(basename(relative), "foobar"); @@ -626,7 +630,7 @@ TEST_F(FilesystemTest, separator) #ifndef _WIN32 EXPECT_EQ("/", ignition::common::separator("")); #else - EXPECT_EQ("\\", ignition::common::separator("")); + EXPECT_EQ("\\", ignition::common::separator("")); #endif } From 5c53d26e8ff7056dd74cc1f3f7cf2b9226654948 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Thu, 3 Mar 2022 11:16:17 -0800 Subject: [PATCH 4/5] Windows tests Signed-off-by: Louise Poubel --- src/Filesystem_TEST.cc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index 89cb1f3ee..57a3d4f03 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -433,22 +433,38 @@ TEST_F(FilesystemTest, decomposition) std::string nobase = joinPaths("baz", ""); EXPECT_EQ(basename(nobase), "baz"); +#ifndef _WIN32 EXPECT_EQ(parentPath(nobase), "baz/"); +#else + EXPECT_EQ(parentPath(nobase), "baz"); +#endif std::string multiple_slash = separator("baz") + separator("") + separator("") + separator(""); EXPECT_EQ(basename(multiple_slash), "baz"); +#ifndef _WIN32 EXPECT_EQ(parentPath(multiple_slash), "baz//"); +#else + EXPECT_EQ(parentPath(multiple_slash), "baz\\\\"); +#endif std::string multiple_slash_middle = separator("") + separator("home") + separator("") + separator("") + separator("bob") + separator("foo"); EXPECT_EQ(basename(multiple_slash_middle), "foo"); +#ifndef _WIN32 EXPECT_EQ(parentPath(multiple_slash_middle), "/home///bob"); +#else + EXPECT_EQ(parentPath(multiple_slash_middle), "\\home\\\\\\bob"); +#endif std::string multiple_slash_start = separator("") + separator("") + separator("") + separator("home") + separator("bob") + separator("foo"); EXPECT_EQ(basename(multiple_slash_start), "foo"); +#ifndef _WIN32 EXPECT_EQ(parentPath(multiple_slash_start), "///home/bob"); +#else + EXPECT_EQ(parentPath(multiple_slash_start), "\\\\\\home\\bob"); +#endif std::string slash_only = separator("") + separator(""); EXPECT_EQ(basename(slash_only), separator("")); @@ -457,7 +473,11 @@ TEST_F(FilesystemTest, decomposition) std::string multiple_slash_only = separator("") + separator("") + separator("") + separator(""); EXPECT_EQ(basename(multiple_slash_only), separator("")); +#ifndef _WIN32 EXPECT_EQ(parentPath(multiple_slash_only), "//"); +#else + EXPECT_EQ(parentPath(multiple_slash_only), "\\\\"); +#endif } ///////////////////////////////////////////////// From d364230b5517d5ed224f18f7516dc3b685088946 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 4 Mar 2022 16:07:32 -0800 Subject: [PATCH 5/5] Windows Signed-off-by: Louise Poubel --- src/Filesystem_TEST.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index 57a3d4f03..2fce02751 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -436,7 +436,7 @@ TEST_F(FilesystemTest, decomposition) #ifndef _WIN32 EXPECT_EQ(parentPath(nobase), "baz/"); #else - EXPECT_EQ(parentPath(nobase), "baz"); + EXPECT_EQ(parentPath(nobase), "baz\\"); #endif std::string multiple_slash = separator("baz") + separator("") + separator("")