Bug 232342 - Review Request: mcs - A configuration file abstraction library
Review Request: mcs - A configuration file abstraction library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-14 16:48 EDT by Ralf Ertzinger
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-01 14:19:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ralf Ertzinger 2007-03-14 16:48:42 EDT
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 18:32:57 EDT
> 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 03:57:33 EDT
(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 04:27:19 EDT
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 08:16:49 EDT
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 10:09:40 EDT
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 10:24:37 EDT
All Must FIX items fixed or explained. Approved by Hans de Goede
Comment 7 Ralf Ertzinger 2007-03-31 11:53:40 EDT
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 12:21:23 EDT
(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 12:29:58 EDT
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 12:40:37 EDT
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 14:14:14 EDT
Any reason why this review hasn't been closed yet?
Comment 12 Ralf Ertzinger 2007-05-01 14:19:26 EDT
None.

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