Properly report missing files for abipkgdiff

Currently, if abipkgdiff is given a path to a nonexistent file,
it propagates all the way until package classification
and ends up being reported as "PKG should be a valid package file",
which doesn't hint it's not there at all.

        * tools/abipkgdiff.cc: (class options): Add the "nonexistent_file" flag
        (parse_command_line): Check if the files given exist.
        (main): Check the nonexistent_file flag. If any of the input
        files don't exist, report it and exit. Also, for present and future test
        uniformity, only show the base names of the packages when using their
        names in error output.
        * tests/test-diff-pkg.cc: Add a new regression test.
        * tests/data/test-diff-pkg/test-nonexistent-report-0.txt: The
        expected output of the above regression test.
        * tests/data/Makefile.am: Add the above file to the list.

Signed-off-by: Ondrej Oprala <ondrej.oprala@gmail.com>

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index eb0748e0f819d83613a50ee38c21a05fed1219a8..68842807cfe9105c3c5cd328fd18c37fc3442ab0 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1144,6 +1144,7 @@ test-diff-pkg/test-rpm-report-2.txt \
 test-diff-pkg/test-rpm-report-3.txt \
 test-diff-pkg/test-rpm-report-4.txt \
 test-diff-pkg/test-rpm-report-5.txt \
+test-diff-pkg/test-nonexistent-report-0.txt \
 test-diff-pkg/libsigc++-2.0-0c2a-dbgsym_2.4.0-1_amd64.ddeb \
 test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt \
 test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64.deb \

diff --git a/tests/data/test-diff-pkg/test-nonexistent-report-0.txt b/tests/data/test-diff-pkg/test-nonexistent-report-0.txt
new file mode 100644 (file)
index 0000000..8c2f0f7
--- /dev/null
+++ b/tests/data/test-diff-pkg/test-nonexistent-report-0.txt
@@ -0,0 +1,2 @@
+abipkgdiff: The input file nonexistent-0.rpm doesn't exist
+try the --help option for more information

diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc
index 3513ec214cc63d90fd815c8f0680aba491aa5fc0..92ca1b1e2bd5b7f89a15db7da9a191223a81a9e8 100644 (file)
--- a/tests/test-diff-pkg.cc
+++ b/tests/test-diff-pkg.cc
@@ -426,6 +426,18 @@ static InOutSpec in_out_specs[] =
     "data/test-diff-pkg/dbus-glib-0.104-3.fc23.x86_64--dbus-glib-0.104-3.fc23.armv7hl-report-0.txt",
     "output/test-diff-pkg/dbus-glib-0.104-3.fc23.x86_64--dbus-glib-0.104-3.fc23.armv7hl-report-0.txt"
   },
+  {
+    "data/test-diff-pkg/nonexistent-0.rpm",
+    "data/test-diff-pkg/nonexistent-1.rpm",
+    "--no-default-suppression",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "data/test-diff-pkg/test-nonexistent-report-0.txt",
+    "output/test-diff-pkg/test-nonexistent-report-0.txt"
+  },
 #endif //WITH_RPM
 
 #ifdef WITH_DEB
@@ -555,7 +567,7 @@ struct test_task : public abigail::workers::task
 
     cmd =
       abipkgdiff + " " + first_in_package_path + " " + second_in_package_path;
-    cmd += " > " + out_abi_diff_report_path;
+    cmd += " > " + out_abi_diff_report_path + " 2>&1";
 
     bool abipkgdiff_ok = true;
     int code = system(cmd.c_str());

diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index a50fb2f3769469e29083f71a08c87a767e09b14a..438ef8f10a8ceb16076128bc8a228cd2e0f1c70c 100644 (file)
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -162,6 +162,7 @@ public:
   bool         display_usage;
   bool         display_version;
   bool         missing_operand;
+  bool         nonexistent_file;
   bool         abignore;
   bool         parallel;
   string       package1;
@@ -188,6 +189,7 @@ public:
       display_usage(),
       display_version(),
       missing_operand(),
+      nonexistent_file(),
       abignore(true),
       parallel(true),
       show_relative_offset_changes(true),
@@ -1846,14 +1848,26 @@ parse_command_line(int argc, char* argv[], options& opts)
       if (argv[i][0] != '-')
         {
           if (opts.package1.empty())
-            opts.package1 = abigail::tools_utils::make_path_absolute(argv[i]).get();
+            {
+              opts.package1 = abigail::tools_utils::make_path_absolute(argv[i]).get();
+              opts.nonexistent_file = !file_exists(opts.package1);
+            }
           else if (opts.package2.empty())
-            opts.package2 = abigail::tools_utils::make_path_absolute(argv[i]).get();
+            {
+              opts.package2 = abigail::tools_utils::make_path_absolute(argv[i]).get();
+              opts.nonexistent_file = !file_exists(opts.package2);
+            }
           else
            {
              opts.wrong_arg = argv[i];
              return false;
            }
+
+          if (opts.nonexistent_file)
+            {
+              opts.wrong_option = argv[i];
+              return true;
+            }
         }
       else if (!strcmp(argv[i], "--debug-info-pkg1")
               || !strcmp(argv[i], "--d1"))
@@ -2000,6 +2014,17 @@ main(int argc, char* argv[])
              | abigail::tools_utils::ABIDIFF_ERROR);
     }
 
+  if (opts.nonexistent_file)
+    {
+      string input_file;
+      base_name(opts.wrong_option, input_file);
+      emit_prefix("abipkgdiff", cerr)
+       << "The input file " << input_file << " doesn't exist\n"
+       "try the --help option for more information\n";
+      return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
+             | abigail::tools_utils::ABIDIFF_ERROR);
+    }
+
   if (opts.display_usage)
     {
       display_usage(argv[0], cout);
@@ -2070,13 +2095,15 @@ main(int argc, char* argv[])
                                "devel_package2",
                                /*pkg_kind=*/package::KIND_DEVEL)));
 
+  string package_name;
   switch (first_package->type())
     {
     case abigail::tools_utils::FILE_TYPE_RPM:
       if (second_package->type() != abigail::tools_utils::FILE_TYPE_RPM)
        {
+         base_name(opts.package2, package_name);
          emit_prefix("abipkgdiff", cerr)
-           << opts.package2 << " should be an RPM file\n";
+           << package_name << " should be an RPM file\n";
          return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
                  | abigail::tools_utils::ABIDIFF_ERROR);
        }
@@ -2085,8 +2112,9 @@ main(int argc, char* argv[])
     case abigail::tools_utils::FILE_TYPE_DEB:
       if (second_package->type() != abigail::tools_utils::FILE_TYPE_DEB)
        {
+         base_name(opts.package2, package_name);
          emit_prefix("abipkgdiff", cerr)
-           << opts.package2 << " should be a DEB file\n";
+           << package_name << " should be a DEB file\n";
          return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
                  | abigail::tools_utils::ABIDIFF_ERROR);
        }
@@ -2095,8 +2123,9 @@ main(int argc, char* argv[])
     case abigail::tools_utils::FILE_TYPE_DIR:
       if (second_package->type() != abigail::tools_utils::FILE_TYPE_DIR)
        {
+         base_name(opts.package2, package_name);
          emit_prefix("abipkgdiff", cerr)
-           << opts.package2 << " should be a directory\n";
+           << package_name << " should be a directory\n";
          return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
                  | abigail::tools_utils::ABIDIFF_ERROR);
        }
@@ -2105,16 +2134,18 @@ main(int argc, char* argv[])
     case abigail::tools_utils::FILE_TYPE_TAR:
       if (second_package->type() != abigail::tools_utils::FILE_TYPE_TAR)
        {
+         base_name(opts.package2, package_name);
          emit_prefix("abipkgdiff", cerr)
-           << opts.package2 << " should be a GNU tar archive\n";
+           << package_name << " should be a GNU tar archive\n";
          return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
                  | abigail::tools_utils::ABIDIFF_ERROR);
        }
       break;
 
     default:
+      base_name(opts.package1, package_name);
       emit_prefix("abipkgdiff", cerr)
-       << opts.package1 << " should be a valid package file \n";
+       << package_name << " should be a valid package file \n";
       return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
              | abigail::tools_utils::ABIDIFF_ERROR);
     }