launchpad-reviewers team mailing list archive

[Hector CAO <mp+443943@xxxxxxxxxxxxxxxxxx>]

Review: Needs Information

great work, some questions

Diff comments:

> diff --git a/gitbuildrecipe/deb_util.py b/gitbuildrecipe/deb_util.py
> index d0a6666..4724de6 100644
> --- a/gitbuildrecipe/deb_util.py
> +++ b/gitbuildrecipe/deb_util.py
> @@ -68,11 +69,20 @@ def extract_upstream_tarball(path, package, version, dest_dir):
>      finally:
>          pristine_tar_list.wait()
>      if dest_filename is not None:
> -        subprocess.check_call(
> -            ["pristine-tar", "checkout",
> -             os.path.abspath(os.path.join(dest_dir, dest_filename))],
> -            cwd=path)
> -    else:
> +        try:
> +            subprocess.check_call(
> +                ["pristine-tar", "checkout",
> +                 os.path.abspath(os.path.join(dest_dir, dest_filename))],
> +                cwd=path)
> +        # ideally we'd triage between pristine-tar issues
> +        except subprocess.CalledProcessError as e:
> +            print("pristine-tar exception")
> +            if not fallback:
> +                raise e
> +            if os.path.exists(dest_filename):

is it worth to move the file removal a little bit upper (before raising the exception) ?

> +                os.remove(dest_filename)

is it better to use os.path.abspath(os.path.join(dest_dir, dest_filename)) instead of dest_filename ?

> +    # no pristine-tar data or pristine-tar failed
> +    if not os.path.exists(dest_filename):
>          tag_names = ["upstream/%s" % version, "upstream-%s" % version]
>          git_tag_list = subprocess.Popen(
>              ["git", "tag"], stdout=subprocess.PIPE, cwd=path)


-- 
https://code.launchpad.net/~lool/git-build-recipe/+git/git-build-recipe/+merge/443943
Your team Launchpad code reviewers is requested to review the proposed merge of ~lool/git-build-recipe:fallback-on-pristine-tar-checkout-failures into git-build-recipe:master.