[Bug 919867] Review Request: billiards - A free cue sports simulator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=919867

--- Comment #4 from Hans de Goede <hdegoede@xxxxxxxxxx> ---
Created attachment 712211
  --> https://bugzilla.redhat.com/attachment.cgi?id=712211&action=edit
billiards.spec with modifications for moving the lc files to libdir

Hi,

(In reply to comment #3)
> Hans, thank you for reviewing the package!
> 
> (In reply to comment #2)
> > Needs work:
> > - rpmlint checks return:
> >  billiards.x86_64: E: no-binary
> >  billiards-debuginfo.x86_64: E: empty-debuginfo-package
> > This is caused by billiards being written completely in lua, this is fine,
> > but the package should be noarch then. Adding: "BuildArch: noarch" to the
> > specfile fixes this.
> 
> The author of Billiards said to me that there is a problem with packaging
> Billiards as an arch-independent package since it contains byte-compiled Lua
> files (the .lc files) which are arch-dependent.

Ah, I see.

> I've filled a bug report about rpmlint not recognizing Lua byte-files, which
> has just recently been fixed upstream:
> https://sourceforge.net/p/rpmlint/code/ci/
> be327c1b08402115aa050a6d8160bd2d05d5efd2/

Good!

> Although this brings a new problem
> (https://bugzilla.redhat.com/show_bug.cgi?id=919869#c1):
> "...you cannot ship the *.lc files in /usr/share, they need to be somewhere
> in /usr/lib(64) instead."
> I don't know what is the best way to solve this?

Move the files to %{_libdir} is the way to solve this. I've quickly whipped a
version
of the package with this done. I'll attach the modified spec file and the
(small) patch to billiards.in here.

> Also, I've disabled the generation of the empty debuginfo package and added
> a comment explaining that.

Good.

> > -%makeinstall should not be used except for broken Makefiles, Use "make
> > install DESTDIR=%{buildroot}" instead.
> 
> I think you misread the .spec file. I used %make_install, which has been
> approved by the Packaging guidelines:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.
> 25makeinstall_macro_should_not_be_used (last line of the section).

Ah, I've never seen anyone do this (use %make_install) I did not know there
is both a %makeinstall and a %make_install, very confusing whomever came
up with that does not get a beer from me the next time I see him.

I would still prefer if you switched to "make install DESTDIR=%{buildroot}"
since that is much more readable, what our packaging templates use, and
thus what most packages use. But you're free the keep things as is.

> > -The info file being gzipped is done by rpmbuild and this may change to
> > another compression format in the future. You should drop the .gz from the
> > scriptlets (install-info will figure it out itself) and the %files entry
> > should be:
> > %{_infodir}/%{name}.info*
> 
> Thanks for pointing this out. Fixed.
> 
> New spec and SRPM files are here:
> http://tadej.fedorapeople.org/billiards.spec
> http://tadej.fedorapeople.org/billiards-0.4.1-2.fc17.src.rpm

Looks good, but as discussed the lc files should be moved to %{_libdir}.

Regards,

Hans

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=ZZmVx3WLSc&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]