Bug 635453

Summary: Review Request: deafshell - Deaf Shell is used for system accounts dedicated to ssh port forwarding
Product: [Fedora] Fedora Reporter: François Cami <fdc>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mario.blaettermann, martin.gieseking, notting, pahan, redhat-bugzilla
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-09-09 14:40:49 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 201449    

Description François Cami 2010-09-19 16:30:15 EDT
Spec URL: http://fcami.fedorapeople.org/srpms/deafshell.spec
SRPM URL: http://fcami.fedorapeople.org/srpms/deafshell-0.1-1.fc13.src.rpm
Deaf Shell is intended to be used as a shell for system accounts dedicated to
OpenSSH port forwarding, as in:
ssh -L [bind_address:]port:host:hostport] user@host
ssh -R [bind_address:]port:host:hostport] user@host
Deaf Shell will print some information and sleep until receiving a termination
signal (Ctrl+C or SIGINT from the user, SIGTERM from the system for instance).
It will also print a character every ten seconds.
Comment 1 Mark McKinstry 2010-09-19 23:34:12 EDT

A few comments:

1. The license should be GPLv3+
2. You could replace the setup and prep sections with just ''%setup -q -n %{name}-%{name}-88674c3'
3. In the build section, it should just be 'make' otherwise rpmlint in unhappy with the unstripped binary
4. In the files section, you should specify the binary as '%{_bindir}deafshell'
Comment 2 François Cami 2010-09-20 02:03:19 EDT
Hi Mark,

Thanks for the comments.

1. Fixed
2. I don't think so, 88674c3 comes from git and is bound to change at each release tag - what I did will alleviate the problem.
3. I don't think so either - if we strip the binaries during the build, rpmbuild won't generate the debuginfo rpm, which is a bug. Do you have the exact error message?
4. Thanks, fixed.

Spec URL: http://fcami.fedorapeople.org/srpms/deafshell.spec
SRPM URL: http://fcami.fedorapeople.org/srpms/deafshell-0.1-2.fc13.src.rpm

Thank you
Comment 3 Mark McKinstry 2010-09-20 08:37:24 EDT

3. Whoops, my bad.
Comment 4 Martin Gieseking 2010-10-08 11:51:37 EDT
Hi François,

here are some more comments on your package:

- Don't repeat the package/program name in the Summary and keep it concise. 
  Maybe something like the following is more appropriate:
  "Shell for system accounts dedicated to OpenSSH port forwarding"

- Since the source tarball is a snapshot release referring to a changeset of the 
  upstream Git repo, you should adapt the Release field according to

- Use the %{version} macro in the URL field.

- Ensure that Fedora's build flags are used, i.e. add CFLAGS='%{optflags} -lm'
  to the make statement.

- Also, add  %{_smp_mflags} to "make" to enable parallel builds.
Comment 5 François Cami 2010-11-07 19:26:36 EST
I've updated the program summary and added CFLAGS. Thank you.

However, the source tarball is not a snapshot release, it is related to the 0.1 git tag of the upstream git repository. See https://github.com/deafshell/deafshell/tree/v0.1 - therefore, I don't think I should change the naming to the changeset, though I agree github's tarball generation messes things up. If this is not acceptable to you, I will revert to using the git hash.

I normally use %{_smp_mflags} but there is only a single C file to compile. I will update the package if upstream ever becomes more than that.

Spec URL: http://fcami.fedorapeople.org/srpms/deafshell.spec
SRPM URL: http://fcami.fedorapeople.org/srpms/deafshell-0.1-3.fc14.src.rpm
Comment 6 Mario Blättermann 2012-08-26 11:18:44 EDT
@François, are you still interested in to work on this package? Your spec and srpm links are dead, and the last commit to upstream's Git was more than two years ago.
Comment 7 Mario Blättermann 2012-09-09 14:40:49 EDT
Closing this review request now, adding FE-DEADREVIEW.