Bug 440676 - Review Request: lua-filesystem - File System Library for Lua
Review Request: lua-filesystem - File System Library for Lua
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
Depends On:
Blocks: 440681
  Show dependency treegraph
Reported: 2008-04-04 09:55 EDT by Tim Niemueller
Modified: 2009-10-03 20:07 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-04-07 17:46:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Tim Niemueller 2008-04-04 09:55:12 EDT
Spec URL: http://fedorapeople.org/~timn/luastuff/lua-filesystem.spec
SRPM URL: http://fedorapeople.org/~timn/luastuff/lua-filesystem-1.4.0-1.fc8.src.rpm
LuaFileSystem is a Lua library developed to complement the set of functions
related to file systems offered by the standard Lua distribution.

LuaFileSystem offers a portable way to access the underlying directory
structure and file attributes.

Website: http://www.keplerproject.org/luafilesystem/
Comment 1 Jason Tibbitts 2008-04-04 18:43:45 EDT
Builds fine and rpmlint is silent.

The compiler is not called with the proper flags.  This leads to a broken
debuginfo package, among other issues.  It looks like you'll need to patch the
"config" file as it overwrites the passed CFLAGS.

* source files match upstream:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
X compiler flags are not correct.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
X debuginfo package is not complete.
* rpmlint is silent.
* final provides and requires are sane:
   lua-filesystem = 1.4.0-1.fc9
   lua >= 5.1
* %check is not present; no test suite upstream.  I do not know how to test this 
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
Comment 2 Tim Niemueller 2008-04-04 18:54:09 EDT
I'm setting the CFLAGS to %{optflags} now (I didn't pass anything yet). I have
to add an additional "-fPIC" or otherwise I get the following compiler error (on
/usr/bin/ld: src/lfs.o: relocation R_X86_64_32 against `a local symbol' can not
be used when making a shared object; recompile with -fPIC
src/lfs.o: could not read symbols: Bad value

Does that satisfy debuginfo generation?

New SRPM is at
Comment 3 Jason Tibbitts 2008-04-04 19:10:30 EDT
Yes, debuginfo is OK now.

Do note that I see the following unpleasant compiler warnings:

src/lfs.c: In function 'get_dir':
src/lfs.c:112: warning: implicit declaration of function 'free'
src/lfs.c:112: warning: incompatible implicit declaration of built-in function

which will probably cause a build failure if we ever turn on
-Werror-implicit-function-declaration by default.

Do note also that your passing of CFLAGS disables the default -I arguments that
the makefile passes.  It turns out that it's only /usr/include so you should be OK.

Anyway, everything looks acceptable now.
Comment 4 Tim Niemueller 2008-04-04 19:23:20 EDT
You are right, I have created a trivial patch to fix the problem and included it
in http://fedorapeople.org/~timn/luastuff/lua-filesystem-1.4.0-3.fc8.src.rpm and
sent it upstream.

Are you willing to take over the other reviews of my Lua packages (440677 to
440681) as well?
Comment 5 Tim Niemueller 2008-04-04 19:25:31 EDT
New Package CVS Request
Package Name: lua-filesystem
Short Description: File System Library for Lua
Owners: timn
Branches: F-7 F-8
Cvsextras Commits: yes
Comment 6 Jason Tibbitts 2008-04-04 19:28:42 EDT
I'm working on lua-posix at the moment.  These packages are mostly trivial so I
should be able to work through them pretty quickly.
Comment 7 Kevin Fenzi 2008-04-05 12:49:11 EDT
cvs done.
Comment 8 Michel Alexandre Salim 2009-10-01 18:05:10 EDT
Discussed with Tim; we'd want to eventually have the entire Lua stack on EPEL.

Package Change Request
Package Name: lua-filesystem
New Branches: EL-4 EL-5
Owners: timn salimma
Comment 9 Kevin Fenzi 2009-10-03 17:34:22 EDT
cvs done.
Comment 10 Fedora Update System 2009-10-03 20:07:06 EDT
lua-filesystem-1.4.2-1.el5 has been submitted as an update for Fedora EPEL 5.

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