Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 173040 - Review Request: rlog - Runtime Logging for C++
Summary: Review Request: rlog - Runtime Logging for C++
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Enrico Scholz
QA Contact: David Lawrence
URL: http://arg0.net/wiki/rlog
Whiteboard:
: 1098460 (view as bug list)
Depends On:
Blocks: FE-ACCEPT 1098472
TreeView+ depends on / blocked
 
Reported: 2005-11-12 21:55 UTC by Peter Lemenkov
Modified: 2014-06-02 11:46 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-12-16 19:31:13 UTC
Type: ---
Embargoed:
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Peter Lemenkov 2005-11-12 21:55:28 UTC
Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

Description: RLog provides a flexible message logging facility for C++ programs and libraries.  It is meant to be fast enough to leave in production code.

RLog needs for EncFS, a filesystem in user-space, which uses FUSE.

Comment 1 Peter Lemenkov 2005-11-13 07:13:12 UTC
Slightly changed spec-file.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

Comment 2 Enrico Scholz 2005-11-13 10:15:17 UTC
* version-release tags should be added to the changelog entry(s) (at least to 
  your ones)
* package checks for 'valgrind' (which is available). afais, it is used in an  
  assertion function only but perhaps it should be added to the BuildRequires?
* 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
  which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
  package?
* upstream ships a GPG signature which should be added to the package (as 
  additional Source) to ease verification of the tarball
* there should be a

  | Requires: /usr/lib/pkgconfig
  or
  | Requires: pkgconfig

  in the -devel package. First variant would be more correct, but can cause
  ambiguities with some core packages 

Comment 3 Peter Lemenkov 2005-11-13 11:58:48 UTC
> * version-release tags should be added to the changelog entry(s) (at least to 
>   your ones)

Done.

> * package checks for 'valgrind' (which is available). afais, it is used in an 
>   assertion function only but perhaps it should be added to the BuildRequires?

Hmmm. It's optional. Would it be more preferable to allow the app to choose
itself (at the './configure'-stage) whether to use Valgrind?

> * upstream ships a GPG signature which should be added to the package (as 
>   additional Source) to ease verification of the tarball

Source1: http://arg0.net/users/vgough/download/rlog-1.3.7.tgz.asc

Done.

> * there should be a
>  | Requires: /usr/lib/pkgconfig
>  or
>  | Requires: pkgconfig

I *temporary* choose second variant. What ambiguities did you mention about? I
don't see any.


Comment 4 Peter Lemenkov 2005-11-13 12:32:30 UTC
(In reply to comment #2)

> * 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
>   which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
>   package?

Done.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

Comment 5 Enrico Scholz 2005-11-13 21:22:56 UTC
> > * package checks for 'valgrind' (which is available). afais, it is
> >   used in an assertion function only but perhaps it should be
> >   added to the BuildRequires?
> 
> Hmmm. It's optional. Would it be more preferable to allow the app to
> choose itself (at the './configure'-stage) whether to use Valgrind?

To make builds reproducible, you should either add the BuildRequires:
or use '--disable-valgrind'. Because it does not seem to introduce
additional dependencies, I do not see reasons to disable it.


> > * there should be a
> >  | Requires: /usr/lib/pkgconfig
> >  or
> >  | Requires: pkgconfig
> 
> I *temporary* choose second variant. What ambiguities did you mention
> about? I don't see any.

'rpm -qf /usr/lib/pkgconfig' shows that this directories is owned by
several core packages. Because yum's depsolver is not very smart, it
will probably choose the wrong one which will perhaps add lots of
unwanted dependencies


> > * 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
> >   which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
> >   package?
> 
> Done.

ok; but
- you should add some more BuildRequires then (e.g. tetex-latex,
  doxygen).
- temporary pdf files (classrlog_*.pdf) should not be shipped but
  'refman.pdf' only


------------------

New issue

* 'INSTALL' should not be packaged; it does not contain anything which
  might be useful for the enduser

Comment 6 Peter Lemenkov 2005-11-13 21:42:54 UTC
(In reply to comment #5)

> To make builds reproducible, you should either add the BuildRequires:
> or use '--disable-valgrind'. Because it does not seem to introduce
> additional dependencies, I do not see reasons to disable it.

OK, added valgrind to BuildRequires

> 'rpm -qf /usr/lib/pkgconfig' shows that this directories is owned by
> several core packages. Because yum's depsolver is not very smart, it
> will probably choose the wrong one which will perhaps add lots of
> unwanted dependencies

Done.
Requires: pkgconfig


> - you should add some more BuildRequires then (e.g. tetex-latex,
>   doxygen).

Done.

> - temporary pdf files (classrlog_*.pdf) should not be shipped but
>   'refman.pdf' only

Done.

> New issue
> 
> * 'INSTALL' should not be packaged; it does not contain anything which
>   might be useful for the enduser

Fixed.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

Comment 7 Enrico Scholz 2005-11-13 23:15:50 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Ok, I APPROVE http://paula.comtv.ru/rlog-1.3.7-1.src.rpm for Fedora Extras

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iQEVAwUBQ3fJSAc3PX/XHFA7AQqG1gf+MygOhOt5++RnJaZPHQ9rPMBFKl74M9gE
4u2nNIP50rNpDzo7HEZ6P8uuMQQMoKh1m7MlkDORV/vBdOz9SPa5tlyYo/pbNRRO
62CxDi0yfmjlj1Deu+yn6g7SQNJ8791NxuT4iZdLCMha7l+OtUdV1NF03xmOK3PW
9MMG7VKElKVmY7Ie+3PQ1ZTyK0wT6VNPkgzFrlqq267qOlY9UFyXI5JfQh/yDax4
JTWZY48/T+JeZ7hNhCA0lBoXKtTfzqDQn9aQH3sWxmLWenL4urZDXI19nOmmekfR
a1nAWoDHQ5slyM0k0KXUNlNszT5dzgtLrM8Fmb+ZCA+TqG7GTIIjtA==
=HzNF
-----END PGP SIGNATURE-----


Comment 8 Peter Lemenkov 2005-11-14 05:11:13 UTC
(In reply to comment #7)

> Ok, I APPROVE http://paula.comtv.ru/rlog-1.3.7-1.src.rpm for Fedora Extras

Thanks. Could you sponsor me?


Comment 9 Warren Togami 2005-12-04 03:47:57 UTC
(In reply to comment #6)
>> To make builds reproducible, you should either add the BuildRequires:
>> or use '--disable-valgrind'. Because it does not seem to introduce
>> additional dependencies, I do not see reasons to disable it.
> OK, added valgrind to BuildRequires

Is that valgrind only used for an optional build-time check?  One major drawback
of requiring valgrind during build is artificially limiting the architectures
that this can be included on.  valgrind's x86_64 support is very new, and it
doesn't run on ppc at all.  I would recommend disabling valgrind explicitly
during rpmbuild for this reason.

I would encourage you to please look into reviewing other contributor's packages
in order to demonstrate to the sponsors that you truly understand the packaging
guidelines and project processes, as that is the main thing we are looking for
in candidates for sponsorship.

I am considering sponsoring you soon, but I am looking more into your
participation first.

Comment 10 Warren Togami 2005-12-04 04:08:20 UTC
Also do you intend on submitting a fuse-encfs package?


Comment 11 Warren Togami 2005-12-04 04:10:49 UTC
Also don't forget to add the dist tag to the Release before this is imported.


Comment 12 Enrico Scholz 2005-12-04 08:45:59 UTC
valgrind should not be a problem; it is used for creating nice stacktraces only
without adding library deps. But I never looked whether these stacktraces are
working also on x86_64 and PPC.

Comment 13 Warren Togami 2005-12-07 16:28:08 UTC
How about ifarch it to include/exclude valgrind only if it is capable on that
architecture?


Comment 14 Peter Lemenkov 2005-12-10 22:33:28 UTC
> I would recommend disabling valgrind explicitly
> during rpmbuild for this reason.

Added %ifarch check.

> Also do you intend on submitting a fuse-encfs package?
 
Yes. More to say - I trying to put rlog-package into extras especially because
of fuse-encfs :).

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

Comment 15 Warren Togami 2005-12-11 13:25:42 UTC
When updating your .src.rpm, please always increment the Release number and add
a changelog entry.

I will test your RPM...

Comment 16 Warren Togami 2005-12-13 21:08:49 UTC
OK APPRPROVED, and I am sponsoring you now.


Comment 17 Warren Togami 2005-12-13 21:53:53 UTC
Please CC me on the new bug when you have a fuse-encfs package for me to review.
 Thanks!


Comment 18 Peter Lemenkov 2005-12-14 22:47:09 UTC
(In reply to comment #16)
> OK APPRPROVED, and I am sponsoring you now.

Thanks!
Added to CVS.

> Please CC me on the new bug when you have a
> fuse-encfs package for me to review.

OK.

Comment 19 Peter Lemenkov 2005-12-16 19:30:23 UTC
Built on devel.

Comment 20 Peter Lemenkov 2009-09-25 12:45:15 UTC
Heh, seems that it's my first review request! :)
It was almost 4 years ago. Anyway, back to serious things:


Package Change Request
======================
Package Name: rlog
New Branches: EL-4 EL-5
Owners: peter

Comment 21 Kevin Fenzi 2009-09-25 16:35:59 UTC
congrats. ;) 

cvs done.

Comment 22 Peter Lemenkov 2014-06-01 15:47:03 UTC
Package Change Request
======================
Package Name: rlog
New Branches: epel7
Owners: salimma
InitialCC:

Comment 23 Peter Lemenkov 2014-06-01 15:47:37 UTC
*** Bug 1098460 has been marked as a duplicate of this bug. ***

Comment 24 Gwyn Ciesla 2014-06-02 11:46:27 UTC
Git done (by process-git-requests).


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