Merge ~athos-ribeiro/ubuntu/+source/python-debian:zstd-compression into ubuntu/+source/python-debian:ubuntu/devel

Hello, is there any reason why we shouldn't fix this in Debian first and let this continue to be a sync. If I am not missing anything, this doesn't seem very Ubuntu-specific, or is it?

Fixing this here is awesome but will require us to keep doing a merge for the rest of our lives, thereby increasing the technical debt. Plus, if the debfile.py is changed in Debian, then it'll demand even more work. I'd be super inclined in having this fixed in Debian and letting this package be a sync here, unless we have a definite reason to not to.

Thanks in advance and for all your work, Athos (and sorry for being the annoying one in raising this here. :D).

Hi Utkarsh,

Thanks for the review!

I did put some thought on whether to submit this patch here on in Debian, sorry for not making this clearer here.

The zstd format support in dpkg is part of our delta, i.e, Debian still does not support it (see [1] for further reference).

Therefore, this change is, at least while [1] is open, quite Ubuntu specific. Still, I did put some more thought on submitting this to Debian anyway, since this just adds additional support for a new compression format.

By looking at how the patched code is crafted (it is restricting the compression algorithms that can be supported - sort of a denylist), I came to the conclusion that adding it as a delta until [1] is accepted was indeed the way forward here.

Finally, regarding the patch itself, one could argue we could take the same approach as the one presented in [2], proposing that dpkg-deb should be used for extraction. This would allow the patch to be forwarded to Debian right away and we would not need to worry about compression algorithms support as long as they were implemented in dpkg.

However, users of python-debian may indeed be in an environment without dpkg. Hence, I am not sure if upstream would be willing to change towards that way (and I am not sure if I agree with this approach either).

Thoughts?

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=892664
[2]https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878900

> Thoughts?

I am convinced. Thank you! :)

However, to review (and sponsor) this MP, I'll need until tomorrow to go through the bug and see the code changes - even though I can see that you basically pulled out some lines into a function and used it accordingly.

I'd like to take a fresher look at this tomorrow (my) morning and I shall be happy to sponsor if everything looks ok. Of course, all this, if somebody doesn't beat me to it, already.

Thanks again, Athos, for the detailed explanation.

Hi Utkarsh,

Did you get the chance to check this one out? I am wondering if we should include this in the impish cycle.

Hi Athos,

> Did you get the chance to check this one out?

Yep, sorry for the delay. Me and Christian pair-reviewed this. So please find the comments below. We can discuss more on this prior to our stand-up.

> I am wondering if we should include this in the impish cycle.

Absolutely. We definitely should. This won't require a FFe and we have 7 days until the beta freeze, so we definitely should target that.

8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<

Though I haven't tested it myself, the patch looks good, great work! However, as you noted, we are taking a different approach here than what debbug #878900 outlines. Furthermore:

> However, users of python-debian may indeed be in an
> environment without dpkg.

That is not true. dpkg-deb comes from src:dpkg which is "Essential: yes", which means in every Debian(-based) distribution, dpkg would always be present. So anyone using python-debian will and should have dpkg, IMHO. Makes sense?

> Hence, I am not sure if upstream would be willing to
> change towards that way (and I am not sure if I
> agree with this approach either).

Hehe. As noted above, dpkg will always be present and so both, me and Christian, are inclined towards making this work with dpkg-deb instead of this approach. Let me also state that this patch is absolutely correct and I don't see any problem will it *but* it'd be better if we can make this work with dpkg-deb for two reasons that I can think of:

1. This can easily be sent to Debian and they'll be happy with this. Win-win.
2. For future compressions (that may find its way into existence), it'd be easier to integrate those w/ dpkg-deb instead of taking this approach, amongst other reasons.

TL;DR: it'd be really nice if we can come up with something that works with dpkg-deb for above reasons.

However, given that we have beta freeze in 7 days, so if we/you can't come up with something that works with dpkg-deb (or is realllyyyy time consuming) until this Friday, then we fall back to this current approach and I'll sponsor this as-is. Does that sounds like a good deal? :)

Let me know what you think? And as I said earlier, I'd be most happy to talk about this {pre,post}-standup, just let me know. TIA! \o/

> Hi Athos,

Hi!

[snip...]

> > However, users of python-debian may indeed be in an
> > environment without dpkg.
>
> That is not true. dpkg-deb comes from src:dpkg which is "Essential: yes",
> which means in every Debian(-based) distribution, dpkg would always be
> present. So anyone using python-debian will and should have dpkg, IMHO. Makes
> sense?

Partially. python-debian is a pure python package, which is also published in PyPI, at https://pypi.org/project/python-debian/ (and is up-to-date there).

Although most use cases would come from systems with dpkg installed, I could imagine one or two cases where that is not necessarily true. Namely, one could use python-debian for static analysis purposes in a non Debian-based environment just by using python-debian to prepare deb packages for analysis (checking metadata, decompressing data, etc).

> > Hence, I am not sure if upstream would be willing to
> > change towards that way (and I am not sure if I
> > agree with this approach either).
>
> Hehe. As noted above, dpkg will always be present and so both, me and
> Christian, are inclined towards making this work with dpkg-deb instead of this
> approach. Let me also state that this patch is absolutely correct and I don't
> see any problem will it *but* it'd be better if we can make this work with
> dpkg-deb for two reasons that I can think of:
>
> 1. This can easily be sent to Debian and they'll be happy with this. Win-win.
> 2. For future compressions (that may find its way into existence), it'd be
> easier to integrate those w/ dpkg-deb instead of taking this approach, amongst
> other reasons.

While (1) may not be entirely true due to the reasons above (python-debian maintainers may frown upon relying solely in dpkg-deb for decompression - also, this could break some workflows), (2) makes a very strong case for actually using dpkg-deb for decompression.

Therefore, we could compromise by implementing this change in such a manner that python-debian tries to use dpkg-deb for decompression and falls back to using its own decompression code path in case dpkg-deb is not present. This could (and most likely will) introduce technical debt in python-debian upstream every time a new compression format is introduced, but it would also avoid breakage in some workflows in those same situations. Moreover, it will also keep the non-Debian-based distro compatibility nature of the package.

WDYT?

Update for audit tail after discussion.
If you can make "dpkg-deb first, fall back to decompressors" without spending too much time then yes that really seems great. If it turns out painful, then tell us and we might use (and submit) this one.

This is the idea:

Given python-debian.debfile module's design, doing "decompressors first, fallback to dpkg-deb" looked better aligned to the code's proposal.

Here is a WIP (it is missing unit tests)

https://salsa.debian.org/athos/python-debian/-/commit/15e38c05b28caa13e0939148bb093e985d38e374

Thoughts?

https://salsa.debian.org/python-debian-team/python-debian/-/merge_requests/65

Thanks - according to the feedback there they might prefer a combined approach of the two that you had. zstd for when dpkg can't handle it (Debian) and dpkg as fallback (Ubuntu or other compression down the road).

Other than that I only see a request to change how the data is passed.

Finally he asked to combine the PRs that you have up

All seem reasonable requests and I'm sure you'll handle them

Depending on the timing we might then have to decide when/if we upload something to Ubuntu ahead of time, but it is already great that the Debian maintainer isn't totally against it.

Can we schedule that (decision if & what to upload early) as a post-standup topic on Thursday?

I filed a FFe bug [1] and subscribed the foundations team to it.

Note that after assessing the requested upstream changes for the upstream patch at [2], I realized that some extra refactoring will be needed in the patched module.

Moreover, [2] is not applicable to python-debian 0.1.39, available in impish: 0.1.40 (Debian unstable) refactored that exact method being patched. Hence, the proposal is to apply this MP introducin zstd to python-debian 0.1.39 for the impish release, and then work with upstream to land [2] for the future releases of python-debian.

[1] https://bugs.launchpad.net/ubuntu/+source/python-debian/+bug/1945205
[2] https://salsa.debian.org/python-debian-team/python-debian/-/merge_requests/65

Agreed Athos and thanks for all the detours. It might seem burdensome but will ensure to not end up badly maintainable in the long term.

I agree that this shall be fixed in impish as proposed right now and know it will be safe with you to continue driving it in Debian to later on become a merge again.

I re-reviewed this but it still is in the state that I've seen before and thereby ok in the context as discussed.

Given that foundations didn't reply on this case yet over all the time the MP is up I guess they are busy and we need to handle this ourselves.

I re-pinged for the FFE in IRC #ubuntu-release and after getting an ack there I plan to sponsor this upload.

Thanks, Christian!

Uploaded:

$ dput python-debian_0.1.39ubuntu1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/python-debian/python-debian_0.1.39ubuntu1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/python-debian/python-debian_0.1.39ubuntu1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading python-debian_0.1.39ubuntu1.dsc: done.
  Uploading python-debian_0.1.39ubuntu1.tar.xz: done.
  Uploading python-debian_0.1.39ubuntu1_source.buildinfo: done.
  Uploading python-debian_0.1.39ubuntu1_source.changes: done.
Successfully uploaded packages.