GNU bug report logs - #37302
[PATCH 0/7] Remove the daemon's libexec helpers

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Wed, 4 Sep 2019 10:22:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37302 in the body.
You can then email your comments to 37302 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:22:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ludovic Courtès <ludo <at> gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Wed, 04 Sep 2019 10:22:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 0/7] Remove the daemon's libexec helpers
Date: Wed,  4 Sep 2019 12:19:35 +0200
Hello Guix!

These patches change the daemon so that it invokes ‘guix substitute’,
‘guix authenticate’, etc. directly, instead of invoking the shell
trampolines under libexec/.

This simplifies things a bit, gets rid of the extra Bash invocation,
which in turn makes sure that we don’t get locale warnings from
Bash, should that happen.  :-)

Initially the libexec trampolines were created primarily so that the
C++ code would remain as close as possible to upstream Nix.  The cost
of this strategy have come to outweigh the benefits, so here we go!

The next step will be to adjust the ‘guix’ and ‘guix-daemon’ packages
accordingly.

Feedback welcome!

Ludo’.

Ludovic Courtès (7):
  daemon: Invoke 'guix gc --list-busy' instead of 'list-runtime-roots'.
  daemon: Run 'guix authenticate' directly.
  daemon: Run 'guix perform-download' directly.
  daemon: Run 'guix offload' directly.
  daemon: Run 'guix substitute' directly and assume a single
    substituter.
  daemon: Remove 'NIX_LIBEXEC_DIR'.
  etc: Remove references to libexec/guix* from SELinux policy.

 build-aux/pre-inst-env.in         |  14 +--
 config-daemon.ac                  |  11 ---
 doc/guix.texi                     |   4 +
 etc/guix-daemon.cil.in            |   4 -
 guix/scripts/gc.scm               |  15 +++
 guix/store/roots.scm              | 129 +++++++++++++++++++++++++-
 nix/libstore/build.cc             |  63 +++++--------
 nix/libstore/builtins.cc          |   4 +-
 nix/libstore/gc.cc                |  11 +--
 nix/libstore/globals.cc           |   2 +-
 nix/libstore/globals.hh           |  11 +--
 nix/libstore/local-store.cc       | 102 ++++++++++++---------
 nix/libstore/local-store.hh       |  12 +--
 nix/local.mk                      |  16 ----
 nix/nix-daemon/guix-daemon.cc     |  20 +---
 nix/nix-daemon/nix-daemon.cc      |  16 +++-
 nix/scripts/authenticate.in       |  11 ---
 nix/scripts/download.in           |  11 ---
 nix/scripts/list-runtime-roots.in | 147 ------------------------------
 nix/scripts/offload.in            |  11 ---
 nix/scripts/substitute.in         |  11 ---
 21 files changed, 260 insertions(+), 365 deletions(-)
 delete mode 100644 nix/scripts/authenticate.in
 delete mode 100644 nix/scripts/download.in
 delete mode 100644 nix/scripts/list-runtime-roots.in
 delete mode 100644 nix/scripts/offload.in
 delete mode 100644 nix/scripts/substitute.in

-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:02 GMT) Full text and rfc822 format available.

Message #8 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 1/7] daemon: Invoke 'guix gc --list-busy' instead of
 'list-runtime-roots'.
Date: Wed,  4 Sep 2019 12:26:57 +0200
* nix/scripts/list-runtime-roots.in: Remove.
* guix/store/roots.scm (%proc-directory): New variable.
(proc-file-roots, proc-exe-roots, proc-cwd-roots)
(proc-fd-roots, proc-maps-roots, proc-environ-roots)
(referenced-files, canonicalize-store-item, busy-store-items): New
procedures, taken from 'list-runtime-roots.in'.
* nix/libstore/globals.hh (Settings)[guixProgram]: New field.
* nix/libstore/globals.cc (Settings::processEnvironment): Initialize
'guixProgram'.
* nix/libstore/gc.cc (addAdditionalRoots): Drop code related to
'NIX_ROOT_FINDER'.  Run "guix gc --list-busy".
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove
'scripts/list-runtime-roots'.
* config-daemon.ac: Don't output nix/scripts/list-runtime-roots.
* build-aux/pre-inst-env.in: Don't set 'NIX_ROOT_FINDER'.
Set 'GUIX'.
* doc/guix.texi (Invoking guix gc): Document '--list-busy'.
* guix/scripts/gc.scm (show-help, %options): Add "--list-busy".
(guix-gc)[list-busy]: New procedure.
Handle the 'list-busy' action.
---
 build-aux/pre-inst-env.in         |   6 +-
 config-daemon.ac                  |   3 -
 doc/guix.texi                     |   4 +
 guix/scripts/gc.scm               |  15 +++
 guix/store/roots.scm              | 129 +++++++++++++++++++++++++-
 nix/libstore/gc.cc                |  11 +--
 nix/libstore/globals.cc           |   1 +
 nix/libstore/globals.hh           |   3 +
 nix/local.mk                      |   1 -
 nix/scripts/list-runtime-roots.in | 147 ------------------------------
 10 files changed, 158 insertions(+), 162 deletions(-)
 delete mode 100644 nix/scripts/list-runtime-roots.in

diff --git a/build-aux/pre-inst-env.in b/build-aux/pre-inst-env.in
index 3efab69e7d..ab1c519d70 100644
--- a/build-aux/pre-inst-env.in
+++ b/build-aux/pre-inst-env.in
@@ -44,15 +44,17 @@ export PATH
 
 # Daemon helpers.
 
-NIX_ROOT_FINDER="$abs_top_builddir/nix/scripts/list-runtime-roots"
 NIX_LIBEXEC_DIR="@abs_top_builddir@/nix/scripts" # for 'authenticate', etc.
 
-export NIX_ROOT_FINDER NIX_LIBEXEC_DIR
+export NIX_LIBEXEC_DIR
 
 NIX_BUILD_HOOK="$abs_top_builddir/nix/scripts/offload"
 @BUILD_DAEMON_OFFLOAD_TRUE <at> export NIX_BUILD_HOOK
 @BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
 @BUILD_DAEMON_OFFLOAD_FALSE <at> unset NIX_BUILD_HOOK
+# The daemon invokes 'guix'; tell it which one to use.
+GUIX="$abs_top_builddir/scripts/guix"
+export GUIX
 
 # The following variables need only be defined when compiling Guix
 # modules, but we define them to be on the safe side in case of
diff --git a/config-daemon.ac b/config-daemon.ac
index f1ad10acff..f1d26af3a7 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -148,9 +148,6 @@ if test "x$guix_build_daemon" = "xyes"; then
   AC_SUBST([GUIX_TEST_ROOT])
 
   GUIX_CHECK_LOCALSTATEDIR
-
-  AC_CONFIG_FILES([nix/scripts/list-runtime-roots],
-    [chmod +x nix/scripts/list-runtime-roots])
   AC_CONFIG_FILES([nix/scripts/download],
     [chmod +x nix/scripts/download])
   AC_CONFIG_FILES([nix/scripts/substitute],
diff --git a/doc/guix.texi b/doc/guix.texi
index de02ad8687..afbeb00f94 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3496,6 +3496,10 @@ This prints nothing unless the daemon was started with
 List the GC roots owned by the user; when run as root, list @emph{all} the GC
 roots.
 
+@item --list-busy
+List store items in use by currently running processes.  These store
+items are effectively considered GC roots: they cannot be deleted.
+
 @item --clear-failures
 Remove the specified store items from the failed-build cache.
 
diff --git a/guix/scripts/gc.scm b/guix/scripts/gc.scm
index 31657326b6..3f20a2e192 100644
--- a/guix/scripts/gc.scm
+++ b/guix/scripts/gc.scm
@@ -56,6 +56,8 @@ Invoke the garbage collector.\n"))
   -D, --delete           attempt to delete PATHS"))
   (display (G_ "
       --list-roots       list the user's garbage collector roots"))
+  (display (G_ "
+      --list-busy        list store items used by running processes"))
   (display (G_ "
       --optimize         optimize the store by deduplicating identical files"))
   (display (G_ "
@@ -174,6 +176,10 @@ is deprecated; use '-D'~%"))
                 (lambda (opt name arg result)
                   (alist-cons 'action 'list-roots
                               (alist-delete 'action result))))
+        (option '("list-busy") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'action 'list-busy
+                              (alist-delete 'action result))))
         (option '("list-dead") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'action 'list-dead
@@ -265,6 +271,12 @@ is deprecated; use '-D'~%"))
                   (newline))
                 roots)))
 
+  (define (list-busy)
+    ;; List store items used by running processes.
+    (for-each (lambda (item)
+                (display item) (newline))
+              (busy-store-items)))
+
   (with-error-handling
     (let* ((opts  (parse-options))
            (store (open-connection))
@@ -305,6 +317,9 @@ is deprecated; use '-D'~%"))
         ((list-roots)
          (assert-no-extra-arguments)
          (list-roots))
+        ((list-busy)
+         (assert-no-extra-arguments)
+         (list-busy))
         ((delete)
          (delete-paths store (map direct-store-path paths)))
         ((list-references)
diff --git a/guix/store/roots.scm b/guix/store/roots.scm
index 4f23ae34e8..58653507f8 100644
--- a/guix/store/roots.scm
+++ b/guix/store/roots.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2017, 2019 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,9 +26,13 @@
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 ftw)
+  #:use-module (rnrs io ports)
   #:re-export (%gc-roots-directory)
   #:export (gc-roots
-            user-owned?))
+            user-owned?
+            busy-store-items))
 
 ;;; Commentary:
 ;;;
@@ -118,3 +122,124 @@ are user-controlled symlinks stored anywhere on the file system."
 
       (= (stat:uid stat) uid))
     (const #f)))
+
+
+;;;
+;;; Listing "busy" store items: those referenced by currently running
+;;; processes.
+;;;
+
+(define %proc-directory
+  ;; Mount point of Linuxish /proc file system.
+  "/proc")
+
+(define (proc-file-roots dir file)
+  "Return a one-element list containing the file pointed to by DIR/FILE,
+or the empty list."
+  (or (and=> (false-if-exception (readlink (string-append dir "/" file)))
+             list)
+      '()))
+
+(define proc-exe-roots (cut proc-file-roots <> "exe"))
+(define proc-cwd-roots (cut proc-file-roots <> "cwd"))
+
+(define (proc-fd-roots dir)
+  "Return the list of store files referenced by DIR, which is a
+/proc/XYZ directory."
+  (let ((dir (string-append dir "/fd")))
+    (filter-map (lambda (file)
+                  (let ((target (false-if-exception
+                                 (readlink (string-append dir "/" file)))))
+                    (and target
+                         (string-prefix? "/" target)
+                         target)))
+                (or (scandir dir string->number) '()))))
+
+(define (proc-maps-roots dir)
+  "Return the list of store files referenced by DIR, which is a
+/proc/XYZ directory."
+  (define %file-mapping-line
+    (make-regexp "^.*[[:blank:]]+/([^ ]+)$"))
+
+  (call-with-input-file (string-append dir "/maps")
+    (lambda (maps)
+      (let loop ((line  (read-line maps))
+                 (roots '()))
+        (cond ((eof-object? line)
+               roots)
+              ((regexp-exec %file-mapping-line line)
+               =>
+               (lambda (match)
+                 (let ((file (string-append "/"
+                                            (match:substring match 1))))
+                   (loop (read-line maps)
+                         (cons file roots)))))
+              (else
+               (loop (read-line maps) roots)))))))
+
+(define (proc-environ-roots dir)
+  "Return the list of store files referenced by DIR/environ, where DIR is a
+/proc/XYZ directory."
+  (define split-on-nul
+    (cute string-tokenize <>
+          (char-set-complement (char-set #\nul))))
+
+  (define (rhs-file-names str)
+    (let ((equal (string-index str #\=)))
+      (if equal
+          (let* ((str (substring str (+ 1 equal)))
+                 (rx  (string-append (regexp-quote %store-directory)
+                                     "/[0-9a-z]{32}-[a-zA-Z0-9\\._+-]+")))
+            (map match:substring (list-matches rx str)))
+          '())))
+
+  (define environ
+    (string-append dir "/environ"))
+
+  (append-map rhs-file-names
+              (split-on-nul
+               (call-with-input-file environ
+                 get-string-all))))
+
+(define (referenced-files)
+  "Return the list of referenced store items."
+  (append-map (lambda (pid)
+                (let ((proc (string-append %proc-directory "/" pid)))
+                  (catch 'system-error
+                    (lambda ()
+                      (append (proc-exe-roots proc)
+                              (proc-cwd-roots proc)
+                              (proc-fd-roots proc)
+                              (proc-maps-roots proc)
+                              (proc-environ-roots proc)))
+                    (lambda args
+                      (let ((err (system-error-errno args)))
+                        (if (or (= ENOENT err)    ;TOCTTOU race
+                                (= ESRCH err)     ;ditto
+                                (= EACCES err))   ;not running as root
+                            '()
+                            (apply throw args)))))))
+              (scandir %proc-directory string->number
+                       (lambda (a b)
+                         (< (string->number a) (string->number b))))))
+
+(define canonicalize-store-item
+  (let* ((store  (string-append %store-directory "/"))
+         (prefix (string-length store)))
+    (lambda (file)
+      "Return #f if FILE is not a store item; otherwise, return the store file
+name without any sub-directory components."
+      (and (string-prefix? store file)
+           (string-append store
+                          (let ((base (string-drop file prefix)))
+                            (match (string-index base #\/)
+                              (#f    base)
+                              (slash (string-take base slash)))))))))
+
+(define (busy-store-items)
+  "Return the list of store items used by the currently running processes.
+
+This code should typically run as root; it allows the garbage collector to
+determine which store items must not be deleted."
+  (delete-duplicates
+   (filter-map canonicalize-store-item (referenced-files))))
diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
index 46171e116c..c466996668 100644
--- a/nix/libstore/gc.cc
+++ b/nix/libstore/gc.cc
@@ -339,14 +339,11 @@ Roots LocalStore::findRoots()
 
 static void addAdditionalRoots(StoreAPI & store, PathSet & roots)
 {
-    Path rootFinder = getEnv("NIX_ROOT_FINDER",
-        settings.nixLibexecDir + "/list-runtime-roots");
+    debug(format("executing `%1% gc --list-busy' to find additional roots")
+	  % settings.guixProgram);
 
-    if (rootFinder.empty()) return;
-
-    debug(format("executing `%1%' to find additional roots") % rootFinder);
-
-    string result = runProgram(rootFinder);
+    const Strings args = { "gc", "--list-busy" };
+    string result = runProgram(settings.guixProgram, false, args);
 
     StringSet paths = tokenizeString<StringSet>(result, "\n");
 
diff --git a/nix/libstore/globals.cc b/nix/libstore/globals.cc
index 6df20e7a52..8f7c976fcb 100644
--- a/nix/libstore/globals.cc
+++ b/nix/libstore/globals.cc
@@ -73,6 +73,7 @@ void Settings::processEnvironment()
     nixLibexecDir = canonPath(getEnv("NIX_LIBEXEC_DIR", NIX_LIBEXEC_DIR));
     nixBinDir = canonPath(getEnv("NIX_BIN_DIR", NIX_BIN_DIR));
     nixDaemonSocketFile = canonPath(nixStateDir + DEFAULT_SOCKET_PATH);
+    guixProgram = canonPath(getEnv("GUIX", nixBinDir + "/guix"));
 }
 
 
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index b073f724b6..0d9315a41a 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -66,6 +66,9 @@ struct Settings {
     /* File name of the socket the daemon listens to.  */
     Path nixDaemonSocketFile;
 
+    /* Absolute file name of the 'guix' program.  */
+    Path guixProgram;
+
     /* Whether to keep temporary directories of failed builds. */
     bool keepFailed;
 
diff --git a/nix/local.mk b/nix/local.mk
index 6d7e60e9fb..fd7379b5ff 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -155,7 +155,6 @@ noinst_HEADERS =						\
 	           (write (get-string-all in) out)))))"
 
 nodist_pkglibexec_SCRIPTS =			\
-  %D%/scripts/list-runtime-roots		\
   %D%/scripts/substitute			\
   %D%/scripts/download
 
diff --git a/nix/scripts/list-runtime-roots.in b/nix/scripts/list-runtime-roots.in
deleted file mode 100644
index 5f2660fb5e..0000000000
--- a/nix/scripts/list-runtime-roots.in
+++ /dev/null
@@ -1,147 +0,0 @@
-#!@GUILE@ -ds
-!#
-;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2017 Ludovic Courtès <ludo <at> gnu.org>
-;;;
-;;; This file is part of GNU Guix.
-;;;
-;;; GNU Guix is free software; you can redistribute it and/or modify it
-;;; under the terms of the GNU General Public License as published by
-;;; the Free Software Foundation; either version 3 of the License, or (at
-;;; your option) any later version.
-;;;
-;;; GNU Guix is distributed in the hope that it will be useful, but
-;;; WITHOUT ANY WARRANTY; without even the implied warranty of
-;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-;;; GNU General Public License for more details.
-;;;
-;;; You should have received a copy of the GNU General Public License
-;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
-
-;;;
-;;; List files being used at run time; these files are garbage collector
-;;; roots.  This is equivalent to `find-runtime-roots.pl' in Nix.
-;;;
-
-(use-modules (ice-9 ftw)
-             (ice-9 regex)
-             (ice-9 rdelim)
-             (ice-9 match)
-             (srfi srfi-1)
-             (srfi srfi-26)
-             (rnrs io ports))
-
-(define %proc-directory
-  ;; Mount point of Linuxish /proc file system.
-  "/proc")
-
-(define %store-directory
-  (or (getenv "NIX_STORE_DIR")
-      "@storedir@"))
-
-(define (proc-file-roots dir file)
-  "Return a one-element list containing the file pointed to by DIR/FILE,
-or the empty list."
-  (or (and=> (false-if-exception (readlink (string-append dir "/" file)))
-             list)
-      '()))
-
-(define proc-exe-roots (cut proc-file-roots <> "exe"))
-(define proc-cwd-roots (cut proc-file-roots <> "cwd"))
-
-(define (proc-fd-roots dir)
-  "Return the list of store files referenced by DIR, which is a
-/proc/XYZ directory."
-  (let ((dir (string-append dir "/fd")))
-    (filter-map (lambda (file)
-                  (let ((target (false-if-exception
-                                 (readlink (string-append dir "/" file)))))
-                    (and target
-                         (string-prefix? "/" target)
-                         target)))
-                (or (scandir dir string->number) '()))))
-
-(define (proc-maps-roots dir)
-  "Return the list of store files referenced by DIR, which is a
-/proc/XYZ directory."
-  (define %file-mapping-line
-    (make-regexp "^.*[[:blank:]]+/([^ ]+)$"))
-
-  (call-with-input-file (string-append dir "/maps")
-    (lambda (maps)
-      (let loop ((line  (read-line maps))
-                 (roots '()))
-        (cond ((eof-object? line)
-               roots)
-              ((regexp-exec %file-mapping-line line)
-               =>
-               (lambda (match)
-                 (let ((file (string-append "/"
-                                            (match:substring match 1))))
-                   (loop (read-line maps)
-                         (cons file roots)))))
-              (else
-               (loop (read-line maps) roots)))))))
-
-(define (proc-environ-roots dir)
-  "Return the list of store files referenced by DIR/environ, where DIR is a
-/proc/XYZ directory."
-  (define split-on-nul
-    (cute string-tokenize <>
-          (char-set-complement (char-set #\nul))))
-
-  (define (rhs-file-names str)
-    (let ((equal (string-index str #\=)))
-      (if equal
-          (let* ((str (substring str (+ 1 equal)))
-                 (rx  (string-append (regexp-quote %store-directory)
-                                     "/[0-9a-z]{32}-[a-zA-Z0-9\\._+-]+")))
-            (map match:substring (list-matches rx str)))
-          '())))
-
-  (define environ
-    (string-append dir "/environ"))
-
-  (append-map rhs-file-names
-              (split-on-nul
-               (call-with-input-file environ
-                 get-string-all))))
-
-(define (referenced-files)
-  "Return the list of referenced store items."
-  (append-map (lambda (pid)
-                (let ((proc (string-append %proc-directory "/" pid)))
-                  (catch 'system-error
-                    (lambda ()
-                      (append (proc-exe-roots proc)
-                              (proc-cwd-roots proc)
-                              (proc-fd-roots proc)
-                              (proc-maps-roots proc)
-                              (proc-environ-roots proc)))
-                    (lambda args
-                      (let ((err (system-error-errno args)))
-                        (if (or (= ENOENT err)    ;TOCTTOU race
-                                (= ESRCH err)     ;ditto
-                                (= EACCES err))   ;not running as root
-                            '()
-                            (apply throw args)))))))
-              (scandir %proc-directory string->number
-                       (lambda (a b)
-                         (< (string->number a) (string->number b))))))
-
-(define canonicalize-store-item
-  (let* ((store  (string-append %store-directory "/"))
-         (prefix (string-length store)))
-    (lambda (file)
-      "Return #f if FILE is not a store item; otherwise, return the store file
-name without any sub-directory components."
-      (and (string-prefix? store file)
-           (string-append store
-                          (let ((base (string-drop file prefix)))
-                            (match (string-index base #\/)
-                              (#f    base)
-                              (slash (string-take base slash)))))))))
-
-(for-each (cut simple-format #t "~a~%" <>)
-          (delete-duplicates
-           (filter-map canonicalize-store-item (referenced-files))))
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:02 GMT) Full text and rfc822 format available.

Message #11 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 2/7] daemon: Run 'guix authenticate' directly.
Date: Wed,  4 Sep 2019 12:26:58 +0200
* nix/scripts/authenticate.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove
scripts/authenticate.
* config-daemon.ac: Don't output 'nix/scripts/authenticate'.
* nix/libstore/local-store.cc (runAuthenticationProgram): Run 'guix
authenticate'.
---
 config-daemon.ac            |  2 --
 nix/libstore/local-store.cc |  5 +++--
 nix/local.mk                |  3 ---
 nix/scripts/authenticate.in | 11 -----------
 4 files changed, 3 insertions(+), 18 deletions(-)
 delete mode 100644 nix/scripts/authenticate.in

diff --git a/config-daemon.ac b/config-daemon.ac
index f1d26af3a7..907457f478 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -152,8 +152,6 @@ if test "x$guix_build_daemon" = "xyes"; then
     [chmod +x nix/scripts/download])
   AC_CONFIG_FILES([nix/scripts/substitute],
     [chmod +x nix/scripts/substitute])
-  AC_CONFIG_FILES([nix/scripts/authenticate],
-    [chmod +x nix/scripts/authenticate])
   AC_CONFIG_FILES([nix/scripts/offload],
     [chmod +x nix/scripts/offload])
 fi
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 892d9300b1..951c35faf3 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -1224,8 +1224,9 @@ static void checkSecrecy(const Path & path)
 
 static std::string runAuthenticationProgram(const Strings & args)
 {
-    return runProgram(settings.nixLibexecDir + "/authenticate",
-		      false, args);
+    Strings fullArgs = { "authenticate" };
+    fullArgs.insert(fullArgs.end(), args.begin(), args.end()); // append
+    return runProgram(settings.guixProgram, false, fullArgs);
 }
 
 void LocalStore::exportPath(const Path & path, bool sign,
diff --git a/nix/local.mk b/nix/local.mk
index fd7379b5ff..cdcd9eb1c2 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -165,9 +165,6 @@ nodist_pkglibexec_SCRIPTS +=			\
 
 endif BUILD_DAEMON_OFFLOAD
 
-nodist_pkglibexec_SCRIPTS +=			\
-  %D%/scripts/authenticate
-
 # The '.service' files for systemd.
 systemdservicedir = $(libdir)/systemd/system
 nodist_systemdservice_DATA = etc/guix-daemon.service etc/guix-publish.service
diff --git a/nix/scripts/authenticate.in b/nix/scripts/authenticate.in
deleted file mode 100644
index 5ce57915f0..0000000000
--- a/nix/scripts/authenticate.in
+++ /dev/null
@@ -1,11 +0,0 @@
-#!@SHELL@
-# A shorthand for "guix authenticate", for use by the daemon.
-
-if test "x$GUIX_UNINSTALLED" = "x"
-then
-    prefix="@prefix@"
-    exec_prefix="@exec_prefix@"
-    exec "@bindir@/guix" authenticate "$@"
-else
-    exec guix authenticate "$@"
-fi
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:03 GMT) Full text and rfc822 format available.

Message #14 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 3/7] daemon: Run 'guix perform-download' directly.
Date: Wed,  4 Sep 2019 12:26:59 +0200
* nix/scripts/download.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove 'scripts/download'.
* config-daemon.ac: Don't output 'nix/scripts/download'.
* nix/libstore/builtins.cc (builtinDownload): Invoke 'guix
perform-download' directly.
---
 config-daemon.ac         |  2 --
 nix/libstore/builtins.cc |  4 ++--
 nix/local.mk             |  3 +--
 nix/scripts/download.in  | 11 -----------
 4 files changed, 3 insertions(+), 17 deletions(-)
 delete mode 100644 nix/scripts/download.in

diff --git a/config-daemon.ac b/config-daemon.ac
index 907457f478..50227e310c 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -148,8 +148,6 @@ if test "x$guix_build_daemon" = "xyes"; then
   AC_SUBST([GUIX_TEST_ROOT])
 
   GUIX_CHECK_LOCALSTATEDIR
-  AC_CONFIG_FILES([nix/scripts/download],
-    [chmod +x nix/scripts/download])
   AC_CONFIG_FILES([nix/scripts/substitute],
     [chmod +x nix/scripts/substitute])
   AC_CONFIG_FILES([nix/scripts/offload],
diff --git a/nix/libstore/builtins.cc b/nix/libstore/builtins.cc
index f7c7d42484..4111ac4760 100644
--- a/nix/libstore/builtins.cc
+++ b/nix/libstore/builtins.cc
@@ -39,7 +39,7 @@ static void builtinDownload(const Derivation &drv,
 
     const char *const argv[] =
       {
-	"download", drvPath.c_str(), output.c_str(), NULL
+	  "guix", "perform-download", drvPath.c_str(), output.c_str(), NULL
       };
 
     /* Tell the script what the store file name is, so that
@@ -50,7 +50,7 @@ static void builtinDownload(const Derivation &drv,
     /* Tell it about options such as "print-extended-build-trace".  */
     setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
 
-    const string program = settings.nixLibexecDir + "/download";
+    const string program = settings.guixProgram;
     execv(program.c_str(), (char *const *) argv);
 
     throw SysError(format("failed to run download program '%1%'") % program);
diff --git a/nix/local.mk b/nix/local.mk
index cdcd9eb1c2..c4c3920fa3 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -155,8 +155,7 @@ noinst_HEADERS =						\
 	           (write (get-string-all in) out)))))"
 
 nodist_pkglibexec_SCRIPTS =			\
-  %D%/scripts/substitute			\
-  %D%/scripts/download
+  %D%/scripts/substitute
 
 if BUILD_DAEMON_OFFLOAD
 
diff --git a/nix/scripts/download.in b/nix/scripts/download.in
deleted file mode 100644
index 4d7088a993..0000000000
--- a/nix/scripts/download.in
+++ /dev/null
@@ -1,11 +0,0 @@
-#!@SHELL@
-# A shorthand for "guix perform-download", for use by the daemon.
-
-if test "x$GUIX_UNINSTALLED" = "x"
-then
-    prefix="@prefix@"
-    exec_prefix="@exec_prefix@"
-    exec "@bindir@/guix" perform-download "$@"
-else
-    exec guix perform-download "$@"
-fi
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:03 GMT) Full text and rfc822 format available.

Message #17 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 4/7] daemon: Run 'guix offload' directly.
Date: Wed,  4 Sep 2019 12:27:00 +0200
* nix/scripts/offload.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS) [BUILD_DAEMON_OFFLOAD]:
Remove 'scripts/offload'.
* config-daemon.ac: Don't output 'nix/scripts/offload'.
* build-aux/pre-inst-env.in: Don't set 'NIX_BUILD_HOOK'.
* nix/libstore/build.cc (HookInstance::HookInstance): Run 'guix
offload'.
(DerivationGoal::tryBuildHook): Remove reference to 'NIX_BUILD_HOOK'.
* nix/nix-daemon/guix-daemon.cc (main) [HAVE_DAEMON_OFFLOAD_HOOK]: Don't
set 'NIX_BUILD_HOOK'.
* nix/nix-daemon/nix-daemon.cc (performOp) [!HAVE_DAEMON_OFFLOAD_HOOK]:
Leave 'settings.useBuildHook' unchanged.
---
 build-aux/pre-inst-env.in     |  4 ----
 config-daemon.ac              |  2 --
 nix/libstore/build.cc         | 11 +++++------
 nix/local.mk                  |  7 -------
 nix/nix-daemon/guix-daemon.cc |  9 +--------
 nix/nix-daemon/nix-daemon.cc  |  8 +++++++-
 nix/scripts/offload.in        | 11 -----------
 7 files changed, 13 insertions(+), 39 deletions(-)
 delete mode 100644 nix/scripts/offload.in

diff --git a/build-aux/pre-inst-env.in b/build-aux/pre-inst-env.in
index ab1c519d70..f96288132d 100644
--- a/build-aux/pre-inst-env.in
+++ b/build-aux/pre-inst-env.in
@@ -48,10 +48,6 @@ NIX_LIBEXEC_DIR="@abs_top_builddir@/nix/scripts" # for 'authenticate', etc.
 
 export NIX_LIBEXEC_DIR
 
-NIX_BUILD_HOOK="$abs_top_builddir/nix/scripts/offload"
-@BUILD_DAEMON_OFFLOAD_TRUE <at> export NIX_BUILD_HOOK
-@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
-@BUILD_DAEMON_OFFLOAD_FALSE <at> unset NIX_BUILD_HOOK
 # The daemon invokes 'guix'; tell it which one to use.
 GUIX="$abs_top_builddir/scripts/guix"
 export GUIX
diff --git a/config-daemon.ac b/config-daemon.ac
index 50227e310c..3d92e8f778 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -150,8 +150,6 @@ if test "x$guix_build_daemon" = "xyes"; then
   GUIX_CHECK_LOCALSTATEDIR
   AC_CONFIG_FILES([nix/scripts/substitute],
     [chmod +x nix/scripts/substitute])
-  AC_CONFIG_FILES([nix/scripts/offload],
-    [chmod +x nix/scripts/offload])
 fi
 
 AM_CONDITIONAL([HAVE_LIBBZ2], [test "x$HAVE_LIBBZ2" = "xyes"])
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index fe7bf79069..9f1f88933a 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -614,9 +614,7 @@ HookInstance::HookInstance()
 {
     debug("starting build hook");
 
-    Path buildHook = getEnv("NIX_BUILD_HOOK");
-    if (string(buildHook, 0, 1) != "/") buildHook = settings.nixLibexecDir + "/nix/" + buildHook;
-    buildHook = canonPath(buildHook);
+    const Path &buildHook = settings.guixProgram;
 
     /* Create a pipe to get the output of the child. */
     fromHook.create();
@@ -642,13 +640,14 @@ HookInstance::HookInstance()
         if (dup2(builderOut.writeSide, 4) == -1)
             throw SysError("dupping builder's stdout/stderr");
 
-        execl(buildHook.c_str(), buildHook.c_str(), settings.thisSystem.c_str(),
+        execl(buildHook.c_str(), buildHook.c_str(), "offload",
+	    settings.thisSystem.c_str(),
             (format("%1%") % settings.maxSilentTime).str().c_str(),
             (format("%1%") % settings.printBuildTrace).str().c_str(),
             (format("%1%") % settings.buildTimeout).str().c_str(),
             NULL);
 
-        throw SysError(format("executing `%1%'") % buildHook);
+        throw SysError(format("executing `%1% offload'") % buildHook);
     });
 
     pid.setSeparatePG(true);
@@ -1581,7 +1580,7 @@ void DerivationGoal::buildDone()
 
 HookReply DerivationGoal::tryBuildHook()
 {
-    if (!settings.useBuildHook || getEnv("NIX_BUILD_HOOK") == "") return rpDecline;
+    if (!settings.useBuildHook) return rpDecline;
 
     if (!worker.hook)
         worker.hook = std::shared_ptr<HookInstance>(new HookInstance);
diff --git a/nix/local.mk b/nix/local.mk
index c4c3920fa3..8e52c77bd9 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -157,13 +157,6 @@ noinst_HEADERS =						\
 nodist_pkglibexec_SCRIPTS =			\
   %D%/scripts/substitute
 
-if BUILD_DAEMON_OFFLOAD
-
-nodist_pkglibexec_SCRIPTS +=			\
-  %D%/scripts/offload
-
-endif BUILD_DAEMON_OFFLOAD
-
 # The '.service' files for systemd.
 systemdservicedir = $(libdir)/systemd/system
 nodist_systemdservice_DATA = etc/guix-daemon.service etc/guix-publish.service
diff --git a/nix/nix-daemon/guix-daemon.cc b/nix/nix-daemon/guix-daemon.cc
index f47d142612..73962af584 100644
--- a/nix/nix-daemon/guix-daemon.cc
+++ b/nix/nix-daemon/guix-daemon.cc
@@ -474,15 +474,8 @@ main (int argc, char *argv[])
       settings.set ("substitute-urls", GUIX_SUBSTITUTE_URLS);
 
 #ifdef HAVE_DAEMON_OFFLOAD_HOOK
-      /* Use our build hook for distributed builds by default.  */
+      /* Use 'guix offload' for distributed builds by default.  */
       settings.useBuildHook = true;
-      if (getenv ("NIX_BUILD_HOOK") == NULL)
-	{
-	  std::string build_hook;
-
-	  build_hook = settings.nixLibexecDir + "/offload";
-	  setenv ("NIX_BUILD_HOOK", build_hook.c_str (), 1);
-	}
 #else
       /* We are not installing any build hook, so disable it.  */
       settings.useBuildHook = false;
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 56137701a1..f29bcd2eab 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -580,8 +580,14 @@ static void performOp(bool trusted, unsigned int clientVersion,
             settings.set("build-max-silent-time", std::to_string(readInt(from)));
         }
 
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 2)
+        if (GET_PROTOCOL_MINOR(clientVersion) >= 2) {
+#ifdef HAVE_DAEMON_OFFLOAD_HOOK
             settings.useBuildHook = readInt(from) != 0;
+#else
+	    readInt(from);			  // ignore the user's setting
+#endif
+	}
+
         if (GET_PROTOCOL_MINOR(clientVersion) >= 4) {
             settings.buildVerbosity = (Verbosity) readInt(from);
             logType = (LogType) readInt(from);
diff --git a/nix/scripts/offload.in b/nix/scripts/offload.in
deleted file mode 100644
index 50faed31c0..0000000000
--- a/nix/scripts/offload.in
+++ /dev/null
@@ -1,11 +0,0 @@
-#!@SHELL@
-# A shorthand for "guix offload", for use by the daemon.
-
-if test "x$GUIX_UNINSTALLED" = "x"
-then
-    prefix="@prefix@"
-    exec_prefix="@exec_prefix@"
-    exec "@bindir@/guix" offload "$@"
-else
-    exec guix offload "$@"
-fi
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:04 GMT) Full text and rfc822 format available.

Message #20 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 7/7] etc: Remove references to libexec/guix* from SELinux
 policy.
Date: Wed,  4 Sep 2019 12:27:03 +0200
* etc/guix-daemon.cil.in: Remove references to libexec/guix*.
---
 etc/guix-daemon.cil.in | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/etc/guix-daemon.cil.in b/etc/guix-daemon.cil.in
index c0c82d8fbb..e0c9113498 100644
--- a/etc/guix-daemon.cil.in
+++ b/etc/guix-daemon.cil.in
@@ -277,9 +277,5 @@
            file (system_u object_r guix_daemon_exec_t (low low)))
   (filecon "@storedir@/.+-(guix-.+|profile)/bin/guix-daemon"
            file (system_u object_r guix_daemon_exec_t (low low)))
-  (filecon "@storedir@/.+-(guix-.+|profile)/libexec/guix-authenticate"
-           file (system_u object_r guix_daemon_exec_t (low low)))
-  (filecon "@storedir@/.+-(guix-.+|profile)/libexec/guix/(.*)?"
-           any (system_u object_r guix_daemon_exec_t (low low)))
   (filecon "@guix_localstatedir@/guix/daemon-socket/socket"
            any (system_u object_r guix_daemon_socket_t (low low))))
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:04 GMT) Full text and rfc822 format available.

Message #23 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 6/7] daemon: Remove 'NIX_LIBEXEC_DIR'.
Date: Wed,  4 Sep 2019 12:27:02 +0200
* nix/libstore/globals.hh (Settings)[nixLibexecDir]: Remove.
* nix/libstore/globals.cc (Settings::processEnvironment): Remove
reference to 'nixLibexecDir'.
* nix/local.mk (libstore_a_CPPFLAGS): Remove -DNIX_LIBEXEC_DIR flag.
* build-aux/pre-inst-env.in: Remove references to 'NIX_LIBEXEC_DIR'.
---
 build-aux/pre-inst-env.in | 6 ------
 nix/libstore/globals.cc   | 1 -
 nix/libstore/globals.hh   | 3 ---
 nix/local.mk              | 1 -
 4 files changed, 11 deletions(-)

diff --git a/build-aux/pre-inst-env.in b/build-aux/pre-inst-env.in
index f96288132d..e0aa7fe868 100644
--- a/build-aux/pre-inst-env.in
+++ b/build-aux/pre-inst-env.in
@@ -42,12 +42,6 @@ export GUILE_LOAD_COMPILED_PATH GUILE_LOAD_PATH
 PATH="$abs_top_builddir/scripts:$abs_top_builddir:$PATH"
 export PATH
 
-# Daemon helpers.
-
-NIX_LIBEXEC_DIR="@abs_top_builddir@/nix/scripts" # for 'authenticate', etc.
-
-export NIX_LIBEXEC_DIR
-
 # The daemon invokes 'guix'; tell it which one to use.
 GUIX="$abs_top_builddir/scripts/guix"
 export GUIX
diff --git a/nix/libstore/globals.cc b/nix/libstore/globals.cc
index 8f7c976fcb..0cc001fbe4 100644
--- a/nix/libstore/globals.cc
+++ b/nix/libstore/globals.cc
@@ -70,7 +70,6 @@ void Settings::processEnvironment()
     nixStateDir = canonPath(getEnv("GUIX_STATE_DIRECTORY", NIX_STATE_DIR));
     nixDBPath = getEnv("GUIX_DATABASE_DIRECTORY", nixStateDir + "/db");
     nixConfDir = canonPath(getEnv("GUIX_CONFIGURATION_DIRECTORY", GUIX_CONFIGURATION_DIRECTORY));
-    nixLibexecDir = canonPath(getEnv("NIX_LIBEXEC_DIR", NIX_LIBEXEC_DIR));
     nixBinDir = canonPath(getEnv("NIX_BIN_DIR", NIX_BIN_DIR));
     nixDaemonSocketFile = canonPath(nixStateDir + DEFAULT_SOCKET_PATH);
     guixProgram = canonPath(getEnv("GUIX", nixBinDir + "/guix"));
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 0069c85956..27616a2283 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -57,9 +57,6 @@ struct Settings {
     /* The directory where configuration files are stored. */
     Path nixConfDir;
 
-    /* The directory where internal helper programs are stored. */
-    Path nixLibexecDir;
-
     /* The directory where the main programs are stored. */
     Path nixBinDir;
 
diff --git a/nix/local.mk b/nix/local.mk
index 18e9ba7604..dc5a8398b2 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -110,7 +110,6 @@ libstore_a_CPPFLAGS =				\
   -DNIX_STATE_DIR=\"$(localstatedir)/guix\"	\
   -DNIX_LOG_DIR=\"$(localstatedir)/log/guix\"	\
   -DGUIX_CONFIGURATION_DIRECTORY=\"$(sysconfdir)/guix\"		\
-  -DNIX_LIBEXEC_DIR=\"$(libexecdir)/guix\"	\
   -DNIX_BIN_DIR=\"$(bindir)\"			\
   -DDEFAULT_CHROOT_DIRS="\"\""
 
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#37302; Package guix-patches. (Wed, 04 Sep 2019 10:28:08 GMT) Full text and rfc822 format available.

Message #26 received at 37302 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 5/7] daemon: Run 'guix substitute' directly and assume a
 single substituter.
Date: Wed,  4 Sep 2019 12:27:01 +0200
The daemon had a mechanism that allows it to handle a list of
substituters and try them sequentially; this removes it.

* nix/scripts/substitute.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove.
* config-daemon.ac: Don't output 'nix/scripts/substitute'.
* nix/libstore/build.cc (SubstitutionGoal)[subs, sub, hasSubstitute]:
Remove.
[tryNext]: Make private.
(SubstitutionGoal::SubstitutionGoal, SubstitutionGoal::init): Remove now
unneeded initializers.
(SubstitutionGoal::tryNext): Adjust to assume a single substituter: call
'amDone' upfront when we couldn't find substitutes.
(SubstitutionGoal::tryToRun): Adjust to run 'guix substitute' via
'settings.guixProgram'.
(SubstitutionGoal::finished): Call 'amDone(ecFailed)' upon failure
instead of setting 'state' to 'tryNext'.
* nix/libstore/globals.hh (Settings)[substituters]: Remove.
* nix/libstore/local-store.cc (LocalStore::~LocalStore): Adjust to
handle a single substituter.
(LocalStore::startSubstituter): Remove 'path' parameter.  Adjust to
invoke 'settings.guixProgram'.  Don't refer to 'run.program', which no
longer exists.
(LocalStore::querySubstitutablePaths): Adjust for 'runningSubstituters'
being a singleton instead of a list.
(LocalStore::querySubstitutablePathInfos): Likewise, and remove
'substituter' parameter.
* nix/libstore/local-store.hh (RunningSubstituter)[program]: Remove.
(LocalStore)[runningSubstituters]: Remove.
[runningSubstituter]: New field.
[querySubstitutablePathInfos]: Remove 'substituter' parameter.
[startSubstituter]: Remove 'substituter' parameter.
* nix/nix-daemon/guix-daemon.cc (main): Remove references to
'settings.substituters'.
* nix/nix-daemon/nix-daemon.cc (performOp): Ignore the user's
"build-use-substitutes" value when 'settings.useSubstitutes' is false.
---
 config-daemon.ac              |  2 -
 nix/libstore/build.cc         | 52 +++++++------------
 nix/libstore/globals.hh       |  5 --
 nix/libstore/local-store.cc   | 97 +++++++++++++++++++----------------
 nix/libstore/local-store.hh   | 12 ++---
 nix/local.mk                  |  3 --
 nix/nix-daemon/guix-daemon.cc | 11 +---
 nix/nix-daemon/nix-daemon.cc  |  8 ++-
 nix/scripts/substitute.in     | 11 ----
 9 files changed, 85 insertions(+), 116 deletions(-)
 delete mode 100644 nix/scripts/substitute.in

diff --git a/config-daemon.ac b/config-daemon.ac
index 3d92e8f778..bf94815966 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -148,8 +148,6 @@ if test "x$guix_build_daemon" = "xyes"; then
   AC_SUBST([GUIX_TEST_ROOT])
 
   GUIX_CHECK_LOCALSTATEDIR
-  AC_CONFIG_FILES([nix/scripts/substitute],
-    [chmod +x nix/scripts/substitute])
 fi
 
 AM_CONDITIONAL([HAVE_LIBBZ2], [test "x$HAVE_LIBBZ2" = "xyes"])
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 9f1f88933a..ad53b81413 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2863,15 +2863,6 @@ private:
     /* The store path that should be realised through a substitute. */
     Path storePath;
 
-    /* The remaining substituters. */
-    Paths subs;
-
-    /* The current substituter. */
-    Path sub;
-
-    /* Whether any substituter can realise this path */
-    bool hasSubstitute;
-
     /* Path info returned by the substituter's query info operation. */
     SubstitutablePathInfo info;
 
@@ -2897,6 +2888,8 @@ private:
     typedef void (SubstitutionGoal::*GoalState)();
     GoalState state;
 
+    void tryNext();
+
 public:
     SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false);
     ~SubstitutionGoal();
@@ -2914,7 +2907,6 @@ public:
 
     /* The states. */
     void init();
-    void tryNext();
     void gotInfo();
     void referencesValid();
     void tryToRun();
@@ -2930,7 +2922,6 @@ public:
 
 SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, bool repair)
     : Goal(worker)
-    , hasSubstitute(false)
     , repair(repair)
 {
     this->storePath = storePath;
@@ -2980,37 +2971,31 @@ void SubstitutionGoal::init()
     if (settings.readOnlyMode)
         throw Error(format("cannot substitute path `%1%' - no write access to the store") % storePath);
 
-    subs = settings.substituters;
-
     tryNext();
 }
 
 
 void SubstitutionGoal::tryNext()
 {
-    trace("trying next substituter");
+    trace("trying substituter");
 
-    if (subs.size() == 0) {
+    SubstitutablePathInfos infos;
+    PathSet dummy(singleton<PathSet>(storePath));
+    worker.store.querySubstitutablePathInfos(dummy, infos);
+    SubstitutablePathInfos::iterator k = infos.find(storePath);
+    if (k == infos.end()) {
         /* None left.  Terminate this goal and let someone else deal
            with it. */
         debug(format("path `%1%' is required, but there is no substituter that can build it") % storePath);
         /* Hack: don't indicate failure if there were no substituters.
            In that case the calling derivation should just do a
            build. */
-        amDone(hasSubstitute ? ecFailed : ecNoSubstituters);
-        return;
+        amDone(ecNoSubstituters);
+	return;
     }
 
-    sub = subs.front();
-    subs.pop_front();
-
-    SubstitutablePathInfos infos;
-    PathSet dummy(singleton<PathSet>(storePath));
-    worker.store.querySubstitutablePathInfos(sub, dummy, infos);
-    SubstitutablePathInfos::iterator k = infos.find(storePath);
-    if (k == infos.end()) { tryNext(); return; }
+    /* Found a substitute.  */
     info = k->second;
-    hasSubstitute = true;
 
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
@@ -3098,7 +3083,8 @@ void SubstitutionGoal::tryToRun()
 
     /* Fill in the arguments. */
     Strings args;
-    args.push_back(baseNameOf(sub));
+    args.push_back("guix");
+    args.push_back("substitute");
     args.push_back("--substitute");
     args.push_back(storePath);
     args.push_back(destPath);
@@ -3111,9 +3097,9 @@ void SubstitutionGoal::tryToRun()
         if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
             throw SysError("cannot dup output pipe into stdout");
 
-        execv(sub.c_str(), stringsToCharPtrs(args).data());
+        execv(settings.guixProgram.c_str(), stringsToCharPtrs(args).data());
 
-        throw SysError(format("executing `%1%'") % sub);
+        throw SysError(format("executing `%1% substitute'") % settings.guixProgram);
     });
 
     pid.setSeparatePG(true);
@@ -3126,7 +3112,9 @@ void SubstitutionGoal::tryToRun()
     state = &SubstitutionGoal::finished;
 
     if (settings.printBuildTrace)
-        printMsg(lvlError, format("@ substituter-started %1% %2%") % storePath % sub);
+	/* The second element in the message used to be the name of the
+	   substituter but we're left with only one.  */
+        printMsg(lvlError, format("@ substituter-started %1% substitute") % storePath);
 }
 
 
@@ -3192,9 +3180,7 @@ void SubstitutionGoal::finished()
                 % storePath % status % e.msg());
         }
 
-        /* Try the next substitute. */
-        state = &SubstitutionGoal::tryNext;
-        worker.wakeUp(shared_from_this());
+	amDone(ecFailed);
         return;
     }
 
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 0d9315a41a..0069c85956 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -115,11 +115,6 @@ struct Settings {
        means infinity.  */
     time_t buildTimeout;
 
-    /* The substituters.  There are programs that can somehow realise
-       a store path without building, e.g., by downloading it or
-       copying it from a CD. */
-    Paths substituters;
-
     /* Whether to use build hooks (for distributed builds).  Sometimes
        users want to disable this from the command-line. */
     bool useBuildHook;
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 951c35faf3..3b08492c64 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -184,13 +184,15 @@ LocalStore::LocalStore(bool reserveSpace)
 LocalStore::~LocalStore()
 {
     try {
-        foreach (RunningSubstituters::iterator, i, runningSubstituters) {
-            if (i->second.disabled) continue;
-            i->second.to.close();
-            i->second.from.close();
-            i->second.error.close();
-            if (i->second.pid != -1)
-                i->second.pid.wait(true);
+	if (runningSubstituter) {
+	    RunningSubstituter &i = *runningSubstituter;
+            if (!i.disabled) {
+		i.to.close();
+		i.from.close();
+		i.error.close();
+		if (i.pid != -1)
+		    i.pid.wait(true);
+	    }
         }
     } catch (...) {
         ignoreException();
@@ -808,11 +810,12 @@ void LocalStore::setSubstituterEnv()
 }
 
 
-void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run)
+void LocalStore::startSubstituter(RunningSubstituter & run)
 {
     if (run.disabled || run.pid != -1) return;
 
-    debug(format("starting substituter program `%1%'") % substituter);
+    debug(format("starting substituter program `%1% substitute'")
+	  % settings.guixProgram);
 
     Pipe toPipe, fromPipe, errorPipe;
 
@@ -829,11 +832,10 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
             throw SysError("dupping stdout");
         if (dup2(errorPipe.writeSide, STDERR_FILENO) == -1)
             throw SysError("dupping stderr");
-        execl(substituter.c_str(), substituter.c_str(), "--query", NULL);
-        throw SysError(format("executing `%1%'") % substituter);
+        execl(settings.guixProgram.c_str(), "guix", "substitute", "--query", NULL);
+        throw SysError(format("executing `%1%'") % settings.guixProgram);
     });
 
-    run.program = baseNameOf(substituter);
     run.to = toPipe.writeSide.borrow();
     run.from = run.fromBuf.fd = fromPipe.readSide.borrow();
     run.error = errorPipe.readSide.borrow();
@@ -889,13 +891,14 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
                 if (errno == EINTR) continue;
                 throw SysError("reading from substituter's stderr");
             }
-            if (n == 0) throw EndOfFile(format("substituter `%1%' died unexpectedly") % run.program);
+            if (n == 0) throw EndOfFile(format("`%1% substitute' died unexpectedly")
+					% settings.guixProgram);
             err.append(buf, n);
             string::size_type p;
             while (((p = err.find('\n')) != string::npos)
 		   || ((p = err.find('\r')) != string::npos)) {
 	        string thing(err, 0, p + 1);
-	        writeToStderr(run.program + ": " + thing);
+	        writeToStderr("substitute: " + thing);
                 err = string(err, p + 1);
             }
         }
@@ -907,7 +910,7 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
                 unsigned char c;
                 run.fromBuf(&c, 1);
                 if (c == '\n') {
-                    if (!err.empty()) printMsg(lvlError, run.program + ": " + err);
+                    if (!err.empty()) printMsg(lvlError, "substitute: " + err);
                     return res;
                 }
                 res += c;
@@ -930,38 +933,47 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
 {
     PathSet res;
 
-    if (!settings.useSubstitutes) return res;
-
-    foreach (Paths::iterator, i, settings.substituters) {
-        if (res.size() == paths.size()) break;
-        RunningSubstituter & run(runningSubstituters[*i]);
-        startSubstituter(*i, run);
-        if (run.disabled) continue;
-        string s = "have ";
-        foreach (PathSet::const_iterator, j, paths)
-            if (res.find(*j) == res.end()) { s += *j; s += " "; }
-        writeLine(run.to, s);
-        while (true) {
-            /* FIXME: we only read stderr when an error occurs, so
-               substituters should only write (short) messages to
-               stderr when they fail.  I.e. they shouldn't write debug
-               output. */
-            Path path = getLineFromSubstituter(run);
-            if (path == "") break;
-            res.insert(path);
-        }
+    if (!settings.useSubstitutes || paths.empty()) return res;
+
+    if (!runningSubstituter) {
+	std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+	runningSubstituter.swap(fresh);
+    }
+
+    RunningSubstituter & run = *runningSubstituter;
+    startSubstituter(run);
+
+    if (!run.disabled) {
+	string s = "have ";
+	foreach (PathSet::const_iterator, j, paths)
+	    if (res.find(*j) == res.end()) { s += *j; s += " "; }
+	writeLine(run.to, s);
+	while (true) {
+	    /* FIXME: we only read stderr when an error occurs, so
+	       substituters should only write (short) messages to
+	       stderr when they fail.  I.e. they shouldn't write debug
+	       output. */
+	    Path path = getLineFromSubstituter(run);
+	    if (path == "") break;
+	    res.insert(path);
+	}
     }
+
     return res;
 }
 
 
-void LocalStore::querySubstitutablePathInfos(const Path & substituter,
-    PathSet & paths, SubstitutablePathInfos & infos)
+void LocalStore::querySubstitutablePathInfos(PathSet & paths, SubstitutablePathInfos & infos)
 {
     if (!settings.useSubstitutes) return;
 
-    RunningSubstituter & run(runningSubstituters[substituter]);
-    startSubstituter(substituter, run);
+    if (!runningSubstituter) {
+	std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+	runningSubstituter.swap(fresh);
+    }
+
+    RunningSubstituter & run = *runningSubstituter;
+    startSubstituter(run);
     if (run.disabled) return;
 
     string s = "info ";
@@ -993,10 +1005,9 @@ void LocalStore::querySubstitutablePathInfos(const Path & substituter,
 void LocalStore::querySubstitutablePathInfos(const PathSet & paths,
     SubstitutablePathInfos & infos)
 {
-    PathSet todo = paths;
-    foreach (Paths::iterator, i, settings.substituters) {
-        if (todo.empty()) break;
-        querySubstitutablePathInfos(*i, todo, infos);
+    if (!paths.empty()) {
+	PathSet todo = paths;
+	querySubstitutablePathInfos(todo, infos);
     }
 }
 
diff --git a/nix/libstore/local-store.hh b/nix/libstore/local-store.hh
index 4e6b4cfc1d..4113fafcb5 100644
--- a/nix/libstore/local-store.hh
+++ b/nix/libstore/local-store.hh
@@ -40,7 +40,6 @@ struct OptimiseStats
 
 struct RunningSubstituter
 {
-    Path program;
     Pid pid;
     AutoCloseFD to, from, error;
     FdSource fromBuf;
@@ -52,8 +51,8 @@ struct RunningSubstituter
 class LocalStore : public StoreAPI
 {
 private:
-    typedef std::map<Path, RunningSubstituter> RunningSubstituters;
-    RunningSubstituters runningSubstituters;
+    /* The currently running substituter or empty.  */
+    std::unique_ptr<RunningSubstituter> runningSubstituter;
 
     Path linksDir;
 
@@ -93,8 +92,8 @@ public:
 
     PathSet querySubstitutablePaths(const PathSet & paths);
 
-    void querySubstitutablePathInfos(const Path & substituter,
-        PathSet & paths, SubstitutablePathInfos & infos);
+    void querySubstitutablePathInfos(PathSet & paths,
+        SubstitutablePathInfos & infos);
 
     void querySubstitutablePathInfos(const PathSet & paths,
         SubstitutablePathInfos & infos);
@@ -261,8 +260,7 @@ private:
 
     void removeUnusedLinks(const GCState & state);
 
-    void startSubstituter(const Path & substituter,
-        RunningSubstituter & runningSubstituter);
+    void startSubstituter(RunningSubstituter & runningSubstituter);
 
     string getLineFromSubstituter(RunningSubstituter & run);
 
diff --git a/nix/local.mk b/nix/local.mk
index 8e52c77bd9..18e9ba7604 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -154,9 +154,6 @@ noinst_HEADERS =						\
 	         (lambda (in)					\
 	           (write (get-string-all in) out)))))"
 
-nodist_pkglibexec_SCRIPTS =			\
-  %D%/scripts/substitute
-
 # The '.service' files for systemd.
 systemdservicedir = $(libdir)/systemd/system
 nodist_systemdservice_DATA = etc/guix-daemon.service etc/guix-publish.service
diff --git a/nix/nix-daemon/guix-daemon.cc b/nix/nix-daemon/guix-daemon.cc
index 73962af584..6f9c404c8d 100644
--- a/nix/nix-daemon/guix-daemon.cc
+++ b/nix/nix-daemon/guix-daemon.cc
@@ -466,8 +466,7 @@ main (int argc, char *argv[])
     {
       settings.processEnvironment ();
 
-      /* Use our substituter by default.  */
-      settings.substituters.clear ();
+      /* Enable substitutes by default.  */
       settings.set ("build-use-substitutes", "true");
 
       /* Use our substitute server by default.  */
@@ -490,14 +489,6 @@ main (int argc, char *argv[])
       printMsg(lvlDebug,
 	       format ("build log compression: %1%") % settings.logCompression);
 
-      if (settings.useSubstitutes)
-	settings.substituters.push_back (settings.nixLibexecDir
-					 + "/substitute");
-      else
-	/* Clear the substituter list to make sure nothing ever gets
-	   substituted, regardless of the client's settings.  */
-	settings.substituters.clear ();
-
       if (geteuid () == 0 && settings.buildUsersGroup.empty ())
 	fprintf (stderr, _("warning: daemon is running as root, so \
 using `--build-users-group' is highly recommended\n"));
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index f29bcd2eab..ffac6cde34 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -596,8 +596,12 @@ static void performOp(bool trusted, unsigned int clientVersion,
         if (GET_PROTOCOL_MINOR(clientVersion) >= 6
             && GET_PROTOCOL_MINOR(clientVersion) < 0x61)
             settings.set("build-cores", std::to_string(readInt(from)));
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 10)
-            settings.set("build-use-substitutes", readInt(from) ? "true" : "false");
+        if (GET_PROTOCOL_MINOR(clientVersion) >= 10) {
+	    if (settings.useSubstitutes)
+		settings.set("build-use-substitutes", readInt(from) ? "true" : "false");
+	    else
+		readInt(from);			// substitutes remain disabled
+	}
         if (GET_PROTOCOL_MINOR(clientVersion) >= 12) {
             unsigned int n = readInt(from);
             for (unsigned int i = 0; i < n; i++) {
diff --git a/nix/scripts/substitute.in b/nix/scripts/substitute.in
deleted file mode 100644
index 5a2eeb7259..0000000000
--- a/nix/scripts/substitute.in
+++ /dev/null
@@ -1,11 +0,0 @@
-#!@SHELL@
-# A shorthand for "guix substitute", for use by the daemon.
-
-if test "x$GUIX_UNINSTALLED" = "x"
-then
-    prefix="@prefix@"
-    exec_prefix="@exec_prefix@"
-    exec "@bindir@/guix" substitute "$@"
-else
-    exec guix substitute "$@"
-fi
-- 
2.23.0





Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Sun, 08 Sep 2019 10:04:02 GMT) Full text and rfc822 format available.

Notification sent to Ludovic Courtès <ludo <at> gnu.org>:
bug acknowledged by developer. (Sun, 08 Sep 2019 10:04:03 GMT) Full text and rfc822 format available.

Message #31 received at 37302-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 37302-done <at> debbugs.gnu.org
Subject: Re: [bug#37302] [PATCH 0/7] Remove the daemon's libexec helpers
Date: Sun, 08 Sep 2019 12:03:46 +0200
Ludovic Courtès <ludo <at> gnu.org> skribis:

>   daemon: Invoke 'guix gc --list-busy' instead of 'list-runtime-roots'.
>   daemon: Run 'guix authenticate' directly.
>   daemon: Run 'guix perform-download' directly.
>   daemon: Run 'guix offload' directly.
>   daemon: Run 'guix substitute' directly and assume a single
>     substituter.
>   daemon: Remove 'NIX_LIBEXEC_DIR'.
>   etc: Remove references to libexec/guix* from SELinux policy.

Pushed at cc98b00857e29074de96a6ed60e325cdfffaea1a.

Now to adjust ‘guix-daemon’ accordingly…

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 06 Oct 2019 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 203 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.