Spec URL: http://ignatenkobrain.fedorapeople.org/for-review/luajit.spec SRPM URL: http://ignatenkobrain.fedorapeople.org/for-review/luajit-2.0.2-2.fc20.src.rpm Description: LuaJIT implements the full set of language features defined by Lua 5.1. The virtual machine (VM) is API- and ABI-compatible to the standard Lua interpreter and can be deployed as a drop-in replacement. Fedora Account System Username: ignatenkobrain
*** Bug 718681 has been marked as a duplicate of this bug. ***
Do you mind me to take over this review?
No ;) Do this!
(In reply to Christopher Meng from comment #2) > Do you mind me to take over this review? (In reply to Igor Gnatenko from comment #3) > No ;) Do this! Just to make things really clear - Igor would like you, Christopher, to review this.
new SPEC: http://ignatenkobrain.fedorapeople.org/for-review/luajit.spec new SRPM: http://ignatenkobrain.fedorapeople.org/for-review/luajit-2.0.2-3.fc20.src.rpm
1. INSTALL_X= install -m 0755 INSTALL_F= install -m 0644 ------- -p option should be inserted also. 2. Package is fine. Only 1 question: Why do this? mv doc/ html/
(In reply to Christopher Meng from comment #6) > 1. INSTALL_X= install -m 0755 > INSTALL_F= install -m 0644 > > ------- > > -p option should be inserted also. Fixed. One minute for new spec/srpm > 2. Package is fine. > > Only 1 question: > > Why do this? > > mv doc/ html/ because this folder contains html docs. I think html folder instead of doc would be good
new SPEC: http://ignatenkobrain.fedorapeople.org/for-review/luajit.spec new SRPM: http://ignatenkobrain.fedorapeople.org/for-review/luajit-2.0.2-4.fc20.src.rpm Don't forget set fedora-review flag ;)
> mv doc/ html/ Please don't do that. It modifies the build dir contents and breaks short-circuit builds. Here repeated calls of "rpmbuild --short-circuit -bi luajit.spec". Even if you don't use any short-circuit builds, other packages do, so please don't break them. Solution: Create a temporary copy of the dir. … rm -rf _tmp_html ; mkdir _tmp_html cp -a doc _tmp_html/html … %files devel %doc _tmp_html/html/ …
s/other packages/other packagers/
(In reply to Michael Schwendt from comment #9) > > mv doc/ html/ > > Please don't do that. It modifies the build dir contents and breaks > short-circuit builds. Here repeated calls of "rpmbuild --short-circuit -bi > luajit.spec". Even if you don't use any short-circuit builds, other packages > do, so please don't break them. > > Solution: Create a temporary copy of the dir. > > … > > rm -rf _tmp_html ; mkdir _tmp_html > cp -a doc _tmp_html/html > > … > > %files devel > %doc _tmp_html/html/ > > … Thank you! Fixed. New SPEC: http://ignatenkobrain.fedorapeople.org/for-review/luajit.spec New SRPM: http://ignatenkobrain.fedorapeople.org/for-review/luajit-2.0.2-5.fc20.src.rpm
Scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=6246247 Hmm.. .pc on x86_64 RPM attached: ------------------------- # Package information for LuaJIT to be used by pkg-config. majver=2 minver=0 relver=2 version=${majver}.${minver}.${relver} abiver=5.1 prefix=/usr exec_prefix=${prefix} libdir=${exec_prefix}/lib libname=luajit-${abiver} includedir=${prefix}/include/luajit-${majver}.${minver} INSTALL_LMOD=${prefix}/share/lua/${abiver} INSTALL_CMOD=/usr/lib64/lua/${abiver} Name: LuaJIT Description: Just-in-time compiler for Lua URL: http://luajit.org Version: ${version} Requires: Libs: -L${libdir} -l${libname} Libs.private: -Wl,-E -lm -ldl Cflags: -I${includedir} ------------------------- So I can't approve it now. Problem found! libdir=${exec_prefix}/lib should be libdir=${exec_prefix}/lib64 on x86_64. But, you can look at VCS repo of luajit, it now has supported lib64 by $(MULTILIB) define. http://repo.or.cz/w/luajit-2.0.git/blob/HEAD:/Makefile http://repo.or.cz/w/luajit-2.0.git/blob/HEAD:/etc/luajit.pc Please package 2.1(You should obey the snapshot package guideline), or add patch to fix that.
Have you watched .spec ? # fix .pc (besser82) sed -i -e 's!${prefix}/lib!%{_libdir}!g' etc/luajit.pc
Yes, I watched it. But the pasted content were got from the koji build 6246247 for x86_64 -devel package. Your sed command changed this on lib64 system from: INSTALL_CMOD=${prefix}/lib/lua/${abiver} to INSTALL_CMOD=/usr/lib64/lua/${abiver} But I still see: libdir=${exec_prefix}/lib Don't you think it should be libdir=${exec_prefix}/lib64?
ah. sorry. I was frozen... Fixed. New SPEC: http://ignatenkobrain.fedorapeople.org/for-review/luajit.spec New SRPM: http://ignatenkobrain.fedorapeople.org/for-review/luajit-2.0.2-6.fc20.src.rpm
PACKAGE APPROVED ------------------- sed -i -e 's!${prefix}/lib!%{_libdir}!g' etc/luajit.pc sed -i -e 's!${exec_prefix}/lib!%{_libdir}!g' etc/luajit.pc Can just be: sed -i -e 's!${.*prefix}/lib!%{_libdir}!g' etc/luajit.pc ;)
Christopher, thank you for review ! ;) New Package SCM Request ======================= Package Name: luajit Short Description: Just-In-Time Compiler for Lua Owners: ignatenkobrain Branches: f19 f20
Git done (by process-git-requests).
luajit-2.0.2-6.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/luajit-2.0.2-6.fc20
luajit-2.0.2-6.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/luajit-2.0.2-6.fc19
luajit-2.0.2-6.fc19 has been pushed to the Fedora 19 testing repository.
Please read the bodhi comments at https://admin.fedoraproject.org/updates/FEDORA-2013-22732/luajit-2.0.2-6.fc20 , I just tested, this package doesn't work. /usr/bin/luajit -> luajit which the latter one doesn't exist.
Created attachment 834456 [details] Don't strip binaries The sed line that tries to disable stripping doesn't actually work, here's a more robust alternative that does.
Thank you! Fxd. http://pkgs.fedoraproject.org/cgit/luajit.git/commit/?id=f05c145cb149f6aa507c7c6c30edd203316dd022
luajit-2.0.2-8.fc19 has been pushed to the Fedora 19 stable repository.
luajit-2.0.2-8.fc20 has been pushed to the Fedora 20 stable repository.