Hello, Thanks. I've included some inline comments below.
I think it would be helpful to work on the spec in tag2upload(5) before
continuing too much with code. It'll make it easier to keep the three
of us on the same page.
On Sat 26 Jul 2025 at 02:12pm +02, Andrea Pappacoda wrote:
> ---
> Ok! Round two.
>
> Here's a summary of the changes from v1. For git-debpush:
>
> - The pristine-tar checking code is now only run if this is a non-native
> package (i.e., "if $upstream").
> - The upstream version is used instead of the Debian-revised one.
ITYM your new code, right?
The old check already had both these properties.
> - Differently from the old pristine-tar check, the code is not run just
> for the first (i.e., -1 or -0.1) revision, but for any upload. This
> way, the t2u service can potentially handle the case where
> a pristine-tar upload was intended, but no orig is available in the
> archive yet. Please let me know if this makes sense or not!
It might make sense, I'm not sure yet. Can you describe a concrete
example that would lead to this being helpful?
> diff --git a/git-debpush b/git-debpush
> index e3a4ba39..78e42fb9 100755
> --- a/git-debpush
> +++ b/git-debpush
> @@ -457,6 +457,30 @@ if $upstream; then
> to_push+=("$upstream_tag")
> fi
>
> +# I obtain the commit ID at the time of the upload, so that I can be sure
> that
> +# the tag2upload service generates the tarball with the expected pristine-tar
> +# branch state
> +pristine_tar_info=''
> +if $upstream; then
> + uversion="${version%-*}"
> +
> + if pristine_tar_commit=$(git rev-parse --verify --quiet
> 'refs/heads/pristine-tar'); then
> + pristine_tar_tarballs=$(git ls-tree -z --name-only --
> 'refs/heads/pristine-tar' \
> + | grep -zF -- "${source}_${uversion}.orig.tar." \
> + | grep -zc -- "\.id$")
> +
> + if [ "$pristine_tar_tarballs" -gt 1 ]; then
> + fail 'more then one pristine-tar orig'
> + fi
> +
> + # If there's no tarball, the user probably stopped using
> pristine-tar a
> + # while ago, but didn't delete the branch. Just ignore it.
> + if [ "$pristine_tar_tarballs" -eq 1 ]; then
> + pristine_tar_info=" pristine-tar=$pristine_tar_commit"
> + fi
> + fi
> +fi
> +
> #**** Useful sanity checks ****
Can you explain why you've put this in at this point in the script? I
think that maybe it should go later, after all the sanity checks.
I take it you switched from invoking pristine-tar itself to calling
git-ls-tree in order to use NUL termination? If so, maybe we should
make that change first to the existing check. Perhaps you could prepare
an MR to that effect.
> @@ -2031,6 +2033,9 @@ END
> "s=$suite",
> "u=$t2u_upstreamc",
> );
> + if (length $t2u_pristine_tar) {
> + push(@obtain_origs, "pristine_tar=$t2u_pristine_tar")
> + }
Generally we avoid parentheses on builtin operators and use poetry
style, so
push @obtain_origs, "pristine_tar=$t2u_pristine_tar"
if $t2u_pristine_tar;
> +# TODO: what about signature files?
Do you think we could extract them and include them in the upload?
I think we can verify them by using the upstream key embedded in the
source package, right? And if that verification fails we should
probably abort the upload -- maintainers who choose to use tarball
signatures had better make sure they verify.
--
Sean Whitton
signature.asc
Description: PGP signature
