Bug 618761

Summary: Review request: gold - Allocation manager
Product: [Fedora] Fedora Reporter: Jessica Jones <fedora>
Component: Package ReviewAssignee: Mark Chappell <tremble>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, tremble
Target Milestone: ---Flags: tremble: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-01-13 10:02:44 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Comment 1 Jessica Jones 2010-07-27 17:00:24 UTC
Gold is an open source accounting system developed by Pacific Northwest
National Laboratory (PNNL) as part of the Department of Energy (DOE) Scalable
Systems Software Project (SSS). It tracks resource usage on High Performance
Computers and acts much like a bank, establishing accounts in order to
pre-allocate user and project resource usage over specific nodes and
timeframe. Gold provides balance and usage feedback to users, managers, and
system administrators.  SQLite is used by default, but Gold can be configured
to use either MySQL or PostgreSQL instead.

There is also a web interface that provides feedback and allows management in a more 'usable' format.  This and the documentation are packaged separately.

Comment 2 Jessica Jones 2010-07-27 17:04:30 UTC
I have also asked the upstream maintainer to make the changes I made via the two patches:

http://www.supercluster.org/pipermail/gold-users/2010-July/000343.html

Comment 3 Mark Chappell 2010-07-28 09:08:37 UTC
 - = N/A
 / = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [/] Package is named according to the Package Naming Guidelines.
 [/] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [/] Package meets the Packaging Guidelines including the Perl specific items
  [/] Versioned MODULE_COMPAT_ Requires
  [-] Non-Versioned CPAN URL tag
 [/] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested: http://koji.fedoraproject.org/koji/taskinfo?taskID=2355956
 [!] Rpmlint output:

gold.noarch: W: spelling-error %description -l en_US pre -> per, ore, pee
gold.noarch: W: spelling-error %description -l en_US timeframe -> time frame, time-frame, timeshare
gold.noarch: W: file-not-utf8 /usr/share/doc/gold-2.1.12.2/LICENSE
gold.noarch: W: non-conffile-in-etc /etc/goldg.conf
gold.noarch: W: file-not-utf8 /usr/share/doc/gold-2.1.12.2/README
gold.noarch: W: non-conffile-in-etc /etc/gold.conf
gold.noarch: W: non-conffile-in-etc /etc/goldd.conf
gold.noarch: W: no-manual-page-for-binary gmkaccount
... Snip many similar messages ...
gold.noarch: W: no-manual-page-for-binary glsproject
gold.src: W: spelling-error %description -l en_US pre -> per, ore, pee
gold.src: W: spelling-error %description -l en_US timeframe -> time frame, time-frame, timeshare
gold.src:96: W: macro-in-comment %{buildroot}
gold.src:96: W: macro-in-comment %{_sysconfdir}
gold.src:98: W: macro-in-comment %{SOURCE1}
gold.src:98: W: macro-in-comment %{buildroot}
gold.src:98: W: macro-in-comment %{_sysconfdir}
gold-doc.noarch: W: file-not-utf8 /usr/share/doc/gold-2.1.12.2/LICENSE
gold-doc.noarch: W: file-not-utf8 /usr/share/doc/gold-doc-2.1.12.2/README
gold-doc.noarch: W: file-not-utf8 /usr/share/doc/gold-doc-2.1.12.2/LICENSE
gold-doc.noarch: W: file-not-utf8 /usr/share/doc/gold-2.1.12.2/README
gold-web.noarch: W: spelling-error %description -l en_US frontend -> fronted, front end, front-end
gold-web.noarch: W: file-not-utf8 /usr/share/doc/gold-web-2.1.12.2/README
gold-web.noarch: W: file-not-utf8 /usr/share/doc/gold-web-2.1.12.2/LICENSE
4 packages and 0 specfiles checked; 0 errors, 62 warnings.

 [/] Package is not relocatable.
 [/] Buildroot is correct  ( Not needed if >= EL6 and >= F13 )
     Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 [/] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [/] License field in the package spec file matches the actual license. 
     License type: BSD (http://lists.fedoraproject.org/pipermail/legal/2010-July/001338.html)
 [/] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
 [/] With any Subpackage installed the license must also be installed (this may belong to another subpackage) 
 [!] Spec file is legible and written in American English.
 [/] Sources used to build the package matches the upstream source, as provided in the spec URL.
     d2cd0943ea4d574f7c101510ad11d02d  gold-2.1.12.2.tar.gz
     d2cd0943ea4d574f7c101510ad11d02d  SOURCES/gold-2.1.12.2.tar.gz

 [/] Package is not known to require ExcludeArch
 [/] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [/] Package must own all directories that it creates.
 [/] Package requires other packages for directories it uses.
 [!] Package does not contain duplicates in %files.
 [/] Permissions on files are set properly.
 [/] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT. ( Not needed if >= EL6 and >= F13 )
 [/] Package consistently uses macros.
 [/] Package contains code, or permissible content.
 [/] Large documentation files are in a -doc subpackage, if required.
 [/] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -static subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [/] Fully versioned dependency in subpackages, if present.
 [/] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [/] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [/] Latest version is packaged.
 [/] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [/] Reviewer should test that the package builds in mock.
     Tested on:  http://koji.fedoraproject.org/koji/taskinfo?taskID=2355956
 [/] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: fedora-rawhide (noarch)
 [?] Package functions as described.
 [/] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [-] %check is present and the tests pass

=== COMMENTS ===

Only the LICENSE file should be included in all the %doc stansas. (the rest should be based on what's appropriate, normally just the main package)

While the spec file is legible, it's supposed to be en_US 
timeframe -> time frame, time-frame

file-not-utf8 (LICENSE, README)
# standard fix (preserving timestamps)
iconv -f iso8859-1 -t utf8 README >README.utf8
touch -r README README.utf8
mv README.utf8 README 

Ignore the lack of man-page messages, upstream don't provide them and there is other documentation.

non-conffile-in-etc (/etc/gold(|g|d).conf
- Use the %conf macro

macro-in-comment
- Ignore, 

Is there a make test rule (that doesn't require a database)?  If so it *should* be used.

It's worth running a recent rpmlint over all the files before submitting, the EL-5 rpmlint is rather out of date.

Comment 4 Jessica Jones 2010-07-28 09:45:08 UTC
(In reply to comment #3)
<snip>
>  [!] Package does not contain duplicates in %files.

<snip>
 
> === COMMENTS ===
> 
> Only the LICENSE file should be included in all the %doc stansas. (the rest
> should be based on what's appropriate, normally just the main package)
> 
> While the spec file is legible, it's supposed to be en_US 
> timeframe -> time frame, time-frame

Is there some way to check this sort of thing (I am finding conflicting sources)?  I copied the description from the website, which is in US English as far as I know.

> file-not-utf8 (LICENSE, README)
> # standard fix (preserving timestamps)
> iconv -f iso8859-1 -t utf8 README >README.utf8
> touch -r README README.utf8
> mv README.utf8 README 

Do you want me to add that to the spec file?

> Ignore the lack of man-page messages, upstream don't provide them and there is
> other documentation.
>
> non-conffile-in-etc (/etc/gold(|g|d).conf
> - Use the %conf macro
> 
> macro-in-comment
> - Ignore, 
> 

Okay will do.

> Is there a make test rule (that doesn't require a database)?  If so it *should*
> be used.

No there isn't.  I already looked into this, and there isn't a make check either.  I have been talking to the upstream maintainer off-list about this and other issues, so it may appear later.

> It's worth running a recent rpmlint over all the files before submitting, the
> EL-5 rpmlint is rather out of date.    

It would be nice if the people.fedoraproject.org servers had the koji client and rpmlint (and rpmbuild for creating SRPMs) on them.  I will have to spend some time setting up a VM so that I can use it to build otherwise.

Comment 5 Mark Chappell 2010-07-28 09:58:44 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > While the spec file is legible, it's supposed to be en_US 
> > timeframe -> time frame, time-frame
> 
> Is there some way to check this sort of thing (I am finding conflicting
> sources)?  I copied the description from the website, which is in US English as
> far as I know.

I rely on RPMlint on the whole, if you've an online US dictionary contradicting this don't worry too much.

> > file-not-utf8 (LICENSE, README)
> 
> Do you want me to add that to the spec file?

Yes, sorry if that wasn't clear.
 
> > It's worth running a recent rpmlint over all the files before submitting, the
> > EL-5 rpmlint is rather out of date.    
> 
> It would be nice if the people.fedoraproject.org servers had the koji client
> and rpmlint (and rpmbuild for creating SRPMs) on them.  I will have to spend
> some time setting up a VM so that I can use it to build otherwise.    

rpmbuild is a deliberate omission, and koji would require your ssh private keys.  Personally I think it would be useful to add rpmlint to the post build scripts in koji...

Comment 6 Jessica Jones 2010-07-28 10:35:22 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > While the spec file is legible, it's supposed to be en_US 
> > > timeframe -> time frame, time-frame
> > 
> > Is there some way to check this sort of thing (I am finding conflicting
> > sources)?  I copied the description from the website, which is in US English as
> > far as I know.
> 
> I rely on RPMlint on the whole, if you've an online US dictionary contradicting
> this don't worry too much.

I have made the changes suggested by rpmlint, although I did not change 'pre'.  I'm finding all of those suggestions are fine according to the online US dictionary, but nevermind.

> > > file-not-utf8 (LICENSE, README)
> > 
> > Do you want me to add that to the spec file?
> 
> Yes, sorry if that wasn't clear.

Okay, done.

> > > It's worth running a recent rpmlint over all the files before submitting, the
> > > EL-5 rpmlint is rather out of date.    
> > 
> > It would be nice if the people.fedoraproject.org servers had the koji client
> > and rpmlint (and rpmbuild for creating SRPMs) on them.  I will have to spend
> > some time setting up a VM so that I can use it to build otherwise.    
> 
> rpmbuild is a deliberate omission, and koji would require your ssh private
> keys.  Personally I think it would be useful to add rpmlint to the post build
> scripts in koji...    

True.  I forget that koji uses ssh keys and not kerberos.

I have updated the spec file and SRPM on my people.fedoraproject space (so same URL as before).  Please could you check again?

Comment 7 Jessica Jones 2010-07-28 10:44:25 UTC
Sorry, as release was incremented, the SRPM changed name.

SRPM: http://people.fedoraproject.org/~zaniyah/gold/gold-2.1.12.2-2.src.rpm

Comment 8 Mark Chappell 2010-07-28 14:20:44 UTC
* The LICENSE file also needs the iconv treatment
* Sorry should have been %config(noreplace) not simply %config

Comment 9 Jessica Jones 2010-07-29 07:53:52 UTC
(In reply to comment #8)
> * The LICENSE file also needs the iconv treatment

Oops, was trying to be quick.

> * Sorry should have been %config(noreplace) not simply %config    

Fixed.

Comment 10 Mark Chappell 2010-07-29 09:15:46 UTC
All blocking issues appear to have been fixed, and no further issues introduced.

You should possibly add a README.Fedora file explaining how to setup the DB and get up and running, as well as an example httpd config file.

In future please bump the release and add changelog entries each time you submit for reconsideration.

APPROVED

Comment 11 Mamoru TASAKA 2010-07-29 09:37:18 UTC
I guess not fedora-cvs+ but fedora-review+.

Comment 12 Mark Chappell 2010-07-29 09:50:01 UTC
Good point

Comment 13 Mamoru TASAKA 2010-08-05 15:46:16 UTC
Jessica, please write git request and set fedora-cvs flag.

Comment 14 Jessica Jones 2010-08-10 12:58:41 UTC
New Package SCM Request
=======================
Package Name:  gold
Short Description:  allocation manager
Owners:  zaniyah
Branches:  f13 f14 el6
InitialCC:  zaniyah

Comment 15 Kevin Fenzi 2010-08-10 17:58:07 UTC
Is this package 'gold' or 'Gold'? 
The review bug summary says "Gold", but the package spec/src.rpm is 'gold'. 

Can you fix the summary and re-request, or fix the package and rerequest?

Comment 16 Jessica Jones 2010-08-11 10:44:20 UTC
(In reply to comment #15)
> Is this package 'gold' or 'Gold'? 
> The review bug summary says "Gold", but the package spec/src.rpm is 'gold'. 
> 
> Can you fix the summary and re-request, or fix the package and rerequest?    

I don't understand what the problem is here?  Many packages have names in various cases, for example, TORQUE, where the rpm is called 'torque' but the software is actually 'TORQUE'.

Comment 17 Mamoru TASAKA 2010-08-11 11:06:30 UTC
Kevin is asking in which name you want to have this software registered
on Fedora package database, and to clarify this is asking to fix
either the summary of _this bugzilla's summary_ (i.e. to change
the summary of this bugzilla entry to 
"Review request: gold - Allocation Manager", or to change the 
package name in SCM request.

Comment 18 Jessica Jones 2010-08-12 09:40:00 UTC
(In reply to comment #17)
> Kevin is asking in which name you want to have this software registered
> on Fedora package database, and to clarify this is asking to fix
> either the summary of _this bugzilla's summary_ (i.e. to change
> the summary of this bugzilla entry to 
> "Review request: gold - Allocation Manager", or to change the 
> package name in SCM request.    

Ah I understand now, thanks.  I did not know there was a requirement for that.  I had assumed that package name == rpm name, rather than software name.  It is not at all clear what most of the fields are from the wiki page on review requests.

I will submit a new SCM request.

Comment 19 Jessica Jones 2010-08-12 09:41:10 UTC
New Package SCM Request
=======================
Package Name:  Gold
Short Description:  allocation manager
Owners:  zaniyah
Branches:  f13 f14 el5 el6
InitialCC:  zaniyah

Comment 20 Jessica Jones 2010-08-12 09:42:51 UTC
I assume that this is the right way around?  The software is called "Gold Allocation Manager" on its home page, and usually referred to as just 'Gold', but the RPM is called 'gold'.  I have assumed from what has been said that the SCM request package name should match the software name, but please let me know if this is not the case.

Comment 21 Mamoru TASAKA 2010-08-12 09:47:34 UTC
Ah, if the srpm name is "gold", SCM request should be filed with
"Package Name: gold" and the summary of this bugzilla should be
changed to "Review request: gold - <short summary>".

At the top of this bugzilla page, you can see
"Bug 618761 - Review request: Gold Allocation Manager for HPC  (edit) ".
Click "edit" and change the summary of this bugzilla.

Comment 22 Jessica Jones 2010-08-12 09:53:50 UTC
(In reply to comment #21)
> Ah, if the srpm name is "gold", SCM request should be filed with
> "Package Name: gold" and the summary of this bugzilla should be
> changed to "Review request: gold - <short summary>".
> 
> At the top of this bugzilla page, you can see
> "Bug 618761 - Review request: Gold Allocation Manager for HPC  (edit) ".
> Click "edit" and change the summary of this bugzilla.    

Thanks, that makes it much easier.  I have changed the summary of this request.  Do I need to fill out the SCM thing again?

Comment 23 Mamoru TASAKA 2010-08-12 12:06:34 UTC
(In reply to comment #22)
>  Do I need to fill out the SCM thing again?    

Yes, please.

Comment 24 Jason Tibbitts 2010-08-12 12:10:57 UTC
The last scm request you filed needs to match what you want, yes.  Currently,
you're requesting "Gold" but the summary says "gold", so the tool we use just
ends up complaining that they don't match.  To save you time, I'll just fix it
up.

I also note that "Allocation manager" is a rather poor package description; I
didn't check the SRPM, but I hope it's better there (or else it shouldn't have
passed review).

Git done (by process-git-requests).

Comment 25 Jessica Jones 2010-08-12 12:17:50 UTC
(In reply to comment #24)
> The last scm request you filed needs to match what you want, yes.  Currently,
> you're requesting "Gold" but the summary says "gold", so the tool we use just
> ends up complaining that they don't match.  To save you time, I'll just fix it
> up.

Thanks.

> I also note that "Allocation manager" is a rather poor package description; I
> didn't check the SRPM, but I hope it's better there (or else it shouldn't have
> passed review).

No, it is not the description, only the summary.

> Git done (by process-git-requests).    

Thanks

Comment 26 Jason Tibbitts 2010-08-12 12:36:27 UTC
Sorry, of course I meant "summary" where I wrote "description" above.

Comment 27 Jessica Jones 2010-08-17 09:34:35 UTC
(In reply to comment #26)
> Sorry, of course I meant "summary" where I wrote "description" above.

Ok, changed.

Comment 28 Mark Chappell 2014-01-13 10:02:44 UTC
Long since reviewed and built.