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.

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/155322

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/155322
Your team Déjà Dup Developers is subscribed to branch lp:deja-dup.

=== modified file 'common/CommonUtils.vala'
--- common/CommonUtils.vala	2013-03-14 20:40:57 +0000
+++ common/CommonUtils.vala	2013-03-25 19:20:33 +0000
@@ -72,20 +72,61 @@
   settings.apply();
 }
 
+public bool parse_version(string version_string, out int major, out int minor,
+                          out int micro)
+{
+  major = 0;
+  minor = 0;
+  micro = 0;
+
+  var ver_tokens = version_string.split(".");
+  if (ver_tokens == null || ver_tokens[0] == null)
+    return false;
+
+  major = int.parse(ver_tokens[0]);
+  // Don't error out if no minor or micro.
+  if (ver_tokens[1] != null) {
+    minor = int.parse(ver_tokens[1]);
+    if (ver_tokens[2] != null)
+      micro = int.parse(ver_tokens[2]);
+  }
+
+  return true;
+}
+
+public bool meets_version(int major, int minor, int micro,
+                          int req_major, int req_minor, int req_micro)
+{
+  return (major > req_major) ||
+         (major == req_major && minor > req_minor) ||
+         (major == req_major && minor == req_minor && micro >= req_micro);
+}
+
 public void run_deja_dup(string args, AppLaunchContext? ctx = null,
                          List<File>? files = null)
 {
   var cmd = "deja-dup %s".printf(args);
 
+  int major, minor, micro;
+  var utsname = Posix.utsname();
+  parse_version(utsname.release, out major, out minor, out micro);
+
   // Check for ionice to be a good disk citizen
   if (Environment.find_program_in_path("ionice") != null) {
-    // lowest priority in best-effort class
-    // (can't use idle class as normal user on <2.6.25)
-    cmd = "ionice -c2 -n7 " + cmd;
+    // In Linux 2.6.25 and up, even normal users can request idle class
+    if (utsname.sysname == "Linux" && meets_version(major, minor, micro, 2, 6, 25))
+      cmd = "ionice -c3 " + cmd; // idle class
+    else
+      cmd = "ionice -c2 -n7 " + cmd; // lowest priority in best-effort class
   }
 
-  if (Environment.find_program_in_path("nice") != null)
-    cmd = "nice " + cmd;
+  // chrt's idle class is more-idle than nice, so prefer it
+  if (utsname.sysname == "Linux" &&
+      meets_version(major, minor, micro, 2, 6, 23) &&
+      Environment.find_program_in_path("chrt") != null)
+    cmd = "chrt --idle 0 " + cmd;
+  else if (Environment.find_program_in_path("nice") != null)
+    cmd = "nice -n19 " + cmd;
 
   var flags = AppInfoCreateFlags.SUPPORTS_STARTUP_NOTIFICATION |
               AppInfoCreateFlags.SUPPORTS_URIS;

=== modified file 'tests/mock/duplicity'
--- tests/mock/duplicity	2012-10-30 00:19:38 +0000
+++ tests/mock/duplicity	2013-03-25 19:20:33 +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:20:33 +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 'tests/unit/unit-tests.vala'
--- tests/unit/unit-tests.vala	2012-12-04 16:08:06 +0000
+++ tests/unit/unit-tests.vala	2013-03-25 19:20:33 +0000
@@ -46,6 +46,38 @@
   assert(DejaDup.parse_dir("file:VIDEOS").equal(File.parse_name("file:VIDEOS")));
 }
 
+void parse_one_version(string str, int maj, int min, int mic)
+{
+  int pmaj, pmin, pmic;
+  assert(DejaDup.parse_version(str, out pmaj, out pmin, out pmic));
+  assert(pmaj == maj);
+  assert(pmin == min);
+  assert(pmic == mic);
+}
+
+void parse_bad_version(string str)
+{
+  int pmaj, pmin, pmic;
+  assert(!DejaDup.parse_version(str, out pmaj, out pmin, out pmic));
+  assert(pmaj == 0);
+  assert(pmin == 0);
+  assert(pmic == 0);
+}
+
+void parse_version()
+{
+  parse_bad_version("");
+  parse_one_version("a", 0, 0, 0);
+  parse_one_version("1", 1, 0, 0);
+  parse_one_version("1.2", 1, 2, 0);
+  parse_one_version("1.2.3", 1, 2, 3);
+  parse_one_version("1.2.3.4", 1, 2, 3);
+  parse_one_version("1.2.3a4", 1, 2, 3);
+  parse_one_version("1.2a3.4", 1, 2, 4);
+  parse_one_version("1.2 3.4", 1, 2, 4);
+  parse_one_version("1.2-3.4", 1, 2, 4);
+}
+
 void setup()
 {
 }
@@ -64,6 +96,7 @@
 
   var unit = new TestSuite("unit");
   unit.add(new TestCase("parse-dir", setup, parse_dir, teardown));
+  unit.add(new TestCase("parse-version", setup, parse_version, teardown));
   TestSuite.get_root().add_suite(unit);
 
   return Test.run();

=== 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:20:33 +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:20:33 +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();

=== modified file 'tools/duplicity/DuplicityPlugin.vala'
--- tools/duplicity/DuplicityPlugin.vala	2013-01-23 22:04:16 +0000
+++ tools/duplicity/DuplicityPlugin.vala	2013-03-25 19:20:33 +0000
@@ -42,25 +42,11 @@
 
     // First token is 'duplicity' and is ignorable.  Second looks like '0.5.03'
     var version_string = tokens[1].strip();
-    var ver_tokens = version_string.split(".");
-    if (ver_tokens == null || ver_tokens[0] == null)
+    int major, minor, micro;
+    if (!DejaDup.parse_version(version_string, out major, out minor, out micro))
       throw new SpawnError.FAILED(_("Could not understand duplicity version ‘%s’.").printf(version_string));
 
-    int major = 0;
-    int minor = 0;
-    int micro = 0;
-    major = int.parse(ver_tokens[0]);
-    // Don't error out if no minor or micro.  Duplicity might not have them?
-    if (ver_tokens[1] != null) {
-      minor = int.parse(ver_tokens[1]);
-      if (ver_tokens[2] != null)
-        micro = int.parse(ver_tokens[2]);
-    }
-
-    var meets = (major > REQUIRED_MAJOR) ||
-                (major == REQUIRED_MAJOR && minor > REQUIRED_MINOR) ||
-                (major == REQUIRED_MAJOR && minor == REQUIRED_MINOR && micro >= REQUIRED_MICRO);
-    if (!meets)
+    if (!DejaDup.meets_version(major, minor, micro, REQUIRED_MAJOR, REQUIRED_MINOR, REQUIRED_MICRO))
       throw new SpawnError.FAILED(_("Déjà Dup Backup Tool requires at least version %d.%d.%.2d of duplicity, but only found version %d.%d.%.2d").printf(REQUIRED_MAJOR, REQUIRED_MINOR, REQUIRED_MICRO, major, minor, micro));
   }