Bug 226353 - Merge Review: quota
Merge Review: quota
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jon Ciesla
Fedora Package Reviews List
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 15:47 EST by Nobody's working on this, feel free to take it
Modified: 2009-04-09 10:42 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-09 10:42:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑review+


Attachments (Terms of Use)
Patch for -devel package, licanse tag, duplicate files (1.50 KB, application/octet-stream)
2008-01-25 08:04 EST, Jon Ciesla
no flags Details

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:47:49 EST
Fedora Merge Review: quota

http://cvs.fedora.redhat.com/viewcvs/devel/quota/
Initial Owner: steved@redhat.com
Comment 1 Jon Ciesla 2008-01-23 08:43:40 EST
rpmlint:

Uses BuiltPreReq.  Not allowed under current policy, not sure what the
workaround is.

Macro in changelog, change %attr to %%attr in lines 281 and 311.

Summary ends with dot.  Remove it.

Patch3 is applied in an ifarch block, this is OK.

Should /usr/include/rpcsvc/rquota.h be in a -devel package?

Other blockers:

License tag should be BSD, GPLv2+.

Replace:
Source0:
http://unc.dl.sourceforge.net/sourceforge/linuxquota/quota-%{version}.tar.gz
with:
Source0: http://downloads.sourceforge.net/linuxquota/%{name}-%{version}.tar.gz

Otherwise, it looks OK.
Comment 2 Steve Dickson 2008-01-24 13:20:42 EST
Fixed in quota-3.15-1.fc9
Comment 3 Jon Ciesla 2008-01-24 13:30:57 EST
The Source0 was fixed, the others remain.
Comment 4 Steve Dickson 2008-01-24 14:13:39 EST
> Macro in changelog, change %attr to %%attr in lines 281 and 311.
Done.

> Summary ends with dot.  Remove it.
Done.

> License tag should be BSD, GPLv2+.
Done

> Should /usr/include/rpcsvc/rquota.h be in a -devel package?
Probably but there is not a -devel patch and there is 
no need of a -devel package. So I would suggest to these
it as is... 

Fixed in  quota-3.15-2.fc9


Comment 5 Robert Scheck 2008-01-24 14:19:32 EST
IMHO there has to be -devel as long there are header files around - Jon, same 
opinion?
Comment 6 Jon Ciesla 2008-01-24 14:41:41 EST
I concur.  A devel package would be what I would expect to have to install to
complie against quota's libraries, and it's not used at runtime, so I think
splitting -devel off would be good.
Comment 7 Steve Dickson 2008-01-25 06:41:56 EST
Are you Serious? You want me to create a whole package for  just one header
file that may or may not be used?? That seems a bit ridiculous.. imho...

If there was an actual library then creating a -devel package would be
no brainer. But why added to the package gloat that Fedora already
suffers from?


Comment 8 Jon Ciesla 2008-01-25 07:16:04 EST
Well, the alternative would be to drop that file.  If it's not used, then that's
even less bloat.  Does anything BuildRequire quota as it is?
Comment 9 Jon Ciesla 2008-01-25 07:17:25 EST
If you'd like me to create a patch for the spec to create the subpackage, I'm
willing to do so. . .
Comment 10 Jon Ciesla 2008-01-25 07:28:56 EST
And actually, a seperate -devel package would actually mean *less* bloat for end
users. . .
Comment 11 Jon Ciesla 2008-01-25 07:55:07 EST
I also misadvised you on the license tag.  It should be:
License: BSD and GPLv2+

You also have duplicate file listed, which I thought I mentioned but neglected to. 

You can replace:

%config(noreplace) %{_sysconfdir}/warnquota.conf
%config(noreplace) %{_sysconfdir}/quotatab
%config(noreplace) %{_sysconfdir}/quotagrpadmins
%attr(0644,root,root) %{_sysconfdir}/*

with:

%config(noreplace) %attr(0644,root,root) %{_sysconfdir}/*
Comment 12 Jon Ciesla 2008-01-25 08:04:54 EST
Created attachment 292939 [details]
Patch for -devel package, licanse tag, duplicate files

Patch for -devel package, licanse tag, duplicate files
Comment 13 Steve Dickson 2008-01-25 13:21:39 EST
> License: BSD and GPLv2+
Done.

> You can replace:
> %config(noreplace) %{_sysconfdir}/warnquota.conf
> %config(noreplace) %{_sysconfdir}/quotatab
> %config(noreplace) %{_sysconfdir}/quotagrpadmins
> %attr(0644,root,root) %{_sysconfdir}/*
> with:
> %config(noreplace) %attr(0644,root,root) %{_sysconfdir}/*
Done.


Its not the point of how to create a -devel package (thats 
easy) its point why should one be created. I still think its a waste
to create a one file package where that may or may not be used and/or
valid.

So as the maintainer of this package I am going respectfully deny 
that request.  
Comment 14 Robert Scheck 2008-01-25 13:42:02 EST
Is it possible to remove that file and to avoid shipping (if nothing depends
on it, of course)? Jon, another maybe more acceptable deal could be to provide 
quota-devel as virtual package, which will make a later shipping of a real 
separate package more easily...
Comment 15 Patrice Dumas 2008-01-25 13:55:12 EST
All the other rpc headers are in glibc-headers. I think that 
this one should be there too.

Otherwise it seems to me that this file is useful, that it is
not so important to have it in a separate devel package 
and that a virtual provides for it should be required, although
it is not clear that quota-devel is what corresponds with these 
files (also rquota.x), maybe quota-headers, or even rquota-headers
would be more appropriate in my opinion.
Comment 16 Jon Ciesla 2008-01-25 15:24:18 EST
I get the impression that if quota-devel is made a virtual package, the real
package will never happen.  Of course, the real quota-devel would require the
main package anyway, so anyone installing -devel wouldn't notice.

How would we move this header to the glibc package?  Unless I misunderstand,
that sounds unnecessarily awkward.  I'm more in favor of the virtual package route.

I'd be ok with the virtual package, but my preference is still the -devel
subpackage.  I guess I'm having a hard time understanding your objection to
that.  Sure, it's an extra package, but it's the same SRPM, and no real extra
maintenance.  It would reduce bloat for end users, and have negligible effect on
developers.
Comment 17 Patrice Dumas 2008-01-25 16:08:07 EST
(In reply to comment #16)
> I get the impression that if quota-devel is made a virtual package, the real
> package will never happen.  Of course, the real quota-devel would require the
> main package anyway, so anyone installing -devel wouldn't notice.

There is no reason why a package containing solely
/usr/include/rpcsvc/rquota.h
/usr/include/rpcsvc/rquota.x
should require the main package. Once again it is not a library API.

> How would we move this header to the glibc package?  Unless I misunderstand,
> that sounds unnecessarily awkward.  I'm more in favor of the virtual package
route.

Headers without library are better in glibc, when they describe 
rpc services. I guess this implies discussing with glibc people.
Look at the files 
/usr/include/rpcsvc/*.x

> I'd be ok with the virtual package, but my preference is still the -devel
> subpackage.  I guess I'm having a hard time understanding your objection to
> that.  Sure, it's an extra package, but it's the same SRPM, and no real extra
> maintenance.  It would reduce bloat for end users, and have negligible effect on
> developers.

It is not a devel header like other devel headers that describes
an api. It describes rpc messages, and in my opinion the deserves
specific treatement.
Comment 18 Patrice Dumas 2008-01-25 17:58:02 EST
As a side note, ypserv also puts
/usr/include/rpcsvc/ypxfrd.x
in here. No other packages (other than glibc-headers) put .x files
in this directory.
Comment 19 Jon Ciesla 2008-01-28 07:17:11 EST
Since it's not an API header (which I did not realize), I agree that something
besides a -devel package is more appropriate.  I'll be curious to see the
responses to your message to the packaging committee.  Once this is resolved,
I'm comfortable approving.
Comment 20 Steve Dickson 2008-01-28 09:02:35 EST
+1 on sending this issue to the packaging committee... 
Comment 21 Jon Ciesla 2008-02-05 14:28:40 EST
No response that I've seen from the Packaging Committee.  Anyone else hear
anythin g off-list?
Comment 22 Steve Dickson 2008-02-05 17:01:26 EST
Add the new Fedora Quota maintainer...  Ondřej Vašík 
Comment 23 Jon Ciesla 2008-05-16 11:05:43 EDT
Any updates?
Comment 24 Jon Ciesla 2008-09-09 13:07:38 EDT
Reviewed current version.

quota.i386: W: symlink-should-be-relative /usr/share/man/man8/quotaoff.8.gz /usr/share/man/man8/quotaon.8.gz
Absolute symlinks are problematic eg. when working with chroot environments.

quota.i386: W: incoherent-version-in-changelog 3.16-4 1:3.16-4.fc9
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

Other than that, we're down to the rquota.h in or not in -devel issue.  Ondrej, any thoughts?
Comment 25 Ondrej Vasik 2008-09-10 06:34:49 EDT
Thanks for review, fixed both objections from rpmlint ...

About that devel subpackage - actually those RPC headers are in glibc tarball but they are not shipped. It is quite ancient thing and I'm not sure why are they intentionally not shipped by glibc. Actually I'm ok with devel subpackage, I would ship the man3 directory in it as well - as documentation for that subpackage should be inside that subpackage. Built as quota-3.16-5.fc10 in rawhide (http://koji.fedoraproject.org/koji/taskinfo?taskID=817361).
Comment 26 Jon Ciesla 2008-09-10 09:41:10 EDT
Sounds good.  Do that, and we're set.
Comment 27 Patrice Dumas 2008-09-10 12:36:07 EDT
(In reply to comment #25)
> Thanks for review, fixed both objections from rpmlint ...
> 
> About that devel subpackage - actually those RPC headers are in glibc tarball
> but they are not shipped. It is quite ancient thing and I'm not sure why are
> they intentionally not shipped by glibc.

In that case it seems to me that the best way is to have them be shipped
by glibc.
Comment 28 Jon Ciesla 2008-09-10 12:48:30 EDT
As you've stated before.  But the Packaging Committee was silent.  In lieu of that, how do propose to accomplish that?  Add another Source tag to glibc's SRPM, and rebuild glibc whenever quota is updated?
Comment 29 Patrice Dumas 2008-09-10 12:59:25 EDT
Unless I am misreading Comment #25 the files are already in glibc but not shipped.
Comment 30 Jon Ciesla 2008-09-10 13:03:57 EDT
Sorry, I misunderstood.  That's a whole different barrel of cod, then. 

CCing glibc maintainer.

Jakub, what do you think?
Comment 31 Jon Ciesla 2008-12-04 14:36:10 EST
Ping?
Comment 32 Robert Scheck 2008-12-20 09:41:59 EST
Jakub, Roland - ping?
Comment 33 Jon Ciesla 2009-03-31 11:26:48 EDT
Ping again?
Comment 34 Michal Nowak 2009-04-09 07:48:27 EDT
Jakub, what you think of delivering

/usr/include/rpcsvc/rquota.h
/usr/include/rpcsvc/rquota.x

files inside glibc package and not here with quota? Since it seems that glibc tarball includes them but we are intentionally not shipping them.
Comment 35 Ondrej Vasik 2009-04-09 09:37:56 EDT
Just want to mention few differences:

glibc:
afaik no manpage
rquota.x size 1569

quota:
manpage rquota.3
rquota.x size 3470

There are several things added in rquota.x shipped with quota...
Comment 36 Michal Nowak 2009-04-09 10:08:44 EDT
Good point! So, let's create -devel and move rquota.x there, since you're not objecting it (comment #25), and close this nice review soon.
Comment 37 Jon Ciesla 2009-04-09 10:17:26 EDT
Works for me. . .
Comment 38 Ondrej Vasik 2009-04-09 10:29:58 EDT
(In reply to comment #36)
> Good point! So, let's create -devel and move rquota.x there, since you're not
> objecting it (comment #25), and close this nice review soon. 

Already done for quite a long time ;) ...

* Wed Sep 10 2008 Ondrej Vasik <ovasik@redhat.com> 1:3.16-5
- fix rpmlint warnings - absolute symlink and not using epoch
  in version in changelog (#226353)
- rquota headers and manpage now in devel subpackage
Comment 39 Jon Ciesla 2009-04-09 10:42:03 EDT
Might've been good to've mentioned that.  :)

APPROVED.

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