Bug 826520 - Review Request: hiera - A simple hierarchical database supporting plugin data sources
Summary: Review Request: hiera - A simple hierarchical database supporting plugin data...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ulrich Schwickerath
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 830587
TreeView+ depends on / blocked
 
Reported: 2012-05-30 12:04 UTC by Steve Traylen
Modified: 2013-09-06 17:30 UTC (History)
6 users (show)

Fixed In Version: hiera-1.0.0-3.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-08-06 00:16:04 UTC
ulrich.schwickerath: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Steve Traylen 2012-05-30 12:04:05 UTC
Spec URL: http://cern.ch/straylen/rpms/hiera/hiera.spec
SRPM URL: http://cern.ch/straylen/rpms/hiera/hiera-1.0.0-0.2.rc3.fc16.src.rpm
Description:
A simple hierarchical database supporting plugin data sources

Comment 1 Ulrich Schwickerath 2012-06-07 12:25:45 UTC
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.

Comment 2 Steve Traylen 2012-06-10 18:52:46 UTC
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.

Comment 3 Steve Traylen 2012-07-01 07:58:25 UTC
I think I answered all your comments?

Comment 4 Ulrich Schwickerath 2012-08-10 07:17:51 UTC
APPROVED


Sorry for the delay.

Comment 5 Steve Traylen 2012-08-10 08:08:45 UTC
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:

Comment 6 Jason Tibbitts 2012-08-10 10:04:09 UTC
No f18 branch?

Comment 7 Steve Traylen 2012-08-10 12:15:29 UTC
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.

Comment 8 Gwyn Ciesla 2012-08-10 12:16:28 UTC
Git done (by process-git-requests).

Added f18.

Comment 9 Fedora Update System 2012-09-27 19:22:09 UTC
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

Comment 10 Fedora Update System 2012-09-27 19:22:22 UTC
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

Comment 11 Fedora Update System 2012-09-27 19:22:35 UTC
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

Comment 12 Fedora Update System 2012-09-27 19:22:45 UTC
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

Comment 13 Fedora Update System 2012-09-28 08:25:05 UTC
hiera-1.0.0-2.fc17 has been pushed to the Fedora 17 testing repository.

Comment 14 Vít Ondruch 2012-10-01 09:39:18 UTC
(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

Comment 15 Vít Ondruch 2012-10-01 09:41:34 UTC
And you should also follow https://fedoraproject.org/wiki/Packaging:Ruby#Ruby_ABI

Comment 16 Steve Traylen 2012-10-01 12:12:25 UTC
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.

Comment 17 Vít Ondruch 2012-10-01 12:37:16 UTC
(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 :)

Comment 18 Steve Traylen 2012-10-01 13:43:34 UTC
(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.

Comment 19 Vít Ondruch 2012-10-01 15:09:32 UTC
(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.

Comment 20 Steve Traylen 2012-10-02 08:12:24 UTC
So what would ever be a pure application?  Under what circumstances
would there ever be files in '/usr/share/ruby/vendor_ruby' ?

Steve.

Comment 21 Moses Mendoza 2012-10-23 17:55:36 UTC
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.

Comment 22 Steve Traylen 2012-10-25 09:38:47 UTC
I am also still very much on the just call the package 'hiera'.

Comment 23 Michael Stahnke 2013-03-14 20:50:41 UTC
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.

Comment 24 Fedora Update System 2013-08-06 00:16:04 UTC
hiera-1.0.0-3.fc18 has been pushed to the Fedora 18 stable repository.

Comment 25 Fedora Update System 2013-09-06 17:30:23 UTC
hiera-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.


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