Bug 458346 - Review Request: gflags - Library for commandline flag processing
Review Request: gflags - Library for commandline flag processing
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Till Maas
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-08-07 14:13 EDT by Rakesh Pandit
Modified: 2014-11-19 06:32 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-09-29 04:54:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
opensource: fedora‑review+
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Rakesh Pandit 2008-08-07 14:13:48 EDT
The gflags package contains a library that implements commandline flags processing. As such it's a replacement for getopt(). It has increased flexibility, including built-in support for C++ types like string, and the ability to define flags in the source file in which they're used.

SPEC: http://rakesh.fedorapeople.org/spec/gflags.spec
SRPM: http://rakesh.fedorapeople.org/srpm/gflags-0.9-1.fc9.src.rpm
Comment 1 Till Maas 2008-08-08 17:06:45 EDT
Is Requires:       automake for the -devel subpackage really needed? I guess not.

Does this really work? Normally --disable-static is used:
%configure --enable-static=no
Comment 2 Rakesh Pandit 2008-08-08 20:58:02 EDT


May you look at them again.

SPEC: http://rakesh.fedorapeople.org/spec/gflags.spec
SRPM: http://rakesh.fedorapeople.org/srpm/gflags-0.9-2.fc9.src.rpm
Comment 3 Till Maas 2008-08-13 16:37:12 EDT
[NOT OK] rpmlint output:
gflags-devel.i386: W: no-documentation
There is a html file in the doc/ dir of the tarball that contains
documenation, that would fit in the -devel subpackage imho.

[OK] Spec in %{name}.spec format

[OK] license allowed: BSD 3 clause
[OK] license matches shortname in License: BSD
[OK] license in tarball and included in %doc:

[OK] package is code or permissive content:
[OK] Source0 is a working URL
[OK] Source0 matches Upstream:
bd4871398e9019b241d89cc21fb62def  gflags-0.9.tar.gz

[OK] Package builds on all platforms:

[OK] BuildRequires are complete (koji builds)
(OK) No file dependencies outside of /etc /bin /sbin /usr/bin /usr/sbin 

[OK] Every (sub)package containing libraries runs ldconfig
[OK] .h (header) files are in -devel subpackage
[OK] .a (static libraries) are in -static subpackage:
no static libraries are packaged
[OK] contains .so.X(.Y) files and .so is in -devel
[OK] -devel subpackage has Requires: %{name} = %{version}-%{release}
[OK] .la files (libtool) are not included

[OK] Prefix: /usr not used (not relocatable)
[OK] Owns all created directories
[OK] no duplicates in %files
[OK] %defattr(-,root,root,-) is in every %files section
[OK] Does not own files or dirs from other packages
[OK] included filenames are in UTF-8

[OK] %clean is rm -rf %{buildroot} or $RPM_BUILD_ROOT 
[OK] %install starts with rm -rf %{buildroot} or $RPM_BUILD_ROOT 
[OK] Consistent macro usage
[OK] %doc does not affect runtime

{OK} no pre-built binaries (.a, .so*, executable)
{OK} well known BuildRoot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
{OK} PreReq not used
{OK} no duplication of system libraries
{OK} no rpath
{NOT OK} Timestamps preserved with cp and install
You should change the make install to:
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

{OK} Uses parallel make (%{?_smp_mflags})
{OK} Requires(pre,post) style notation not used
{OK} only writes to tmp /var/tmp $TMPDIR %{_tmppath} %{_builddir} (and %{buildroot} on %install and %clean)
{OK} no Conflicts
{OK} nothing installed in /srv
{OK} Changelog in allowed format
<OK> Sane Provides: and Requires:

{OK} Follows Naming Guidelines

Other stuff:
- this line should be removed, because no static libraries are build:
find $RPM_BUILD_ROOT -type f -name "*.a" -delete

- Can you maybe submit an patch to upstream that changes the installation of the
  header files within the automake stuff or file a bug? Then you could remove
  these lines:
mkdir -p $RPM_BUILD_ROOT%{_includedir}/%{name}
mv $RPM_BUILD_ROOT%{_includedir}/google/*.h $RPM_BUILD_ROOT%{_includedir}/%{name}/

- On my system no rpath is set in the libraries, why did you use chrpath?
- Why don't you install the python stuff?
- not the NOT OK for rpmlint and timestamps above
Comment 4 Till Maas 2008-08-13 16:41:19 EDT
There is a unittest running script in the python directory, maybe you can integrate it into the spec somehow (you can use a section named %check). I tried to just run it in %check and it failed, but without using rpmbuild it did not. Not sure, what can be done here.
Comment 5 Rakesh Pandit 2008-08-14 15:45:06 EDT
I have reported upstream, about header files not going to proper place and an extra folder being created which doesn't make sense. I assume it as a cosmetic issue and non-blocking one.

Other all issues are taken care.

1. rpmlint & timestamp
2. python module & unittest
3. removed useless stuff

SPEC: http://rakesh.fedorapeople.org/spec/gflags.spec
SRPM: http://rakesh.fedorapeople.org/srpm/gflags-0.9-3.fc9.src.rpm
Comment 6 Rakesh Pandit 2008-08-26 06:21:32 EDT
> Can you maybe submit an patch to upstream that changes the installation of
>  header files within the automake stuff or file a bug? Then you could remove
>  these lines:
>mkdir -p $RPM_BUILD_ROOT%{_includedir}/%{name}
>mv $RPM_BUILD_ROOT%{_includedir}/google/*.h

I had a discussion with upstream and had a look at some other packages. It seems to me that it is wrong to move header files to some other place as it may break some other useful stuff.
  Few other packages have similar structure and they don't move header files.

I will like to keep them in place.

Will update ASAP.

Comment 8 Rakesh Pandit 2008-08-31 01:48:10 EDT

Waiting impatiently ;)
Comment 9 Rakesh Pandit 2008-09-03 08:41:43 EDT
ping Again?
Comment 10 Rakesh Pandit 2008-09-03 19:16:25 EDT
Thanks Till

I have fixed the issues. I have disabled test suite because 2 out of 17 where failing on x86_64 on rawhide.

Package build on F9 and rawhide on most archs -

SPEC: http://rakesh.fedorapeople.org/spec/gflags.spec
SRPM: http://rakesh.fedorapeople.org/srpm/gflags-0.9-5.fc10.src.rpm
Comment 11 Rakesh Pandit 2008-09-04 01:09:26 EDT
It failed on F-8 only - because the Python eggs are not created in Fedora 8 by default.

Will update shortly.

Comment 12 Rakesh Pandit 2008-09-04 05:57:05 EDT

SPEC: http://rakesh.fedorapeople.org/spec/gflags.spec
SRPM: http://rakesh.fedorapeople.org/srpm/gflags-0.9-6.fc10.src.rpm

Build on F-8 test-box also.
Comment 13 Till Maas 2008-09-10 11:40:20 EDT
[OK] rpmlint output: silent
{OK} Timestamps preserved with cp and install

Other issues from comment 3: OK

(In reply to comment #10)

> I have fixed the issues. I have disabled test suite because 2 out of 17 where
> failing on x86_64 on rawhide.

Did you report this to upstream? Are you sure that the test suite is buggy and not the package itself, i.e. does it really work as intended on x86_64? There is also a binary testsuite that can be run with "make test", but it uses unsecure tempfiles: It always uses the same directories in /tmp to perform the test, which can maybe exploited.

I consider the testsuite issues not to be a blocker, therefore this package is now  APPROVED. Nevertheless, please try to get upstream to make the testsuite workable within Fedora builds.
Comment 14 Rakesh Pandit 2008-09-10 12:04:22 EDT
I have reported it upstream and will be working on it upstream.

New Package CVS Request
Package Name: gflags
Short Description: Library for commandline flag processing
Owners: rakesh
Branches: F-8 F-9
InitialCC: rakesh
Comment 15 Jason Tibbitts 2008-09-10 20:03:56 EDT
Did you mean to include yourself as both the owner and initialcc?  Because that's pointless.  I've just ignored that line; if there's some other user who wants to be CC'd, they can add themselves in pkgdb.

CVS done.
Comment 16 Rakesh Pandit 2008-09-29 03:52:48 EDT
The build issue is fixed upstream and will include test suite in 1.0 version to be released soon.

Comment 17 Fedora Update System 2008-09-29 04:47:30 EDT
gflags-0.9-6.fc8 has been submitted as an update for Fedora 8.
Comment 18 Fedora Update System 2008-09-29 04:49:16 EDT
gflags-0.9-6.fc9 has been submitted as an update for Fedora 9.
Comment 19 Rakesh Pandit 2008-09-29 04:54:37 EDT
build and imported
Will update to latest 1.0 after its release in few days.

Comment 20 Fedora Update System 2008-10-24 19:52:23 EDT
gflags-0.9-6.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Fedora Update System 2008-10-24 19:54:02 EDT
gflags-0.9-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Peter Lemenkov 2013-08-06 08:55:13 EDT
Package Change Request
Package Name: gflags
New Branches: el6
Owners: peter
Comment 23 Gwyn Ciesla 2013-08-06 09:03:29 EDT
Git done (by process-git-requests).
Comment 24 Peter Lemenkov 2014-11-19 04:55:47 EST
Package Change Request
Package Name: gflags
New Branches: epel7
Owners: peter ivaxer
Comment 25 Gwyn Ciesla 2014-11-19 06:32:14 EST
Git done (by process-git-requests).

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