Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the unix:// store on Windows #10556

Merged
merged 2 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/libmain/shared.cc
Expand Up @@ -173,12 +173,13 @@ void initNix()
everybody. */
umask(0022);

#ifndef _WIN32
/* Initialise the PRNG. */
struct timeval tv;
gettimeofday(&tv, 0);
#ifndef _WIN32
srandom(tv.tv_usec);
#endif
srand(tv.tv_usec);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it not be:

#ifndef _WIN32
    srandom(tv.tv_usec);
#else
    srand(tv.tv_usec);
#endif

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it use rand on all platforms in the one spot above, so srand is likewise needed: on Unix we now use all 4 functions, on Windows we use two.



}
Expand Down
44 changes: 44 additions & 0 deletions src/libstore/indirect-root-store.cc
@@ -0,0 +1,44 @@
#include "indirect-root-store.hh"

namespace nix {

void IndirectRootStore::makeSymlink(const Path & link, const Path & target)
{
/* Create directories up to `gcRoot'. */
createDirs(dirOf(link));

/* Create the new symlink. */
Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), rand());
createSymlink(target, tempLink);

/* Atomically replace the old one. */
renameFile(tempLink, link);
}


Path IndirectRootStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot)
{
Path gcRoot(canonPath(_gcRoot));

if (isInStore(gcRoot))
throw Error(
"creating a garbage collector root (%1%) in the Nix store is forbidden "
"(are you running nix-build inside the store?)", gcRoot);

/* Register this root with the garbage collector, if it's
running. This should be superfluous since the caller should
have registered this root yet, but let's be on the safe
side. */
addTempRoot(storePath);

/* Don't clobber the link if it already exists and doesn't
point to the Nix store. */
if (pathExists(gcRoot) && (!isLink(gcRoot) || !isInStore(readLink(gcRoot))))
throw Error("cannot create symlink '%1%'; already exists", gcRoot);
makeSymlink(gcRoot, printStorePath(storePath));
addIndirectRoot(gcRoot);

return gcRoot;
}

}
3 changes: 3 additions & 0 deletions src/libstore/indirect-root-store.hh
Expand Up @@ -67,6 +67,9 @@ struct IndirectRootStore : public virtual LocalFSStore
* The form this weak-reference takes is implementation-specific.
*/
virtual void addIndirectRoot(const Path & path) = 0;

protected:
void makeSymlink(const Path & link, const Path & target);
};

}
3 changes: 3 additions & 0 deletions src/libstore/local.mk
Expand Up @@ -21,6 +21,9 @@ libstore_LDFLAGS += $(SQLITE3_LIBS) $(LIBCURL_LIBS) $(THREAD_LDFLAGS)
ifdef HOST_LINUX
libstore_LDFLAGS += -ldl
endif
ifdef HOST_WINDOWS
libstore_LDFLAGS += -lws2_32
endif

$(foreach file,$(libstore_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/sandbox)))

Expand Down
Expand Up @@ -2,16 +2,20 @@
#include "unix-domain-socket.hh"
#include "worker-protocol.hh"

#include <cstring>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>

#include <cstring>

#ifdef _WIN32
# include <winsock2.h>
Copy link
Member

@qknight qknight Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# include vs. #include, i like it to be consistent, this happens multiple times in multiple documents. i wonder is this a _WIN32 thing we want to have?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked the indentation to make nested if..endif easier to follow

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modern IDEs detect the _WIN32 switch an grey out others but so far didn't manage this in my visual studio code ssh IDE to pick up the compiler settings. for clion this required cmake to work properly and for other build systems at least one complete compile run and then IDEs sometimes pick up these settings.

this level of support would be nice for nix hacking!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qknight We have instructions for the clangd LSP if that's of use to you.
In support of this, we have make compile_commands.json. Maybe CLion supports that format too?

bear or similar isn't needed anymore since recently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that clang-format is configured to do this sort of indentation now.

Copy link
Member

@qknight qknight May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ericson2314 @roberth interesting! So i would have to use the stdenv.cc.isClang for formatting? Or would I install nix-env -i clang-format into my shell before nix develop .#devShells.x86_64-linux.x86_64-w64-mingw32?

  nativeBuildInputs = attrs.nativeBuildInputs or []
    # TODO: Remove the darwin check once
    # https://github.com/NixOS/nixpkgs/pull/291814 is available
     ++ lib.optional (stdenv.cc.isClang && !stdenv.buildPlatform.isDarwin) pkgs.buildPackages.bear
     ++ lib.optional (stdenv.cc.isClang && stdenv.hostPlatform == stdenv.buildPlatform) pkgs.buildPackages.clang-tools;

Tried to get my visual studio code setup to work with make compile_commands.json but the problem is that I would need to make the login shell (it uses ssh to attach to the machine) have what the nix develop ... brings in and I couldn't make this happen. There is no field in the configuration, https://code.visualstudio.com/docs/remote/ssh, where one could set env vars or commands to run prior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just run make format, it is in all of Nix's dev shells.

# include <afunix.h>
#else
# include <sys/socket.h>
# include <sys/un.h>
#endif

namespace nix {

Expand Down Expand Up @@ -57,7 +61,7 @@ std::string UDSRemoteStore::getUri()

void UDSRemoteStore::Connection::closeWrite()
{
shutdown(fd.get(), SHUT_WR);
shutdown(toSocket(fd.get()), SHUT_WR);
}


Expand All @@ -68,7 +72,7 @@ ref<RemoteStore::Connection> UDSRemoteStore::openConnection()
/* Connect to a daemon that does the privileged work for us. */
conn->fd = createUnixDomainSocket();

nix::connect(conn->fd.get(), path ? *path : settings.nixDaemonSocketFile);
nix::connect(toSocket(conn->fd.get()), path ? *path : settings.nixDaemonSocketFile);

conn->from.fd = conn->fd.get();
conn->to.fd = conn->fd.get();
Expand Down
40 changes: 0 additions & 40 deletions src/libstore/unix/gc.cc
Expand Up @@ -35,20 +35,6 @@ static std::string gcSocketPath = "/gc-socket/socket";
static std::string gcRootsDir = "gcroots";


static void makeSymlink(const Path & link, const Path & target)
{
/* Create directories up to `gcRoot'. */
createDirs(dirOf(link));

/* Create the new symlink. */
Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), random());
createSymlink(target, tempLink);

/* Atomically replace the old one. */
renameFile(tempLink, link);
}


void LocalStore::addIndirectRoot(const Path & path)
{
std::string hash = hashString(HashAlgorithm::SHA1, path).to_string(HashFormat::Nix32, false);
Expand All @@ -57,32 +43,6 @@ void LocalStore::addIndirectRoot(const Path & path)
}


Path IndirectRootStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot)
{
Path gcRoot(canonPath(_gcRoot));

if (isInStore(gcRoot))
throw Error(
"creating a garbage collector root (%1%) in the Nix store is forbidden "
"(are you running nix-build inside the store?)", gcRoot);

/* Register this root with the garbage collector, if it's
running. This should be superfluous since the caller should
have registered this root yet, but let's be on the safe
side. */
addTempRoot(storePath);

/* Don't clobber the link if it already exists and doesn't
point to the Nix store. */
if (pathExists(gcRoot) && (!isLink(gcRoot) || !isInStore(readLink(gcRoot))))
throw Error("cannot create symlink '%1%'; already exists", gcRoot);
makeSymlink(gcRoot, printStorePath(storePath));
addIndirectRoot(gcRoot);

return gcRoot;
}


void LocalStore::createTempRootsFile()
{
auto fdTempRoots(_fdTempRoots.lock());
Expand Down
@@ -1,24 +1,31 @@
#include "file-system.hh"
#include "processes.hh"
#include "unix-domain-socket.hh"
#include "util.hh"

#include <sys/socket.h>
#include <sys/un.h>
#ifdef _WIN32
# include <winsock2.h>
# include <afunix.h>
#else
# include <sys/socket.h>
# include <sys/un.h>
# include "processes.hh"
#endif
#include <unistd.h>

namespace nix {

AutoCloseFD createUnixDomainSocket()
{
AutoCloseFD fdSocket = socket(PF_UNIX, SOCK_STREAM
AutoCloseFD fdSocket = toDescriptor(socket(PF_UNIX, SOCK_STREAM
#ifdef SOCK_CLOEXEC
| SOCK_CLOEXEC
#endif
, 0);
, 0));
if (!fdSocket)
throw SysError("cannot create Unix domain socket");
#ifndef _WIN32
closeOnExec(fdSocket.get());
#endif
return fdSocket;
}

Expand All @@ -32,16 +39,15 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode)
if (chmod(path.c_str(), mode) == -1)
throw SysError("changing permissions on '%1%'", path);

if (listen(fdSocket.get(), 100) == -1)
if (listen(toSocket(fdSocket.get()), 100) == -1)
throw SysError("cannot listen on socket '%1%'", path);

return fdSocket;
}


static void bindConnectProcHelper(
std::string_view operationName, auto && operation,
int fd, const std::string & path)
Socket fd, const std::string & path)
{
struct sockaddr_un addr;
addr.sun_family = AF_UNIX;
Expand All @@ -54,6 +60,9 @@ static void bindConnectProcHelper(
auto * psaddr = reinterpret_cast<struct sockaddr *>(&addr);

if (path.size() + 1 >= sizeof(addr.sun_path)) {
#ifdef _WIN32
throw Error("cannot %s to socket at '%s': path is too long", operationName, path);
#else
Pipe pipe;
pipe.create();
Pid pid = startProcess([&] {
Expand Down Expand Up @@ -83,6 +92,7 @@ static void bindConnectProcHelper(
errno = *errNo;
throw SysError("cannot %s to socket at '%s'", operationName, path);
}
#endif
} else {
memcpy(addr.sun_path, path.c_str(), path.size() + 1);
if (operation(fd, psaddr, sizeof(addr)) == -1)
Expand All @@ -91,15 +101,15 @@ static void bindConnectProcHelper(
}


void bind(int fd, const std::string & path)
void bind(Socket fd, const std::string & path)
{
unlink(path.c_str());

bindConnectProcHelper("bind", ::bind, fd, path);
}


void connect(int fd, const std::string & path)
void connect(Socket fd, const std::string & path)
{
bindConnectProcHelper("connect", ::connect, fd, path);
}
Expand Down
84 changes: 84 additions & 0 deletions src/libutil/unix-domain-socket.hh
@@ -0,0 +1,84 @@
#pragma once
///@file

#include "types.hh"
#include "file-descriptor.hh"

#ifdef _WIN32
# include <winsock2.h>
#endif
#include <unistd.h>

namespace nix {

/**
* Create a Unix domain socket.
*/
AutoCloseFD createUnixDomainSocket();

/**
* Create a Unix domain socket in listen mode.
*/
AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode);


/**
* Often we want to use `Descriptor`, but Windows makes a slightly
* stronger file descriptor vs socket distinction, at least at the level
* of C types.
*/
using Socket =
#ifdef _WIN32
SOCKET
#else
int
#endif
;

#ifdef _WIN32
/**
* Windows gives this a different name
*/
# define SHUT_WR SD_SEND
#endif

/**
* Convert a `Socket` to a `Descriptor`
*
* This is a no-op except on Windows.
*/
static inline Socket toSocket(Descriptor fd)
{
#ifdef _WIN32
return reinterpret_cast<Socket>(fd);
#else
return fd;
#endif
}

/**
* Convert a `Socket` to a `Descriptor`
*
* This is a no-op except on Windows.
*/
static inline Descriptor fromSocket(Socket fd)
{
#ifdef _WIN32
return reinterpret_cast<Descriptor>(fd);
#else
return fd;
#endif
}


/**
* Bind a Unix domain socket to a path.
*/
void bind(Socket fd, const std::string & path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't "public" use of types like Socket and Descriptor cause type errors that are basically only caught in CI? (When forgetting to call toSocket, for example.)

I'd prefer a distinct type so that the compiler helps us get it right.

struct Socket {
#ifdef _WIN32
  SOCKET socket;
#else
  int socket;
#endif
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you feel about this (for both types) being a follow up PR? I agree it is better, but I was wary of the "but now Unix needs to jump through more hoops" counter argument.


/**
* Connect to a Unix domain socket.
*/
void connect(Socket fd, const std::string & path);

}
31 changes: 0 additions & 31 deletions src/libutil/unix/unix-domain-socket.hh

This file was deleted.