Bug 1019770 - Review Request: lua-term - lua module for manipulating terminal [NEEDINFO]
Review Request: lua-term - lua module for manipulating terminal
Status: CLOSED DUPLICATE of bug 1093503
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-16 07:29 EDT by Jiri Machala
Modified: 2014-05-01 18:52 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-05-01 18:52:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
orion: needinfo? (george.machala+rhbugzilla)


Attachments (Terms of Use)

  None (edit)
Description Jiri Machala 2013-10-16 07:29:27 EDT
Spec URL: http://www.fi.muni.cz/~xmachal4/fedorapackages/lua-term.spec
SRPM URL: http://www.fi.muni.cz/~xmachal4/fedorapackages/lua-term-0.02-1.fc19.src.rpm
Description: lua-term is a Lua module for manipulating a terminal
Fedora Account System Username: jmachala

koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=6064842

Hello,
this is my first fedora package (I'm trying to put into practice what I learned week ago on fedora packaging workshop :) ), so I'll need a sponsor for it. It's really simple lua module so I suppose there shouldn't be any problems with it.
I need it as a dependency for a software(Lmod), I'd like to package next.
Comment 1 Johan Swensson 2013-10-19 05:22:22 EDT
A few comments.

drop %clean

You have duplicate file listings.
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#DuplicateFiles
Comment 2 Michael Schwendt 2013-10-21 11:04:26 EDT
Right.

[...]

The file at "Spec URL" and the spec file in the src.rpm are not the same. The diff is:

-%{lualibdir}/term/core.so
+%attr(644, -, -) %{lualibdir}/term/core.so

The %changelog ought to have mentioned that. ;)

So far, we keep shared libs executable, since that's a requirement for the automatic -debuginfo generation and stripping done by rpmbuild. We must not turn them -x in %install. Setting them to -x via %attr works _currently_, because apparently it sets the file attribute at a sufficiently later point.  Though, it's widely accepted practise to restrict usage of %attr to setting really special/unusual permissions (e.g. setuid, setgid, g-rx) and owner/group changes, so special attributes set with %attr really stick out (especially when using syntax highlighting). 
There's nothing in the packaging guidelines about it yet. Better not get used to 
using %attr for ordinary permissions that could be set in %install. In packages with many more files, if you needed to "fix" permissions, overusing %attr would reduce readability of the spec file a lot.
Comment 3 Orion Poplawski 2014-04-02 00:48:36 EDT
Jiri - Are you still interested in packaging this?  I can sponsor you if so, but you should address the above concerns fist.
Comment 4 Orion Poplawski 2014-04-02 01:14:22 EDT
Package doesn't work on Fedora 20:

# lua
Lua 5.2.2  Copyright (C) 1994-2013 Lua.org, PUC-Rio
> local term = require 'term'
error loading module 'term.cursor' from file '/usr/share/lua/5.2/term/cursor.lua':
        /usr/share/lua/5.2/term/cursor.lua:24: unexpected symbol near 'goto'
stack traceback:
        [C]: in ?
        [C]: in function 'require'
        /usr/share/lua/5.2/term/init.lua:43: in main chunk
        [C]: in function 'require'
        stdin:1: in main chunk
        [C]: in ?

I've filed https://github.com/hoelzro/lua-term/issues/7
Comment 5 Orion Poplawski 2014-04-02 22:56:45 EDT
Upstream has now released 0.3 that fixes this.
Comment 6 Orion Poplawski 2014-04-14 18:45:27 EDT
Jiri - Are you still interested in this?  I need this ASAP for Lmod, and will submit my own review soon if I don't here back from you.  Thanks.
Comment 7 Orion Poplawski 2014-05-01 18:52:26 EDT

*** This bug has been marked as a duplicate of bug 1093503 ***

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