Bug 510822 - Review Request: maloc - Minimal Abstraction Layer for Object-oriented C/C++ programs
Summary: Review Request: maloc - Minimal Abstraction Layer for Object-oriented C/C++ p...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 497622
TreeView+ depends on / blocked
 
Reported: 2009-07-11 00:40 UTC by Tim Fenn
Modified: 2014-11-06 12:52 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-05 23:09:38 UTC
Type: ---
Embargoed:
j: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Tim Fenn 2009-07-11 00:40:32 UTC
Spec URL: http://www.stanford.edu/~fenn/packs/maloc.spec
SRPM URL: http://www.stanford.edu/~fenn/packs/maloc-0.2-1.fc10.src.rpm

Description: MALOC is a small, portable, abstract C environment library for
object-oriented C programming. MALOC is used as the foundation layer
for a number of scientific applications, including MC, SG, and
APBS. MALOC can be used as a small stand-alone abstraction environment
for writing portable C programs which need access to resources which
are typically architecture-dependent, such as INET sockets, timing
routines, and so on. MALOC provides abstract datatypes, memory
management routines, timing routines, machine epsilon, access to UNIX
and INET sockets, MPI, and so on. All things that can vary from one
architecture to another are abstracted out of an application code and
placed in MALOC.

MALOC is also useful for APBS, which acts as a poisson-boltzmann solver for pymol.

Comment 1 Jason Tibbitts 2009-07-11 00:59:04 UTC
Builds fine; rpmlint says: 
  maloc.x86_64: W: no-documentation
  maloc-devel.x86_64: W: no-documentation
which are OK, and

  maloc.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmaloc.so.1.0.0
   /lib64/libncurses.so.5
Which is odd; why is this linked against ncurses when it doesn't call ncurses?  I guess this has something to do with readline.  I wouldn't say it's a problem because libncurses is going to come in either way.

Full review forthcoming.

Comment 2 Tim Fenn 2009-07-11 01:14:04 UTC
(In reply to comment #1)
> 
>   maloc.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmaloc.so.1.0.0
>    /lib64/libncurses.so.5
> Which is odd; why is this linked against ncurses when it doesn't call ncurses? 
> I guess this has something to do with readline.  I wouldn't say it's a problem
> because libncurses is going to come in either way.
> 

Noticed that - the configure.ac specifically checks for ncurses, I don't know why.  I'll mention it to upstream.

Comment 3 Jason Tibbitts 2009-07-11 01:28:04 UTC
I'm a bit confused about the versioning.  Is the "-1" in the tarball indicative of some sub-version thing?  Will they release 0.2-2 in the future?  What would you call this package in that case?  Upstream also produces RPMs and seems to use the dashed portion of the version as their Release: (or perhaps that's just coincidental), but it should be obvious that we can't do that.

The blank line at the start of %description does make it into the final package and should probably be removed.

Did you intend not to build and package the documentation?  It seems like that would be a good idea, but I haven't checked that the documentation is actually useful.

Why does this require pkgconfig?  I don't see any .pc files in the package.

* source files match upstream.  sha256sum:        
   9b29c4b6401adf20ce1ab3c47fe71066ca7952eb10db4b1e6b1440973f616cda
   maloc-0.2-1.tar.gz
? name is OK; not sure about package versioning.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK.                                                          
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
? final provides and requires:
  maloc-0.2-1.fc12.x86_64.rpm
   libmaloc.so.1()(64bit)
   maloc = 0.2-1.fc12
   maloc(x86-64) = 0.2-1.fc12
  =
   /sbin/ldconfig
   libmaloc.so.1()(64bit)
   libncurses.so.5()(64bit)
   libreadline.so.5()(64bit)

  maloc-devel-0.2-1.fc12.x86_64.rpm
   maloc-devel = 0.2-1.fc12
   maloc-devel(x86-64) = 0.2-1.fc12
  =
   libmaloc.so.1()(64bit)
   maloc = 0.2-1.fc12
?  pkgconfig

* %check is present; no test suite upstream.
* shared libraries are installed:
   ldconfig called properly.
   unversioned .so links are in the -devel package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets are OK (ldconfig).
* code, not content.
* headers are in the -devel package.
* no pkgconfig files (but maybe there should be?)
* no static libraries.
* no libtool .la files.

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 4 Tim Fenn 2009-07-11 21:41:40 UTC
(In reply to comment #3)
> I'm a bit confused about the versioning.  Is the "-1" in the tarball 
> indicative of some sub-version thing?  Will they release 0.2-2 in the 
> future?  What would you call this package in that case?  Upstream also 
> produces RPMs and seems to use the dashed portion of the version as their 
> Release: (or perhaps that's just coincidental), but it should be obvious that 
> we can't do that.
> 

I'm not sure what that indicates, and its entirely reasonable to assume a "-2" version will be made at some point. Is it possible/OK to just transmogrify "0.2-1" into "0.2.1"? Perhaps its best to ask upstream WTF that number represents?

> The blank line at the start of %description does make it into the final package
> and should probably be removed.
> 

done.

> Did you intend not to build and package the documentation?  It seems like that
> would be a good idea, but I haven't checked that the documentation is actually
> useful.
> 

Oh, yeah - i kind of whipped this up in a jif so I could test the apbs builds I was doing - I'll add that in.

> Why does this require pkgconfig?  I don't see any .pc files in the package.
> 

Oops - removed.

> 
> The package review process needs reviewers!  If you haven't done any package
> reviews recently, please consider doing one.  

When I get enough free time, I'll jump in!  :)

Comment 5 Jason Tibbitts 2009-07-12 20:22:45 UTC
Since a dash isn't legal in a version, I would transform it into an underscore.  You never know; they might actually release 0.2.1-1 in the future.  (Although unfortunately 0.2_2 sorts newer than 0.2.1_1, so if they did that you would have to resort to using Epoch: to get things to sort properly.)

Honestly the best thing to do is ask them.

Comment 6 Tim Fenn 2009-07-28 22:40:32 UTC
2 emails, no reply on the version numbering.  :(

Comment 7 Jason Tibbitts 2009-07-29 17:04:54 UTC
If you don't hear from upstream, there are a couple of possibilities.  If they won't reply to a simple question like that, how are they going to reply to bug reports?  Is it worth packaging software with an absent upstream?  If you're still sure you want to package it, you could just make a guess.  Honestly I'd just drop the bit after the dash and later add it into the version somehow if you get a response from upstream that indicates it's meaningful.  You can always rely on Epoch: to force an ordering if you need to.

Comment 8 Tim Fenn 2009-07-30 01:19:33 UTC
(In reply to comment #7)
> If you don't hear from upstream, there are a couple of possibilities.  If they
> won't reply to a simple question like that, how are they going to reply to bug
> reports?  Is it worth packaging software with an absent upstream?

They've been responsive to other issues, so I'm assuming this issue is just below their radar.

Spec URL: http://www.stanford.edu/~fenn/packs/maloc.spec
SRPM URL: http://www.stanford.edu/~fenn/packs/maloc-0.2-2.fc10.src.rpm

Comment 9 Jason Tibbitts 2009-08-01 15:22:34 UTC
Unfortunately that package doesn't build for me.

+ make -C doc/doxygen
make: Entering directory `/builddir/build/BUILD/maloc/doc/doxygen'
doxygen maloc.dox
make: execvp: doxygen: Permission denied

That's kind of odd, but the package doesn't seem to have any build dependency on doxygen, so most likely it's simply not there.

Comment 10 Tim Fenn 2009-08-06 18:39:23 UTC
(In reply to comment #9)
> Unfortunately that package doesn't build for me.
> 
> + make -C doc/doxygen
> make: Entering directory `/builddir/build/BUILD/maloc/doc/doxygen'
> doxygen maloc.dox
> make: execvp: doxygen: Permission denied
> 
> That's kind of odd, but the package doesn't seem to have any build dependency
> on doxygen, so most likely it's simply not there.  

Added doxygen to the buildreq:

Spec URL: http://www.stanford.edu/~fenn/packs/maloc.spec
SRPM URL: http://www.stanford.edu/~fenn/packs/maloc-0.2-3.fc10.src.rpm

Comment 11 Jason Tibbitts 2009-08-19 18:49:15 UTC
OK, builds fine, rpmlint is as previously discussed above and the versioning loooks good to me.

APPROVED

Comment 12 Tim Fenn 2009-08-20 20:48:57 UTC
New Package CVS Request
=======================
Package Name: maloc
Short Description: Minimal Abstraction Layer for Object-oriented C/C++ programs
Owners: timfenn
Branches: F-11 EL-5
InitialCC: timfenn

Comment 13 Dennis Gilmore 2009-08-21 18:27:37 UTC
CVS done

Comment 14 Tim Fenn 2009-08-21 20:04:16 UTC
builds for EL-5, F-11 and devel done.

Comment 15 Jason Tibbitts 2009-11-05 22:48:37 UTC
Is there any reason for this ticket to remain open?

Comment 16 Tim Fenn 2009-11-05 23:09:05 UTC
no, should be closed.

Comment 17 Tim Fenn 2014-11-06 03:30:08 UTC
Package Change Request
======================
Package Name: maloc
New Branches: epel7
Owners: timfenn
InitialCC: timfenn

Comment 18 Gwyn Ciesla 2014-11-06 12:52:26 UTC
Git done (by process-git-requests).


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