Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 234488 - Review Request: yum-presto - Yum plugin to download deltarpms rather than full rpms
Summary: Review Request: yum-presto - Yum plugin to download deltarpms rather than ful...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 6
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tim Lauridsen
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-29 16:32 UTC by Jonathan Dieter
Modified: 2007-11-30 22:12 UTC (History)
3 users (show)

Fixed In Version: 0.3.8-1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-04-06 20:13:08 UTC
tla: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Jonathan Dieter 2007-03-29 16:32:52 UTC
Spec URL: http://www.lesbg.com/jdieter/presto/yum-presto.spec
SRPM URL: http://www.lesbg.com/jdieter/presto/yum-presto-0.3.2-1.src.rpm
Description: Yum-presto is a plugin for yum that looks for deltarpms rather than rpms whenever they are available.  This has the potential of saving a lot of bandwidth when downloading updates.

A Deltarpm is the difference between two rpms.  If you already have foo-1.0 installed and foo-1.1 is available, yum-presto will download the deltarpm for foo-1.0 => 1.1 rather than the full foo-1.1 rpm, and then build the full foo-1.1 package from your installed foo-1.0 and the downloaded deltarpm.

Comment 1 Jonathan Dieter 2007-03-29 17:04:40 UTC
This is my first package, so I need a sponsor.

Comment 2 Maxime Carron 2007-03-29 23:12:58 UTC
%{_datadir}/presto/* should be %{_datadir}/presto thus presto is known as the
owner of this directory.


Comment 4 Jonathan Dieter 2007-03-30 16:20:44 UTC
New release: 0.3.3-1
Spec URL: http://www.lesbg.com/jdieter/presto/yum-presto.spec
SRPM URL: http://www.lesbg.com/jdieter/presto/yum-presto-0.3.3-1.src.rpm

Comment 5 Tim Lauridsen 2007-04-03 10:45:33 UTC
rpmlint Output:

[tim@naboo ~]$ rpmlint yum-presto-0.3.3-1.noarch.rpm 
E: yum-presto only-non-binary-in-usr-lib
[tim@naboo ~]$ rpmlint yum-presto-0.3.3-1.src.rpm 
W: yum-presto macro-in-%changelog _datadir
   - Change %{_datadir} to %%{_datadir} in Chagelog entry
W: yum-presto no-%build-section
   - Not a blocker, but a empty %build section coulf be added to silence rpmlint


Comment 6 Tim Lauridsen 2007-04-03 10:48:09 UTC
MUST:
* package is named appropriately
* it is legal for Fedora to distribute this
* license field matches the actual license.
* license is open source-compatible.
* specfile name matches %{name}
* verify source and patches (md5sum matches upstream, know what the patches do)
* summary and description fine
* correct buildroot
* %{?dist} is used
* license text included in package and marked with %doc
* package meets FHS (http://www.pathname.com/fhs/)
X changelog format fine (see earlier bug comments)
* Packager tag not used
* Vendor tag not used
* Distribution tag not used
* License used and not Copyright 
* Summary tag does not end in a period
* if possible, replace PreReq with Requires(pre) and/or Requires(
* Source match upstream
  $ md5sum yum-presto-0.3.3.tar.bz2 
  fd1984365bdbe61aca8114ab47eccefa  yum-presto-0.3.3.tar.bz2
  $ md5sum rpmbuild/SOURCES/yum-presto-0.3.3.tar.bz2
  fd1984365bdbe61aca8114ab47eccefa  rpmbuild/SOURCES/yum-presto-0.3.3.tar.bz2
* specfile is legible
* package successfully compiles and builds on at least x86
* BuildRequires are proper
* make sure lines are <= 80 characters
* specfile written in American English
* no -doc sub-package necessary
* no libraries
* no rpath
* config files uses %config(noreplace)
* not a GUI app
* no -devel sub-package necessary
* macros used appropriately and consistently
* no %makeinstall
* install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot}
* no locale data
* no cp usage so no need to worry about -p
* split Requires(pre,post) into two separate lines
* package not relocatable
* package contains code
* package owns all directories and files
* no %files duplicates
* file permissions fine
? %defattrs present
 - should it be %defattr(-, root, root, -)
* %clean present
* %doc files do not affect runtime
* not a web app
* verify the final provides and requires of the binary RPMs
  $ rpm -q --requires -p rpmbuild/RPMS/noarch/yum-presto-0.3.3-1.noarch.rpm 
  config(yum-presto) = 0.3.3-1
  deltarpm >= 3.4
  python >= 2.4
  rpmlib(CompressedFileNames) <= 3.0.4-1
  rpmlib(PayloadFilesHavePrefix) <= 4.0-1
  yum >= 3.0


  $  rpm -q --provides -p rpmbuild/RPMS/noarch/yum-presto-0.3.3-1.noarch.rpm 
  config(yum-presto) = 0.3.3-1
  yum-presto = 0.3.3-1

* run rpmlint on the binary RPMs
 - see previous bug comments

SHOULD:
* package should include license text in the package and mark it with %doc
* package should build on i386
? package should build in mock
 - I haven't tried, but I don't think it'll be a problem


Comment 7 Tim Lauridsen 2007-04-03 10:54:14 UTC
SUMMARY:
E: yum-presto only-non-binary-in-usr-lib
This is fine because yum plugins is stored in /usr/lib/yum-plugins.

W: yum-presto macro-in-%changelog _datadir
Change %{_datadir} to %%{_datadir} in Changelog entry

use %defattr(-, root, root, -)

Fix these issues and i will approve it, i'm not a sponsor, so we need somebody
to sponsor you.
 


Comment 8 Jonathan Dieter 2007-04-03 13:43:16 UTC
New release: 0.3.4-1
Spec URL: http://www.lesbg.com/jdieter/presto/yum-presto.spec
SRPM URL: http://www.lesbg.com/jdieter/presto/yum-presto-0.3.4-1.src.rpm

This fixes all the mistakes you pointed out, Tim (including the %build one). 
Thanks much for the review!

Comment 9 Tim Lauridsen 2007-04-03 17:41:28 UTC
The spec link is wrong, it point to 0.3.3 spec file, not the 0.3.4 one

$ rpmlint rpmbuild/RPMS/noarch/yum-presto-0.3.4-1.fc7.noarch.rpm 
E: yum-presto only-non-binary-in-usr-lib
$ rpmlint Download/yum-presto-0.3.4-1.src.rpm 
silent. 


Comment 10 Tim Lauridsen 2007-04-03 17:54:51 UTC
The spec file in the SRPM is ok.

APPROVED.


Comment 11 Jonathan Dieter 2007-04-03 18:03:21 UTC
Thanks!  The spec file has been changed on the server, but it may take a refresh
to come through.

Is there anything I should be doing to get a sponsor's attention, or should I
just wait patiently?

Comment 12 Tim Lauridsen 2007-04-03 18:09:30 UTC
You should just wait for a sponsor to pop up. :-)

Comment 13 Jonathan Dieter 2007-04-03 18:14:50 UTC
Sweet!  Thanks for the review!  I'll just wait and hope someone pops up.

Comment 14 Kevin Fenzi 2007-04-04 04:50:01 UTC
Hey Jonathan. I'd be happy to sponsor you. 

Thanks for the nice review here Tim. 

I do see a build failure on x86_64 however... 

Processing files: yum-presto-0.3.4-1.fc7
error: File not found by glob:
/var/tmp/yum-presto-0.3.4-1.fc7-root-mockbuild/usr/lib64/yum-plugins/presto.py*

Looks like: 
%{_libdir}/yum-plugins/presto.py*

needs to be: 

/usr/lib/yum-plugins/presto.py*

Can you make that change? I have a x86_64 test box if you need one. 


Comment 15 Tim Lauridsen 2007-04-04 05:32:49 UTC
(In reply to comment #14)
> Hey Jonathan. I'd be happy to sponsor you. 
> 
> Thanks for the nice review here Tim. 
> 
> I do see a build failure on x86_64 however... 
> 
> Processing files: yum-presto-0.3.4-1.fc7
> error: File not found by glob:
> /var/tmp/yum-presto-0.3.4-1.fc7-root-mockbuild/usr/lib64/yum-plugins/presto.py*
> 
> Looks like: 
> %{_libdir}/yum-plugins/presto.py*
> 
> needs to be: 
> 
> /usr/lib/yum-plugins/presto.py*
> 
> Can you make that change? I have a x86_64 test box if you need one. 
> 

It's right, it need to be '/usr/lib/yum-plugins/presto.py*', because yum look
for plugins in /usr/lib/yum-plugins/ for all archs.


Comment 16 Jonathan Dieter 2007-04-04 14:53:33 UTC
Thanks for pointing that out!  I really should have tested this on my wife's
x86_64 system.

A note though: I haven't mirrored FC6 x86_64 updates or extras yet, so you won't
get any deltarpms through it yet.

New release: 0.3.5-1
Spec URL: http://www.lesbg.com/jdieter/presto/yum-presto.spec
SRPM URL: http://www.lesbg.com/jdieter/presto/yum-presto-0.3.5-1.src.rpm




Comment 17 Kevin Fenzi 2007-04-04 16:40:44 UTC
Yeah, that looks good now here on x86_64... 

Are you looking for mirror space/BW for the x86_64 rpms? 
Or just haven't had time to generate them yet? Feel free to drop me an email 
about mirror space, I might be able to scare some up depending on what you need. 

Please continue the process from: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b

Feel free to email me if you have any questions or problems, or find me on irc: 
nirik on #fedora-devel on irc.freenode.net. 

Comment 18 Rex Dieter 2007-04-04 17:55:06 UTC
NOTE: (Just in case if it wasn't obvious or clear)  When/if imported into
Extras, the default config(s) must not point to repos/servers outside of the
(usual) fedoraproject infrastructure.  

Comment 19 Jonathan Dieter 2007-04-04 18:04:17 UTC
Thank you for pointing that out.  I hadn't thought about it, but I guess it
should have been obvious to me.

When/if we get presto into extras, I don't think we'll point it at any
repositories.  Rather, it will be up to the repository owners (or the user) to
add the deltaurl line to their .repo files.

Thanks for the heads up.

Comment 20 Kevin Fenzi 2007-04-04 18:08:36 UTC
Totally agreed. 

Jonathan: Can you make sure the version you check in has a commented presto.conf? 
ie, so it doesn't enable anything by default and the user must enable it, or
wait until it's enabled in their repo files?

Comment 21 Jonathan Dieter 2007-04-04 18:24:15 UTC
It shouldn't be hard to comment the conf file.  The plugin will be enabled, but
there won't be any presto repositories in the conf file (as there are now).  If
there aren't any presto repositories in the conf file and none in the user's
.repo files, then it's just as if the plugin is disabled.

I had never originally planned on having the conf file contain repository
information at all.  It just turned out to be an easy feature to add and one
that was invaluable during testing.

I'm hoping that, as interest in yum-presto grows, Fedora Infrastructure will
start hosting deltarpms and modify their .repo files accordingly (the deltaurl
option will be ignored by yum if yum-presto isn't installed).

So, to reiterate, the conf file will not point to *any* repositories.  If
there's anything else you would like me to do with this, let me know.

Jonathan

Comment 22 Jonathan Dieter 2007-04-05 04:04:59 UTC
New Package CVS Request
=======================
Package Name: yum-presto
Short Description: Deltarpm plugin for yum
Owners: jdieter@gmail.com
Branches: FC-6
InitialCC: 

Comment 23 Warren Togami 2007-04-05 04:22:39 UTC
CVS is approved, please wait about 10 minutes before checking in.


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