[PATCH] daemon: Split CHROOT_ENABLED into CHROOT_ENABLED and CONTAINER_ENABLED.

[PATCH] daemon: Split CHROOT_ENABLED into CHROOT_ENABLED and CONTAINER_ENABLED.

[Manolis Ragkousis]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

With this patch, the daemon can perform chrooted builds on Hurd, without
creating problems to other parts of the daemon that can't be supported.

So as Mark said, the cases are:

1. CONTAINER_ENABLED and CHROOT_ENABLED are both true.
In this case, the daemon works as expected, which is what happens in Linux now.

2. CONTAINER_ENABLED is false and CHROOT_ENABLED is true.
Here, things like namespaces cannot be supported, but we can still
perform chrooted builds.

3. CONTAINER_ENABLED and CHROOT_ENABLED are both false.
Here, the daemon is unusable on the system, as it should.

[-- Attachment #2: 0001-daemon-Split-CHROOT_ENABLED-into-CHROOT_ENABLED-and-.patch --]
[-- Type: text/x-patch, Size: 1948 bytes --]

From 9faae6784c63a47f3cc8faa160c208f60dad1e9c Mon Sep 17 00:00:00 2001
From: Manolis Ragkousis <manolis837@gmail.com>
Date: Thu, 20 Aug 2015 13:50:04 +0300
Subject: [PATCH] daemon: Split CHROOT_ENABLED into CHROOT_ENABLED and
 CONTAINER_ENABLED.

* nix/libstore/build.cc (CHROOT_ENABLED): Split.
  (DerivationGoal::startBuilder): Replace CHROOT_ENABLED with CONTAINER_ENABLED.
  (DerivationGoal::runChild): Same.
---
 nix/libstore/build.cc | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index a9eedce..7cde735 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -51,7 +51,15 @@
 #include <linux/fs.h>
 #endif
 
-#define CHROOT_ENABLED HAVE_CHROOT && HAVE_UNSHARE && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
+/* In non Linux systems we can still support chroot builds, even
+   though <sys/mount.h> doesn't exist.*/
+#if __linux__
+#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H
+#else
+#define CHROOT_ENABLED HAVE_CHROOT
+#endif
+
+#define CONTAINER_ENABLED CHROOT_ENABLED && defined(MS_BIND) && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
 
 #if CHROOT_ENABLED
 #include <sys/socket.h>
@@ -1946,7 +1954,7 @@ void DerivationGoal::startBuilder()
        - The UTS namespace ensures that builders see a hostname of
          localhost rather than the actual hostname.
     */
-#if CHROOT_ENABLED
+#if CONTAINER_ENABLED
     if (useChroot) {
 	char stack[32 * 1024];
 	int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | SIGCHLD;
@@ -1994,7 +2002,7 @@ void DerivationGoal::runChild()
 
         commonChildInit(builderOut);
 
-#if CHROOT_ENABLED
+#if CONTAINER_ENABLED
         if (useChroot) {
             /* Initialise the loopback interface. */
             AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP));
-- 
2.5.0

Re: [PATCH] daemon: Split CHROOT_ENABLED into CHROOT_ENABLED and CONTAINER_ENABLED.

[Mark H Weaver]

Manolis Ragkousis <manolis837@gmail.com> writes:

> With this patch, the daemon can perform chrooted builds on Hurd, without
> creating problems to other parts of the daemon that can't be supported.
>
> So as Mark said, the cases are:
>
> 1. CONTAINER_ENABLED and CHROOT_ENABLED are both true.
> In this case, the daemon works as expected, which is what happens in Linux now.
>
> 2. CONTAINER_ENABLED is false and CHROOT_ENABLED is true.
> Here, things like namespaces cannot be supported, but we can still
> perform chrooted builds.
>
> 3. CONTAINER_ENABLED and CHROOT_ENABLED are both false.
> Here, the daemon is unusable on the system, as it should.
>
> From 9faae6784c63a47f3cc8faa160c208f60dad1e9c Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Thu, 20 Aug 2015 13:50:04 +0300
> Subject: [PATCH] daemon: Split CHROOT_ENABLED into CHROOT_ENABLED and
>  CONTAINER_ENABLED.
>
> * nix/libstore/build.cc (CHROOT_ENABLED): Split.
>   (DerivationGoal::startBuilder): Replace CHROOT_ENABLED with CONTAINER_ENABLED.
>   (DerivationGoal::runChild): Same.
> ---
>  nix/libstore/build.cc | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
> index a9eedce..7cde735 100644
> --- a/nix/libstore/build.cc
> +++ b/nix/libstore/build.cc
> @@ -51,7 +51,15 @@
>  #include <linux/fs.h>
>  #endif
>  
> -#define CHROOT_ENABLED HAVE_CHROOT && HAVE_UNSHARE && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
> +/* In non Linux systems we can still support chroot builds, even
> +   though <sys/mount.h> doesn't exist.*/
> +#if __linux__
> +#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H
> +#else
> +#define CHROOT_ENABLED HAVE_CHROOT
> +#endif
> +
> +#define CONTAINER_ENABLED CHROOT_ENABLED && defined(MS_BIND) && defined(MS_PRIVATE) && defined(CLONE_NEWNS) && defined(SYS_pivot_root)
>  
>  #if CHROOT_ENABLED
>  #include <sys/socket.h>
> @@ -1946,7 +1954,7 @@ void DerivationGoal::startBuilder()
>         - The UTS namespace ensures that builders see a hostname of
>           localhost rather than the actual hostname.
>      */
> -#if CHROOT_ENABLED
> +#if CONTAINER_ENABLED
>      if (useChroot) {
>  	char stack[32 * 1024];
>  	int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | SIGCHLD;
> @@ -1994,7 +2002,7 @@ void DerivationGoal::runChild()
>  
>          commonChildInit(builderOut);
>  
> -#if CHROOT_ENABLED
> +#if CONTAINER_ENABLED
>          if (useChroot) {
>              /* Initialise the loopback interface. */
>              AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP));

This still doesn't look right.  What about all the code starting from
line 2040 that sets up /dev and /etc in the chroot, as well as all the
store directories, and notably the code starting from line 2123 that
actually does the chroot?

As you have it now, I don't think it's actually doing the chroot at all
when CONTAINER_ENABLED is false.  Am I missing something?

      Mark