From f5dd84c92672e7788a9e3fb58439507445683c93 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 23 Sep 2024 04:19:28 +0000 Subject: [PATCH] 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. --- CMakeLists.txt | 1 + src/PackCommand.cc | 77 +++++++++++++++++++++++-------------------- src/test/pack_dir.run | 31 +++++++++++++++++ src/test/util.sh | 6 ++-- 4 files changed, 77 insertions(+), 38 deletions(-) create mode 100644 src/test/pack_dir.run diff --git a/CMakeLists.txt b/CMakeLists.txt index 6c067caa90e..14372b855da 100644 --- 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 index debc027bef7..4059cf1b6cf 100644 --- a/src/PackCommand.cc +++ b/src/PackCommand.cc @@ -52,9 +52,13 @@ class PackCommand : public Command { PackCommand PackCommand::singleton( "pack", - " rr pack [OPTION]... []\n" + " rr pack [OPTION]... [...]\n" " --symlink Create symlinks to all mmapped files\n" " instead of copying them.\n" + " --pack-dir= 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& 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& 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 canonical_symlink_map = - compute_canonical_symlink_map(abspath); - rewrite_mmaps(canonical_symlink_map, abspath); - delete_unnecessary_files(canonical_symlink_map, abspath); - } else { - map 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 canonical_symlink_map = + compute_canonical_symlink_map(abspath); + rewrite_mmaps(canonical_symlink_map, abspath); + delete_unnecessary_files(canonical_symlink_map, abspath); + } else { + map 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& args, PackFlags& flags) { } int PackCommand::run(vector& args) { - bool found_dir = false; - string trace_dir; PackFlags flags; while (parse_pack_arg(args, flags)) { } + vector 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 new file mode 100644 index 00000000000..ceafbcbdd1a --- /dev/null +++ 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 index 56d29bf7349..8da3d7e6a09 100644 --- 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