Bug 549296 - Review Request: lshell - Python-based limited Shell
Summary: Review Request: lshell - Python-based limited Shell
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Thomas Spura
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-12-21 08:56 UTC by Fabian Affolter
Modified: 2011-05-18 21:17 UTC (History)
5 users (show)

Fixed In Version: lshell-0.9.12-2.fc12
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-07-13 07:36:26 UTC
Type: ---
tomspur: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Fabian Affolter 2009-12-21 08:56:57 UTC
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/lshell.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/lshell-0.9.8-1.fc12.src.rpm

Project URL: http://ghantoos.org/limited-shell-lshell

Description:
lshell provides a limited shell configured per user. The configuration
is done quite simply using a configuration file.

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1882672

rpmlint output:
[fab@localhost noarch]$ rpmlint lshell-0.9.8-1.fc12.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[fab@localhost SRPMS]$ rpmlint lshell-0.9.8-1.fc12.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Thomas Spura 2010-01-22 18:22:58 UTC
This builds just fine with python3, because you don't need to have python-setuptools (the egg is builded by distutils).

How about building python3 only?

If you want to package this for F-12 and/or EPEL you need to be more explicit, see https://fedoraproject.org/wiki/PackagingDrafts/Python3#Guidelines_for_adding_python3_subpackages_to_an_existing_package


$ diff -u lshell.spec py3.spec 
--- lshell.spec	2009-12-21 09:53:01.000000000 +0100
+++ py3.spec	2010-01-22 14:39:42.835184038 +0100
@@ -1,5 +1,3 @@
-%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
-
 Name:           lshell
 Version:        0.9.8
 Release:        1%{?dist}
@@ -12,8 +10,7 @@
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 
-BuildRequires:  python-devel
-BuildRequires:  python-setuptools
+BuildRequires:  python3-devel
 
 
 %description
@@ -27,15 +24,18 @@
 chmod -x CHANGES
 #Remove shebang
 sed -i -e '/^#!\//, 1d' lshellmodule/lshell.py
+# Add python3 shebang
+sed -i -e 's|^#!/usr/bin/env python|#!/usr/bin/python3|g' bin/lshell
 
 
 %build
-%{__python} setup.py build
+2to3-3 -w .
+%{__python3} setup.py build
 
 
 %install
 rm -rf %{buildroot}
-%{__python} setup.py install -O1 --skip-build --root=%{buildroot}
+%{__python3} setup.py install -O1 --skip-build --root=%{buildroot}
 #doc files at the wrong place
 rm %{buildroot}%{_defaultdocdir}/lshell/{CHANGES,COPYING,README}
 
@@ -50,8 +50,8 @@
 %{_mandir}/man*/*.gz
 %{_bindir}/lshell
 %config(noreplace) %{_sysconfdir}/lshell.conf
-%{python_sitelib}/lshell.py*
-%{python_sitelib}/lshell*.egg-info
+%{python3_sitelib}/lshell.py*
+%{python3_sitelib}/lshell*.egg-info
 
 
 %changelog

Comment 2 Dave Malcolm 2010-01-22 18:39:01 UTC
(In reply to comment #1)
> This builds just fine with python3, because you don't need to have
> python-setuptools (the egg is builded by distutils).

This may seem like a silly question, but do the built packages actually _work_ with python3?

Comment 3 Thomas Spura 2010-04-24 19:23:03 UTC
Ping.

You didn't remove yourself from vacation [1] yet, but according to the page should be back again.

Are you?


[1] http://fedoraproject.org/wiki/Vacation

Comment 4 Fabian Affolter 2010-04-28 11:09:43 UTC
Sorry for the delay.  Yes, I'm back.  0.9.10 was released. Time for an update and a test with python3 according your comment.

Comment 5 Fabian Affolter 2010-06-06 16:24:17 UTC
I guess that I will wait till upstream goes Python 3.

But thanks for the comment #1. 

Here are the updated files:

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/lshell.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/lshell-0.9.12-1.fc13.src.rpm

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2233599

Comment 6 Thomas Spura 2010-06-06 17:19:31 UTC
I don't see anything about python2 only or python3, but ok.

So here is the review:

Good:
- macros everywhere
- name/version ok
- group ok
- arch: noarch ok
- %prep ok
- %build ok
- %install ok
- %clean there
- buildroot ok
- license ok
- no libs
- no *.la
- requires look ok (none)


Needswork:
- $ rpmlint -I non-conffile-in-etc
non-conffile-in-etc:
A non-executable file in your package is being installed in /etc, but is not a
configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

- too much BR: setuptools is not needed, because the egg is build with distuils
  (buils in koji without setuptools: http://koji.fedoraproject.org/koji/taskinfo?taskID=2233657)

##############################################################################

Not much todo...

##############################################################################

APPROVED

Comment 7 Fabian Affolter 2010-06-15 11:13:49 UTC
Thanks for the review Thomas.

Comment 8 Fabian Affolter 2010-06-15 11:17:03 UTC
New Package CVS Request
=======================
Package Name: lshell 
Short Description: Python-based limited Shell
Owners: fab
Branches: F-12 F-13

Comment 9 Jason Tibbitts 2010-06-16 22:06:19 UTC
CVS done (by process-cvs-requests.py).

Comment 10 Fedora Update System 2010-07-04 22:45:18 UTC
lshell-0.9.12-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/lshell-0.9.12-2.fc12

Comment 11 Fedora Update System 2010-07-04 22:45:23 UTC
lshell-0.9.12-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/lshell-0.9.12-2.fc13

Comment 12 Fedora Update System 2010-07-06 17:16:57 UTC
lshell-0.9.12-2.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update lshell'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/lshell-0.9.12-2.fc13

Comment 13 Fedora Update System 2010-07-06 17:21:40 UTC
lshell-0.9.12-2.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update lshell'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/lshell-0.9.12-2.fc12

Comment 14 Fedora Update System 2010-07-13 07:36:20 UTC
lshell-0.9.12-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Fedora Update System 2010-07-13 07:50:52 UTC
lshell-0.9.12-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fabian Affolter 2011-05-17 06:54:57 UTC
Package Change Request
======================
Package Name: lshell
New Branches: el5 el6
Owners: fab

Comment 17 Jason Tibbitts 2011-05-18 21:17:15 UTC
Git done (by process-git-requests).


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