10518 pkgdep should check that manifest is a file, proto is a dir
authorJohn Sonnenschein <John.Sonnenschein@Sun.COM>
Mon, 12 Oct 2009 13:41:34 -0700
changeset 1416 1d30d2b8e0f1
parent 1415 8a6ef6448292
child 1417 cb6277369ea4
10518 pkgdep should check that manifest is a file, proto is a dir 11517 pkgdep resolve needs to catch bad opts exceptions 11829 pkgdep needs to catch bad manifest actions
src/pkgdep.py
src/tests/baseline.txt
src/tests/cli/t_pkgdep.py
--- a/src/pkgdep.py	Tue Oct 13 09:51:06 2009 -0700
+++ b/src/pkgdep.py	Mon Oct 12 13:41:34 2009 -0700
@@ -121,14 +121,22 @@
         manf = pargs[0]
         proto_dir = pargs[1]
 
+        if (not os.path.isdir(proto_dir)) or (not os.path.isfile(manf)):
+            usage(retcode=1)
+
         try:
                 ds, es, ms = dependencies.list_implicit_deps(manf, proto_dir,
                     remove_internal_deps)
         except IOError, e:
                 if e.errno == errno.ENOENT:
-                        error("Could not find manifest file %s" % manf)
+                        error(_("Could not find manifest file %s") % manf)
                         return 1
                 raise
+        except actions.MalformedActionError, e:
+                error(_("Could not parse manifest %(manifest)s because of the "
+                    "following line:\n%(line)s") % { 'manifest': manf ,
+                    'line': e.actionstr})
+                return 1
 
         if echo_manf:
                 fh = open(manf, "rb")
@@ -156,7 +164,10 @@
         echo_manifest = False
         output_to_screen = False
         suffix = None
-        opts, pargs = getopt.getopt(args, "d:mos:")
+        try:
+            opts, pargs = getopt.getopt(args, "d:mos:")
+        except getopt.GetoptError, e:
+            usage(_("illegal global option -- %s") % e.opt)
         for opt, arg in opts:
                 if opt == "-d":
                         out_dir = arg
@@ -172,8 +183,14 @@
 
         manifest_paths = [os.path.abspath(fp) for fp in pargs]
 
+        for manifest in manifest_paths:
+            if not os.path.isfile(manifest):
+                usage()
+
         if out_dir:
                 out_dir = os.path.abspath(out_dir)
+                if not os.path.isdir(out_dir):
+                    usage()
 
         if img_dir is None:
                 try:
@@ -203,8 +220,15 @@
                     progress.QuietProgressTracker(), None, PKG_CLIENT_NAME)
         except api_errors.ImageNotFoundException, e:
                 error(_("'%s' is not an install image") % e.user_dir)
-                return 1                
-        pkg_deps, errs = dependencies.resolve_deps(manifest_paths, api_inst)
+                return 1
+
+        try:
+            pkg_deps, errs = dependencies.resolve_deps(manifest_paths, api_inst)
+        except actions.MalformedActionError, e:
+            error(_("Could not parse one or more manifests because of the " +
+                "following line:\n%s") % e.actionstr)
+            return 1
+
         ret_code = 0
         
         if output_to_screen:
--- a/src/tests/baseline.txt	Tue Oct 13 09:51:06 2009 -0700
+++ b/src/tests/baseline.txt	Mon Oct 12 13:41:34 2009 -0700
@@ -508,6 +508,9 @@
 cli.t_pkgdep.py TestPkgdepBasics.test_opts|pass
 cli.t_pkgdep.py TestPkgdepBasics.test_output|pass
 cli.t_pkgdep.py TestPkgdepBasics.test_resolve_screen_out|pass
+cli.t_pkgdep.py TestPkgdepBasics.test_bug_10518|pass
+cli.t_pkgdep.py TestPkgdepBasics.test_bug_11517|pass
+cli.t_pkgdep.py TestPkgdepBasics.test_bug_11829|pass
 cli.t_pkgdep_resolve.py TestApiDependencies.test_bug_11518|pass
 cli.t_pkgdep_resolve.py TestApiDependencies.test_resolve_cross_package|pass
 cli.t_pkgdep_resolve.py TestApiDependencies.test_resolve_mix|pass
--- a/src/tests/cli/t_pkgdep.py	Tue Oct 13 09:51:06 2009 -0700
+++ b/src/tests/cli/t_pkgdep.py	Mon Oct 12 13:41:34 2009 -0700
@@ -168,6 +168,19 @@
 variant.foo:baz variant.num:three
 """
 
+        usage_msg = """\
+Usage:
+        pkgdep [options] command [cmd_options] [operands]
+
+Subcommands:
+        pkgdep generate [-IMm] manifest proto_dir
+        pkgdep [options] resolve [-dMos] manifest ...
+
+Options:
+        -R dir
+        --help or -?
+Environment:
+        PKG_IMAGE"""
         
         @staticmethod
         def make_manifest(str):
@@ -435,3 +448,58 @@
                                 portable.remove(m3_path)
                         if m4_path:
                                 portable.remove(m4_path)
+
+        def test_bug_10518(self):
+
+                m_path = None
+
+                try:
+                        m_path = self.make_manifest(self.test_manf_1)
+
+                        self.pkgdep("generate / %s " % m_path,
+                            use_proto=False, exit=1)
+                        self.check_res(self.usage_msg, self.errout)
+
+                        self.pkgdep("resolve -o / ", use_proto=False, exit=2)
+                        self.check_res(self.usage_msg, self.errout)
+                finally:
+                        if m_path:
+                                portable.remove(m_path)
+
+        def test_bug_11517(self):
+
+                m_path = None
+                ill_usage = 'pkgdep: illegal global option -- M\n'
+                try:
+                        m_path = self.make_manifest(self.test_manf_1)
+
+                        self.pkgdep("resolve -M -o %s " % m_path,
+                            use_proto=False, exit=2)
+                        self.check_res(ill_usage + self.usage_msg,
+                            self.errout)
+                finally:
+                        if m_path:
+                                portable.remove(m_path)
+
+        def test_bug_11829(self):
+
+                m_path = None
+                nonsense = "This is a nonsense manifest"
+                try:
+                        m_path = self.make_manifest(nonsense)
+                        
+                        self.pkgdep("generate %s /" % m_path,
+                            use_proto=False, exit=1)
+                        self.check_res('pkgdep: Could not parse manifest ' + 
+                            '%s because of the following line:\n' % m_path +
+                            nonsense, self.errout)
+
+                        self.pkgdep("resolve -o %s " % m_path, 
+                            use_proto=False, exit=1)
+                        self.check_res("pkgdep: Could not parse one or more " +
+                            "manifests because of the following line:\n" +
+                            nonsense, self.errout)
+                finally:
+                        if m_path:
+                                portable.remove(m_path)
+