deja-dup-team team mailing list archive

[Michael Terry <michael.terry@xxxxxxxxxxxxx>]

Michael Terry has proposed merging lp:~mterry/deja-dup/tempdir-archive-dir into lp:deja-dup with lp:~mterry/deja-dup/more-idle as a prerequisite.

Requested reviews:
  Robert Bruce Park (robru)
Related bugs:
  Bug #1159749 in Déjà Dup: "When testing a restore, don't blindly use /tmp"
  https://bugs.launchpad.net/deja-dup/+bug/1159749

For more details, see:
https://code.launchpad.net/~mterry/deja-dup/tempdir-archive-dir/+merge/155323

A little while back, for 25.5, I added code to tell duplicity to use a tempdir that is on the same partition as the source files, so that a user with /tmp as a tmpfs partition wouldn't fill it up.

However, there is one code path where we end up using /tmp anyway, that I didn't notice.  If we are testing a restore in "nag mode", which is where we don't use any cached information (we re-download all the metadata and ask for the password again, hence the "nag"), we were making a call to g_dir_make_tmp.  This uses /tmp, and we were filling /tmp up with our downloaded metadata.

So the solution is to use g_mkdtemp, which lets us specify a tempdir, and we'll re-use the one we create for passing to duplicity.

There's a bit of cleanup in the tests/ directory to match this new usage and to better test it.
-- 
https://code.launchpad.net/~mterry/deja-dup/tempdir-archive-dir/+merge/155323
Your team Déjà Dup Developers is subscribed to branch lp:deja-dup.

=== modified file 'tests/mock/duplicity'
--- tests/mock/duplicity	2012-10-30 00:19:38 +0000
+++ tests/mock/duplicity	2013-03-25 19:21:23 +0000
@@ -108,7 +108,8 @@
       if split[1].find("/cache/") != -1:
         print("TESTFAIL: expected random /tmp archive dir", file=logfd)
         sys.exit(-1)
-      sys.argv[i] = "--archive-dir=?"
+      # Chop off random string at end, for reproducable tests
+      sys.argv[i] = sys.argv[i][:-6] + "?"
 
 if expected_args != sys.argv[1:]:
   print("TESTFAIL: expected\n%s\nvs\n%s" % (expected_args, sys.argv[1:]), file=logfd)

=== modified file 'tests/runner.vala'
--- tests/runner.vala	2013-02-04 20:39:45 +0000
+++ tests/runner.vala	2013-03-25 19:21:23 +0000
@@ -140,13 +140,12 @@
   var backupdir = Path.build_filename(test_home, "backup");
   var restoredir = Path.build_filename(test_home, "restore");
 
-  var archive = tmp_archive ? "?" : "%s/deja-dup".printf(cachedir);
-
   string enc_str = "";
   if (!encrypted)
     enc_str = "--no-encryption ";
 
   var tempdir = Path.build_filename(test_home, "tmp");
+  var archive = tmp_archive ? "%s/deja-dup-?".printf(tempdir) : "%s/deja-dup".printf(cachedir);
 
   var end_str = "%s'--verbosity=9' '--gpg-options=--no-use-agent' '--archive-dir=%s' '--tempdir=%s' '%s'".printf(enc_str, archive, tempdir, make_fd_arg(as_root));
 

=== modified file 'tools/duplicity/DuplicityInstance.vala'
--- tools/duplicity/DuplicityInstance.vala	2013-01-24 14:31:08 +0000
+++ tools/duplicity/DuplicityInstance.vala	2013-03-25 19:21:23 +0000
@@ -93,12 +93,10 @@
     // Cache signature files
     var cache_dir = forced_cache_dir;
     if (cache_dir == null)
-      cache_dir = Environment.get_user_cache_dir();
-    if (cache_dir != null) {
-      cache_dir = Path.build_filename(cache_dir, Config.PACKAGE);
-      if (DejaDup.ensure_directory_exists(cache_dir))
-        argv.append("--archive-dir=" + cache_dir);
-    }
+      cache_dir = Path.build_filename(Environment.get_user_cache_dir(),
+                                      Config.PACKAGE);
+    if (cache_dir != null && DejaDup.ensure_directory_exists(cache_dir))
+      argv.append("--archive-dir=" + cache_dir);
 
     // Specify tempdir
     var tempdir = yield DejaDup.get_tempdir();

=== modified file 'tools/duplicity/DuplicityJob.vala'
--- tools/duplicity/DuplicityJob.vala	2013-01-20 19:54:44 +0000
+++ tools/duplicity/DuplicityJob.vala	2013-03-25 19:21:23 +0000
@@ -110,9 +110,6 @@
 
   ~DuplicityJob() {
     DejaDup.Network.get().notify["connected"].disconnect(network_changed);
-
-    if (forced_cache_dir != null)
-      new DejaDup.RecursiveDelete(File.new_for_path(forced_cache_dir)).start_async.begin();
   }
 
   public override void start()
@@ -130,24 +127,21 @@
     if (mode == DejaDup.ToolJob.Mode.BACKUP)
       process_include_excludes();
 
+    var settings = DejaDup.get_settings();
+    delete_age = settings.get_int(DejaDup.DELETE_AFTER_KEY);
+
+    async_setup.begin();
+  }
+
+  async void async_setup()
+  {
     /* Fake cache dir if we need to */
     if ((flags & DejaDup.ToolJob.Flags.NO_CACHE) != 0) {
-      try {
-        forced_cache_dir = DirUtils.make_tmp("deja-dup-XXXXXX");
-      }
-      catch (Error e) {
-        warning("%s\n", e.message);
-      }
+      var template = Path.build_filename(yield DejaDup.get_tempdir(), "deja-dup-XXXXXX");
+      forced_cache_dir = DirUtils.mkdtemp(template);
     }
 
-    var settings = DejaDup.get_settings();
-    delete_age = settings.get_int(DejaDup.DELETE_AFTER_KEY);
-
-    get_envp.begin();
-  }
-
-  async void get_envp()
-  {
+    /* Get custom environment from backend, if needed */
     try {
       backend.envp_ready.connect(continue_with_envp);
       yield backend.get_envp();