https://github.com/ninja-build/ninja/pull/2692 From 9f1f01d7d03b84342dda511aad0ef3e3fcf4ea7c Mon Sep 17 00:00:00 2001 From: Sam James Date: Sat, 22 Nov 2025 12:49:35 +0000 Subject: [PATCH] Don't require a FIFO to be identifiable as such The jobserver specification [0] currently suggests that the FIFO must be a genuine FIFO. For some work we're doing [1][2], we're emulating a FIFO using CUSE/FUSE to allow tracking when consumers disappear to avoid lost tokens. nixos had a similar idea in the past too [3]. There doesn't seem to be a good reason to check that any FIFO passed by the user is actually identifiable as such by `stat()`, so drop the check. make already does not perform such a check, just the specification isn't clear about it, so we've asked them to clarify it [4]. [0] https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html [1] https://codeberg.org/amonakov/guildmaster [2] https://gitweb.gentoo.org/proj/steve.git/ [3] https://github.com/NixOS/nixpkgs/pull/314888 [4] https://savannah.gnu.org/bugs/index.php?67726 --- src/jobserver-posix.cc | 12 ------------ src/jobserver_test.cc | 10 +--------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/jobserver-posix.cc b/src/jobserver-posix.cc index 0e3c7e250c..95208c09b6 100644 --- a/src/jobserver-posix.cc +++ b/src/jobserver-posix.cc @@ -26,13 +26,6 @@ namespace { -// Return true if |fd| is a fifo or pipe descriptor. -bool IsFifoDescriptor(int fd) { - struct stat info; - int ret = ::fstat(fd, &info); - return (ret == 0) && ((info.st_mode & S_IFMT) == S_IFIFO); -} - // Implementation of Jobserver::Client for Posix systems class PosixJobserverClient : public Jobserver::Client { public: @@ -89,11 +82,6 @@ class PosixJobserverClient : public Jobserver::Client { std::string("Error opening fifo for reading: ") + strerror(errno); return false; } - if (!IsFifoDescriptor(read_fd_)) { - *error = "Not a fifo path: " + fifo_path; - // Let destructor close read_fd_. - return false; - } write_fd_ = ::open(fifo_path.c_str(), O_WRONLY | O_NONBLOCK | O_CLOEXEC); if (write_fd_ < 0) { *error = diff --git a/src/jobserver_test.cc b/src/jobserver_test.cc index 850a8b13fd..9e180e6e39 100644 --- a/src/jobserver_test.cc +++ b/src/jobserver_test.cc @@ -379,21 +379,13 @@ TEST(Jobserver, PosixFifoClientWithWrongPath) { ASSERT_GE(fd, 0) << "Could not create file: " << strerror(errno); ::close(fd); - // Create new client instance, passing the file path for the fifo. + // Create new client instance, with an empty file path. Jobserver::Config config; config.mode = Jobserver::Config::kModePosixFifo; - config.path = file_path; - std::string error; std::unique_ptr client = Jobserver::Client::Create(config, &error); - EXPECT_FALSE(client.get()); - EXPECT_FALSE(error.empty()); - EXPECT_EQ("Not a fifo path: " + file_path, error); - // Do the same with an empty file path. - error.clear(); - config.path.clear(); client = Jobserver::Client::Create(config, &error); EXPECT_FALSE(client.get()); EXPECT_FALSE(error.empty());