Bug 197649 - Review Request: gnustep-make - GNUstep makefile package
Review Request: gnustep-make - GNUstep makefile package
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dominik 'Rathann' Mierzejewski
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 201000
  Show dependency treegraph
 
Reported: 2006-07-05 05:59 EDT by Axel Thimm
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-03 05:10:57 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Axel Thimm 2006-07-05 05:59:28 EDT
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-3.at.src.rpm
Description:
The makefile package is a simple, powerful and extensible way to write
makefiles for a GNUstep-based project.  It allows the user to write a
project without having to deal with the complex issues associated with
configuration, building, installation, and packaging.  It also allows
the user to easily create cross-compiled binaries.
Comment 1 Axel Thimm 2006-07-05 06:06:30 EDT
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.
Comment 2 Ville Skyttä 2006-07-05 11:59:51 EDT
(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).
Comment 3 Axel Thimm 2006-07-05 14:45:37 EDT
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). :)
Comment 4 Ville Skyttä 2006-07-05 15:04:34 EDT
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 :)
Comment 5 Parag AN(पराग) 2006-07-06 02:47:48 EDT
== 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. 
   
Comment 6 Axel Thimm 2006-07-08 06:02:34 EDT
(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.
Comment 7 Ville Skyttä 2006-07-08 06:33:59 EDT
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.
Comment 8 Ville Skyttä 2006-07-08 06:44:35 EDT
Duh, the example in comment 7 may be a bit suboptimal (apples/oranges), but I
hope that the intention is clear.
Comment 9 Axel Thimm 2006-07-08 06:50:54 EDT
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 :)
Comment 10 Ville Skyttä 2006-07-08 07:44:54 EDT
(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.
Comment 11 Axel Thimm 2006-07-08 08:42:57 EDT
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.
Comment 12 Axel Thimm 2006-07-19 07:11:13 EDT
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@ATrpms.net> - 1.12.0-4
- Remove default -lobjc-fd2 switch.
- Disable flat hierarchy to allow for different library combos.
Comment 13 Axel Thimm 2006-08-02 01:15:32 EDT
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.
Comment 14 Dominik 'Rathann' Mierzejewski 2006-08-09 15:45:36 EDT
So is anyone reviewing this formally? If not, I'm interested, as this solves one
of the dependencies for oolite.
Comment 15 Rex Dieter 2006-08-09 16:11:09 EDT
Dominik, it's still blocking FE-NEW, so no one has formally reviewed this yet. 
Go for it.
Comment 16 Dominik 'Rathann' Mierzejewski 2006-08-09 18:02:23 EDT
Very well, I'll review this soon.
Comment 17 Dominik 'Rathann' Mierzejewski 2006-08-09 20:18:41 EDT
Corrected to block FE-REVIEW.
Comment 18 Dominik 'Rathann' Mierzejewski 2006-08-28 10:34:59 EDT
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.
Comment 19 Axel Thimm 2006-08-28 19:56:37 EDT
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.
Comment 20 Dominik 'Rathann' Mierzejewski 2006-08-29 16:47:40 EDT
All right then, I'm satisfied. APPROVED.
Comment 21 Axel Thimm 2006-09-03 05:10:57 EDT
Thanks, Dominik. Built for FC6 and FC5.

Note You need to log in before you can comment on or make changes to this bug.