Bug 173040 - Review Request: rlog - Runtime Logging for C++
Review Request: rlog - Runtime Logging for C++
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Enrico Scholz
David Lawrence
http://arg0.net/wiki/rlog
:
: 1098460 (view as bug list)
Depends On:
Blocks: FE-ACCEPT 1098472
  Show dependency treegraph
 
Reported: 2005-11-12 16:55 EST by Peter Lemenkov
Modified: 2014-06-02 07:46 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-12-16 14:31:13 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Lemenkov 2005-11-12 16:55:28 EST
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 02:13:12 EST
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 05:15:17 EST
* 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 06:58:48 EST
> * 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 07:32:30 EST
(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 16:22:56 EST
> > * 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 16:42:54 EST
(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 18:15:50 EST
-----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 00:11:13 EST
(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-03 22:47:57 EST
(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-03 23:08:20 EST
Also do you intend on submitting a fuse-encfs package?
Comment 11 Warren Togami 2005-12-03 23:10:49 EST
Also don't forget to add the dist tag to the Release before this is imported.
Comment 12 Enrico Scholz 2005-12-04 03:45:59 EST
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 11:28:08 EST
How about ifarch it to include/exclude valgrind only if it is capable on that
architecture?
Comment 14 Peter Lemenkov 2005-12-10 17:33:28 EST
> 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 08:25:42 EST
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 16:08:49 EST
OK APPRPROVED, and I am sponsoring you now.
Comment 17 Warren Togami 2005-12-13 16:53:53 EST
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 17:47:09 EST
(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 14:30:23 EST
Built on devel.
Comment 20 Peter Lemenkov 2009-09-25 08:45:15 EDT
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 12:35:59 EDT
congrats. ;) 

cvs done.
Comment 22 Peter Lemenkov 2014-06-01 11:47:03 EDT
Package Change Request
======================
Package Name: rlog
New Branches: epel7
Owners: salimma
InitialCC:
Comment 23 Peter Lemenkov 2014-06-01 11:47:37 EDT
*** Bug 1098460 has been marked as a duplicate of this bug. ***
Comment 24 Jon Ciesla 2014-06-02 07:46:27 EDT
Git done (by process-git-requests).

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