Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 185215
Summary: | Review Request: adime - Allegro Dialogs Made Easy | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Hans de Goede <hdegoede> | ||||||
Component: | Package Review | Assignee: | Wart <wart> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | wart | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2006-03-13 21:51:04 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779, 185257 | ||||||||
Attachments: |
|
Description
Hans de Goede
2006-03-11 22:50:35 UTC
Michael, I'm hoping that you're willing to review this hence the CC, this is a dependency for a game: http://home.exetel.com.au/tjaden/raidem/ Which I'm busy packaging. Its turning out to be quite a pain. It needs a lot of libs. To be fair it does come with all these libs (including a few already in FE) in the source package, but I want todo this right and thus package the libs seperatly so that they can be used later for other games too. Its late (bedtime) overhere now, so I'll take a look at your Doom work tommorow. Thanks. MUST ==== * Source tarball matches upstream aa71fbe7661e56421cab42e6bca70c7c adime-2.2.1.tar.gz * Package (and .spec) named properly * License file (zlib) included * Spec file readable, in Am. English * Compiles and builds cleanly on FC-4 x86_64 * No locale files * No excessive BR: or Requires: * ldconfig used in %post/%postun as is necessary * Not relocatable * Does not create any directories that it needs to own * buildroot cleaned up in %install and %clean * No duplicate files * permissions ok * Macro usage consistent * Contains code, not content * headers, api docs, and .so library in -devel * No .la archives * No desktop file needed SHOULD ====== * Packages builds in mock (FC4-i386, FC4-x86_64, FC5-i386) * Did not test the package as I don't have anything useful to build against it. NONBLOCKING =========== * License is actually the zlib license, which is very similar to BSD. However, since the zlib package itself claims a BSD license (rpm -qi zlib), this is not a blocker. MUSTFIX ======= * "make docs" generates warnings and fails to build the html/rtf documentation because the $srcdir/docs/html and $srcdir/docs/rtf directories are missing. I suggest adding the following to %prep to suppress these warnings: mkdir docs/html mkdir docs/rtf ...and including docs/html and docs/rtf in %doc * BuildRequires: texinfo is needed to pull in 'makeinfo' to build the info files. mock builds fail without this. * rpmlint complains on i386 packages: E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0 W: adime-debuginfo objdump-failed objdump: /tmp/adime-debuginfo-2.2.1-1.fc4.i386.rpm.30986/usr/lib/debug/usr/lib/libadime.so.0.debug: Invalid operation ...and on x86_64 packages: W: adime-debuginfo objdump-failed E: adime-devel only-non-binary-in-usr-lib Many thanks, and sorry I only ran rpmlint on the mainpackage (x86_64). NONBLOCKING =========== * License is actually the zlib license, which is very similar to BSD. However, since the zlib package itself claims a BSD license (rpm -qi zlib), this is not a blocker. I actually had "zlib License" as License tag first, which is for example what mandriva uses and rpmlint upstream accepts. Our rpmlint package however overrides the list of valid licenses used by upstream and does not include zlib hence the change to BSD, which is indeed what zlib uses too. So I can either file an RFE against rpmlint to add "zlib License" or leave it as is, waht do you think? MUSTFIX ======= * "make docs" generates warnings and fails to build the html/rtf documentation because the $srcdir/docs/html and $srcdir/docs/rtf directories are missing. I suggest adding the following to %prep to suppress these warnings: mkdir docs/html mkdir docs/rtf ...and including docs/html and docs/rtf in %doc Done. * BuildRequires: texinfo is needed to pull in 'makeinfo' to build the info files. mock builds fail without this. Done. * rpmlint complains on i386 packages: E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0 Ah an actual problem found by rpmlint how nice (sorry bit sarcastic, see below), I fixed this by not using allegro-config --libs as that drags in -lalleg_unshareable, which is non-PIC and thus should only be in the application and not in a .so file. This means all applications which use us must be linked with -Wl,--export-dynamic -lalleg_unsharable (or `allegro-config --libs`) W: adime-debuginfo objdump-failed objdump: /tmp/adime-debuginfo-2.2.1-1.fc4.i386.rpm.30986/usr/lib/debug/usr/lib/libadime.so.0.debug: Invalid operation rpmlint bug, see bug 185227 ...and on x86_64 packages: W: adime-debuginfo objdump-failed Same rpmlint bug 185227 E: adime-devel only-non-binary-in-usr-lib And another rpmlint bug, what fun see bug 185228 New attemp (get the src.rpm, the patches changed too) at: Spec Name or Url: http://home.zonnet.nl/jwrdegoede/adime.spec SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/adime-2.2.1-2.src.rpm I noticed that it was missing a "Requires: allegro-devel" for the -devel package. I've fixed this and oploaded updated versions to the 2 files linked to in comment 3. (In reply to comment #3) > NONBLOCKING > =========== > * License is actually the zlib license, which is very similar to BSD. > However, since the zlib package itself claims a BSD license (rpm -qi zlib), > this is not a blocker. > > I actually had "zlib License" as License tag first, which is for example what > mandriva uses and rpmlint upstream accepts. Our rpmlint package however > overrides the list of valid licenses used by upstream and does not include zlib > hence the change to BSD, which is indeed what zlib uses too. So I can either > file an RFE against rpmlint to add "zlib License" or leave it as is, waht do you > think? I prefer to have the License: tag reflect the actual license as best as it can. On my FC-4 system /usr/share/rpmlint/TagsCheck.py claims that "zlib License" is a valid license, but /etc/rpmlint/config doesn't. It's a valid GPL compatible license, so rpmlint should be configured to accept it. Change the License: tag and file a RFE against rpmlint. > MUSTFIX > ======= [...] > * rpmlint complains on i386 packages: > E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0 > Ah an actual problem found by rpmlint how nice (sorry bit sarcastic, see below), > I fixed this by not using allegro-config --libs as that drags in > -lalleg_unshareable, which is non-PIC and thus should only be in the application > and not in a .so file. This means all applications which use us must be linked> with -Wl,--export-dynamic -lalleg_unsharable (or `allegro-config --libs`) Now the Makefile has a hardcoded version for the allegro library: LIB_FLAGS = -lalleg-4.2.0 ...making it more sensitive to changes in the allegro version, and also breaking the build on FC-4. You could try stripping the offending -lalleg_unsharable flag in the Makefile: LIB_FLAGS= $(shell allegro-config --libs | sed 's/-lalleg_unsharable//') Created attachment 126035 [details]
New so-fixes patch without hardcoded allero version
Good call,
Attached is a new version of so-fixes patch, sorry no new SRPM as I'm at work
and I don't have upload access to my homepage from here.
Created attachment 126040 [details]
spec with updated license, release and changelog
Here is an updated spec with the requested license change and a changelog entry
for the modified so-fixes patch.
The new spec/patch look good. I'd prefer a final SRPM before giving approval, but given your ISP issues that's not necessary this time. APPROVED Imported and Build, thanks! |