Bug 865915 - (python-py9p) Review Request: python-py9p - Pure Python implementation of 9p protocol
Review Request: python-py9p - Pure Python implementation of 9p protocol
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-12 15:39 EDT by Saveliev Peter
Modified: 2013-10-31 20:59 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-11-07 12:12:49 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Saveliev Peter 2012-10-12 15:39:47 EDT
Spec URL: http://peet.spb.ru/archives/python-py9p.spec
SRPM URL: http://peet.spb.ru/archives/python-py9p-1.0.1-1.fc16.src.rpm
Description:

Protocol 9P is developed for Plan9 operating system from Bell Labs.
It is used for remote file access, and since files are key objects
in Plan9, 9P can be used also for composite file access, RPC etc.

github: https://github.com/svinota/py9p

Fedora Account System Username: psavelye
Comment 1 Peter Lemenkov 2012-10-13 02:10:18 EDT
I'll review it (and will sponsor you apparently).
Comment 2 Peter Lemenkov 2012-10-13 02:14:50 EDT
> psavelye

I wonder why Red Hat still restricts user names to 8 symbols? What year is in Brno now?
Comment 3 Peter Lemenkov 2012-10-13 02:15:59 EDT
Lifting FE-NEEDSPONSOR - I've just sponsored Peter.
Comment 4 Peter Lemenkov 2012-10-13 02:47:06 EDT
Koji scratchbuild for Rawhide:

* http://koji.fedoraproject.org/koji/taskinfo?taskID=4586979

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+/- rpmlint is NOT silent

sulaco ~/rpmbuild/SPECS: rpmlint ~/rpmbuild/SRPMS/python-py9p-1.0.1-1.fc19.src.rpm ~/rpmbuild/RPMS/noarch/python-py9p-1.0.1-1.fc19.noarch.rpm 
python-py9p.src: W: spelling-error %description -l en_US pyvfs -> payoffs
python-py9p.noarch: W: spelling-error %description -l en_US pyvfs -> payoffs

^^^ false positive

python-py9p.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/py9p/py9p.py 0644L /usr/bin/env
python-py9p.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/py9p/pki.py 0644L /usr/bin/env
python-py9p.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/py9p/sk1.py 0644L /usr/bin/env

^^^ I don't think this is a blocker but I don't see any reason why these files should be executed directly. So could you, please, add something like this to the %prep section:

sed -i -e "1d" py9p/pki.py py9p/py9p.py py9p/sk1.py

2 packages and 0 specfiles checked; 3 errors, 2 warnings.
sulaco ~/rpmbuild/SPECS: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (MIT).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum py9p-1.0.1.tar.gz*
9286733887750eeeec629fff6eca33db0a1e0449efbd73cff2d466127164d2e2  py9p-1.0.1.tar.gz
9286733887750eeeec629fff6eca33db0a1e0449efbd73cff2d466127164d2e2  py9p-1.0.1.tar.gz.1
sulaco ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package consistently uses macros. Well, to be honest in order to fully comply to this requirement you should replace the only $RPM_BUILD_ROOT instance to %{buildroot} but I wouldn't really insist on this.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ All filenames in rpm packages are valid UTF-8.


I don't see any issues (except the small almost cosmetic one with /usr/bin/enb noted above) so this package is

APPROVED.
Comment 5 Saveliev Peter 2012-10-13 06:20:25 EDT
(In reply to comment #4)
<skip />
> python-py9p.noarch: E: non-executable-script
> /usr/lib/python2.7/site-packages/py9p/sk1.py 0644L /usr/bin/env
> 
> ^^^ I don't think this is a blocker but I don't see any reason why these
> files should be executed directly. So could you, please, add something like
> this to the %prep section:
> 
> sed -i -e "1d" py9p/pki.py py9p/py9p.py py9p/sk1.py

Since I maintain this fork, I think, we can just remove the line from the source code: it is of no use here.

<skip /> 
> I don't see any issues (except the small almost cosmetic one with
> /usr/bin/enb noted above) so this package is
> 
> APPROVED.

Thanks. Can I remove the «env» clause in this build, or should do in the next one?
Comment 6 Saveliev Peter 2012-10-13 06:22:53 EDT
(In reply to comment #2)
> > psavelye
> 
> I wonder why Red Hat still restricts user names to 8 symbols? What year is
> in Brno now?

For me it is no problem as long as they keep my short mail alias :)
Comment 7 Saveliev Peter 2012-10-13 06:27:10 EDT
Peter, should I remove FE_NEEDSPONSOR from this package — https://bugzilla.redhat.com/show_bug.cgi?id=865630 ? Or, maybe, can I abuse you with its review also?
Comment 8 Saveliev Peter 2012-10-13 06:47:04 EDT
But right now there is a little help from the short username… fedora-cvs flag is not available for me, despite the mail in my Fedora accaunt is the same as in this bug, and there I'm in Fedora Bugs Group.
Comment 9 Peter Lemenkov 2012-10-13 07:08:04 EDT
(In reply to comment #5)
> 
> Thanks. Can I remove the «env» clause in this build, or should do in the
> next one?

If you don't have any objections against it, then I think it's better to remove it asap - in the current build.

(In reply to comment #8)
> But right now there is a little help from the short username… fedora-cvs
> flag is not available for me, despite the mail in my Fedora accaunt is the
> same as in this bug, and there I'm in Fedora Bugs Group.

Unfortunately I can't help much here - you should poke some of your fellow redhatters for that (informal, fast, and efficient way). I suspect that there is something extra required in case of RedHat accounts but I could be wrong.
Comment 10 Saveliev Peter 2012-10-13 08:31:41 EDT
removed «env» clauses and fixed pep8 issues and 99% of pyflakes warnings (two remaining issues need some more time to fix, but the code works)

uploaded new tgz and rpms
Comment 11 Saveliev Peter 2012-10-13 09:25:54 EDT
New Package SCM Request
=======================
Package Name: python-py9p
Short Description: Pure Python implementation of 9P protocol (Plan9)
Owners: psavelye
Branches: f16 f17 f18 el6
InitialCC:
Comment 12 Gwyn Ciesla 2012-10-14 21:49:54 EDT
Git done (by process-git-requests).

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