From c6df09941b9fb225788e9efa276b7aa92e19d3c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Mon, 25 Aug 2014 00:16:49 +0200
Subject: [PATCH] guix-register: By default, attempt to deduplicate registered
 items.

* nix/guix-register/guix-register.cc (GUIX_OPT_DEDUPLICATE): New macro.
  (options): Add --no-deduplication.
  (deduplication): New variable.
  (parse_opt): Handle GUIX_OPT_DEDUPLICATE.
  (register_validity): Add 'optimize' parameter and honor it.
  (main): Move 'store' instanatiation after 'settings.nixStore'
  assignment.
* tests/guix-register.sh: Add test for deduplication.
* guix/nar.scm (finalize-store-file): Update comment above
  'register-path' call.
---
 guix/nar.scm                       |  3 +--
 nix/guix-register/guix-register.cc | 34 +++++++++++++++++++++++++-----
 tests/guix-register.sh             | 19 +++++++++++++++--
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/guix/nar.scm b/guix/nar.scm
index 0a7187c2dd..b95cbd648d 100644
--- a/guix/nar.scm
+++ b/guix/nar.scm
@@ -324,8 +324,7 @@ held."
         (rename-file source target)
 
         ;; Register TARGET.  As a side effect, it resets the timestamps of all
-        ;; its files, recursively.  However, it doesn't attempt to deduplicate
-        ;; its files like 'importPaths' does (FIXME).
+        ;; its files, recursively, and runs a deduplication pass.
         (register-path target
                        #:references references
                        #:deriver deriver))
diff --git a/nix/guix-register/guix-register.cc b/nix/guix-register/guix-register.cc
index ed5ab23e41..92eedab9f5 100644
--- a/nix/guix-register/guix-register.cc
+++ b/nix/guix-register/guix-register.cc
@@ -57,6 +57,7 @@ information about which store files are valid, and what their \
 references are.";
 
 #define GUIX_OPT_STATE_DIRECTORY 1
+#define GUIX_OPT_DEDUPLICATE 2
 
 static const struct argp_option options[] =
   {
@@ -64,6 +65,8 @@ static const struct argp_option options[] =
       "Open the store that lies under DIRECTORY" },
     { "state-directory", GUIX_OPT_STATE_DIRECTORY, "DIRECTORY", 0,
       "Use DIRECTORY as the state directory of the target store" },
+    { "no-deduplication", GUIX_OPT_DEDUPLICATE, 0, 0,
+      "Disable automatic deduplication of registered store items" },
     { 0, 0, 0, 0, 0 }
   };
 
@@ -71,6 +74,9 @@ static const struct argp_option options[] =
 /* Prefix of the store being populated.  */
 static std::string prefix;
 
+/* Whether to deduplicate the registered store items.  */
+static bool deduplication = true;
+
 /* Parse a single option. */
 static error_t
 parse_opt (int key, char *arg, struct argp_state *state)
@@ -97,6 +103,10 @@ parse_opt (int key, char *arg, struct argp_state *state)
 	break;
       }
 
+    case GUIX_OPT_DEDUPLICATE:
+      deduplication = false;
+      break;
+
     case ARGP_KEY_ARG:
       {
 	std::ifstream *file;
@@ -136,6 +146,7 @@ static struct argp argp = { options, parse_opt, 0, doc };
    This is really meant as an internal format.  */
 static void
 register_validity (LocalStore *store, std::istream &input,
+		   bool optimize = true,
 		   bool reregister = true, bool hashGiven = false,
 		   bool canonicalise = true)
 {
@@ -176,6 +187,19 @@ register_validity (LocalStore *store, std::istream &input,
     }
 
   store->registerValidPaths (infos);
+
+  /* XXX: When PREFIX is non-empty, store->linksDir points to the original
+     store's '.links' directory, which means 'optimisePath' would try to link
+     to that instead of linking to the target store.  Thus, disable
+     deduplication in this case.  */
+  if (optimize && prefix.empty ())
+    {
+      /* Make sure deduplication is enabled.  */
+      settings.autoOptimiseStore = true;
+
+      foreach (ValidPathInfos::const_iterator, i, infos)
+	store->optimisePath (i->path);
+    }
 }
 
 
@@ -200,17 +224,17 @@ main (int argc, char *argv[])
     {
       argp_parse (&argp, argc, argv, 0, 0, 0);
 
-      /* Instantiate the store.  This creates any missing directories among
-	 'settings.nixStore', 'settings.nixDBPath', etc.  */
-      LocalStore store;
-
       if (!prefix.empty ())
 	/* Under the --prefix tree, the final name of the store will be
 	   NIX_STORE_DIR.  Set it here so that the database uses file names
 	   prefixed by NIX_STORE_DIR and not PREFIX + NIX_STORE_DIR.  */
 	settings.nixStore = NIX_STORE_DIR;
 
-      register_validity (&store, *input);
+      /* Instantiate the store.  This creates any missing directories among
+	 'settings.nixStore', 'settings.nixDBPath', etc.  */
+      LocalStore store;
+
+      register_validity (&store, *input, deduplication);
     }
   catch (std::exception &e)
     {
diff --git a/tests/guix-register.sh b/tests/guix-register.sh
index 3f261d7bef..e99f5c6075 100644
--- a/tests/guix-register.sh
+++ b/tests/guix-register.sh
@@ -43,13 +43,28 @@ $new_file
 0
 EOF
 
-# Make sure it's valid, and delete it.
+# Register an idendical file, and make sure it gets deduplicated.
+new_file2="$new_file-duplicate"
+cat "$new_file" > "$new_file2"
+guix-register <<EOF
+$new_file2
+
+0
+EOF
+
+guile -c "
+  (exit (= (stat:ino (stat \"$new_file\"))
+           (stat:ino (stat \"$new_file2\"))))"
+
+# Make sure both are valid, and delete them.
 guile -c "
    (use-modules (guix store))
    (define s (open-connection))
    (exit (and (valid-path? s \"$new_file\")
+              (valid-path? s \"$new_file2\")
               (null? (references s \"$new_file\"))
-              (pair? (delete-paths s (list \"$new_file\")))))"
+              (null? (references s \"$new_file2\"))
+              (pair? (delete-paths s (list \"$new_file\" \"$new_file2\")))))"
 
 
 #