[Bug 511276] New: Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Summary: Review Request: comoonics-base-py - Comoonics minimum
                    baselibraries written in Python
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: low
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

Spec URL:
http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py.s...
SRPM URL:
http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0...
Description:

This is my first package. Therefore I need a sponsor. Thanks !!

This library is required for the open-sharedroot feature: 
https://fedoraproject.org/wiki/Features/Opensharedroot

Comoonics minimum baselibraries written in Python
Those are classes used by more other packages.
Functionality provided here are
comoonics.ComDataObject: abstract basic DOM-Based class that is base for any
other DOM-Based class
comooncis.ComExceptions: the library provides a base class for all comoonics
exceptions.
comoonics.DictTools:     Tools for helping with dicts.
comoonics.ComProperties: Property implementation for DataObjects
comoonics.ComPath:       DataObject class representing a path.
comoonics.ComLog:        library for some commonly used logging functions
comoonics.ComSystem:     library for some commonly used functions to execute
commands.
comoonics.XmlTools:      some xml library functions used by other modules.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Marc Grimme  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |511277

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #1 from Bill Nottingham   2009-07-14 10:58:33 EDT ---
This is the part that concerns me about this feature. I understand some of the
sysvinit/halt interactions, even if I don't like them. I don't understand or
like the idea that setting up a gfs2 root requires a large python
infrastructure.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #2 from Mark Hlawatschek   2009-07-14 11:48:12 EDT ---
The open-sharedroot feature is about setting up and managing a sharedroot
cluster. I.e. multiple nodes share the same root filesystem.
When a single root filesystem is concurrently used for multiple nodes the
following topics have to be considered:

1. A shared or cluster filesystem is needed.

I agree, we basically do not need python scripts to mount the shared filesystem
inside the initrd. 
In our approach, some configuration items can be defined inside the cluster
configuration itself. E.g. network configuration for the cluster/storage
interconnect. To be able to query the cluster configuration in a general way
(abstraction layer), the comoonics-cluster-py package is needed.  
In the NFS sharedroot case the whole redhat cluster stack is not needed.
Nevertheless the comoonics-cluster-py is used to query a basic configuration
file. Another abstracted implementation is going to be ldap queries.

2. The shared root filesystem has to provide host dependent parts for every
cluster node. E.g. /etc/sysconfig/network needs to be host dependent.

Context dependent symbolic links (cdsl) are provided using --bind mounts. 
I.e. # mount --bind /cluster/cdsl/[nodeid] /cdsl.local
To be able to do this, the id of the current node has to be queried. 
Also, utilities for managing the cdsls are needed. We provide two utils
com-mkcdslinfrastructure and com-mkcdsl for the cdsl management
(comoonics-cdsl-py). 
The cluster query (nodeid) and the cdsl management are implemented in those
python packages.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #3 from Jason Tibbitts   2009-07-14 16:51:21 EDT ---
*** Bug 511352 has been marked as a duplicate of this bug. ***

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #4 from Jason Tibbitts   2009-07-14 16:51:27 EDT ---
*** Bug 511351 has been marked as a duplicate of this bug. ***

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Nils Philippsen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail],
                   |                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #5 from Nils Philippsen   2009-07-15 08:17:51 EDT ---
I'll take this.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #6 from Nils Philippsen   2009-07-20 05:53:50 EDT ---
First round :-). I'd appreciate if you would check the issues listed below in
your other packages pending review, this would make their reviews so much
simpler. Thanks!

Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this
package.
Items marked "CHECK" aren't covered by the guidelines but you should check and
fix them anyway in my opinion.
Items marked "BAD" violate the guidelines in some point and need to be fixed.

- BAD: rpmlint run on comoonics-base-py-0-1-1.src.rpm flags errors/warnings:

comoonics-base-py.src: E: description-line-too-long comoonics.ComDataObject:
abstract basic DOM-Based class that is base for any other DOM-Based class
comoonics-base-py.src: E: description-line-too-long comooncis.ComExceptions:
the library provides a base class for all comoonics exceptions.
comoonics-base-py.src: E: description-line-too-long comoonics.ComSystem:    
library for some commonly used functions to execute commands.

--> description lines must be 79 characters or shorter, please shorten (and fix
the typo "comooncis.ComExceptions:..." while you're at it)

comoonics-base-py.src: E: no-changelogname-tag

--> start and maintain a package changelog, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

comoonics-base-py.src: W: invalid-license GPL

--> I've looked at a few source files which state that they are licensed under
GPL version 3 or later, this would make the license "GPLv3+". It would be nice
if the package contained a README making this explicit for the whole package as
well as a copy of the license (since you're upstream, this should be no
problem). Be sure to bump the upstream/tarball version when you do this though.

comoonics-base-py.src: W: non-coherent-filename comoonics-base-py-0-1-1.src.rpm
comoonics-base-py-0.1-1.src.rpm

--> I think this is only some typo from copying the file over to your
webserver, correct?

comoonics-base-py.src:17: W: hardcoded-packager-tag Marc

--> please get rid of the Packager:/Vendor: lines, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags

comoonics-base-py.src:38: W: setup-not-quiet

--> use "%setup -q" -- by the way, what's the business with
%version/%unmangled_version? As it is, they're the same and packages should
have the same version number as their upstream as well. If you're thinking
about alpha/beta versions, please see
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Version for
more info -- you (as upstream) will make your life (as packager) much easier
then. Also, using "%define name foo", then "Name: %name" and the like is
unnecessary, if you just define name, versino, release the normal way, the
corresponding macros will be set as well.

comoonics-base-py.src: E: no-cleaning-of-buildroot %install
1 packages and 0 specfiles checked; 5 errors, 4 warnings.

--> clean the build root as described in
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags (just like in %clean)
before installing

- GOOD: package name according to guidelines:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28...
(only fix the source rpm file name please)
- GOOD: spec file named properly
- GOOD/CHECK: licensing mostly clear (see above, an added README would be good)
and according to licensing guidelines
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
- CHECK: license files are not shipped as documentation, but they aren't
shipped in the upstream tarball, see
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text --
since you're upstream you really should ship it though ;-)
- GOOD: the spec file is written in American English
- GOOD: the spec file is legible
- CHECK: no source tarball URL, it would be good if the tarball used were
directly available -- see https://fedoraproject.org/wiki/Packaging/SourceURL
- BAD: doesn't build in mock for x86_64/Rawhide
- BAD: no build dependencies listed

--> at least python-devel is missing as a build dependency (and a dependency of
the generated installable package), see
http://fedoraproject.org/wiki/Packaging:Python

- PASS: doesn't ship locale files
- PASS: no libraries shipped
- BAD/CHECK: package is made relocatable (Prefix: ...), please remove or
justify its use,
https://fedoraproject.org/wiki/Packaging/Guidelines#RelocatablePackages

- BAD: not all shipped directories owned by package, direct dependency or
filesystem

--> instead of using the file list generated by your setup.py, define
%python_sitelib at the top of your spec file and simply list the directory
containing your module, see http://fedoraproject.org/wiki/Packaging:Python :

%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib()")}
[...]
%files
%defattr (-, root, root, -)
%{python_sitelib}/comoonics

- GOOD: no duplicates in %files
- CHECK: permissions on files

--> There are three files in the comoonics module which have mode 0644, but a
"#!/usr/bin/python" line at the top of the file:

comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/XmlTools.py 0644 /usr/bin/python
    comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/DictTools.py 0644 /usr/bin/python
    comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/ComProperties.py 0644
/usr/bin/python

Are they supposed to be executed directly (-> fix mode) or not (remove the
line)?

- GOOD: package has a %clean section
- GOOD: package uses macros consistently
- GOOD: the package contains code, not content
- PASS: no large documentation files
- GOOD: %doc doesn't affect runtime
- PASS: no header files
- PASS: no static libraries
- PASS: no pkgconfig files
- PASS: no libraries included
- PASS: no devel package
- GOOD: no *.la libtool archives
- PASS: no desktop file
- GOOD: doesn't own files or directories owned by other packages
- BAD: build root isn't cleaned at the beginning of %install (see rpmlint
comment above)
- GOOD: all file names are valid UTF-8

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #7 from Mark Hlawatschek   2009-07-21 06:18:36 EDT ---
Released new versions at:

http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-...

http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-...

http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-...

http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-...

All except the mock thing should work now.

What do you think?

Mark

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #8 from Mark Hlawatschek   2009-07-21 08:01:09 EDT ---
I just verified that the latest src rpm builds in mock.
-Mark

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #9 from Nils Philippsen   2009-07-21 08:36:04 EDT ---
I still found some issues:

- BAD: rpmlint still warns about the non-coherent filename:

nils@gibraltar:~/devel/fedora-review/comoonics-base-py> rpmlint comoonics-
   base-py-0-1-2-src.rpm 
   comoonics-base-py.src: W: non-coherent-filename comoonics-base-py-0-1-2-
   src.rpm comoonics-base-py-0.1-2.src.rpm
   1 packages and 0 specfiles checked; 0 errors, 1 warnings.

--> the file name should contain the version as "0.1", not "0-1" -- do you
have any fancy RPM macros which change the resulting file names when building
packages?

- BAD: the spec file still contains the vendor tag, cf.
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags: "The Vendor tag
should not be used. It is set automatically by the build system."

- CHECK: you still %define name, version, release, then later use "Name:
%{name}" etc. This is unnecessary and potentially confusing, just use "Name:
comoonics", "Version: 0.1", "Release: 2" and the macros should be set
accordingly.

- CHECK/BAD: no source tarball URL, it would be good if the tarball used were
directly available -- see https://fedoraproject.org/wiki/Packaging/SourceURL.
Needless to say, if the tarball contents change (e.g. license, readme added),
the version has to be bumped (e.g. to 0.1.1 in this case).

- BAD: not all shipped directories owned by package, direct dependency or
filesystem

--> Requires: python

These slipped past me in the first review:

- BAD: The BuildRoot value MUST be below %{_tmppath}/ and MUST contain at least
%{name}, %{version} and %{release}. It may invoke mktemp since this is
guaranteed to exist on every system. From there, packagers are expected to use
a sane BuildRoot. See
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag for details
and recommended values.

- CHECK: You use "%setup -q -n %{name}-%{version}" in the spec file, plainly
using "%setup -q" is sufficient.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

--- Comment #10 from Mark Hlawatschek   2009-07-21 10:05:52 EDT ---
Thanks for your feedback.
I modified the packages according to your suggestions.
You'll find the latest version at:

http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0...

http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py.s...

http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0...

http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0...

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Nils Philippsen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #11 from Nils Philippsen   2009-07-21 10:46:15 EDT ---
This package is APPROVED. You can now continue with the process, i.e. request
CVS to be set up, see http://fedoraproject.org/wiki/Package_Review_Process

Some thoughts:

- if you want, you can change the source URL to:

http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-...

This way you won't have to change it if you bump the version.

- if you can configure your web server to serve spec files as text/plain, it
would be most helpful for the other reviews, thanks ;-).

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Mark Hlawatschek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #12 from Mark Hlawatschek   2009-07-21 11:26:53 EDT ---
New Package CVS Request
=======================
Package Name: comoonics-base-py
Short Description: Comoonics minimum baselibraries
Owners: markhla elcody02
Branches: 
InitialCC:

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Jason Tibbitts  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #13 from Jason Tibbitts   2009-07-23 12:35:17 EDT ---
CVS done.

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Nils Philippsen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=511276

Nils Philippsen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugzilla.redhat.com
                   |                            |/show_bug.cgi?id=727541