Bug 226353 - Merge Review: quota
Summary: Merge Review: quota
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:47 UTC by Nobody's working on this, feel free to take it
Modified: 2009-04-09 14:42 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-09 14:42:03 UTC
Type: ---
Embargoed:
gwync: fedora-review+


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

Description Nobody's working on this, feel free to take it 2007-01-31 20:47:49 UTC
Fedora Merge Review: quota

http://cvs.fedora.redhat.com/viewcvs/devel/quota/
Initial Owner: steved

Comment 1 Gwyn Ciesla 2008-01-23 13:43:40 UTC
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 18:20:42 UTC
Fixed in quota-3.15-1.fc9

Comment 3 Gwyn Ciesla 2008-01-24 18:30:57 UTC
The Source0 was fixed, the others remain.

Comment 4 Steve Dickson 2008-01-24 19:13:39 UTC
> 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 19:19:32 UTC
IMHO there has to be -devel as long there are header files around - Jon, same 
opinion?

Comment 6 Gwyn Ciesla 2008-01-24 19:41:41 UTC
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 11:41:56 UTC
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 Gwyn Ciesla 2008-01-25 12:16:04 UTC
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 Gwyn Ciesla 2008-01-25 12:17:25 UTC
If you'd like me to create a patch for the spec to create the subpackage, I'm
willing to do so. . .

Comment 10 Gwyn Ciesla 2008-01-25 12:28:56 UTC
And actually, a seperate -devel package would actually mean *less* bloat for end
users. . .

Comment 11 Gwyn Ciesla 2008-01-25 12:55:07 UTC
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 Gwyn Ciesla 2008-01-25 13:04:54 UTC
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 18:21:39 UTC
> 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 18:42:02 UTC
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 18:55:12 UTC
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 Gwyn Ciesla 2008-01-25 20:24:18 UTC
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 21:08:07 UTC
(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 22:58:02 UTC
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 Gwyn Ciesla 2008-01-28 12:17:11 UTC
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 14:02:35 UTC
+1 on sending this issue to the packaging committee... 

Comment 21 Gwyn Ciesla 2008-02-05 19:28:40 UTC
No response that I've seen from the Packaging Committee.  Anyone else hear
anythin g off-list?

Comment 22 Steve Dickson 2008-02-05 22:01:26 UTC
Add the new Fedora Quota maintainer...  Ondřej Vašík 

Comment 23 Gwyn Ciesla 2008-05-16 15:05:43 UTC
Any updates?

Comment 24 Gwyn Ciesla 2008-09-09 17:07:38 UTC
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 10:34:49 UTC
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 Gwyn Ciesla 2008-09-10 13:41:10 UTC
Sounds good.  Do that, and we're set.

Comment 27 Patrice Dumas 2008-09-10 16:36:07 UTC
(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 Gwyn Ciesla 2008-09-10 16:48:30 UTC
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 16:59:25 UTC
Unless I am misreading Comment #25 the files are already in glibc but not shipped.

Comment 30 Gwyn Ciesla 2008-09-10 17:03:57 UTC
Sorry, I misunderstood.  That's a whole different barrel of cod, then. 

CCing glibc maintainer.

Jakub, what do you think?

Comment 31 Gwyn Ciesla 2008-12-04 19:36:10 UTC
Ping?

Comment 32 Robert Scheck 2008-12-20 14:41:59 UTC
Jakub, Roland - ping?

Comment 33 Gwyn Ciesla 2009-03-31 15:26:48 UTC
Ping again?

Comment 34 Michal Nowak 2009-04-09 11:48:27 UTC
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 13:37:56 UTC
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 14:08:44 UTC
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 Gwyn Ciesla 2009-04-09 14:17:26 UTC
Works for me. . .

Comment 38 Ondrej Vasik 2009-04-09 14:29:58 UTC
(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> 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 Gwyn Ciesla 2009-04-09 14:42:03 UTC
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.