Spec: http://rpm.greysector.net/extras/libtlen.spec SRPM: http://rpm.greysector.net/extras/libtlen-0-0.1.20041113.src.rpm Description: libtlen is a library providing an API for client programs which want to use Tlen.pl, an Instant Messanging protocol based on Jabber, but with some modifications.
Created attachment 122897 [details] patch to spec file It seems to be your first package in FE, so only your sponsor can make an official review of this RPM. I can only point out some problems with the spec file. - wrong âGroupâ tag. Should be âSystem Environment/Librariesâ http://fedoraproject.org/wiki/RPMGroups - you can use â%{?dist}â in âReleaseâ tag. It make life much easier :) http://fedoraproject.org/wiki/DistTag#head-6853fe5b797b146350e0d6b4caf57d517504449f - remove âEpochâ tag - this package was never available in FC or FE - superflous BR. You can delete: BuildRequires: autoconf BuildRequires: automake BuildRequires: libstdc++-devel libstdc++-devel is a dependency of gcc-c++ http://fedoraproject.org/wiki/PackagingGuidelines#Exceptions - wrong âBuildRootâ http://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot - in âdevelâ subpackage summary: s/developping/developing/ - use parallel make: http://fedoraproject.org/wiki/PackagingGuidelines#parallelmake - kill static libs. They are evil from the security point of view ;] http://fedoraproject.org/wiki/PackagingGuidelines#StaticLibraries http://people.redhat.com/drepper/no_static_linking.html - please provide css file in devel subpackage - change encoding of %doc files to UTF-8 (like in man-pages-pl or vim SRPMs) Proposal changes are in attached patch. I have also modified order of tags so now they are like in /usr/share/fedora/spectemplate-minimal.spec from fedora-rpmdevtools package. I also changed $RPM_BUILD_ROOT to %{buildroot} because its shorter. If you don't like these to changes, you can revert them ;) http://fedoraproject.org/wiki/PackagingGuidelines#UsingBuildRootOptFlags rpmlint warnings which should be corrected: [rpm-build@X i386]$ rpmlint libtlen-0-0.1.20041113.i386.rpm W: libtlen no-version-in-last-changelog [rpm-build@X i386]$ rpmlint libtlen-devel-0-0.1.20041113.i386.rpm W: libtlen-devel no-version-in-last-changelog [rpm-build@X i386]$ It would be nice if you could contact with upstream and force them to include license in tarball ;) Package compiles with lots of warnings. Developers screwed the job in lib/libtlen.h:452 and lib/libtlen.h:457. They check some macros but they do not set them in config.h via configure script :D
Fixed, but what's the point of adding %{dist} to Release?
(In reply to comment #2) > Fixed It does not build now :/ rpmbuild does not recognize â%doc(pl)â macro on my machine (fully updated FC4 and Rawhide box): Processing files: libtlen-0-0.1.20041113 error: File must begin with "/": %doc(pl) error: File must begin with "/": docs/AUTHORS error: File must begin with "/": docs/TODO error: File must begin with "/": ChangeLog Processing files: libtlen-devel-0-0.1.20041113 error: File must begin with "/": %doc(pl) error: File must begin with "/": docs/*.{html,css} Processing files: libtlen-debuginfo-0-0.1.20041113 Provides: libtlen.so.1.5.debug Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 RPM build errors: user dominik does not exist - using root user dominik does not exist - using root File must begin with "/": %doc(pl) File must begin with "/": docs/AUTHORS File must begin with "/": docs/TODO File must begin with "/": ChangeLog File must begin with "/": %doc(pl) File must begin with "/": docs/*.{html,css} [rpm-build@X SPECS]$ Aghh, I forgot about ChangeLog file. It has wrong encoding. It should be also fixed. > but what's the point of adding %{dist} to Release? It's not mandatory but it makes life of developer much easier (at least for me). You can have the same spec file for FC-3, FC-4 and devel branch and it makes very clear which distribution package was built for. It can also help a bit in upgrade path from FC(n) to FC(n+1). You can take a look at this thread â http://www.redhat.com/archives/fedora-extras-list/2006-January/msg00334.html
Doh! It should've been %lang(pl) %doc, fixed.
Dropping FE-NEEDSPONSOR. Tom Callaway offered sponsorship in bug 177235 (and bugzilla change-several-bugs-at-once feature requires me to add/edit something).
uhm, bugzilla is broken :(
I know this is an old ticket; is there still interest in getting it in? I'll be happy to review it if so.
I will close this bug if there is no response soon.
Is there anything wrong with the current submission?
I haven't reviewed it yet. I just wanted to make sure that my time wouldn't be wasted, given that it had been six weeks since my original review offer with no response. I'll go ahead and review it now.
You've said on IRC that you'll add the dist tag, so no worries there. The major issue I see is with the license. The only file I see that actually has a license statement is lib/asciitab.h which is GPL/MPL. The only thing that says LGPL is the sourceforge page, which unfortunately really isn't sufficient. Perhaps I'm just not looking in the proper place, though; am I missing a license statement somewhere? * source files match upstream: b77c0a3234a21d1b79df8a8b9a9b9534 libtlen-20041113.tar.gz * package meets naming and packaging guidelines. * Prerelease snapshot naming is correct: 0-0.1.20041113 * specfile is properly named, is cleanly written and uses macros consistently. X dist tag is not present. * build root is correct. ? license field matches the actual license. * latest version is being packaged. * BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (development, x86_64). * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: libtlen-0-0.1.20041113.x86_64.rpm libtlen.so.1()(64bit) libtlen = 0-0.1.20041113 = /sbin/ldconfig libtlen.so.1()(64bit) libtlen-devel-0-0.1.20041113.x86_64.rpm libtlen-devel = 0-0.1.20041113 = libtlen = 0-0.1.20041113 libtlen.so.1()(64bit) * %check is not present; no test suite upstream. * shared libraries are present; ldconfig is called as necessary and unversioned .so links are in the -devel package. * package is not relocatable. * 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 -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * headers are in the -devel package. * no pkgconfig files. * no libtool .la droppings.
http://rpm.greysector.net/extras/libtlen.spec http://rpm.greysector.net/extras/libtlen-0-0.2.20041113.src.rpm added dist tag and a patch to fix 64bit builds
or was it to fix 32bit build on 64bit fedora...
Yes, that was it. In the meantime I've found a newer release on http://tleenx.sf.net/ and updated once again. ;) Also, I sent an e-mail to the developers about the license. http://rpm.greysector.net/extras/libtlen.spec http://rpm.greysector.net/extras/libtlen-0-0.3.20060309.src.rpm
Got a response from upstream. They've included the license text (LGPL) inside. http://rpm.greysector.net/extras/libtlen.spec http://rpm.greysector.net/extras/libtlen-0-0.4.20060309.src.rpm
OK, the updated package builds fine and still looks OK. The dist tag is there, as is the license file. APPROVED
Package imported, devel build successful, FC-4 and FC-5 branches requested.
Branches built successfully.