Bug 481141
Summary: | Review Request: pycryptsetup - Python bindings for cryptsetup tool | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Martin Sivák <msivak> |
Component: | Package Review | Assignee: | Matěj Cepl <mcepl> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dan, fedora-package-review, mcepl, notting |
Target Milestone: | --- | Flags: | mcepl:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-02-09 10:56:53 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Martin Sivák
2009-01-22 12:17:17 UTC
Firstly if your package should be also in other distro, please reconsider inclusion of spec file. That's for your upstream package ;-) Now Fedora packaging: FAIL source files match upstream You should create tar file somewhere on the project homepage. FAIL package meets naming and versioning guidelines. Use python-%{name} FAIL specfile is properly named, is cleanly written and uses macros consistently. macro %name-%version should be %{name}-%{version}. Without {} take some global constants. Anyway brackets are used in most cases. OK dist tag is present. OK build root is correct. OK license field matches the actual license. OK license is open source-compatible. License text not included upstream. OK latest version is being packaged. OK BuildRequires are proper. OK %clean is present. OK package builds in mock (Rawhide/x86_64). OK debuginfo package is need. FAIL rpmlint is silent. later OK final provides and requires look sane. OK no shared libraries are added to the regular linker search paths. OK owns the directories it creates. OK no duplicates in %files. OK file permissions are appropriate. OK no scriptlets present. OK code, not content. OK documentation is small, so no -docs subpackage is necessary. OK %docs are not necessary for the proper functioning of the package. OK no headers. OK no pkgconfig files. OK no libtool .la droppings. pycryptsetup.src: E: no-changelogname-tag pycryptsetup.x86_64: E: no-changelogname-tag pycryptsetup-debuginfo.x86_64: E: no-changelogname-tag Docs are usually package in %files section like %doc CHANGES etc. Ok, modified spec and srpm: http://msivak.fedorapeople.org/pycryptsetup/python-cryptsetup.spec http://msivak.fedorapeople.org/pycryptsetup/python-cryptsetup-0.0.1-1.fc8.src.rpm We do not have homepage at the moment though, but this project is related to https://fedoraproject.org/wiki/Anaconda/StorageRewrite (so i set the Url temporarily to this page). First of all: > varování: Nalezeny instalované, ale nezabalené soubory: > /usr/lib/pycryptsetup/examples/selftest.py > /usr/lib/pycryptsetup/examples/selftest.pyc > /usr/lib/pycryptsetup/examples/selftest.pyo Just let it be uninstalled (setup.py would leave it alone) and put examples/ into %doc. And now about the spec file: > %{!?python_sitelib: %define python_sitelib %(%{__python} -c "from distutils.sysc onfig import get_python_lib; print get_python_lib()")} > %{!?python_sitearch: %define python_sitearch %(%{__python} -c "from distutils.sy sconfig import get_python_lib; print get_python_lib(1)")} Only one of these, please. > #I don't want the unpackaged file check > %define _unpackaged_files_terminate_build 0 Why? There is no reason in the world, I would know about, why you don't want it, and actually it would save your from the above blunder. Either explain pretty well in the comment why you need, or get rid of it. > Requires: cryptsetup-luks Needless Requires should be avoided. [matej@viklef redhat]$ rpm -qRp RPMS/i386/pycryptsetup-0.0.1-1.fc10.i386.rpm libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3) libcryptsetup.so.0 libpthread.so.0 libpython2.5.so.1.0 python(abi) = 2.5 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) [matej@viklef redhat]$ rpm -q --whatprovides libcryptsetup.so.0 cryptsetup-luks-1.0.6-6.fc10.i386 [matej@viklef redhat]$ > BuildRequires: python-devel Needless BuildRequires should be avoided as well: [matej@viklef ~]$ rpm -qR python-setuptools-devel /usr/bin/python python(abi) = 2.5 python-devel python-setuptools = 0.6c9-1.fc10 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 [matej@viklef ~]$ > %attr(0644,root,root) %{_datadir}/doc/%name-%version/COPYING This should be %doc COPYING --- Please replace all occurrences of %{__rm}, %{__install}, %{__mv} and others with direct calls to the real command. (OK, you won with %{__python}) --- Still unhappy rpmlint: [matej@viklef redhat]$ rpmlint -i SRPMS/pycryptsetup-0.0.1-1.fc10.src.rpm [matej@viklef redhat]$ rpmlint -i SRPMS/pycryptsetup-0.0.1-1.fc10.src.rpm pycryptsetup.src: E: no-changelogname-tag There is no %changelog tag in your spec file. To insert it, just insert a '%changelog' in your spec file and rebuild it. pycryptsetup.src: W: strange-permission pycryptsetup.spec 0600 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. 1 packages and 0 specfiles checked; 1 errors, 1 warnings. [matej@viklef redhat]$ --- Source0 should be URL -- https://fedoraproject.org/wiki/Packaging/SourceURL Cannot compare MD5 checksum with upstream. URL: field doesn't make sense as well. Please, make the upstream for it and THEN file a version of the package for review. --- It does build in koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=1074220) but it shouldn't because of broken %files -- see the top of this comment. Please fix all the above problems, build upstream, and then we can go through the next round of review. Still not silent rpmlint: > python-cryptsetup.src: W: strange-permission python-cryptsetup.spec 0600 > A file that you listed to include in your package has strange permissions. > Usually, a file should have 0644 permissions. That's from your playing with git ;-) and on the binary package: > python-cryptsetup.x86_64: W: spurious-executable-perm /usr/share/doc/python- cryptsetup-0.0.1/selftest.py > python-cryptsetup.x86_64: W: incoherent-version-in-changelog 0.0.1 ['0.0.1-1.fc10', '0.0.1-1'] > python-cryptsetup.x86_64: W: unstripped-binary-or-object /usr/lib64/python2.5/site-packages/cryptsetup.so > python-cryptsetup.x86_64: W: doc-file-dependency /usr/share/doc/python-cryptsetup-0.0.1/selftest.py /usr/bin/python Please, get rid of these (or at least most of them -- the last probably goes away if the examples are not executable). No idea, about unscript binary object. > #Requires: cryptsetup-luks It is considered bad habit to comment out in spec files -- CVS is not an ideal VCS, but we have previous versions recorded. > Requires: python Don't you care about version? Will python 1.5 work? (OK, maybe not that, but people may want to rebuild this on RHEL for example). What happened to examples/ ? Still no URL in Source. Stuff you have in URL: field should be in Source: However, MD5 is OK d6b3486e096097c52e939b40c33aa130 So, yes, we need one more round to get this together. (In reply to comment #4) > Still not silent rpmlint: > > > python-cryptsetup.src: W: strange-permission python-cryptsetup.spec 0600 > > A file that you listed to include in your package has strange permissions. > > Usually, a file should have 0644 permissions. > > That's from your playing with git ;-) > > and on the binary package: > > python-cryptsetup.x86_64: W: spurious-executable-perm /usr/share/doc/python- cryptsetup-0.0.1/selftest.py > > python-cryptsetup.x86_64: W: incoherent-version-in-changelog 0.0.1 ['0.0.1-1.fc10', '0.0.1-1'] > > python-cryptsetup.x86_64: W: unstripped-binary-or-object /usr/lib64/python2.5/site-packages/cryptsetup.so > > python-cryptsetup.x86_64: W: doc-file-dependency /usr/share/doc/python-cryptsetup-0.0.1/selftest.py /usr/bin/python > > Please, get rid of these (or at least most of them -- the last probably goes > away if the examples are not executable). No idea, about unscript binary > object. > unstripped-binary-or-object is usually result of missing the executable bits on that object All fixed. APPROVED!!! New Package CVS Request ======================= Package Name: python-cryptsetup Short Description: Python libraries to ease LUKS manipulation from python code Owners: msivak Branches: Devel InitialCC: cvs done. Martin, you should close this bug as CLOSED/NEXTRELEASE after importing and building the package. Ah, thanks, will do |