Re: [Launchpad-reviewers] [Merge] ~lool/git-build-recipe:fallback-on-pristine-tar-checkout-failures into git-build-recipe:master

2023-06-01 Thread Hector CAO
Review: Needs Information

great work, some questions

Diff comments:

> diff --git a/gitbuildrecipe/deb_util.py b/gitbuildrecipe/deb_util.py
> index d0a..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.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] ~lool/git-build-recipe:fallback-on-pristine-tar-checkout-failures into git-build-recipe:master

2023-06-01 Thread Loïc Minier
Loïc Minier has proposed merging 
~lool/git-build-recipe:fallback-on-pristine-tar-checkout-failures into 
git-build-recipe:master.

Commit message:
Trying to build 
https://code.launchpad.net/~lool/+recipe/linux-nvidia-tegra-sidecar-daily I 
first run into issues with missing refs for pristine-tar to work, but fixing 
these 
(https://code.launchpad.net/~lool/git-build-recipe/+git/git-build-recipe/+merge/443942)
 I run into a pristine-tar issue:
tar: /tmp/pristine-tar.CAMo9gloTn/recreatetarball: Wrote only 8192 of 10240 
bytes
tar: Error is not recoverable: exiting now
pristine-tar: command failed: tar cf 
/tmp/pristine-tar.CAMo9gloTn/recreatetarball --owner 0 --group 0 
--numeric-owner -C /tmp/pristine-tar.CAMo9gloTn/workdir --no-recursion --mode 
0644 --verbatim-files-from --files-from /tmp/pristine-tar.CAMo9gloTn/manifest
pristine-tar: failed to generate tarball

The code doesn't fallback to generating an archive from git tags or from the 
source package if pristine-tar checkout fails, this is my somewhat clumsy 
attempt at doing this

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
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.
diff --git a/gitbuildrecipe/deb_util.py b/gitbuildrecipe/deb_util.py
index d0a..4724de6 100644
--- a/gitbuildrecipe/deb_util.py
+++ b/gitbuildrecipe/deb_util.py
@@ -48,13 +48,14 @@ def debian_source_package_name(control_path):
 return control["Source"]
 
 
-def extract_upstream_tarball(path, package, version, dest_dir):
+def extract_upstream_tarball(path, package, version, dest_dir, fallback=True):
 """Extract the upstream tarball from a Git repository.
 
 :param path: Path to the Git repository
 :param package: Package name
 :param version: Package version
 :param dest_dir: Destination directory
+:param fallback: Extract from tag if pristine-tar fails
 """
 prefix = "%s_%s.orig.tar." % (package, version)
 dest_filename = None
@@ -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):
+os.remove(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)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp