From 08cc572f8935f9078ef3747187c65b904d5675c7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 24 Aug 2021 12:31:49 +0200 Subject: [PATCH 1/6] Revert "Shorten the test drv name" This reverts commit 5ec873b127139ca90cc31967c25c9a34fb4cc3e4. --- flake.nix | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/flake.nix b/flake.nix index 43123ca7e..91341c97d 100644 --- a/flake.nix +++ b/flake.nix @@ -156,10 +156,7 @@ testNixVersions = pkgs: client: daemon: with commonDeps pkgs; pkgs.stdenv.mkDerivation { NIX_DAEMON_PACKAGE = daemon; NIX_CLIENT_PACKAGE = client; - # Must keep this name short as OSX has a rather strict limit on the - # socket path length, and this name appears in the path of the - # nix-daemon socket used in the tests - name = "nix-tests"; + name = "nix-tests-${client.version}-against-${daemon.version}"; inherit version; src = self; From 43d4d75e22ea1f7ee2936fe725c1f7ea7a5005e6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 24 Aug 2021 13:52:55 +0200 Subject: [PATCH 2/6] Connect/bind Unix domain sockets in a child process In the child process, we can do a chdir() and avoid the problem of the path not fitting into sockaddr_un. --- src/libstore/uds-remote-store.cc | 11 +---- src/libutil/util.cc | 71 +++++++++++++++++++++++++++----- src/libutil/util.hh | 8 +++- 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index cfadccf68..02e81b022 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -65,16 +65,7 @@ ref UDSRemoteStore::openConnection() throw SysError("cannot create Unix domain socket"); closeOnExec(conn->fd.get()); - string socketPath = path ? *path : settings.nixDaemonSocketFile; - - struct sockaddr_un addr; - addr.sun_family = AF_UNIX; - if (socketPath.size() + 1 >= sizeof(addr.sun_path)) - throw Error("socket path '%1%' is too long", socketPath); - strcpy(addr.sun_path, socketPath.c_str()); - - if (::connect(conn->fd.get(), (struct sockaddr *) &addr, sizeof(addr)) == -1) - throw SysError("cannot connect to daemon at '%1%'", socketPath); + nix::connect(conn->fd.get(), path ? *path : settings.nixDaemonSocketFile); conn->from.fd = conn->fd.get(); conn->to.fd = conn->fd.get(); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index bc841f425..445875541 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1686,16 +1686,7 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode) closeOnExec(fdSocket.get()); - struct sockaddr_un addr; - addr.sun_family = AF_UNIX; - if (path.size() + 1 >= sizeof(addr.sun_path)) - throw Error("socket path '%1%' is too long", path); - strcpy(addr.sun_path, path.c_str()); - - unlink(path.c_str()); - - if (bind(fdSocket.get(), (struct sockaddr *) &addr, sizeof(addr)) == -1) - throw SysError("cannot bind to socket '%1%'", path); + bind(fdSocket.get(), path); if (chmod(path.c_str(), mode) == -1) throw SysError("changing permissions on '%1%'", path); @@ -1707,6 +1698,66 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode) } +void bind(int fd, const std::string & path) +{ + unlink(path.c_str()); + + struct sockaddr_un addr; + addr.sun_family = AF_UNIX; + + if (path.size() + 1 >= sizeof(addr.sun_path)) { + Pid pid = startProcess([&]() { + auto dir = dirOf(path); + if (chdir(dir.c_str()) == -1) + throw SysError("chdir to '%s' failed", dir); + std::string base(baseNameOf(path)); + if (base.size() + 1 >= sizeof(addr.sun_path)) + throw Error("socket path '%s' is too long", base); + strcpy(addr.sun_path, base.c_str()); + if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) + throw SysError("cannot bind to socket '%s'", path); + _exit(0); + }); + int status = pid.wait(); + if (status != 0) + throw Error("cannot bind to socket '%s'", path); + } else { + strcpy(addr.sun_path, path.c_str()); + if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) + throw SysError("cannot bind to socket '%s'", path); + } +} + + +void connect(int fd, const std::string & path) +{ + struct sockaddr_un addr; + addr.sun_family = AF_UNIX; + + if (path.size() + 1 >= sizeof(addr.sun_path)) { + Pid pid = startProcess([&]() { + auto dir = dirOf(path); + if (chdir(dir.c_str()) == -1) + throw SysError("chdir to '%s' failed", dir); + std::string base(baseNameOf(path)); + if (base.size() + 1 >= sizeof(addr.sun_path)) + throw Error("socket path '%s' is too long", base); + strcpy(addr.sun_path, base.c_str()); + if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) + throw SysError("cannot connect to socket at '%s'", path); + _exit(0); + }); + int status = pid.wait(); + if (status != 0) + throw Error("cannot connect to socket ar '%s'", path); + } else { + strcpy(addr.sun_path, path.c_str()); + if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) + throw SysError("cannot connect to socket at '%s'", path); + } +} + + string showBytes(uint64_t bytes) { return fmt("%.2f MiB", bytes / (1024.0 * 1024.0)); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index bee77b53f..6d3e64949 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -259,7 +259,7 @@ void killUser(uid_t uid); pid to the caller. */ struct ProcessOptions { - string errorPrefix = "error: "; + string errorPrefix = ""; bool dieWithParent = true; bool runExitHandlers = false; bool allowVfork = true; @@ -574,6 +574,12 @@ void commonChildInit(Pipe & logPipe); /* Create a Unix domain socket in listen mode. */ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode); +/* Bind a Unix domain socket to a path. */ +void bind(int fd, const std::string & path); + +/* Connect to a Unix domain socket. */ +void connect(int fd, const std::string & path); + // A Rust/Python-like enumerate() iterator adapter. // Borrowed from http://reedbeta.com/blog/python-like-enumerate-in-cpp17. From b299560872e56fb8aed2d720f9c0b3df9c6265e3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 25 Aug 2021 13:28:36 +0200 Subject: [PATCH 3/6] Typo --- src/libutil/util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 445875541..feea9f48e 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1749,7 +1749,7 @@ void connect(int fd, const std::string & path) }); int status = pid.wait(); if (status != 0) - throw Error("cannot connect to socket ar '%s'", path); + throw Error("cannot connect to socket at '%s'", path); } else { strcpy(addr.sun_path, path.c_str()); if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) From e4a5d64a81bcefe8a08015a877492235b079a9e2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Oct 2021 12:26:04 +0200 Subject: [PATCH 4/6] Show failing PID --- src/libutil/util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index feea9f48e..0b261ca60 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -903,7 +903,7 @@ int Pid::wait() return status; } if (errno != EINTR) - throw SysError("cannot get child exit status"); + throw SysError("cannot get exit status of PID %d", pid); checkInterrupt(); } } From b14bc06955885f661e4af5f04513086d74f2cab7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Oct 2021 13:23:16 +0200 Subject: [PATCH 5/6] Don't ignore SIGCHLD in createUnixDomainSocket() --- src/nix/daemon.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 2cf2a04c9..6a40a0bd3 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -156,9 +156,6 @@ static void daemonLoop() if (chdir("/") == -1) throw SysError("cannot change current directory"); - // Get rid of children automatically; don't let them become zombies. - setSigChldAction(true); - AutoCloseFD fdSocket; // Handle socket-based activation by systemd. @@ -176,6 +173,9 @@ static void daemonLoop() fdSocket = createUnixDomainSocket(settings.nixDaemonSocketFile, 0666); } + // Get rid of children automatically; don't let them become zombies. + setSigChldAction(true); + // Loop accepting connections. while (1) { From 223ab254c29313fff2a9af463baa2f0b68d7d83f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Oct 2021 14:50:55 +0200 Subject: [PATCH 6/6] Compatibility --- flake.nix | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/flake.nix b/flake.nix index 91341c97d..d3486db4c 100644 --- a/flake.nix +++ b/flake.nix @@ -126,8 +126,7 @@ '' mkdir -p $out/nix-support - # Converts /nix/store/50p3qk8kka9dl6wyq40vydq945k0j3kv-nix-2.4pre20201102_550e11f/bin/nix - # To 50p3qk8kka9dl6wyq40vydq945k0j3kv/bin/nix + # Converts /nix/store/50p3qk8k...-nix-2.4pre20201102_550e11f/bin/nix to 50p3qk8k.../bin/nix. tarballPath() { # Remove the store prefix local path=''${1#${builtins.storeDir}/} @@ -153,10 +152,15 @@ echo "file installer $out/install" >> $out/nix-support/hydra-build-products ''; - testNixVersions = pkgs: client: daemon: with commonDeps pkgs; pkgs.stdenv.mkDerivation { + testNixVersions = pkgs: client: daemon: with commonDeps pkgs; with pkgs.lib; pkgs.stdenv.mkDerivation { NIX_DAEMON_PACKAGE = daemon; NIX_CLIENT_PACKAGE = client; - name = "nix-tests-${client.version}-against-${daemon.version}"; + name = + "nix-tests" + + optionalString + (versionAtLeast daemon.version "2.4pre20211005" && + versionAtLeast client.version "2.4pre20211005") + "-${client.version}-against-${daemon.version}"; inherit version; src = self;