Spec URL: http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py.spec SRPM URL: http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0-1-1.src.rpm 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.
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.
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 511352 has been marked as a duplicate of this bug. ***
*** Bug 511351 has been marked as a duplicate of this bug. ***
I'll take this.
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_.28python_modules.29 (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
Released new versions at: http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-py.spec http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0-1-2-src.rpm http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0-1-2-noarch.rpm http://www.open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0-1-tar.gz All except the mock thing should work now. What do you think? Mark
I just verified that the latest src rpm builds in mock. -Mark
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.
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.1.tar.gz http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py.spec http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0.1-3.src.rpm http://open-sharedroot.org/development/comoonics-base-py/comoonics-base-py-0.1-3.noarch.rpm
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-py-%{version}.tar.gz 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 ;-).
New Package CVS Request ======================= Package Name: comoonics-base-py Short Description: Comoonics minimum baselibraries Owners: markhla elcody02 Branches: InitialCC:
CVS done.