diff --git a/src/lang/io/include/sourcemeta/core/io.h b/src/lang/io/include/sourcemeta/core/io.h index fb2736f710..c57c049cf8 100644 --- a/src/lang/io/include/sourcemeta/core/io.h +++ b/src/lang/io/include/sourcemeta/core/io.h @@ -57,7 +57,8 @@ auto canonical(const std::filesystem::path &path) -> std::filesystem::path; /// @ingroup io /// /// A safe variant of `std::filesystem::weakly_canonical` that takes into -/// account platform-specific oddities like FIFO on GNU/Linux. For example: +/// account platform-specific oddities like FIFO on GNU/Linux, always resolving +/// relative paths against the current working directory. For example: /// /// ```cpp /// #include diff --git a/src/lang/io/io.cc b/src/lang/io/io.cc index 322e7967b7..9a6527be6f 100644 --- a/src/lang/io/io.cc +++ b/src/lang/io/io.cc @@ -130,15 +130,25 @@ auto weakly_canonical(const std::filesystem::path &path) return path; } + // C++ [fs.op.weakly.canonical] defines the result in terms of "the leading + // elements of p that exist, if any", leaving the case where nothing exists + // unspecified. Standard libraries disagree there: some resolve against the + // current working directory while others hand the input back untouched. + // Absolutising first makes the result absolute on every platform, and is a + // no-op when any leading element exists, as canonicalising that prefix + // already yields an absolute path + const auto absolute_path{ + path.is_absolute() ? path : std::filesystem::absolute(path)}; + // On Linux, FIFO files (like /dev/fd/XX due to process substitution) // cannot be made canonical // See https://github.com/sourcemeta/jsonschema/issues/252 - if (std::filesystem::is_fifo(path)) { - return normalize(path); + if (std::filesystem::is_fifo(absolute_path)) { + return normalize(absolute_path); } try { - return normalize(std::filesystem::weakly_canonical(path)); + return normalize(std::filesystem::weakly_canonical(absolute_path)); } catch (const std::filesystem::filesystem_error &error) { if (error.code() == std::errc::no_such_file_or_directory) { throw IOFileNotFoundError{path}; diff --git a/test/io/io_weakly_canonical_test.cc b/test/io/io_weakly_canonical_test.cc index 4ce256067c..0aca6c9f6f 100644 --- a/test/io/io_weakly_canonical_test.cc +++ b/test/io/io_weakly_canonical_test.cc @@ -1,6 +1,12 @@ #include #include +#include // std::filesystem + +#if !defined(_WIN32) +#include // mkfifo, S_IRUSR, S_IWUSR +#endif + TEST(test_txt) { const auto path{sourcemeta::core::weakly_canonical( std::filesystem::path{STUBS_DIRECTORY} / "test.txt")}; @@ -18,6 +24,43 @@ TEST(empty) { std::filesystem::path{}); } +TEST(relative_that_does_not_exist) { + const auto path{sourcemeta::core::weakly_canonical( + std::filesystem::path{"sourcemeta-core-does-not-exist.txt"})}; + EXPECT_TRUE(path.is_absolute()); + EXPECT_EQ(path, + std::filesystem::current_path() / + std::filesystem::path{"sourcemeta-core-does-not-exist.txt"}); +} + +TEST(relative_with_missing_leading_component) { + const auto path{sourcemeta::core::weakly_canonical( + std::filesystem::path{"sourcemeta-core-nope"} / + std::filesystem::path{"also-nope.txt"})}; + EXPECT_TRUE(path.is_absolute()); + EXPECT_EQ(path, std::filesystem::current_path() / + std::filesystem::path{"sourcemeta-core-nope"} / + std::filesystem::path{"also-nope.txt"}); +} + +TEST(relative_with_dot_segments_that_does_not_exist) { + const auto path{sourcemeta::core::weakly_canonical( + std::filesystem::path{"."} / + std::filesystem::path{"sourcemeta-core-nope"} / + std::filesystem::path{".."} / + std::filesystem::path{"sourcemeta-core-also-nope.txt"})}; + EXPECT_TRUE(path.is_absolute()); + EXPECT_EQ(path, std::filesystem::current_path() / + std::filesystem::path{"sourcemeta-core-also-nope.txt"}); +} + +TEST(relative_that_exists) { + const auto path{sourcemeta::core::weakly_canonical( + std::filesystem::path{"."} / std::filesystem::path{"."})}; + EXPECT_TRUE(path.is_absolute()); + EXPECT_EQ(path, std::filesystem::current_path()); +} + #ifndef _WIN32 TEST(posix_trailing_slash) { @@ -95,4 +138,19 @@ TEST(posix_no_change_for_clean_path) { std::filesystem::path{"/foo/bar/baz"}); } +TEST(posix_relative_fifo) { + const auto fifo_path{std::filesystem::path{BUILD_DIRECTORY} / + "sourcemeta_core_io_weakly_canonical_fifo"}; + std::filesystem::remove(fifo_path); + EXPECT_EQ(::mkfifo(fifo_path.c_str(), S_IRUSR | S_IWUSR), 0); + const auto relative_fifo_path{std::filesystem::relative(fifo_path)}; + const auto path{sourcemeta::core::weakly_canonical(relative_fifo_path)}; + const auto expected{sourcemeta::core::weakly_canonical(fifo_path)}; + std::filesystem::remove(fifo_path); + + EXPECT_TRUE(relative_fifo_path.is_relative()); + EXPECT_TRUE(path.is_absolute()); + EXPECT_EQ(path, expected); +} + #endif