Bug 653378
Summary: | Review Request: gtk-theme-engine-clearlooks - Clearlooks GTK+ theme engine | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Cosimo Cecchi <ccecchi> |
Component: | Package Review | Assignee: | Matthias Clasen <mclasen> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mclasen, notting, petersen |
Target Milestone: | --- | Flags: | mclasen:
fedora-review+
j: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-01-25 17:32:32 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: |
Description
Cosimo Cecchi
2010-11-15 10:17:05 UTC
builds fine in mock. rpmlint output: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/gtk-theme-engine-clearlooks-*.rpm gtk-theme-engine-clearlooks.src:13: W: unversioned-explicit-obsoletes gtk3-engines gtk-theme-engine-clearlooks.src: W: no-cleaning-of-buildroot %install gtk-theme-engine-clearlooks.src: W: no-cleaning-of-buildroot %clean gtk-theme-engine-clearlooks.src: W: no-buildroot-tag gtk-theme-engine-clearlooks.src: W: no-%clean-section gtk-theme-engine-clearlooks.x86_64: W: obsolete-not-provided gtk3-engines gtk-theme-engine-clearlooks.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/gtk-theme-engine-clearlooks-3.pc 3 packages and 0 specfiles checked; 0 errors, 7 warnings. The obsoletes is questionable anyway, in my opinion. We should leave it out. The .pc file indeed needs to go into a sucky, single-file -devel package. I'm going to advocate for dropping it from the upstream tarball, since it is useless. package name: ok spec file name: ok packaging guidelines: minor issues: - the explicit dep on gtk3 should be dropped, since it will get pulled in by library deps anyway. - as mentioned, the .pc file needs to go in a -devel package (until we get rid of it upstream) license: ok license field: copying says GPLv3... license file: should be included, after verifying what license was intended here spec language: ok spec readable: ok upstream sources: ok buildable: ok excludearch: ok build requires: ok locale handling: ok ldconfig: ok system libraries: ok relocatable: ok directory ownership: must own %{_datadir}/gtk-theme-engine-clearlooks duplicate files: ok file permissions: ok macro use: ok permissible content: ok large docs: ok doc content: ok headers: ok static libs: ok shared libs: ok -devel deps: ok libtool archives: the .la file needs to be dropped gui apps: ok duplicate ownership: ok utf8 filenames: ok Matthias: thanks for the review! Updated spec: http://people.gnome.org/~cosimoc/gtk-theme-engine-clearlooks-pkg/gtk-theme-engine-clearlooks.spec New SRPM: http://people.gnome.org/~cosimoc/gtk-theme-engine-clearlooks-pkg/gtk-theme-engine-clearlooks-2.91.3-2.fc15.src.rpm Some comments... License: I double-checked and it seems that upstream has no COPYING file at all. We should probably provide a patch for upstream with it and add it to %doc for the next upstream release. Anyway, I assumed the gtk-engines COPYING was valid for this package too, and that's LGPLv2; the source code headers say LGPLv2 too. Where did you find GPLv3 mentioned for this package? Obsoletes: if I understood correctly how spec files work, it's actually required to obsolete gtk3-engines, at least in its current form, as both ship the same libclearlooks.so & friends files. The best approach for this would be probably to provide again a patch for upstream which removes the clearlooks engine from gtk-engines, so we can still package both without conflicts. For the license: since this is basically all just copied clearlooks code, it certainly needs to stay under the same license as gtk3-engines, and the failure to include COPYING in git just gives us that automake bug where it just makes up a license. I'll get the license added upstream. For the obsoletes, what needs to happen is that we remove clearlooks from gtk3-engines at the same time that we introduce this package. We can probably make the transition a bit smoother by having this package conflict with gtk3-engines <= {current version}. The coming merge of the gtk-style-context branch will make gtk3-engines obsolete anyway (at least until somebody ports the engines). I have now - added the correct license upstream - removed clearlooks from the gtk3-engines package So I think we are almost ready to go here. Some changes I would still like to see: 1) %doc AUTHORS NEWS README should be in the main package, not in -devel 2) add a comment next to it explaining that the COPYING in the tarball is an automake artifact, and the correct license file has been added upstream Updated spec: http://people.gnome.org/~cosimoc/gtk-theme-engine-clearlooks-pkg/gtk-theme-engine-clearlooks.spec New SRPM: http://people.gnome.org/~cosimoc/gtk-theme-engine-clearlooks-pkg/gtk-theme-engine-clearlooks-2.91.3-3.fc15.src.rpm I have updated the spec according to your comments, should be alright now. I don't think that Obsoletes is right. I'd just make that a Conflicts. Everything else looks good now. I'll approve it, please remove or replace the Obsoletes before building it. Ok, I just removed the Obsoletes then, as we already have a new gtk3-engines package without clearlooks in rawhide. I already set approved+, so feel free to request git (don't forget to ask for an f14 branch, otherwise the initial import will fail) New Package SCM Request ======================= Package Name: gtk-theme-engine-clearlooks Short Description: Clearlooks GTK+ theme engine Owners: cosimoc Branches: f14 f15 InitialCC: cosimoc It is too early to request f15 branches; I have made only the f14 branch. Git done (by process-git-requests). |