Follow up addition of --pack-dir

This follows up the recently merged #3735 by:

1. Adding --pack-dir to the help
2. Allowing `pack` to take multiple directories to pack to make
   the argument useful.
3. Adding a test for the option.

Author: Keno

CMakeLists.txt

@@ -1720,6 +1720,7 @@ set(TESTS_WITHOUT_PROGRAM
   nested_detach_kill
   nested_detach_stop
   nested_release
+  pack_dir
   parent_no_break_child_bkpt
   parent_no_stop_child_crash
   post_exec_fpu_regs

src/PackCommand.cc

@@ -52,9 +52,13 @@ class PackCommand : public Command {
 
 PackCommand PackCommand::singleton(
     "pack",
-    " rr pack [OPTION]... [<trace-dir>]\n"
+    " rr pack [OPTION]... [<trace-dirs>...]\n"
     "  --symlink                  Create symlinks to all mmapped files\n"
     "                             instead of copying them.\n"
+    "  --pack-dir=<path>          Specify a directory in which to pack common files.\n"
+    "                             This helps conserve space when packing multiple traces\n"
+    "                             with common files. Both the trace dir and pack dir\n"
+    "                             (at the same relative path) are required for replay\n"
     "\n"
     "Eliminates duplicate files in the trace directory, and copies files into\n"
     "the trace directory as necessary to ensure that all needed files are in\n"
@@ -671,36 +675,38 @@ static void delete_unnecessary_files(const map<string, string>& file_map,
   }
 }
 
-static int pack(const string& trace_dir, const PackFlags& flags) {
-  string dir;
-  {
-    // validate trace and produce default trace directory if trace_dir is empty
-    TraceReader reader(trace_dir);
-    dir = reader.dir();
-  }
+static int pack(const vector<string>& trace_dirs, const PackFlags& flags) {
+  for (const string &trace_dir : trace_dirs) {
+    string dir;
+    {
+      // validate trace and produce default trace directory if trace_dir is empty
+      TraceReader reader(trace_dir);
+      dir = reader.dir();
+    }
 
-  PackDir pack_dir(flags.pack_dir);
-  char buf[PATH_MAX];
-  char* ret = realpath(dir.c_str(), buf);
-  if (!ret) {
-    FATAL() << "realpath failed on " << dir;
-  }
-  string abspath(buf);
+    PackDir pack_dir(flags.pack_dir);
+    char buf[PATH_MAX];
+    char* ret = realpath(dir.c_str(), buf);
+    if (!ret) {
+      FATAL() << "realpath failed on " << dir;
+    }
+    string abspath(buf);
 
-  if (flags.symlink) {
-    map<string, string> canonical_symlink_map =
-        compute_canonical_symlink_map(abspath);
-    rewrite_mmaps(canonical_symlink_map, abspath);
-    delete_unnecessary_files(canonical_symlink_map, abspath);
-  } else {
-    map<string, string> canonical_mmapped_files =
-        compute_canonical_mmapped_files(abspath, pack_dir);
-    rewrite_mmaps(canonical_mmapped_files, abspath);
-    delete_unnecessary_files(canonical_mmapped_files, abspath);
-  }
+    if (flags.symlink) {
+      map<string, string> canonical_symlink_map =
+          compute_canonical_symlink_map(abspath);
+      rewrite_mmaps(canonical_symlink_map, abspath);
+      delete_unnecessary_files(canonical_symlink_map, abspath);
+    } else {
+      map<string, string> canonical_mmapped_files =
+          compute_canonical_mmapped_files(abspath, pack_dir);
+      rewrite_mmaps(canonical_mmapped_files, abspath);
+      delete_unnecessary_files(canonical_mmapped_files, abspath);
+    }
 
-  if (!probably_not_interactive(STDOUT_FILENO)) {
-    printf("rr: Packed trace directory `%s'.\n", dir.c_str());
+    if (!probably_not_interactive(STDOUT_FILENO)) {
+      printf("rr: Packed trace directory `%s'.\n", dir.c_str());
+    }
   }
 
   return 0;
@@ -733,23 +739,22 @@ static bool parse_pack_arg(vector<string>& args, PackFlags& flags) {
 }
 
 int PackCommand::run(vector<string>& args) {
-  bool found_dir = false;
-  string trace_dir;
   PackFlags flags;
 
   while (parse_pack_arg(args, flags)) {
   }
 
+  vector<string> trace_dirs;
   while (!args.empty()) {
-    if (!found_dir && parse_optional_trace_dir(args, &trace_dir)) {
-      found_dir = true;
-      continue;
+    string trace_dir;
+    if (!parse_optional_trace_dir(args, &trace_dir)) {
+      print_help(stderr);
+      return 1;
     }
-    print_help(stderr);
-    return 1;
+    trace_dirs.push_back(trace_dir);
   }
 
-  return pack(trace_dir, flags);
+  return pack(trace_dirs, flags);
 }
 
 } // namespace rr

src/test/pack_dir.run

@@ -0,0 +1,31 @@
+source `dirname $0`/util.sh
+
+record simple$bitness
+record simple$bitness
+
+mkdir the_pack_dir
+
+# Pack both directories with a common pack directory
+pack --pack-dir the_pack_dir simple$bitness-$nonce-0 simple$bitness-$nonce-1
+
+# Test that we can still replay both traces
+replay simple$bitness-$nonce-0
+check EXIT-SUCCESS
+rm replay.err replay.out
+
+replay simple$bitness-$nonce-1
+check EXIT-SUCCESS
+rm replay.err replay.out
+
+# Check that we can move all three directories somewhere else and replay
+# still works.
+mkdir a_subdirectory
+mv the_pack_dir simple$bitness-$nonce-0 simple$bitness-$nonce-1 a_subdirectory
+
+replay a_subdirectory/simple$bitness-$nonce-0
+check EXIT-SUCCESS
+rm replay.err replay.out
+
+replay a_subdirectory/simple$bitness-$nonce-1
+check EXIT-SUCCESS
+rm replay.err replay.out

src/test/util.sh

@@ -328,9 +328,9 @@ function rerun { rerunflags=$1
         $RR_EXE $GLOBAL_OPTIONS rerun $rerunflags 1> rerun.out 2> rerun.err
 }
 
-function pack { packflags=$1
+function pack {
     _RR_TRACE_DIR="$workdir" test-monitor $TIMEOUT pack.err \
-        $RR_EXE $GLOBAL_OPTIONS pack $packflags 1> pack.out 2> pack.err
+        $RR_EXE $GLOBAL_OPTIONS pack $@ 1> pack.out 2> pack.err
 }
 
 function do_ps { psflags=$1