Hi, Steve,

Issues:
-------
rpmlint produces some errors on the source rpm:

hiera.src: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging
hiera.src: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging
hiera.src:2: E: hardcoded-library-path in /usr/lib/ruby/site_ruby/1.8}

Maybe http://fedoraproject.org/wiki/Packaging:Ruby#ruby_applications is useful.
These guidelines also specify that ruby applications should be installed into %{_datadir}

Review:
-------
[x]  Package is named according to the Package Naming Guidelines. 
[x]  Spec file name must match the base package %{name}, in the format %{name}.spec.
[x]  Spec file is legible and written in American English.
[x]  Spec file lacks Packager, Vendor, PreReq tags.
[!]  Spec uses macros instead of hard-coded directory names.
[!]  Package consistently uses macros.
[x]  Macros in Summary, %description expandable at SRPM build time.
[x]  PreReq is not used.
[x]  Requires correct, justified where necessary.
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. 
[x]  Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)).
[x]  Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the beginning of %install.
[x]  Package use %makeinstall only when ``make install DESTDIR=...'' doesn't work.
[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[-]  The spec file handles locales properly.
[x]  Changelog in prescribed format.
[-]  Rpmlint output is silent.
[x]  License field in the package spec file matches the actual license.
[x]  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.
[-]  License file installed when any subpackage combination is installed.
[x]  Sources contain only permissible code or content.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
     MD5SUM this package     : 429f5bb30834183b3a3b7237e636364b
     MD5SUM upstream package : 429f5bb30834183b3a3b7237e636364b
[-]  Compiler flags are appropriate.
Nothing to compile
[-]  ldconfig called in %post and %postun if required.
[x]  Package must own all directories that it creates.
[x]  Package does not own files or directories owned by other packages.
[x]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.
[x]  Each %files section contains %defattr.
[x]  No %config files under /usr.
[x]  %config files are marked noreplace or the reason is justified.
[-]  Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application. 
[-]  Package contains a valid .desktop file.
[x]  Package contains code, or permissable content.
[-]  Package contains a SysV-style init script if in need of one.
[x]  File names are valid UTF-8.
[-]  Large documentation files are in a -doc subpackage, if required.
[x]  Package uses nothing in %doc for runtime.
[x]  Package contains no bundled libraries.
[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -static subpackage, if present.
[x]  Package contains no static executables.
[-]  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).
[-]  Useful -debuginfo package or justification otherwise.
package contains only scripts
[x]  Rpath absent or only used for internal libs.
[x]  Package does not generate any conflict.
[x]  Package does not contains kernel modules.
[x]  Package is not relocatable.
[x]  Package successfully compiles and builds into binary rpms on at least one supported architecture.
[x]  Package is not known to require ExcludeArch.
[x]  Package installs properly.
[x]  Package obeys FHS, except libexecdir and /usr/target.
[x]  Package meets the Packaging Guidelines.

Hi(In reply to comment #1)
 
> hiera.src: W: spelling-error Summary(en_US) plugin -> plug in, plug-in,
> plugging

plugin is in very common usage I would say but I am happy to change
to plug-in. I will change at next package iteration or before import.

> hiera.src:2: E: hardcoded-library-path in /usr/lib/ruby/site_ruby/1.8}

Yes its hard coded but I've hard coded also the operating systems also
when I use the path. For non specified list of OSes, rhel>=7 and fc>=17 it
will use a variable %{ruby_vendorlibdir} as supplied by ruby-devel.

 
> Maybe http://fedoraproject.org/wiki/Packaging:Ruby#ruby_applications is
> useful.
> These guidelines also specify that ruby applications should be installed
> into %{_datadir}

I don't think this is an "application" as such but a non-gem package with
a single executable.

http://fedoraproject.org/wiki/Packaging:Ruby#Non-Gem_Packages

Other applications e.g puppet will run a 'require hiera'

So the library installs in '%{ruby_vendorlibdir}' which on rhel>=7
or fc>=17 is set to /usr/share/ruby/vendor_ruby  which is 
of course inside %{_datadir} of course.

But on fc<17 or rhel<17 then modules have been installed in
/usr/lib/ruby/site_ruby/1.8 as far as I can tell hence the hard code
of this variable.

Steve.



> Review:
> -------
> [x]  Package is named according to the Package Naming Guidelines. 
> [x]  Spec file name must match the base package %{name}, in the format
> %{name}.spec.
> [x]  Spec file is legible and written in American English.
> [x]  Spec file lacks Packager, Vendor, PreReq tags.
> [!]  Spec uses macros instead of hard-coded directory names.
> [!]  Package consistently uses macros.
> [x]  Macros in Summary, %description expandable at SRPM build time.
> [x]  PreReq is not used.
> [x]  Requires correct, justified where necessary.
> [x]  All build dependencies are listed in BuildRequires, except for any that
> are listed in the exceptions section of Packaging Guidelines. 
> [x]  Buildroot is correct
> (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)).
> [x]  Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the beginning
> of %install.
> [x]  Package use %makeinstall only when ``make install DESTDIR=...'' doesn't
> work.
> [x]  Package has a %clean section, which contains rm -rf %{buildroot} (or
> $RPM_BUILD_ROOT).
> [-]  The spec file handles locales properly.
> [x]  Changelog in prescribed format.
> [-]  Rpmlint output is silent.
> [x]  License field in the package spec file matches the actual license.
> [x]  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.
> [-]  License file installed when any subpackage combination is installed.
> [x]  Sources contain only permissible code or content.
> [x]  Sources used to build the package matches the upstream source, as
> provided in the spec URL.
>      MD5SUM this package     : 429f5bb30834183b3a3b7237e636364b
>      MD5SUM upstream package : 429f5bb30834183b3a3b7237e636364b
> [-]  Compiler flags are appropriate.
> Nothing to compile
> [-]  ldconfig called in %post and %postun if required.
> [x]  Package must own all directories that it creates.
> [x]  Package does not own files or directories owned by other packages.
> [x]  Package requires other packages for directories it uses.
> [x]  Package does not contain duplicates in %files.
> [x]  Permissions on files are set properly.
> [x]  Each %files section contains %defattr.
> [x]  No %config files under /usr.
> [x]  %config files are marked noreplace or the reason is justified.
> [-]  Package contains a properly installed %{name}.desktop using
> desktop-file-install file if it is a GUI application. 
> [-]  Package contains a valid .desktop file.
> [x]  Package contains code, or permissable content.
> [-]  Package contains a SysV-style init script if in need of one.
> [x]  File names are valid UTF-8.
> [-]  Large documentation files are in a -doc subpackage, if required.
> [x]  Package uses nothing in %doc for runtime.
> [x]  Package contains no bundled libraries.
> [-]  Header files in -devel subpackage, if present.
> [-]  Static libraries in -static subpackage, if present.
> [x]  Package contains no static executables.
> [-]  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).
> [-]  Useful -debuginfo package or justification otherwise.
> package contains only scripts
> [x]  Rpath absent or only used for internal libs.
> [x]  Package does not generate any conflict.
> [x]  Package does not contains kernel modules.
> [x]  Package is not relocatable.
> [x]  Package successfully compiles and builds into binary rpms on at least
> one supported architecture.
> [x]  Package is not known to require ExcludeArch.
> [x]  Package installs properly.
> [x]  Package obeys FHS, except libexecdir and /usr/target.
> [x]  Package meets the Packaging Guidelines.

I think I answered all your comments?

APPROVED


Sorry for the delay.

New Package SCM Request
=======================
Package Name: hiera
Short Description: A simple hierarchical database supporting plugin data sources
Owners: stevetraylen
Branches: f17 f16 el6 el5
InitialCC:

No f18 branch?

New Package SCM Request
=======================
Package Name: hiera
Short Description: A simple hierarchical database supporting plugin data sources
Owners: stevetraylen
Branches: f18 f17 f16 el6 el5
InitialCC:


Thanks.

Git done (by process-git-requests).

Added f18.

hiera-1.0.0-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/hiera-1.0.0-2.el6

hiera-1.0.0-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/hiera-1.0.0-2.fc17

hiera-1.0.0-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/hiera-1.0.0-2.el5

hiera-1.0.0-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/hiera-1.0.0-2.fc18

hiera-1.0.0-2.fc17 has been pushed to the Fedora 17 testing repository.

(In reply to comment #2)
> > Maybe http://fedoraproject.org/wiki/Packaging:Ruby#ruby_applications is
> > useful.
> > These guidelines also specify that ruby applications should be installed
> > into %{_datadir}
> 
> I don't think this is an "application" as such but a non-gem package with
> a single executable.
> 
> http://fedoraproject.org/wiki/Packaging:Ruby#Non-Gem_Packages
> 
> Other applications e.g puppet will run a 'require hiera'

In that case, the package should be named ruby-hiera [1] ...


[1] https://fedoraproject.org/wiki/Packaging:Ruby#Naming_Guidelines

And you should also follow https://fedoraproject.org/wiki/Packaging:Ruby#Ruby_ABI

Hi Vit,

I do not want to rename this package from hiera, its an application
where you can import that application into some other application.

If this was renamed then so should puppet for instance and mcollective
since you can use the backend of puppet as a library in your own application.

Are these packages also wrong?.. Maybe I am misunderstanding what a
#Non-Gem-Package is?

Re the ABI not sure where I got 
ruby(abi) >= 1.9.3
from, bizarre.

(In reply to comment #16)
> Hi Vit,
> 
> I do not want to rename this package from hiera, its an application
> where you can import that application into some other application.
> 
> If this was renamed then so should puppet for instance and mcollective
> since you can use the backend of puppet as a library in your own application.
> 
> Are these packages also wrong?. Maybe I am misunderstanding what a
> #Non-Gem-Package is?

Well, I agree that the application vs library might be a bit fuzzy. But if you take a look on Puppet, non of its files are installed into Ruby's %{ruby_vendorlibdir}, while all of hiera's files are installed there. In this case, it seems that hiera is more library then application and therefore it should have ruby- prefix.

May be it could be split into two packages? Something like hiera, which contains the executable and may be something more and the ruby-hiera, which would contain the library part? Not sure if that is not overkill though :)

(In reply to comment #17)
> Well, I agree that the application vs library might be a bit fuzzy. But if
> you take a look on Puppet, non of its files are installed into Ruby's
> %{ruby_vendorlibdir}, while all of hiera's files are installed there. In
> this case, it seems that hiera is more library then application and
> therefore it should have ruby- prefix.

I don't make the same observation? I see puppet and hiera as from a 
packaging point of view identical.

On Fedora 17

Puppet installs as 

/usr/share/ruby/vendor_ruby/puppet.rb
/usr/share/ruby/vendor_ruby/puppet/*

and hiera installs as 

/usr/share/ruby/vendor_ruby/hiera.rb
/usr/share/ruby/vendor_ruby/hiera

on fedora < 17, epel <= 6 it installs in the old location via the following:

%if 0%{?el5}%{?el6}%{?fc16}
%{!?ruby_vendorlibdir: %global ruby_vendorlibdir /usr/lib/ruby/site_ruby/1.8}
%endif

which is exactly what puppet does also: EPEL6 puppet-2.6.17-2.el6.noarch.rpm

/usr/lib/ruby/site_ruby/1.8/puppet.rb
/usr/lib/ruby/site_ruby/1.8/puppet/*

> May be it could be split into two packages? Something like hiera, which
> contains the executable and may be something more and the ruby-hiera, which
> would contain the library part? Not sure if that is not overkill though :)

Indeed.

(In reply to comment #18)
> (In reply to comment #17)
> > Well, I agree that the application vs library might be a bit fuzzy. But if
> > you take a look on Puppet, non of its files are installed into Ruby's
> > %{ruby_vendorlibdir}, while all of hiera's files are installed there. In
> > this case, it seems that hiera is more library then application and
> > therefore it should have ruby- prefix.
> 
> I don't make the same observation? I see puppet and hiera as from a 
> packaging point of view identical.

Actually, I was wrong. I was looking into -sever subpackage instead :/ But anyway, if I did the review for puppet, I would suggest them to split the package into puppet and ruby-puppet as well.

So what would ever be a pure application?  Under what circumstances
would there ever be files in '/usr/share/ruby/vendor_ruby' ?

Steve.

I don't think it should be split into two packages or renamed, but I agree this is a gray area. My interpretation (please correct me if I'm mistaken) of the ruby naming guidelines is that if a package is and only is a library/extension package, then it should be named ruby-$UPSTREAM (as opposed to "mostly is" a library). In that sense, Hiera shouldn't be named ruby-hiera because it is not explicitly a library only - it provides a user-level tool also. But it is primarily a library, in that, anecdotally at least, most users use it that way and not as a standalone application, so it's not really explicitly an "Application" either, and it would probably be less appropriate if it was packaged as such. My two cents is that hiera should probably qualify as a 'pure ruby package,' and %{ruby_vendorlibdir} is appropriate. As for splitting into two packages, I also think it's overkill. However, it would ensure that when packages such as puppet bring in hiera for a dependency, they only have to bring in the library and not the binary. But I personally feel this is a sledge-hammer to fly kind of solution and that the current setup is fairly sane.

I am also still very much on the just call the package 'hiera'.

I think it should just be called hiera. How it's distributed (gem, or what language it's written in is not relevant). 

Think about Amarok, we don't call that ruby-amarok. 

Hiera is usable stand-alone, and people do use it that way.

hiera-1.0.0-3.fc18 has been pushed to the Fedora 18 stable repository.

hiera-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.