Updated to http://nbecker.dyndns.org:8080/unuran-0.7.2-2.src.rpm

Mass-block FE-NEEDSPONSOR for the six review requests of Neal Becker. Neal, 
when you get sponsorship, you will have to unblock it for all your requests.

Neal,

It seems (from the large amount of Review Requests) that you're seriously
interested in becoming an FE contributer. However although serious interest is a
very good start you must understand that things are currently organised in FE in
such a way that once you are sponsored you get full CVS access to all packages.
Thus before anyone can sponsor you we (he) should first get to know you a little
bit.

I'm able to sponsor people and as said I believe that you're seriously
interested (good!) So I would like to sponsor you once I get to know you a
little better. For this I would like to suggest the following:
-you choice 3 of the submitted packages for me to review
-we work together to get these 3 packages approved
-once these 3 are approved you can create an account in the account system and
 I'll sponsor you.

Notice that if because of interdeps 3 is a bad number 4 or 5 also is ok :)

Does this sound like a plan?

Neal, ping?

OK, this sounds fine.  Sorry for the delay, I thought I had already replied to 
this but it seems not.

I will collect 3 packages for review.  I think that one that had progressed 
the farthest is kdiff3.  I will also suggest 2 others and get back to you.

Besides kdiff one other is enough, this one counts too :)

Erm, never mind I spoke too soon, I'll start a review on kdiff soonish (I just
started a new job) and then I'll hear 2 others which you would like to get
reviewed for the sponsor process from you.

Removing FE-NEEDSPONSOR, as submitter was sponsored in 205023.

I can fetch neither the spec nor either of the above linked SRPMs.

Moved to:
http://nbecker.dyndns.org:8080/RPM/unuran.spec
http://nbecker.dyndns.org:8080/RPM/unuran-0.7.2-2.src.rpm

A few issues I noticed.

The package won't install:
   /usr/share/info/dir from install of unuran-0.7.2-2.fc6 conflicts with file
from package info-4.8-11.1
You'll need to delete that file, or %exclude it.  This also causes a few rpmlint
warnings:
   E: unuran standard-dir-owned-by-package /usr/share/info
   E: unuran info-dir-file /usr/share/info/dir

You don't call install-info to properly install the info files.  See:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
   E: unuran postin-without-install-info /usr/share/info/unuran.info-2.gz
   E: unuran postin-without-install-info /usr/share/info/unuran.info-2.gz
   E: unuran postin-without-install-info /usr/share/info/unuran.info.gz
   E: unuran postin-without-install-info /usr/share/info/unuran.info.gz
   E: unuran postin-without-install-info /usr/share/info/unuran.info-1.gz
   E: unuran postin-without-install-info /usr/share/info/unuran.info-1.gz
   E: unuran postin-without-install-info /usr/share/info/dir
   E: unuran postin-without-install-info /usr/share/info/dir

These will go away when you fix the install-info bit:
   W: unuran one-line-command-in-%post /sbin/ldconfig
   W: unuran one-line-command-in-%postun /sbin/ldconfig

Headers and unversioned .so files need to be in a -devel subpackage:
   W: unuran devel-file-in-non-devel-package /usr/lib64/libunuran.so
   W: unuran devel-file-in-non-devel-package /usr/include/unuran_config.h

Finally, static libraries are not generally permitted at all.  Passing
--disable-static and removing the .a file from %files gets rid of it.

Hmm, no comments in eight months.  I'll close this ticket soon unless there is
some progress here.

OK, I'm not really that interested in this package anymore, and apparently 
nobody else is either.

I think that static libs could be interesting for the usual
reasons for math libraries. This seems to be a very interesting
package to me. I can't what makes you think that there is no
interest in it, Jason made a pre-review.

I'm not sure how you can say that nobody's interested since review comments were
made.  But I'll go ahead and close this; if anyone else wants to submit it,
they're more than welcome to do so.

Reopen.  Update to unuran-1.1.0.

rpmlint looks OK to me.
mock build passed on F8-x86_64.

https://nbecker.dyndns.org/RPM/unuran.spec
https://nbecker.dyndns.org/RPM/unuran-1.1.0-1.fc8.src.rpm

Fix up install-info:

https://nbecker.dyndns.org/RPM/unuran.spec
https://nbecker.dyndns.org/RPM/unuran-1.1.0-2.fc8.src.rpm

This fails to build for me:

error: Installed (but unpackaged) file(s) found:
   /usr/share/info/dir

The usual way to deal with this is to remove $RPM_BUILD_ROOT/%{_infodir}/dir at
the end of %install.  install-info will properly manage the info catalog in your
scriptlets.

After doing that, I can build and check this; rpmlint has some complaints:
  unuran.src: W: mixed-use-of-spaces-and-tabs (spaces: line 25, tab: line 1)
Not a big deal; fix it if you like.

  unuran.x86_64: W: devel-file-in-non-devel-package 
   /usr/include/unuran_urng_prng.h
  unuran.x86_64: W: devel-file-in-non-devel-package 
   /usr/include/unuran_urng_rngstreams.h
  unuran.x86_64: W: devel-file-in-non-devel-package
   /usr/include/unuran.h
  unuran.x86_64: W: devel-file-in-non-devel-package 
   /usr/include/unuran_urng_gsl.h
These must go in a -devel package.

  unuran.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libunuran.so
The unversioned .so file must be in a -devel package.

The pdf documentation should probably be in the -devel package as well, since
it's not of much use at runtime and it's half the size of the package.

Fortunately the include files have sufficiently distinct names that they don't
need to be relocated to a subdirectory of %{_includedir}.

It really looks to me like the license is GPLv2+, not just GPLv2; running
  grep -r -B1 'any later version' .
in the unpacked source directory shows plenty of hits.

The scriptlets need a couple of tweaks.  First, you need to do install-info in
%preun, not %postun, and you need to conditionalize it for uninstalls only:

  %preun
  if [ $1 = 0 ]; then
    /sbin/install-info --delete %{_infodir}/unuran.info.gz %{_infodir}/dir || :
  fi

  %postun -p /sbin/ldconfig

And unfortunately rpm doesn't generate dependencies for multi-line scriptlets,
so you will need some dependencies:
  Requires(post): /sbin/ldconfig, /sbin/install-info
  Requires(preun): /sbin/install-info

Otherwise I think things look OK.

This is nothing but devel.  If the headers, objects and pdf are moved to a 
devel package, that leaves really nothing.  Are you sure that's a good idea?

Here's the result of that suggestion:
rpm -qlp RPMS/x86_64/unuran-1.1.0-3.fc8.x86_64.rpm 
/usr/share/doc/unuran-1.1.0
/usr/share/doc/unuran-1.1.0/AUTHORS
/usr/share/doc/unuran-1.1.0/COPYING
/usr/share/doc/unuran-1.1.0/KNOWN-PROBLEMS
/usr/share/doc/unuran-1.1.0/NEWS
/usr/share/doc/unuran-1.1.0/README
/usr/share/doc/unuran-1.1.0/THANKS
/usr/share/doc/unuran-1.1.0/UPGRADE

The object files should remain in the main package. But the
.so link should be in the devel package. I personally think that
static libraries, in a subpackage, could be useful, I have
put the reasons on:

http://fedoraproject.org/wiki/PatriceDumas

but you may disagree.

Also I would suggest passing INSTALL='install -p' or similar to
make install DESTDIR=$RPM_BUILD_ROOT
to keep header files timestamps, and I would also suggest dropping 
the .gz suffixes in the install-info scriptlets, install-info
should figure them out automatically.

What about the library unuran optionally depends on? (gsl, 
rngstream and prng). Is it useful to have them used 
during the build?

I also think that it could be nice to have 

%check
make check

Also maybe you could ship the examples directory.

(In reply to comment #19)
> This fails to build for me:
> 
> error: Installed (but unpackaged) file(s) found:
>    /usr/share/info/dir
> 

I can't duplicate this.  Can you recheck this with the latest srpm?

> The usual way to deal with this is to remove $RPM_BUILD_ROOT/%{_infodir}/dir 
at
> the end of %install.  install-info will properly manage the info catalog in 
your
> scriptlets.
> 
> After doing that, I can build and check this; rpmlint has some complaints:
>   unuran.src: W: mixed-use-of-spaces-and-tabs (spaces: line 25, tab: line 1)
> Not a big deal; fix it if you like.
> 
>   unuran.x86_64: W: devel-file-in-non-devel-package 
>    /usr/include/unuran_urng_prng.h
>   unuran.x86_64: W: devel-file-in-non-devel-package 
>    /usr/include/unuran_urng_rngstreams.h
>   unuran.x86_64: W: devel-file-in-non-devel-package
>    /usr/include/unuran.h
>   unuran.x86_64: W: devel-file-in-non-devel-package 
>    /usr/include/unuran_urng_gsl.h
> These must go in a -devel package.
> 

I moved to devel package, this is all that would be left:
 rpm -qlp RPMS/x86_64/unuran-1.1.0-3.fc8.x86_64.rpm 
/usr/share/doc/unuran-1.1.0
/usr/share/doc/unuran-1.1.0/AUTHORS
/usr/share/doc/unuran-1.1.0/COPYING
/usr/share/doc/unuran-1.1.0/KNOWN-PROBLEMS
/usr/share/doc/unuran-1.1.0/NEWS
/usr/share/doc/unuran-1.1.0/README
/usr/share/doc/unuran-1.1.0/THANKS
/usr/share/doc/unuran-1.1.0/UPGRADE
/usr/share/info/unuran.info.gz

This is devel package:
rpm -qlp RPMS/x86_64/unuran-devel-1.1.0-3.fc8.x86_64.rpm 
/usr/include/unuran.h
/usr/include/unuran_urng_gsl.h
/usr/include/unuran_urng_prng.h
/usr/include/unuran_urng_rngstreams.h
/usr/lib64/libunuran.so
/usr/lib64/libunuran.so.7
/usr/lib64/libunuran.so.7.1.0
/usr/share/doc/unuran-devel-1.1.0
/usr/share/doc/unuran-devel-1.1.0/unuran.pdf

OK I have:
* split into devel package
* install -p
* add examples
* fix pre,post stuff

I don't think adding the gsl, etc stuff is useful (at least not for me).  Does 
anyone want it?

rpmlint is (almost) silent - only complaint is:
 rpmlint RPM/RPMS/x86_64/unuran-devel-1.1.0-4.fc8.x86_64.rpm 
unuran-devel.x86_64: W: 
hidden-file-or-dir /usr/share/doc/unuran-devel-1.1.0/examples/.deps
unuran-devel.x86_64: W: 
hidden-file-or-dir /usr/share/doc/unuran-devel-1.1.0/examples/.deps

I think this is ignorable - I suspect the examples are not too useful without 
this.

Please try:

https://nbecker.dyndns.org/RPM/unuran.spec
https://nbecker.dyndns.org/RPM/unuran-1.1.0-4.fc8.src.rpm

This still doesn't build for me due to the unpackaged /usr/share/info/dir file
which you will need to delete or exclude.  I can't imagine that there's
something special about my setup that causes this file to appear, and just to be
certain I went ahead and did a koji scratch build to show you how the buildsys
would deal with it.  The result is a failure on all architectures:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=356336
You can do a scratch build yourself with
  koji build --scratch dist-f9 unuran-1.1.0-4.fc8.src.rpm

(In reply to comment #25)
OK, fixed.  Please try
https://nbecker.dyndns.org/RPM/unuran.spec
https://nbecker.dyndns.org/RPM/unuran-1.1.0-5.fc8.src.rpm

Note that oddly, this doesn't occur when building on my f8 system.  The file 
is never installed.  I simply added rm -f to the install, so that should cover 
it.

This one build fine.  Perhaps you're not building in mock; the dir file is
probably probably only created in specific circumstances you'd see on a clean
system.

So, to recap the rpmlint complaints:

  unuran.src: W: mixed-use-of-spaces-and-tabs (spaces: line 26, tab: line 1)
Not a big deal; fix it if you like.

  unuran-devel.x86_64: W: hidden-file-or-dir 
   /usr/share/doc/unuran-devel-1.1.0/examples/.deps
  unuran-devel.x86_64: W: hidden-file-or-dir 
   /usr/share/doc/unuran-devel-1.1.0/examples/.deps
I don't think this really needs to be packaged at all; it's just makedep output
and all of the files there contain nothing but the string "# dummy".  Unless you
can point to something that doesn't work without it, of course.

There's a rather extensive test suite included here, which I think should be
run.  It does take quite some time but I don't expect that this will be built
all that often, and it might turn up issues on the platforms on which most of us
can't easily test.  Just add this after the %install section:
  %check
  make check
However, doing this turns up a problem: the tests actually fail for me (sorry
for wrapping; the test suite output isn't very readable):

distr_condi: [new  ==> ok] [set  ==> ok] [get  ==> ok] [chg  ==> ok] [init  ==>
ok] [reinit  ==> ok] [sample  ==> ok] [validate
(verify hat)  condi_standardmultinormal_3++++++++++
condi_standardmultinormal_4+++++ condi_multinormal_random(!+)++++
condi_standardmultinormal_domain+++++++++.+++++ ==> failed]
FAIL: t_distr_condi

gibbs: [new  ==> ok] [set  ==> ok] [get  ==> ok] [chg  ==> ok] [init  ==> ok]
[reinit  ==> ok] [sample  ==> ok] [validate
(chi^2)  multinormal+++++++++++++++?(!+)++++
multinormal_ar1++++++..++..++..++..++..++..++++
multinormal_constantrho++..++..++++++.. multicauchy00++00++00++
multicauchy_ar100?++00++00++00?+?(!+) multistudent_ar100++00++00++00++00++00++
multinormal+.+.+.+.+.+.0000 ==> failed]
FAIL: t_gibbs

hinv: [new  ==> ok] [set  ==> ok] [get  ==> ok] [chg  ==> ok] [init  ==> ok]
[reinit  ==> ok] [sample  ==> ok] [validate
mock.Root.build: (chi^2) 
beta+++++++++++++++++++++.+++++++++++++++++.++++++++++++++++++++++++++++++++++++++++++++++++++
cauchy++++++++++++++++++ exponential++++++++++++++++++
gamma+++++++++++++++++++++++++++++++++++++++.+++.++++.++++++++.+++.++++.+++.+++++++++++++++++.+
laplace++++++++++++++++.+ normal++++++++++++.+++.++++++++++
uniform++++++++++++.+++-+ F++++++++++++++++++++++++++++++++++++++(!+)+(!+)++++
cauchy+++++++.. beta++++++++. unknown++++++.+.+++++.++.
triangular-string+++...... triangular-invalid-string000......000...... ==>
failed] [special
test maximal
u-error++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
test unur_chg_truncated():  normal++++++++++++++++++++++++++++++++++++++++
sinus1++++++++++++++++++++++++++++++++++++++++
sinus2++++++++++++++++++++++++++++++++++++++++ ==> ok]
FAIL: t_hinv

===============================================
3 of 54 tests failed
Please report to unuran.ac.at
===============================================

Perhaps you could investigate these with upstream.  Maybe there's some x86_64 issue?


* source files match upstream:
   a297f28a717b8ddd50bf06fb98301b780ed4e2246dfa5427b02dbf6d40caa879  
   unuran-1.1.0.tar.gz
* package meets naming and versioning guidelines.
* 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 included in package.
* latest version is being packaged.
* BuildRequires are proper (none)
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has a valid complaint.
* final provides and requires are sane:
  unuran-1.1.0-5.fc9.x86_64.rpm
   libunuran.so.7()(64bit)
   unuran = 1.1.0-5.fc9
  =
   /bin/sh
   /sbin/install-info
   /sbin/ldconfig
   libunuran.so.7()(64bit)

  unuran-devel-1.1.0-5.fc9.x86_64.rpm
   unuran-devel = 1.1.0-5.fc9
  =
   libunuran.so.7()(64bit)
   unuran = 1.1.0-5.fc9

X %check is not present, but there seems to be a test suite.  When added, the 
   test suite fails.
* shared libraries present; ldconfig is called as necessary and unversioned .so 
   files 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.
* scriptlets look OK (ldconfig and info files)
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Same test failures here (also x86_64), I have sent upstream.

Regarding examples/.deps, Simply rm during %install causes check to fail.

I don't know how to fix this.  examples get installed by:

%doc examples

Not sure how to rm .deps without screwing up %check.  Any ideas?

Hmm, good point; I didn't actually test deleting it.

I trued using %exclude:
  %exclude %{_docdir}/%{name}-devel-%{version}/examples/.deps
but that doesn't work, and I've no idea why.

Nothing else I can come up with is remotely savory, so I'm inclined to ignore it.

What remains is the test suite.  I don't know what the failures mean, honestly,
so I can't judge whether the failures represent some error accumulation down in
the noise or if there's a serious issue.

Created attachment 294463 [details]
spec file fixes

Minor fixes (dot end description, INSTALL='install -p' at the right
place no -f for rm for files that are to be here to be noticed when 
they are not here anymore, failsafe install-info script).

And a fix for examples to clean it and remove Makefile* while not
touching the sources.

Thanks for the patches.  Looks OK.

I'm leaving %check off for now.  Upstream has been contacted and says he'll 
look into it, but he's busy for now.

https://nbecker.dyndns.org/RPM/unuran.spec
https://nbecker.dyndns.org/RPM/unuran-1.1.0-6.fc8.src.rpm

I've been meaning to ask: is there some specific reason all of your links go to
an https site with an invalid certificate?  It's annoying to keep having to pass
--no-check-certificate to wget and I can't imagine there's any reason why I
would need to get a specfile over an encrypted channel.

So, to the package.  It still builds fine and rpmlint is down to just the
spaces-and-tabs thing.

If you're comfortable with having the test suite disabled and still believe that
the software is useful and gives correct results even while some tests are known
to fail the then I'm OK with that.  I guess it would be preferable to disable
the tests we know are failing so that we at least get some coverage, but I
honestly don't know how difficult that would be to do.  So,

APPROVED

Sorry for any annoyance, the reason is that my ISP is blocking port 80, so 
that was just a simple workaround.

New Package CVS Request
=======================
Package Name: unuran
Short Description: Universal Non-Uniform Random number generator
Owners: nbecker
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: tibbs.edu

What do you mean by "Cvsextras Commits: tibbs.edu" ? You want tibbs to
be in InitialCC? The Cvsextras Commits: field should be 'yes' or 'no'. ;) 

Actually, please don't CC me; I do too many reviews to be CC'd on all of the
packages.

New Package CVS Request
=======================
Package Name: unuran
Short Description: Universal Non-Uniform Random number generator
Owners: nbecker
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

cvs done.

Anything happening with this package?  I notice it's been built on rawhide, F9
and F7 but not F8, and it was pushed to F7 and F9 testing but there was no
request to push to stable.  I don't expect that this package will see much
testing, so you'll almost certainly have to manually request that it go out as a
stable release.

Also, why was it not built for F8?

Me also confused. Ping, Neal!

unuran-1.2.4p1-1.fc8 has been submitted as an update for Fedora 8

Ok, thanks.
Seems that we can close this Review Request now.

unuran-1.2.4p1-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

With reference to the discussion in bug #618699

Package Change Request
======================
Package Name: unuran
New Branches: EL-5 EL-6
Owners: ellert

Git done (by process-git-requests).