Bug 1123217 - Review Request: obix - ONEDC toolkit
Summary: Review Request: obix - ONEDC toolkit
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Meng
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-25 06:50 UTC by Sam Wilson
Modified: 2014-08-15 14:26 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
i: fedora-review?


Attachments (Terms of Use)

Description Sam Wilson 2014-07-25 06:50:35 UTC
Spec URL: https://cycloptivity.fedorapeople.org/obix/1.0.3-0/obix.spec
SRPM URL: https://cycloptivity.fedorapeople.org/obix/1.0.3-0/obix-1.0.3-0.fc19.src.rpm
Description: An open source project derived from the C oBIX Tools (CoT) project, an open source project dedicated to the development of embedded Building Automation solutions based on oBIX standard (http://www.obix.org).
Fedora Account System Username: cycloptivity

Koji builds http://koji.fedoraproject.org/koji/taskinfo?taskID=7194133

Comment 1 Christopher Meng 2014-07-25 06:59:32 UTC
Who is Andrew Ross? Where are you?

Comment 2 Sam Wilson 2014-07-25 07:07:05 UTC
(In reply to Christopher Meng from comment #1)
> Who is Andrew Ross? Where are you?

Andrew and I work together on the packaging side [1], but his not yet sponsored so I am pushing the reviewing instead.

1 - https://www.onedc.com/pipermail/obix-devel/2014-July/thread.html

Cheers,

Sam

Comment 3 Christopher Meng 2014-07-25 07:41:37 UTC
1. Drop BuildRequires:  glibc-devel.

2. The -doc is just a crap if only contains README.md COPYING CODING_GUIDELINES.md. Put them in the main package, and -libs, remember the way depends on your needs(e.g CODING_GUIDELINES.md is only intended for development IMO so put it into -devel), but must put license file in main package.

3. Use %cmake macro, and drop -DLIB_DIR="%{_libdir}"

4. %post libs
-p /sbin/ldconfig

Weird.

%post libs
/sbin/ldconfig

5. %if 0%{?rhel}
%defattr(-, root, root)

rm -rf %{buildroot}

RHEL doesn't need them.

6. %attr(0755,root,root) %dir %{_sysconfdir}/obix
%attr(0755,lighttpd,lighttpd) %dir %{_sharedstatedir}/obix/histories

755 is redundant. Why did you set it?

7. ln -sf %{_sharedstatedir}/obix/histories %{buildroot}/%{_sysconfdir}/obix/res/server/

Reason?

8. Better to leave the Source tag as:

https://github.com/ONEDC/obix/archive/%{version}.tar.gz#/obix-%{version}.tar.gz

9. Requires:       lighttpd
Requires:       lighttpd-fastcgi

lighttpd-fastcgi depends on lighttpd, drop the explicit Requires of lighttpd.

And you should append the bits macro like you've done to the other packages:

Requires:       lighttpd-fastcgi%{?_isa}

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

10. -DPROJECT_DOC_DIR_SUFFIX="%{name}-doc-%{version}"

F20+, use %{_pkgdocdir}

We use unversioned docdir just because it can help save the bookmarks, if you keep it versioned still, after the update the bookmark will be useless. Think of it.

Comment 4 Michael Schwendt 2014-08-15 14:26:28 UTC
> 4. %post libs
> -p /sbin/ldconfig
> 
> Weird.

Well, it's nearly correct. Just the second line should be appended at the end of the first line:

  %post libs -p /sbin/ldconfig

As to answer a "Why?": The "-p" is an option that overrides the program that shall run the scriptlet. By default, it would be /bin/sh. Here, it's /sbin/ldconfig that will be executed with an empty scriptlet.


> %post libs
> /sbin/ldconfig

That would run /bin/sh with a shell script that executes /sbin/ldconfig. A little bit superfluous if only running ldconfig is needed.


> %package        libs
> Summary:        Shared library files for %{name}
> Requires:       %{name}%{?_isa} = %{version}-%{release}

So, if the -libs package requires the base package, then why split off this -libs package at all? It could not be installed independently.  The base package also doesn't explicitly require the -libs package, so it seems you've got the dependencies mixed up. An independent library package sort of becomes the real "base" package your other [sub-]packages should depend on explicitly (a reverse case of https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package ).



> %files server

Looks like a few case of
  https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
and:
  https://fedoraproject.org/wiki/Packaging:UnownedDirectories


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