Bug 1597287 - Review Request: zchunk - Compressed file format that allows easy deltas
Summary: Review Request: zchunk - Compressed file format that allows easy deltas
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Igor Raits
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-07-02 13:30 UTC by Jonathan Dieter
Modified: 2018-08-09 17:41 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-07-06 14:48:44 UTC
Type: ---
Embargoed:
igor.raits: fedora-review+


Attachments (Terms of Use)

Description Jonathan Dieter 2018-07-02 13:30:23 UTC
Spec URL: https://www.jdieter.net/downloads/zchunk/zchunk.spec
SRPM URL: https://www.jdieter.net/downloads/zchunk/zchunk-0.7.4-2.fc28.src.rpm
Description:
zchunk is a compressed file format that splits the file into independent
chunks.  This allows you to only download the differences when downloading a
new version of the file, and also makes zchunk files efficient over rsync.
zchunk files are protected with strong checksums to verify that the file you
downloaded is in fact the file you wanted.

Fedora Account System Username: jdieter

Comment 1 Jonathan Dieter 2018-07-02 13:32:39 UTC
rpmlint output:
zchunk.src: W: spelling-error %description -l en_US rsync -> sync, r sync
zchunk.src: W: spelling-error %description -l en_US checksums -> check sums, check-sums, checks
zchunk.x86_64: W: spelling-error %description -l en_US rsync -> sync, r sync
zchunk.x86_64: W: spelling-error %description -l en_US checksums -> check sums, check-sums, checks
zchunk.x86_64: W: no-manual-page-for-binary unzck
zchunk.x86_64: W: no-manual-page-for-binary zck
zchunk.x86_64: W: no-manual-page-for-binary zck_delta_size
zchunk.x86_64: W: no-manual-page-for-binary zck_read_header
zchunk.x86_64: W: no-manual-page-for-binary zckdl
zchunk-devel.x86_64: W: spelling-error Summary(en_US) libzck -> lick
zchunk-devel.x86_64: W: spelling-error %description -l en_US rsync -> sync, r sync
zchunk-devel.x86_64: W: spelling-error %description -l en_US checksums -> check sums, check-sums, checks
zchunk-devel.x86_64: W: spelling-error %description -l en_US libzck -> lick
5 packages and 1 specfiles checked; 0 errors, 13 warnings.

Comment 2 Neal Gompa 2018-07-02 13:52:43 UTC
Please split out the libraries into a separate libs subpackage, so that multilibbing will work correctly.

Comment 3 Jonathan Dieter 2018-07-02 13:57:43 UTC
Something like zchunk-libs?  Should zchunk-devel be renamed to zchunk-libs-devel?

Comment 4 Igor Raits 2018-07-02 14:00:30 UTC
Will review shortly.

Comment 5 Igor Raits 2018-07-02 14:07:49 UTC
So the licensing is complicated here:
* It should be BSD and MIT and Public Domain
* Provides: bundled(buzhash) = VERSION OF BUZHASH
There seems to be some bundled code about multipart parsing...
* Provides: bundled(something) = VERSION OF THIS SOMETHING
* Provides: bundled(sha1) = …
* Provides: bundled(sha2) = …

But please, can you unbundle those?

Comment 6 Igor Raits 2018-07-02 14:19:15 UTC
* Source0:        https://github.com/zchunk/%{name}/archive/%{version}/%{name}-%{version}.tar.gz
You could replace this by Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz

* Missing BuildRequires: gcc

* This package contains the headers necessary for building against the zchunk
library, libzck
Fullstop here, please.

* %{_libdir}/libzck*.so.*
%{_libdir}/libzck.so.*

* %{_libdir}/libzck*.so
%{_libdir}/libzck.so

* %{_libdir}/pkgconfig/zck*
%{_libdir}/pkgconfig/zck.pc

* %{_includedir}/*
%{_includedir}/zck.h

* Summary:        A compressed file format that allows easy deltas
No need for "A", Summary: Compressed file format that allows easy deltas

+ licensing story mentioned above.

Comment 7 Jonathan Dieter 2018-07-02 14:23:30 UTC
(In reply to Igor Gnatenko from comment #5)
> So the licensing is complicated here:
> * It should be BSD and MIT and Public Domain
> * Provides: bundled(buzhash) = VERSION OF BUZHASH

I had to modify the original code, so it can't really be unbundled.  I'll go ahead and add the provides.

> There seems to be some bundled code about multipart parsing...

There was, but it didn't do what I needed, so I had to start from scratch.  I obviously forgot to delete the original README.md, so I'll take care of that

> * Provides: bundled(something) = VERSION OF THIS SOMETHING
> * Provides: bundled(sha1) = …
> * Provides: bundled(sha2) = …
> 
> But please, can you unbundle those?

These were bundled to keep dependencies at a minimum because I see zchunk as having universal use, even in places where openssl might not be available.

Having said that, if you feel strongly about this, I will change zchunk so openssl is an optional dependency, that, if available, will provide the hashing functions rather than the built-in hashing functions.

Do you feel that that would be worth the effort?

Comment 8 Jonathan Dieter 2018-07-02 14:40:22 UTC
(In reply to Jonathan Dieter from comment #7)
> Having said that, if you feel strongly about this, I will change zchunk so
> openssl is an optional dependency, that, if available, will provide the
> hashing functions rather than the built-in hashing functions.
> 
> Do you feel that that would be worth the effort?

Having looked at OpenSSL's API, this should be a very easy adaptation, so I think I'll go ahead and do it.  It's reaching the end of the day here, though, so it probably won't be ready until tomorrow.

Comment 9 Igor Raits 2018-07-02 14:51:21 UTC
(In reply to Jonathan Dieter from comment #7)
> (In reply to Igor Gnatenko from comment #5)
> > So the licensing is complicated here:
> > * It should be BSD and MIT and Public Domain
> > * Provides: bundled(buzhash) = VERSION OF BUZHASH
> 
> I had to modify the original code, so it can't really be unbundled.  I'll go
> ahead and add the provides.
> 
> > There seems to be some bundled code about multipart parsing...
> 
> There was, but it didn't do what I needed, so I had to start from scratch. 
> I obviously forgot to delete the original README.md, so I'll take care of
> that

Why did you do that? Any reason not to use some other dependency which meet your needs?

> > * Provides: bundled(something) = VERSION OF THIS SOMETHING
> > * Provides: bundled(sha1) = …
> > * Provides: bundled(sha2) = …
> > 
> > But please, can you unbundle those?
> 
> These were bundled to keep dependencies at a minimum because I see zchunk as
> having universal use, even in places where openssl might not be available.
> 
> Having said that, if you feel strongly about this, I will change zchunk so
> openssl is an optional dependency, that, if available, will provide the
> hashing functions rather than the built-in hashing functions.
> 
> Do you feel that that would be worth the effort?

Just saying that it can't be dependency of dnf because in enterprise any crypto-related code has to be checked and certified. While if you use openssl, this is already done. So yes, please use openssl.

Comment 10 Jonathan Dieter 2018-07-02 15:39:52 UTC
(In reply to Igor Gnatenko from comment #9)
> (In reply to Jonathan Dieter from comment #7)
> > (In reply to Igor Gnatenko from comment #5)
> > > So the licensing is complicated here:
> > > * It should be BSD and MIT and Public Domain
> > > * Provides: bundled(buzhash) = VERSION OF BUZHASH
> > 
> > I had to modify the original code, so it can't really be unbundled.  I'll go
> > ahead and add the provides.
> > 
> > > There seems to be some bundled code about multipart parsing...
> > 
> > There was, but it didn't do what I needed, so I had to start from scratch. 
> > I obviously forgot to delete the original README.md, so I'll take care of
> > that
> 
> Why did you do that? Any reason not to use some other dependency which meet
> your needs?

I looked, but couldn't find anything that would do it. I don't think many pieces of software use multiple range requests in a single http request, so I just wrote the code myself.

> > > * Provides: bundled(something) = VERSION OF THIS SOMETHING
> > > * Provides: bundled(sha1) = …
> > > * Provides: bundled(sha2) = …
> > > 
> > > But please, can you unbundle those?
> > 
> > These were bundled to keep dependencies at a minimum because I see zchunk as
> > having universal use, even in places where openssl might not be available.
> > 
> > Having said that, if you feel strongly about this, I will change zchunk so
> > openssl is an optional dependency, that, if available, will provide the
> > hashing functions rather than the built-in hashing functions.
> > 
> > Do you feel that that would be worth the effort?
> 
> Just saying that it can't be dependency of dnf because in enterprise any
> crypto-related code has to be checked and certified. While if you use
> openssl, this is already done. So yes, please use openssl.

Ok, will do

Comment 12 Jonathan Dieter 2018-07-03 11:42:46 UTC
Ok, in this updated spec, the only bundled code is buzhash from the urlblock project.  It's used to figure out when to automatically end a chunk.

There's a new dependency on openssl, and I think I've finished the rest of the list of smaller items, including the change on the License tag.

Comment 13 Neal Gompa 2018-07-03 22:26:59 UTC
> %ldconfig_scriptlets

This needs to be `%ldconfig_scriptlets libs`

Comment 14 Jonathan Dieter 2018-07-04 09:43:40 UTC
(In reply to Neal Gompa from comment #13)
> > %ldconfig_scriptlets
> 
> This needs to be `%ldconfig_scriptlets libs`

Good catch!  Fixed in 0.7.5-2

Spec URL: https://www.jdieter.net/downloads/zchunk/zchunk.spec
SRPM URL: https://www.jdieter.net/downloads/zchunk/zchunk-0.7.5-2.fc28.src.rpm

Comment 15 Neal Gompa 2018-07-04 12:14:00 UTC
(In reply to Jonathan Dieter from comment #14)
> (In reply to Neal Gompa from comment #13)
> > > %ldconfig_scriptlets
> > 
> > This needs to be `%ldconfig_scriptlets libs`
> 
> Good catch!  Fixed in 0.7.5-2
> 
> Spec URL: https://www.jdieter.net/downloads/zchunk/zchunk.spec
> SRPM URL:
> https://www.jdieter.net/downloads/zchunk/zchunk-0.7.5-2.fc28.src.rpm

You should leave it as devel instead of libs-devel. Otherwise, looks fine to me.

Comment 16 Jonathan Dieter 2018-07-04 12:23:09 UTC
(In reply to Neal Gompa from comment #15)
> You should leave it as devel instead of libs-devel. Otherwise, looks fine to me.

Fixed in 0.7.5-3

Spec URL: https://www.jdieter.net/downloads/zchunk/zchunk.spec
SRPM URL: https://www.jdieter.net/downloads/zchunk/zchunk-0.7.5-3.fc28.src.rpm

Comment 17 Igor Raits 2018-07-04 12:37:05 UTC
* Missing BuildRequires: gcc
* Please pass -Dwith-openssl=true or something like that, same for zstd. Just to be sure that they are enabled

* %{_libdir}/libzck*.so.*
%{_libdir}/libzck.so.*

* %{_libdir}/libzck*.so
%{_libdir}/libzck.so

* %{_libdir}/pkgconfig/zck*
%{_libdir}/pkgconfig/zck.pc

* %{_includedir}/*
%{_includedir}/zck.h

This is trivial to fix during the import. So APPROVED.

Comment 18 Jonathan Dieter 2018-07-04 18:13:33 UTC
Igor, thanks so much for the review, and sorry for missing the BuildRequires and file globs the first time around.  I've got them fixed in the spec that will be imported, along with the explicit openssl and zstd options.

Neal, thanks for your input.

Comment 19 Gwyn Ciesla 2018-07-05 13:53:39 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/zchunk

Comment 20 Jonathan Dieter 2018-07-06 14:48:44 UTC
Zchunk is now available in F29+, and F27, F28 and EPEL7 builds are in progress.

Comment 21 Fedora Update System 2018-07-07 12:28:18 UTC
zchunk-0.7.5-4.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-3dd6fc56e5

Comment 22 Fedora Update System 2018-07-07 12:29:05 UTC
zchunk-0.7.5-4.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-845969b9d8

Comment 23 Fedora Update System 2018-07-07 12:29:56 UTC
zchunk-0.7.5-4.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-56581ffa1c

Comment 24 Fedora Update System 2018-07-07 20:51:21 UTC
zchunk-0.7.5-4.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-845969b9d8

Comment 25 Fedora Update System 2018-07-07 22:26:47 UTC
zchunk-0.7.5-4.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-56581ffa1c

Comment 26 Fedora Update System 2018-07-07 23:43:41 UTC
zchunk-0.7.5-4.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-3dd6fc56e5

Comment 27 Fedora Update System 2018-07-12 21:54:32 UTC
zchunk-0.7.6-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-4f9b786b03

Comment 28 Fedora Update System 2018-07-13 07:38:28 UTC
zchunk-0.7.6-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-7743cb12b7

Comment 29 Fedora Update System 2018-07-13 07:39:15 UTC
zchunk-0.7.6-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-330d8dd1bf

Comment 30 Fedora Update System 2018-07-13 17:06:20 UTC
zchunk-0.7.6-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-330d8dd1bf

Comment 31 Fedora Update System 2018-07-13 17:33:52 UTC
zchunk-0.7.6-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-4f9b786b03

Comment 32 Fedora Update System 2018-07-13 19:29:41 UTC
zchunk-0.7.6-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-7743cb12b7

Comment 33 Fedora Update System 2018-07-25 17:15:29 UTC
zchunk-0.9.1-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-4be6fd5ece

Comment 34 Fedora Update System 2018-07-25 17:16:37 UTC
zchunk-0.9.1-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-15d2af4533

Comment 35 Fedora Update System 2018-07-25 17:17:07 UTC
zchunk-0.9.1-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-199cebafad

Comment 36 Fedora Update System 2018-07-26 12:19:28 UTC
zchunk-0.9.1-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-199cebafad

Comment 37 Fedora Update System 2018-07-26 14:16:56 UTC
zchunk-0.9.1-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-15d2af4533

Comment 38 Fedora Update System 2018-07-26 16:28:28 UTC
zchunk-0.9.1-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-4be6fd5ece

Comment 39 Fedora Update System 2018-08-01 13:12:51 UTC
zchunk-0.9.5-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-afde6f4b99

Comment 40 Fedora Update System 2018-08-01 13:19:43 UTC
zchunk-0.9.5-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-fabad2927e

Comment 41 Fedora Update System 2018-08-01 15:45:55 UTC
zchunk-0.9.5-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-fabad2927e

Comment 42 Fedora Update System 2018-08-01 18:26:45 UTC
zchunk-0.9.5-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-afde6f4b99

Comment 43 Fedora Update System 2018-08-09 16:51:23 UTC
zchunk-0.9.5-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.

Comment 44 Fedora Update System 2018-08-09 17:41:02 UTC
zchunk-0.9.5-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.