Bug 209025
Summary: | Review Request: xfce4-dev-tools - Xfce developer tools | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Christoph Wickert <christoph.wickert> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | pertusus |
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-10-08 11:02:27 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 |
Description
Christoph Wickert
2006-10-03 00:42:59 UTC
I'll go ahead and review this, so we can hopefully get Xfce 4.4rc1 in later this week. ;) Look for a full review in a few here. OK - Package name OK - Spec file matches base package name. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: dc6403caf82edfb896eb3878385b439a xfce4-dev-tools-4.3.99.1.tar.bz2 dc6403caf82edfb896eb3878385b439a xfce4-dev-tools-4.3.99.1.tar.bz2.1 OK - Package compiles and builds on at least one arch. OK - BuildRequires correct See below - Package owns all the directories it creates. OK - Package has no duplicate files in %files. OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Spec has consistant macro usage. OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package doesn't own any directories other packages own. See below - No rpmlint output. SHOULD Items: OK - Should include License or ask upstream to include it. OK - Should build in mock. Issues: 1. rpmlint says: E: xfce4-dev-tools explicit-lib-dependency libtool This can be ignored I think. rpmlint is just looking for anything starting with lib... However, there might be some more Requires. I see it checking for the following at runtime: ## Check for a suitable make ## Check for autoconf, first trying autoconf-2.59, then autoconf-2.58, then ## Check for intltoolize ## Check for libtoolize ## Check for glib-gettextize ## Check for gtkdocize ## Check for aclocal, first trying aclocal-1.9, then aclocal-1.8, and finally ## Check for autoheader, first trying autoheader-2.59, then autoheader-2.58, ## Check for automake, first trying automake-1.9, then automake-1.8, and finally 2. Should require the Xfce package that owns datadir/xfce4... Should perhaps be xfwm4? (which I need to fix to own that dir). It seems to me that the m4 macros should also be in /usr/share/aclocal such that it is possible to simply use autoreconf and the like. In my opinion this package shouldn't depend on any other xfce package, such that it may be used to bootstrap them from svn. So I think that the directory datadir/xfce4 should also be owned by xfce4-dev-tools (unless there is another package that provides the xfce filesystem). (In reply to comment #2) > SHOULD Items: > > OK - Should include License or ask upstream to include it. will do. > However, there might be some more Requires. > I see it checking for the following at runtime: > > ## Check for a suitable make added make > ## Check for autoconf, first trying autoconf-2.59, then autoconf-2.58, then libtool already requires autoconf >= 2.50 and Core 5 comes with autoconf-2.59 > ## Check for intltoolize provided by intltool > ## Check for libtoolize provided by libtool > ## Check for glib-gettextize added glib2-devel > ## Check for gtkdocize added gtk-doc > ## Check for aclocal, first trying aclocal-1.9, then aclocal-1.8, and finally already provided by automake > ## Check for autoheader, first trying autoheader-2.59, then autoheader-2.58, already provided by autoconf > ## Check for automake, first trying automake-1.9, then automake-1.8, and finally libtool requires automake >= 1.4, Core 5 comes with 1.9 > > 2. Should require the Xfce package that owns datadir/xfce4... > Should perhaps be xfwm4? (which I need to fix to own that dir). In this case I agree with Patrice, so I made the new package own %dir %{_datadir}/xfce4 New Package: SRPMs at http://home.arcor.de/christoph.wickert/fedora/extras/review/noarch/ SPEC at http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-dev-tools.spec * Tue Oct 03 2006 Christoph Wickert <fedora christoph-wickert de> - 4.3.99.1-2 - Some more requires: glib2-devel, make and gtk-doc. - Own %%{_datadir}/xfce4 (In reply to comment #3) > It seems to me that the m4 macros should also be in > /usr/share/aclocal > such that it is possible to simply use autoreconf and the > like. I think most macros should already be in there now, but when I run xdt-autogen I still see: Please add the files codeset.m4 gettext.m4 glibc21.m4 iconv.m4 isc-posix.m4 lcmessage.m4 progtest.m4 from the /usr/share/aclocal directory to your autoconf macro directory or directly to your aclocal.m4 file. Any idea where to get the missing files from? glib21.m4 looks like a version/naming problem to me. The current glib2-devel-2.10.3 on Core 5 only provides glib-2.0.m4. (In reply to comment #4) > (In reply to comment #3) > > It seems to me that the m4 macros should also be in > > /usr/share/aclocal > > such that it is possible to simply use autoreconf and the > > like. > > I think most macros should already be in there now, It is not what I meant. I said that it would be nice to have the xfce4-dev-tools m4 macros also in /usr/share/aclocal, that is have xdt-depends.m4 xdt-features.m4... also in /usr/share/aclocal. > but when I run xdt-autogen I > still see: > > Please add the files > codeset.m4 gettext.m4 glibc21.m4 iconv.m4 isc-posix.m4 lcmessage.m4 > progtest.m4 > from the /usr/share/aclocal directory to your autoconf macro directory > or directly to your aclocal.m4 file. > > Any idea where to get the missing files from? glib21.m4 looks like a > version/naming problem to me. The current glib2-devel-2.10.3 on Core 5 only > provides glib-2.0.m4. It is not a naming problem, and all those autoconf macros are from gettext-devel. (In reply to comment #5) > I said that it would be nice to have the > xfce4-dev-tools m4 macros also in /usr/share/aclocal, that is > have xdt-depends.m4 xdt-features.m4... also in /usr/share/aclocal. Ah, I see. Done. > It is not a naming problem, and all those autoconf macros are > from gettext-devel. Thanks for the pointer. I have updated the package to require gettext-devel. New SRPMs http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-dev-tools-4.3.99.1-3.fc5.src.rpm http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-dev-tools-4.3.99.1-3.fc6.src.rpm Updated SPEC: http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-dev-tools.spec * It seems to be wrong, in description: "In addition, this package contains the Xfce developer's handbook." Some comments (not blockers) * you could add -p to the install invocation to keep timestamps of autoconf macros. * You could add a trailing / in %files to %{_datadir}/xfce4/dev-tools/ to show that it is a directory. Otherwise the package seems fine to me. (In reply to comment #7) > * It seems to be wrong, in description: > > "In addition, this package contains the Xfce developer's handbook." I think this shoud change in the final 4.4 version, but I'll remove it till then. > Some comments (not blockers) > * you could add -p to the install invocation to keep timestamps > of autoconf macros. Will do > * You could add a trailing / in %files to > %{_datadir}/xfce4/dev-tools/ > to show that it is a directory. yeah, usually I do, thanks > Otherwise the package seems fine to me. If there are no objections for Kevin to approve the package I'll import release 3 tomorrow and fix the issues you mentioned directly after import into CVS. Thanks for you help and feedback. All the changes look good to me, and I don't see any further blockers, so this package is APPROVED. Don't forget to close this bug NEXTRELEASE once it's been imported and built. This package appears to have been imported and built, but I don't see it in owners.list yet. ;) Can you add it there and close this NEXTRELEASE? (In reply to comment #10) > This package appears to have been imported and built, but I don't see it in > owners.list yet. ;) It wasn't in comps.xml ether. ;( Thanks for the reminder, both fixed now. |