diff mbox series

[bug#45253,6/6] daemon: Delegate deduplication to 'guix substitute'.

Message ID 20201215095730.10954-6-ludo@gnu.org
State Accepted
Headers show
Series Pipeline substitute integrity check, deduplication, and canonicalization | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Ludovic Courtès Dec. 15, 2020, 9:57 a.m. UTC
This removes the main source of latency between subsequent downloads.

* nix/libstore/build.cc (SubstitutionGoal::tryToRun): Add a
"deduplicate" key to ENV.
(SubstitutionGoal::finished): Remove call to 'optimisePath'.
* guix/scripts/substitute.scm (process-substitution)[destination-in-store?]
[dump-file/deduplicate*]: New variables.
Pass #:dump-file to 'restore-file'.
* guix/scripts/substitute.scm (guix-substitute)[deduplicate?]: New
variable.
Pass #:deduplicate? to 'process-substitution'.
* guix/serialization.scm (dump-file): Export and augment 'dump-file'.
---
 guix/scripts/substitute.scm | 31 ++++++++++++++++++++++++++-----
 guix/serialization.scm      |  8 ++++++--
 nix/libstore/build.cc       | 13 ++++++++-----
 3 files changed, 40 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index 17d0002b9f..38702d0c4b 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -28,7 +28,8 @@ 
   #:use-module (guix records)
   #:use-module (guix diagnostics)
   #:use-module (guix i18n)
-  #:use-module ((guix serialization) #:select (restore-file))
+  #:use-module ((guix serialization) #:select (restore-file dump-file))
+  #:autoload   (guix store deduplication) (dump-file/deduplicate)
   #:autoload   (guix scripts discover) (read-substitute-urls)
   #:use-module (gcrypt hash)
   #:use-module (guix base32)
@@ -1045,15 +1046,27 @@  one.  Return #f if URI's scheme is 'file' or #f."
   (call-with-cached-connection uri (lambda (port) exp ...)))
 
 (define* (process-substitution store-item destination
-                               #:key cache-urls acl print-build-trace?)
+                               #:key cache-urls acl
+                               deduplicate? print-build-trace?)
   "Substitute STORE-ITEM (a store file name) from CACHE-URLS, and write it to
 DESTINATION as a nar file.  Verify the substitute against ACL, and verify its
-hash against what appears in the narinfo.  Print a status line on the current
-output port."
+hash against what appears in the narinfo.  When DEDUPLICATE? is true, and if
+DESTINATION is in the store, deduplicate its files.  Print a status line on
+the current output port."
   (define narinfo
     (lookup-narinfo cache-urls store-item
                     (cut valid-narinfo? <> acl)))
 
+  (define destination-in-store?
+    (string-prefix? (string-append (%store-prefix) "/")
+                    destination))
+
+  (define (dump-file/deduplicate* . args)
+    ;; Make sure deduplication looks at the right store (necessary in test
+    ;; environments).
+    (apply dump-file/deduplicate
+           (append args (list #:store (%store-prefix)))))
+
   (unless narinfo
     (leave (G_ "no valid substitute for '~a'~%")
            store-item))
@@ -1100,7 +1113,11 @@  output port."
                   ((hashed get-hash)
                    (open-hash-input-port algorithm input)))
       ;; Unpack the Nar at INPUT into DESTINATION.
-      (restore-file hashed destination)
+      (restore-file hashed destination
+                    #:dump-file (if (and destination-in-store?
+                                         deduplicate?)
+                                    dump-file/deduplicate*
+                                    dump-file))
       (close-port hashed)
       (close-port input)
 
@@ -1248,6 +1265,9 @@  default value."
       ((= string->number number) (> number 0))
       (_ #f)))
 
+  (define deduplicate?
+    (find-daemon-option "deduplicate"))
+
   ;; The daemon's agent code opens file descriptor 4 for us and this is where
   ;; stderr should go.
   (parameterize ((current-error-port (if (%error-to-file-descriptor-4?)
@@ -1307,6 +1327,7 @@  default value."
                  (process-substitution store-path destination
                                        #:cache-urls (substitute-urls)
                                        #:acl (current-acl)
+                                       #:deduplicate? deduplicate?
                                        #:print-build-trace?
                                        print-build-trace?)
                  (loop))))))
diff --git a/guix/serialization.scm b/guix/serialization.scm
index 9e2dce8bb0..59cd93fb18 100644
--- a/guix/serialization.scm
+++ b/guix/serialization.scm
@@ -51,7 +51,8 @@ 
             write-file
             write-file-tree
             fold-archive
-            restore-file))
+            restore-file
+            dump-file))
 
 ;;; Comment:
 ;;;
@@ -458,7 +459,10 @@  depends on TYPE."
            (&nar-read-error (port port) (file file) (token x)))))))))
 
 (define (dump-file file input size type)
-  "Dump SIZE bytes from INPUT to FILE."
+  "Dump SIZE bytes from INPUT to FILE.
+
+This procedure is suitable for use as the #:dump-file argument to
+'restore-file'."
   (call-with-output-file file
     (lambda (output)
       (dump input output size))))
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index ea809c6971..20d83fea4a 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2984,7 +2984,12 @@  void SubstitutionGoal::tryToRun()
 
     if (!worker.substituter) {
 	const Strings args = { "substitute", "--substitute" };
-	const std::map<string, string> env = { { "_NIX_OPTIONS", settings.pack() } };
+	const std::map<string, string> env = {
+	    { "_NIX_OPTIONS",
+	      settings.pack() + "deduplicate="
+	      + (settings.autoOptimiseStore ? "yes" : "no")
+	    }
+	};
 	worker.substituter = std::make_shared<Agent>(settings.guixProgram, args, env);
     }
 
@@ -3085,10 +3090,8 @@  void SubstitutionGoal::finished()
 
     if (repair) replaceValidPath(storePath, destPath);
 
-    /* Note: 'guix substitute' takes care of resetting timestamps and
-       permissions on 'destPath', so no need to do it here.  */
-
-    worker.store.optimisePath(storePath); // FIXME: combine with hashPath()
+    /* Note: 'guix substitute' takes care of resetting timestamps and of
+       deduplicating 'destPath', so no need to do it here.  */
 
     ValidPathInfo info2;
     info2.path = storePath;