Bug 194011

Summary: Review Request: curry - Münster Curry compiler
Product: [Fedora] Fedora Reporter: Gérard Milmeister <gemi>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
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-06-14 14:31:06 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 Gérard Milmeister 2006-06-04 15:32:15 UTC
Spec URL: http://math.ifi.unizh.ch/fedora/spec/curry.spec
SRPM URL: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/curry-0.9.10-1.src.rpm 
Description:
The Münster Curry compiler is a native code compiler for the
declarative multi-paradigm language Curry.

Comment 1 Jason Tibbitts 2006-06-06 18:53:08 UTC
The license seems to be essentially identical to the 3-clause BSD license, so I
think License: BSD is appropriate here.

I had build failures when using parallel make; everything works fine after
disabling parallel make.  Sure takes a while to build, though.

rpmlint warnings:

W: curry invalid-license Freely distributable
and many of these:
W: curry devel-file-in-non-devel-package /usr/lib64/curry/files.h

Since this is a compiler, header files are acceptable in the base package.  I
wonder, however, if there are libraries or something that applications compiled
by this compiler will need and if it would be reasonable to split those out to a
separate -libs or -runtime subpackage.  I do see this massive static library: 
19435080 Jun  6 13:23 /usr/lib64/curry/libcurry_g.a that I suppose is simply
linked to every application.

Now, static libs are really strongly disliked in Extras, so I'm going to need to
 ask for some guidance here.

Comment 2 Gérard Milmeister 2006-06-06 19:12:20 UTC
(In reply to comment #1)
> The license seems to be essentially identical to the 3-clause BSD license, so I
> think License: BSD is appropriate here.
Ok.

> I had build failures when using parallel make; everything works fine after
> disabling parallel make.  Sure takes a while to build, though.
Ok.

> W: curry devel-file-in-non-devel-package /usr/lib64/curry/files.h
> 
> Since this is a compiler, header files are acceptable in the base package.  I
> wonder, however, if there are libraries or something that applications compiled
> by this compiler will need and if it would be reasonable to split those out to a
> separate -libs or -runtime subpackage.  I do see this massive static library: 
> 19435080 Jun  6 13:23 /usr/lib64/curry/libcurry_g.a that I suppose is simply
> linked to every application.
> 
> Now, static libs are really strongly disliked in Extras, so I'm going to need to
>  ask for some guidance here.
This is in fact similar to ghc package itself, which contains a lot
of static libraries. I think, therefore, it is ok. However you can ask
the maintainer of ghc in Extras, Jens Petersen (petersen),
about this.


Comment 3 Ralf Corsepius 2006-06-07 06:17:22 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > I do see this massive static library: 
> > 19435080 Jun  6 13:23 /usr/lib64/curry/libcurry_g.a that I suppose is simply
> > linked to every application.
> > 
> > Now, static libs are really strongly disliked in Extras, so I'm going to need to
> >  ask for some guidance here.
> This is in fact similar to ghc package itself, which contains a lot
> of static libraries. I think, therefore, it is ok. 
Without having looked into the details lib*_g.a indicates a static-debug variant
of a library. 

This is problematic twice: static + debug.
If "static" isn't avoidable (This often is the case for exotic compilers), then
package it ...

"debug" is a different issue: Such libaries are rarely needed by anybody and
often are of doubtful benefits. One would have to check for for what they are
really needed and if shipping a "non-debug variant" isn't sufficient. In most
cases a "-O2 -g" compiled "non-debug" variant + debug-info-rpms render "debug
variants" superfluous. But with exotic compilers, one never knows.

So I recommend to check for 
* if it is really required and if a non-debug variant + debuginfos are sufficient.
* if existance of this file is mandatory for the toolchain. If not, I'd
recommend removing it, or packaging it into a separate *-debug package (cf. how
gcc and glibc are being packaged).
If yes, if this library can't be replaced with a set of "lib*_g.a -> lib*.a"
symlinks (This approach is used by newlib. It supports a *_g.a libs but by
default doesn't build it, and therefore needs these links)

 

Comment 4 Gérard Milmeister 2006-06-07 08:26:48 UTC
(In reply to comment #3)
> "debug" is a different issue: Such libaries are rarely needed by anybody and
> often are of doubtful benefits. One would have to check for for what they are
> really needed and if shipping a "non-debug variant" isn't sufficient. In most
> cases a "-O2 -g" compiled "non-debug" variant + debug-info-rpms render "debug
> variants" superfluous. But with exotic compilers, one never knows.
I looked into this a little deeper: libcurry.a is the normal runtime library,
that is linked into every standalone program. libcurry_g.a is NOT
the same library compiled using gcc -g, but contains the code for the
declarative debugger of curry (this is similar to the standard
debugger in Prolog). If you compile using "cyc --debug" this library is
linked in addition to libcurry.a. I could imagine putting this library into
a separate package curry-debugger or curry-debug. However, then
cyc --debug fails.

Comment 5 Jason Tibbitts 2006-06-09 00:46:39 UTC
I've been thinking about this a bit and took a look at a couple of things.

Obviously the best possible solution would be for the curry compiler to
dynamically link the executables it creates, and then package the runtime
separately.  The question is, how feasible is this?  I don't think anything
prevents the runtime from being made into a .so; it would just be a few lines in
runtime/Makefile.in.  And the curry compiler (cyc) is really just a shell script
that turns the curry source into C, then calls GCC to compile it and then links it:

$exec $CC $debug $ccopts $ldopts /tmp/cyc$$.c $linkfiles $libs $dbglib -lcurry
@LDFLAGS@ @LIBS@

So it may be possible to actually make things work without an insane amount of
effort.

However, assuming that we can't achieve curry packaging Nirvana, is the current
package behavior acceptable?   Obviously we dislike static libraries and static
linking, but if there's no other way to make it work then we either have to
package the compiler as is or not package it at all.  The standard argument
against static linking is one of security: you have to fix every package that
linked in the bad library individually.  The curry compiler is mostly of
research interest and isn't going to be used to produce a large number of
security-sensitive programs, so it's probably reasonable to discount the
security argument.

What remains is Ralf's comment about splitting out the _g library.  It is large
and not a terrible idea, but there's the issue of cymake failing.  However,
cymake is just a shell script which calls cyc; it should be easy to patch the
appropriate one to check for the existence of the debug library and simply bail
with a useful error if it doesn't exist.

So the real issue is how much work is involved to make things a bit cleaner. 
What do you think?

Comment 6 Jason Tibbitts 2006-06-14 14:23:42 UTC
Adding back in several comments that were lost in the crash:

------- Additional Comments From gemi  2006-06-09 03:21 EST -------
I remember reading that shared libraries with ghc were either not
possible or problematic, probably this applies to curry too.
In any case, the only point I would really consider is splitting
out the _g library. It would be possible too patch cyc and cymake
to check for the library (when invoked using --debug) and bail out
with an error when it is not present.



Comment 7 Jason Tibbitts 2006-06-14 14:24:21 UTC
------- Additional Comments From gemi  2006-06-09 10:56 EST -------
I split off a curry-debugger package which only contains libcurry_g.a
I patche cyc so that when invoked with --debug with the debugger
package installed, it will give an error and a hint that
the curry-debugger package should be installed.

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/curry-0.9.10-2.src.rpm



Comment 8 Jason Tibbitts 2006-06-14 14:25:18 UTC
------- Additional Comments From tibbs.edu  2006-06-10 14:41 EST -------
Hmmm.  You shouldn't use PreReq:; use Requires: instead.  This solves the following:

W: curry prereq-use curry = %{version}-%{release}

Other than that I'm happy with the package.  An additional couple of ignorable
rpmlint warnings popped up for the -debugger package:

W: curry-debugger no-documentation
W: curry-debugger devel-file-in-non-devel-package /usr/lib64/curry/libcurry_g.a

but these are no big deal.

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* source files match upstream:
   ae56a087dd6e174cc865e701657876a0  curry-0.9.10.tar.gz
   ae56a087dd6e174cc865e701657876a0  curry-0.9.10.tar.gz-srpm
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (development, x86_64).
O rpmlint has some ignorable complaints.
* final provides and requires are sane:
  curry-0.9.10-2.fc6.x86_64.rpm
   curry = 0.9.10-2.fc6
  =
   /bin/sh
   gcc
   libgmp.so.3()(64bit)

  curry-debugger-0.9.10-2.fc6.x86_64.rpm
   curry-debugger = 0.9.10-2.fc6
  =
   curry = 0.9.10-2.fc6

* no shared libraries are present.
* 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.
* %clean is present.
* %check is not present; no test suite upstream.
* 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.
O plenty of headers, but this is a compiler.
* no pkgconfig files.
* no libtool .la droppings.
O static libraries, but this is a compiler and there's no reasonable way to
eliminate them.
* not a GUI app.

APPROVED



Comment 9 Jason Tibbitts 2006-06-14 14:26:05 UTC
------- Additional Comments From tibbs.edu  2006-06-10 14:47 EST -------
Oops, I just noticed that the debuginfo package came out empty.  I don't see
anything being stripped and the compiler is being called with -g so I'm not sure
what's up here.  Perhaps it's best to just disable the -debuginfo package
generation.


Comment 10 Jason Tibbitts 2006-06-14 14:26:48 UTC
------- Additional Comments From gemi  2006-06-10 15:14 EST -------
(In reply to comment #8)
> Hmmm.  You shouldn't use PreReq:; use Requires: instead.  This solves the
following:
> 
> W: curry prereq-use curry = %{version}-%{release}
Ah yes, I tried this to so that uninstalling curry and curry-debugger
first uninstalls curry-debugger, so that the /usr/lib/curry directory
is not left behind. This didn't work out and I simply made curry-debugger
also own /usr/lib/curry.

However after giving some thought to the package splitting, I would
rather not split it after all. Remember, this is a development system
with an integrated debugger. A compiled binary doesn't need any runtime
library anymore, therefore the curry package isn't needed anymore either.

I will however accept if your insist on it.

> APPROVED


(In reply to comment #9)
> Oops, I just noticed that the debuginfo package came out empty.  I don't see
> anything being stripped and the compiler is being called with -g so I'm not sure
> what's up here.  Perhaps it's best to just disable the -debuginfo package
> generation.
I will try to find what's happening.
How is it disabled?


Comment 11 Jason Tibbitts 2006-06-14 14:27:31 UTC
------- Additional Comments From ville.skytta  2006-06-10 15:16 EST -------
Re: useless debuginfo: from the install-dir target in Makefile.in:

        $(INSTALL_PROGRAM) -s cycc $(DESTDIR)$(libdir)/curry
        $(INSTALL_PROGRAM) -s cymk $(DESTDIR)$(libdir)/curry
        $(INSTALL_PROGRAM) -s newer $(DESTDIR)$(libdir)/curry

Those -s's look suspicious.


Comment 12 Jason Tibbitts 2006-06-14 14:28:06 UTC
------- Additional Comments From gemi  2006-06-10 15:30 EST -------
(In reply to comment #11)
> Re: useless debuginfo: from the install-dir target in Makefile.in:
> 
>         $(INSTALL_PROGRAM) -s cycc $(DESTDIR)$(libdir)/curry
>         $(INSTALL_PROGRAM) -s cymk $(DESTDIR)$(libdir)/curry
>         $(INSTALL_PROGRAM) -s newer $(DESTDIR)$(libdir)/curry
> 
> Those -s's look suspicious.
Yep, I am patching them away.


Comment 13 Jason Tibbitts 2006-06-14 14:28:48 UTC
------- Additional Comments From tibbs.edu  2006-06-10 15:36 EST -------
(In reply to comment #10)
> Ah yes, I tried this to so that uninstalling curry and curry-debugger
> first uninstalls curry-debugger, so that the /usr/lib/curry directory
> is not left behind. This didn't work out and I simply made curry-debugger
> also own /usr/lib/curry.

This is an rpm bug; there's a ticket open on it somewhere but there are so many
rpm bugz that I can't find it right now.

> However after giving some thought to the package splitting, I would
> rather not split it after all.

I don't really think it is necessary; it was just one of the ideas I tossed out.
It's not the static+debug situation that Ralf mentioned earlier, it's just a
library with additional stuff linked in.


 Remember, this is a development system
> with an integrated debugger. A compiled binary doesn't need any runtime
> library anymore, therefore the curry package isn't needed anymore either.
> 
> I will however accept if your insist on it.
> 
> > APPROVED
> 
> 
> (In reply to comment #9)
> > Oops, I just noticed that the debuginfo package came out empty.  I don't see
> > anything being stripped and the compiler is being called with -g so I'm not sure
> > what's up here.  Perhaps it's best to just disable the -debuginfo package
> > generation.
> I will try to find what's happening.
> How is it disabled?
> 



Comment 14 Jason Tibbitts 2006-06-14 14:29:45 UTC
------- Additional Comments From gemi  2006-06-10 16:25 EST -------
I reverted to the non-split version and added a patch to disable
stripping:
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/curry-0.9.10-2.src.rpm
However, I doubt that that debuginfo package is really useful,
after all it is written mostly in Haskell, only the runtime library is C.
There are no C files in the debuginfo package. There are only .debug
versions of the binaries.

Should I still import?


Comment 15 Jason Tibbitts 2006-06-14 14:30:28 UTC
------- Additional Comments From tibbs.edu  2006-06-10 17:18 EST -------
I think it's fine as is; at least now you can install the -debuginfo package and
get symbols if the compiler coredumps.  (You might argue that nobody would
bother, but then we'd just turn off debuginfo generation on most other packages
as well.)

I think it's fine for you to go ahead and import at this point.


Comment 16 Jason Tibbitts 2006-06-14 14:31:06 UTC
------- Additional Comments From gemi  2006-06-11 13:09 EST -------
Built on FC4, FC5 and FC6. Added entry in owners.list file.
Thanks again for the review!