Follow up addition of --pack-dir

This follows up the recently merged rr-debugger#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.
JuliaLang · Sep 23, 2024 · f5dd84c · f5dd84c
1 parent 4dfbb1c
commit f5dd84c
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 38 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1719,6 +1719,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

diff --git a/src/PackCommand.cc b/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
diff --git a/src/test/pack_dir.run b/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
diff --git a/src/test/util.sh b/src/test/util.sh
@@ -327,9 +327,11 @@ 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" echo test-monitor $TIMEOUT pack.err \
+        $RR_EXE $GLOBAL_OPTIONS 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