Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 232342 - Review Request: mcs - A configuration file abstraction library
Summary: Review Request: mcs - A configuration file abstraction library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-14 20:48 UTC by Ralf Ertzinger
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-05-01 18:19:26 UTC
hdegoede: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Ralf Ertzinger 2007-03-14 20:48:42 UTC
Spec URL: http://www.skytale.net/files/mcs/mcs.spec
SRPM URL: http://www.skytale.net/files/mcs/mcs-0.4.1-0.1.sky.src.rpm
Description:
mcs is a library and set of userland tools which abstract the storage of
configuration settings away from userland applications.

It is hoped that by using mcs, that the applications which use it will
generally have a more congruent feeling in regards to settings.

There have been other projects like this before (such as GConf), but unlike
those projects, mcs strictly handles abstraction. It does not impose any
specific data storage requirement, nor is it tied to any desktop environment or
software suite.



Note: this package blocks an update to audacious (the new version depends on mcs)

Comment 1 Michael Schwendt 2007-03-26 22:32:57 UTC
> Requires(post): /sbin/ldconfig

Is automatic already because of "-p /sbin/ldconfig" in %post.

[...]

What I don't like about mcs is that they ship an autoheader/autoconf
config header renamed to mcs_config.h and include it *always* from
within their main API header. The definitions inside can cause
unwanted redefinitions for applications that use mcs and autotools.
I wanted to contact upstream about it when I tried mcs for Audacious 1.3,
but have forgotten to do so.

Comment 2 Hans de Goede 2007-03-27 07:57:33 UTC
(In reply to comment #1)
> > Requires(post): /sbin/ldconfig
> 
> Is automatic already because of "-p /sbin/ldconfig" in %post.
> 
> [...]
> 
> What I don't like about mcs is that they ship an autoheader/autoconf
> config header renamed to mcs_config.h and include it *always* from
> within their main API header. The definitions inside can cause
> unwanted redefinitions for applications that use mcs and autotools.
> I wanted to contact upstream about it when I tried mcs for Audacious 1.3,
> but have forgotten to do so.

Yes, that is bad, autoheader files should not be installed. But allas, things
happen. I always fix this by looking through the other header files and see what
defines (if any) from the autoheaderfile they use. And then I manually create a
very minimal config.h with those defines + any defines which might be of
interest to software using the lib, those are usually fine as the should have a
packagename prefix, (for example MCS_VERSION)

Ralf if you could fix the autoheader as I just described, then I'm more then
willing todo a review.



Comment 3 Ralf Ertzinger 2007-03-27 08:27:19 UTC
Hmmm. A quick glance over the header file shows that mcs_config.h is being
included in mcs.h, but no definitions from it are used anywhere.

So I'll just drop mcs_config.h and remove the #include from mcs.h.

Comment 4 Hans de Goede 2007-03-27 12:16:49 UTC
MUST:
=====
0 rpmlint output is:
W: mcs incoherent-version-in-changelog 0.4.1-1.fc7 0.4.1-0.1
W: mcs no-documentation
W: mcs-devel no-dependency-on mcs
W: mcs-devel no-documentation
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel x86_64
* BR: ok
* No locales
* ldconfig run for shared libraries
* Not relocatable
* Package owns / or requires all dirs
0 No duplicate files & Permissions
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* no .desktop file required

Must FIX
========
* The following rpmlint messages:
W: mcs incoherent-version-in-changelog 0.4.1-1.fc7 0.4.1-0.1
W: mcs no-documentation
  For the docs, add: "%doc AUTHORS COPYING README TODO"
* Remove the unnecessart: "Requires(post): /sbin/ldconfig"
* Remove msc_config.h and patch the other .h files to not include it


Comment 5 Ralf Ertzinger 2007-03-27 14:09:40 UTC
The changelog version will be coherent once the package hit the build system
(call it weird but I'd like the first package to be released with -1.fc7 :). If
you insist I will change this for the review package, however.

The documentations is contained in the -libs subpackage. Rationale for this is
that the only package actually using mcs (audacious) will pull in just the
libraries, thus the -libs package. The "main" package is not strictly required
to be installed.

New version fixing the other MUST FIX available at
http://www.skytale.net/files/mcs

Thanks for reviewing this.

Comment 6 Hans de Goede 2007-03-27 14:24:37 UTC
All Must FIX items fixed or explained. Approved by Hans de Goede


Comment 7 Ralf Ertzinger 2007-03-31 15:53:40 UTC
Hans, since I am currently fighting the fedora account system, could you request
the CVS module for this package to be created, so that I can trigger a build?

Thanks.

Comment 8 Hans de Goede 2007-03-31 16:21:23 UTC
(In reply to comment #7)
> Hans, since I am currently fighting the fedora account system, could you request
> the CVS module for this package to be created, so that I can trigger a build?
> 
> Thanks.

I don't understand, have you read:
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

All you need todo is add a comment here and set the cvs flag to ?
You should have the necessary rights for that, anyone in the cvsextras group should.


Comment 9 Ralf Ertzinger 2007-03-31 16:29:58 UTC
In theory, yes.

In reality bugzilla tells me that I do not have the right to change fedora-cvs.
I suspect that this is due to different mail addresses in the fedora account
system and in bugzilla.

Please see
https://www.redhat.com/archives/fedora-extras-list/2007-March/msg00278.html

Comment 10 Hans de Goede 2007-03-31 16:40:37 UTC
New Package CVS Request
=======================
Package Name:      mcs
Short Description: A configuration file abstraction library
Owners:            redhat-bugzilla@camperquake.de
Branches:          FC-5 FC-6 devel
InitialCC:         <empty>


Comment 11 Hans de Goede 2007-05-01 18:14:14 UTC
Any reason why this review hasn't been closed yet?

Comment 12 Ralf Ertzinger 2007-05-01 18:19:26 UTC
None.


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