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 197649
Summary: | Review Request: gnustep-make - GNUstep makefile package | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Axel Thimm <axel.thimm> |
Component: | Package Review | Assignee: | Dominik 'Rathann' Mierzejewski <dominik> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | che666, gauret |
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-09-03 09:10:57 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, 201000 |
Description
Axel Thimm
2006-07-05 09:59:28 UTC
Note that rpmlint will produce the following expected output: E: gnustep-make dir-or-file-in-usr-local /usr/local/lib/GNUstep E: gnustep-make non-executable-script /usr/lib/GNUstep/Library/Makefiles/executable.template 0644 E: gnustep-make non-executable-script /usr/lib/GNUstep/Library/Makefiles/java-executable.template 0644 o The /usr/local entry is where user built packages will install into. If this doesn't exist simple "make install"s can fail. o The two non-executable-script are just templates (as the name implies) they shouldn't be executables. (In reply to comment #1) > o The /usr/local entry is where user built packages will install into. If this > doesn't exist simple "make install"s can fail. I don't have a clue about this package or gnustep, but can't this be fixed by patching things in the package so that the required dirs are always created? This sounds like something that would also cause problems with staged installs (eg. during rpmbuild of packages that use this stuff). The /usr/local/GNUstep entry is for pure user actions, e.g. no package should install anything there. It is just a parallel to the typical /usr vs /usr/local idiom, /usr belongs to packages and /usr/local is the default configure prefix in pristine upstream sources. GNUstep is a bit peculiar, there is even a network root that is mapped by default on the local root. They also refuse to adhere to the FHS (normally GNUStep wants to use /usr/GNUstep/{System,Local} instead of the above), since they were first on the planet and FHS is only for one of their many targets (Linux/Unix). :) I did not mean that any packages should be installed to /usr/local, but just that if the stuff is set up so that things are blindly installed to some dirs without creating the dirs, /usr/local/GNUstep is probably not the only case affected. Think for example "make install DESTDIR=..." (if this package provides something like that). Thus, including it as a special case in the package sounds like a dubious workaround for one specific symptom, leaving the cause untouched. Caveats in comment 2 still apply :) == Not an official review as I'm not yet sponsored == * Mock build for development i386 is sucessfull with errors as ** No gnustep-make installation found, attempting to create a local/temporary one. ** make[2]: texi2pdf: Command not found make[2]: texi2html: Command not found I really got confused over why such errors was shown besides addding texinfo in BuildRequires. MUST Items: - MUST: rpmlint shows same ERRORS as posted by author of package. - MUST: dist tag is present - MUST: The package is named according to the Package Naming Guidelines. - MUST: The spec file name matching the base package gnustep-make, in the format gnustep-make.spec - MUST: This package meets the Packaging Guidelines. - MUST: The package is licensed with an open-source compatible license GPL. - MUST: License file COPYING is included in package. - MUST: The sources used to build the package matches the upstream source, as provided in the spec URL. md5sum is correct (1883a6387405e51ff4c384fb5cc547a7). - MUST: This package have a %clean section, which contains rm -rf $RPM_BUILD_ROOT. - MUST: This package used macros. - MUST: Document files are included like README. - MUST: Package did NOT contained any .la libtool archives. * Source URL is present and working. * BuildRoot is correct BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * BuildRequires is correct Also it will be good to move /usr/local to /usr directory if there is no such requirement. (In reply to comment #4) > I did not mean that any packages should be installed to /usr/local, but just > that if the stuff is set up so that things are blindly installed to some dirs > without creating the dirs, /usr/local/GNUstep is probably not the only case > affected. You mean subdirectories? In the case of the pure non-fhs setup, again only the root directory of the "local root" is created. So all I did is adjust the path. The purest packaging would dwell into it and fully convert it to FHS, and that's what I was aiming for at the beginning. But I followed up archived discussions on the pain and "impossibility" involved, as well as the "strong opinion" of upstream to not accept patches for Linux/Unix targets. Debian for example maintained a larger fhs-patch which needed adjustments on each upstream release and after the package was never being updated they consider dropping gnustep-make due to that burden. If it weren't needed as a build dependency for further packages, I would had done the same. :/ In a nutshell: I need to move out the /usr/local like bits of gnustep-make to avoid default user installs under /usr and this seems the least intrusive way to do so. (In reply to comment #5) > == Not an official review as I'm not yet sponsored == Sponsored means that you are allowed to do a first review of a new contributor and to pull in this contributor into the Fedora family. No, I don't mean *any* dir in particular. If 3rd party packages that use functionality from this package and due to issues/intrinsics of this package end up blindly installing files into dirs that don't exist without creating the dirs as part of the install process, that sounds like the root cause that needs fixing. Think for example stuff using autotools, if you run "./configure --prefix=/foo/bar ; make ; make install DESTDIR=/quux", things Just Work wrt creation of the /quux/foo/bar hierarchy and none of those dirs need to exist beforehand. If gnustep-make would work like that, there would be no need to create nor own the /usr/local/GNUstep dir. Duh, the example in comment 7 may be a bit suboptimal (apples/oranges), but I hope that the intention is clear. The local root (be it /usr/GNUstep/Local or /usr/local/GNUstep) is created by upstream make install of this package, I'm not explicitly creating it and including it to the package. So the upstream model is to provide the local root dir and let the gnustep-make using software create the subdirs. Whether all such gnustep-make based packages would also have a safeguard to try to create the root dir if it's missing I can't say, but why risk it? About the autoconf example: In that case /usr/local itself is not needed beforehand :) (In reply to comment #9) > About the autoconf example: In that case /usr/local itself is not needed > beforehand :) Exactly. And thus should not be, and is not, included/owned in the autotools packages, considering that is /usr/local which is reserved to local stuff (no matter if it's owned by other packages or not). Anyway, I think you got at least most of the point, so I'll shut up now. But the autotools packages are not BuildRequired on, gnustep-make is. The /usr/local/GNUstep folder should theoretically be part of a "filesystem" type category package, but that would be overkill. So, if you like, gnustep-make is also some kind of "filesystem-gnustep". And the argument on /usr/local would apply to "filesystem", not autotools (assuming all packages installing under /usr/local are autotooled, which is not true, but since we're in the apples/orange business anyway, let's pass this, too ;). I did get your point, I just don't know how to improve the situation short of dropping the package or making it fully FHS conformant, which (currently) means that it would become unmaintainable very soon. :/ I'll look into some more FHS vs GNUstep discussions, perhaps there is some nice compromise somehwere, and I'm certainly open to any suggestions. Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/gnustep-make/gnustep-make.spec SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/gnustep-make/gnustep-make-1.12.0-4.at.src.rpm * Tue Jul 11 2006 Axel Thimm <Axel.Thimm> - 1.12.0-4 - Remove default -lobjc-fd2 switch. - Disable flat hierarchy to allow for different library combos. Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/gnustep-make/gnustep-make.spec SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/gnustep-make/gnustep-make-1.12.0-5.at.src.rpm Teach gnustep-make some FHS. Also removes the /usr/local/.../GNUstep special handling. So is anyone reviewing this formally? If not, I'm interested, as this solves one of the dependencies for oolite. Dominik, it's still blocking FE-NEW, so no one has formally reviewed this yet. Go for it. Very well, I'll review this soon. Corrected to block FE-REVIEW. Here's the review: 1. package meets naming and packaging guidelines, however, I'd like to see some reasoning why /usr/libexec/gnustep is used instead of %{_libdir}/gnustep and, similarly, %{_datadir}/gnustep/Libraries instead of %{_libdir}/gnustep/Libraries 2. specfile is properly named, is cleanly written and uses macros consistently. 3. dist tag is present. 4. build root NOT correct, should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 5. license field matches the actual license. 6. license is open source-compatible. License text included in package. 7. source files match upstream: 1883a6387405e51ff4c384fb5cc547a7 gnustep-make-1.12.0.tar.gz 8. latest version is being packaged. 9. BuildRequires are proper. 10. package builds in mock (fc5,devel). 11. rpmlint shows errors, but they can be ignored, as discussed in comment #1: E: gnustep-make non-executable-script /usr/share/gnustep/makefiles/executable.template 0644 E: gnustep-make non-executable-script /usr/share/gnustep/makefiles/java-executable.template 0644 12. final provides and requires are sane: config(gnustep-make) = 1.12.0-5.fc6 gnustep-make = 1.12.0-5.fc6 = /bin/csh /bin/echo /bin/sh config(gnustep-make) = 1.12.0-5.fc6 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) rtld(GNU_HASH) 13. no shared libraries are present. 14. package is not relocatable. 15. owns the directories it creates. 16. doesn't own any directories it shouldn't. 17. no duplicates in %files. 18. file permissions are appropriate 19. %clean is present. 20. %check is not necessary. 21. no scriptlets present. 22. code, not content. 23. documentation is small, so no -docs subpackage is necessary. 24. %docs are not necessary for the proper functioning of the package. 25. no headers. 26. no pkgconfig files. 27. no libtool .la droppings. 28. not a GUI app. 29. not a web app. To summarize: all seems fine except 1. and 4. Thanks for the review! About 1. > 1. package meets naming and packaging guidelines, however, I'd like to see some > reasoning why /usr/libexec/gnustep is used instead of %{_libdir}/gnustep and, > similarly, %{_datadir}/gnustep/Libraries instead of %{_libdir}/gnustep/Libraries libexec is chosen because the parts placed in there are called by parts of gnustep-make, e.g. which_lib. The choice about FHS'izing gnustep-make is a bit arbitrary since upstream hasn't yet properly addressed this, but most parts are derived from mail exchanges between gnustep-make authors and ogo authors. datadir instead of libdir is chosen because there is nothing arch-dependent in there. About 4. > 4. build root NOT correct, should be: > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) This is the "preferred buildroot" which is not a must-buildroot. This issue was brought up some time ago in the packaging group and consensus was mostly, that if it works it passes. All right then, I'm satisfied. APPROVED. Thanks, Dominik. Built for FC6 and FC5. |