cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/tools/read-dependencies b/tools/read-dependencies
> index 4ba2c1b..f22f4a5 100755
> --- a/tools/read-dependencies
> +++ b/tools/read-dependencies
> @@ -144,12 +146,18 @@ def main(distro):
>          topd = os.path.realpath(os.environ.get('CLOUD_INIT_TOP_D'))
>      else:
>          topd = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
> -    req_path = os.path.join(topd, args.req_file)
> -    if not os.path.isfile(req_path):
> -        sys.stderr.write("Unable to locate '%s' file that should "
> -                         "exist in cloud-init root directory." % req_path)
> -        return 1
> -    pip_pkg_names = parse_pip_requirements(req_path)
> +
> +    if args.req_files is None:
> +        args.req_files = [os.path.join(topd, DEFAULT_REQUIREMENTS)]

this falls over if someone specifies the full path to the file. But it's a simple tool so I think we are fine dealing w/ that (as we'll error out just below).

> +        if not os.path.isfile(args.req_files[0]):

Since we support multiple requirenments files now any objection to the following?
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -149,11 +149,14 @@ def main(distro):
 
     if args.req_files is None:
         args.req_files = [os.path.join(topd, DEFAULT_REQUIREMENTS)]
-        if not os.path.isfile(args.req_files[0]):
-            sys.stderr.write("Unable to locate '%s' file that should "
-                             "exist in cloud-init root directory." %
-                             args.req_files[0])
-            sys.exit(1)
+    for req_file in args.req_files:
+        bad_files = [
+            req_file for req_file in args.req_files
+            if not os.path.isfile(req_file)]
+    if bad_files:
+        sys.stderr.write(
+            "Unable to find requirements files: %s\n" % ','.join(bad_files))
+        sys.exit(1)
 
     pip_pkg_names = set()
     for req_path in args.req_files:

> +            sys.stderr.write("Unable to locate '%s' file that should "
> +                             "exist in cloud-init root directory." %
> +                             args.req_files[0])
> +            sys.exit(1)
> +
> +    pip_pkg_names = set()
> +    for req_path in args.req_files:
> +        pip_pkg_names.update(set(parse_pip_requirements(req_path)))
>      deps_from_json = get_package_deps_from_json(topd, args.distro)
>      renames = deps_from_json.get('renames', {})
>      translated_pip_names = translate_pip_to_system_pkg(


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/325679
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/fix-ci-redhat into cloud-init:master.