Bug 522980 - Review Request: lua-json - JSON Parser/Constructor for Lua
Summary: Review Request: lua-json - JSON Parser/Constructor for Lua
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tim Niemueller
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 522979
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-09-12 23:13 UTC by Michel Alexandre Salim
Modified: 2009-10-21 00:42 UTC (History)
3 users (show)

Fixed In Version: 1.0-1.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-21 00:41:12 UTC
tim: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Michel Alexandre Salim 2009-09-12 23:13:39 UTC
Spec URL: http://salimma.fedorapeople.org/specs/funpl/lua-json.spec
SRPM URL: http://salimma.fedorapeople.org/specs/funpl/lua-json-1.0-1.fc12.src.rpm
Description:
LuaJSON is a customizable JSON decoder/encoder, using LPEG for parsing.

Comment 1 Tim Niemueller 2009-09-21 22:17:35 UTC
MUST

* OK: rpmlint
# rpmlint lua-json.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
# rpmlint lua-json-1.0-1.fc11.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# rpmlint lua-json-1.0-1.fc11.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

* OK: package name
* OK: package version and release
* OK: spec file name
* OK: package guideline-compliant
* OK: license complies with guidelines
* OK: license field accurate
* OK: license file not deleted
* OK: spec in US English
* OK: spec legible
* ?: source matches upstream
  sha256sum 32650b8f35ff57c0ce79aca2ed659f1fe50c50b5237c520ee61518b6c4a05949
* OK: builds under >= 1 archs, others excluded
* OK: dependencies (requires)
* OK: build dependencies complete
* N/A: locales handled using %find_lang, no %{_datadir}/locale
* N/A: library -> ldconfig
* N/A: relocatable: give reason
* FAIL: own all directories
  Must contain in %files:
  %dir %{luapkgdir}/json
  %dir %{luapkgdir}/json/decode
  %dir %{luapkgdir}/json/encode
* OK: no dupes in %files
* OK: permission
* OK: %clean RPM_BUILD_ROOT
* FAIL: macros used consistently
  Most of the time you use %{dir} macros, but then you use $RPM_BUILD_ROOT,
  should be %{buildroot}
* OK: Package contains code
* N/A: large docs => -doc
* OK: doc not runtime dependent
* N/A: headers in -devel
* N/A: static in -static
* N/A: if contains *.pc, req pkgconfig
* N/A: if libfiles are suffixed, the non-suffixed goes to devel
* N/A: devel requires versioned base package
* N/A: desktop file uses desktop-file-install
* OK: clean buildroot before install
* OK: filenames UTF-8

SHOULD
* OK: if license text missing, ask upstream to include it
* N/A: desc and summary contain translations if available
* N/A: package build in mock on all architectures
  lua-lunit not tagged in koji build root, therefore no scratch build possible
* ?: package functioned as described
  %check suggests so, haven't checked myself.
* OK: scriplets are sane
  If %check fails, you should cat testlog.txt such that the user knows what
  happened.
* N/A: other subpackages should require versioned base
* N/A: if main pkg is development-wise, pkgconfig can go in main package
* OK: require package not files

Preventing approval:
- Directories must be owned
- Consistent macro usage

Comment 2 Michel Alexandre Salim 2009-09-21 22:31:42 UTC
(In reply to comment #1)
> * ?: source matches upstream
>   sha256sum 32650b8f35ff57c0ce79aca2ed659f1fe50c50b5237c520ee61518b6c4a05949
Not sure how that happens. I just redownloaded twice:
spectool -gf lua-json.spec

and verified the old SRPM, and the two downloads. All of them check out:

$ sha1sum luajson-1.0.tar.bz2.try1 luajson-1.0.tar.bz2 /home/michel/rpmbuild/SOURCES/luajson-1.0.tar.bz2 
ca09374ec38b94573232e82e8b27c960c6f5cd9e  luajson-1.0.tar.bz2.try1
ca09374ec38b94573232e82e8b27c960c6f5cd9e  luajson-1.0.tar.bz2
ca09374ec38b94573232e82e8b27c960c6f5cd9e  /home/michel/rpmbuild/SOURCES/luajson-1.0.tar.bz2

> * FAIL: own all directories
>   Must contain in %files:
>   %dir %{luapkgdir}/json
>   %dir %{luapkgdir}/json/decode
>   %dir %{luapkgdir}/json/encode
Not really: I do own everything under %luapkgdir}. The %dir directive would
instruct RPM to own the directory only, without the files within, necessitating
additional lines for %{luapkgdir}/json/decode/* and %{luapkgdir}/json/encode/*

Best way to check is to do rpm -qpl on the binary RPM itself

> * FAIL: macros used consistently
>   Most of the time you use %{dir} macros, but then you use $RPM_BUILD_ROOT,
>   should be %{buildroot}
Hm. RPM_BUILD_ROOT is allowed, I think.

See http://fedoraproject.org/wiki/Packaging:Guidelines#macros

what's not allowed is mixing up %{buildroot} and $RPM_BUILD_ROOT within the same spec file.

> Preventing approval:
> - Directories must be owned
> - Consistent macro usage  

These two are, I think, non-issues. Do let me know; I'll use %{buildroot} if you insist :)

Comment 3 Tim Niemueller 2009-09-21 23:03:10 UTC
(In reply to comment #2)

> and verified the old SRPM, and the two downloads. All of them check out:
> 
> $ sha1sum luajson-1.0.tar.bz2.try1 luajson-1.0.tar.bz2

I had given the sha256sum, while you used sha1sum... My bad, I simply forgot to replace ? by OK. So yes, the checksums of your SRPM and the original source do match!

> > * FAIL: own all directories
> >   Must contain in %files:
> >   %dir %{luapkgdir}/json
> >   %dir %{luapkgdir}/json/decode
> >   %dir %{luapkgdir}/json/encode
> Not really: I do own everything under %luapkgdir}. The %dir directive would
> instruct RPM to own the directory only, without the files within, necessitating
> additional lines for %{luapkgdir}/json/decode/* and %{luapkgdir}/json/encode/*

I remember being instructed once to explicitly name all directories. But checking some of my other packages confirms your statement. The %dir was used in "special" situations (e.g. in the lua-sql package). So this seems to be ok then. I did check the package contains, but was unsure about possible meta data %dir might record.

> what's not allowed is mixing up %{buildroot} and $RPM_BUILD_ROOT within the
> same spec file.

Yes, that's what I thought before I read the guidelines and how many of my spec files actually look. I leave it up to you. And as said in #522979 I agree with your interpretation of the buildroot guideline (now after I have thought about it a second time...).

Since the %dir issue is actually none:

APPROVED

Comment 4 Michel Alexandre Salim 2009-09-23 11:48:05 UTC
Thanks!

New Package CVS Request
=======================
Package Name: lua-json
Short Description: JSON Parser/Constructor for Lua
Owners: salimma
Branches: EL-5 F-10 F-11
InitialCC:

Comment 5 Kevin Fenzi 2009-09-24 04:37:29 UTC
cvs done.

Comment 6 Fedora Update System 2009-09-28 20:00:50 UTC
lua-json-1.0-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/lua-json-1.0-1.fc10

Comment 7 Fedora Update System 2009-09-28 20:00:56 UTC
lua-json-1.0-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/lua-json-1.0-1.fc11

Comment 8 Fedora Update System 2009-09-30 01:34:13 UTC
lua-json-1.0-1.fc10 has been pushed to the Fedora 10 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 lua-json'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-10099

Comment 9 Fedora Update System 2009-09-30 01:35:51 UTC
lua-json-1.0-1.fc11 has been pushed to the Fedora 11 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 lua-json'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10102

Comment 10 Fedora Update System 2009-10-21 00:41:03 UTC
lua-json-1.0-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 11 Fedora Update System 2009-10-21 00:42:25 UTC
lua-json-1.0-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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