Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/989 add zstandard support #1002

Closed

Conversation

ahaswell
Copy link

@ahaswell ahaswell commented Dec 9, 2021

added zstandard decompression to support ubuntu 21.10

#965 seems to be abandoned so raising this pr to get #989 done.

Included support for control file as per @svanzoest request.

deb/deb.go Outdated Show resolved Hide resolved
@eilandert
Copy link

eilandert commented Dec 15, 2021

Hi,

I am not sure how this works, as I am new to aptly. I cloned your fork but it doesn't work out of the box. I have scripted to rsync my public reprepro dir and add it to my local (testing) aptly.

This is a snippet of the output

Local repo [jammy] successfully added.
You can run 'aptly repo add jammy ...' to add packages to repository.
Loading packages...
[!] Unable to read file /tmp/aptly/jammy/apache2-bin_2.4.51-5myguard12+jammy_amd64.deb: unsupported tar compression in /tmp/aptly/jammy/apache2-bin_2.4.51-5myguard12+jammy_amd64.deb: control.tar.zst

Thanks,
Thijs

@jvperrin
Copy link

Hi @eilandert! Apologies for the delay, but it's been working fine for us with packages that have a control.tar.zst and/or a data.tar.zst. For instance, loading the 0ad package (it's early when sorting a list of packages and has zst control/data) from https://packages.ubuntu.com/jammy/0ad as a test, it shows this:

$ ar t 0ad_0.0.25b-1_amd64.deb
debian-binary
control.tar.zst
data.tar.zst

$ cp 0ad_0.0.25b-1_amd64.deb /tmp/packages/jammy/incoming/0ad_0.0.25b-1_amd64.deb

$ /usr/bin/aptly -config="/etc/aptly.conf.jammy" repo --remove-files=true add jammy-yelp /tmp/packages/jammy/incoming/
Loading packages...
[+] 0ad_0.0.25b-1_amd64 added

We've built the aptly .deb to install it by using make release and installed as that deb package, does that match how you are building/installing it?

@eilandert
Copy link

Hi @jvperrin

I've updated the debian package with a git clone with the PR included, as I have a complete build pipeline with pbuilder for my own packages/repositories

As I have totally no experience with go, there will be more adjusting to do to my package. I skimmed go.mod and at first glance I am missing some modules, not sure yet. I guess I have to invest some to investigate and learn some go basics.
Another solution would be to tag a new release, so the debian maintainers can pick up the changes and do the work ;-)

In the meantime I've adjusted my packages so everything will be build with xz so I'm in the clear now.

Thanks!

-Thijs

Hi @eilandert! Apologies for the delay, but it's been working fine for us with packages that have a control.tar.zst and/or a data.tar.zst. For instance, loading the 0ad package (it's early when sorting a list of packages and has zst control/data) from https://packages.ubuntu.com/jammy/0ad as a test, it shows this:

$ ar t 0ad_0.0.25b-1_amd64.deb
debian-binary
control.tar.zst
data.tar.zst

$ cp 0ad_0.0.25b-1_amd64.deb /tmp/packages/jammy/incoming/0ad_0.0.25b-1_amd64.deb

$ /usr/bin/aptly -config="/etc/aptly.conf.jammy" repo --remove-files=true add jammy-yelp /tmp/packages/jammy/incoming/
Loading packages...
[+] 0ad_0.0.25b-1_amd64 added

We've built the aptly .deb to install it by using make release and installed as that deb package, does that match how you are building/installing it?

@eilandert
Copy link

@jvperrin , i've got it working. had to create a new package first, golang-github-klauspost-compress, and add it in debian/control. I've now functional zstd aptly packages (and dockers) and can confirm it as working.

As soon as this is merged I can switch back to the aptly-dev/aptly repo

Thanks.

@jbigot
Copy link

jbigot commented Jan 18, 2022

This is great news, I will try the golang-github-klauspost-compress repo until I can switch back to the official repo!

@lbolla lbolla mentioned this pull request Jan 28, 2022
6 tasks
@lbolla
Copy link
Contributor

lbolla commented Jan 28, 2022

@ahaswell Hey! Could you please rebase against master, so we can run the revived unittests? Cheers

Also, you'll have to add some unittests for this feature before merging. Thanks!

Copy link
Contributor

@Vitexus Vitexus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Work for me:

aptly repo add impish /var/tmp/deb/phar-composer_1.1.1~impish~53_all.deb
Loading packages...
[+] phar-composer_1.1.1~impish~53_all added

The package wass added into repository:

jenkins@vyvojar:~$ aptly repo show -with-packages impish 
Name: impish
Comment: Impish packages
Default Distribution: impish
Default Component: main
Number of packages: 2
Packages:
  apt-repo-vitexsoftware_0.3~impish~21_all
  phar-composer_1.1.1~impish~53_all

vs current master (without zstd support):

aptly repo add impish /var/tmp/deb/apt-repo-vitexsoftware_0.3~impish~19_all.deb
Loading packages...
[!] Unable to read file /var/tmp/deb/apt-repo-vitexsoftware_0.3~impish~19_all.deb: unsupported tar compression in /var/tmp/deb/apt-repo-vitexsoftware_0.3~impish~19_all.deb: control.tar.zst
[!] Some files were skipped due to errors:
  /var/tmp/deb/apt-repo-vitexsoftware_0.3~impish~19_all.deb
ERROR: some files failed to be added

Maintainers please merge this pullrequest.

@eilandert
Copy link

In the meanwhile, I have a rebased patch for master on https://github.com/eilandert/dockerized/blob/master/aptly/zstd.patch

@lbolla
Copy link
Contributor

lbolla commented Feb 16, 2022

In the meanwhile, I have a rebased patch for master on https://github.com/eilandert/dockerized/blob/master/aptly/zstd.patch

Once the PR is rebased against the latest master, I can run the test suite. Tests for this specific feature are still missing: we need them in order to be able to merge it, as per guidelines.

tobiasherzke added a commit to HoerTech-gGmbH/tascar that referenced this pull request Mar 15, 2022
aptly cannot handle zst compression yet, see T1813 and
aptly-dev/aptly#1002.

Work around by setting compression method to xz.
@mbearup mbearup mentioned this pull request Mar 22, 2022
6 tasks
@mbearup
Copy link
Contributor

mbearup commented Mar 22, 2022

Since this PR seems to be stalled waiting on a rebase, I've opened a fresh PR here with the same changes.
#1050

@randombenj randombenj closed this Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants