Also, I've sent my rpmlint warnings to the developer upstream re: the absence of a manpage and exit().

Also this is my first package so I need a sponsor. I've also submitted 2 other packages allowing a user to interact from R with JAGS. These review requests are 

https://bugzilla.redhat.com/show_bug.cgi?id=600517
https://bugzilla.redhat.com/show_bug.cgi?id=600529

I've updated the SRPM and the spec file. 
Spec URL: http://dl.dropbox.com/u/1501309/jags.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/jags-2.1.0-2.fc13.src.rpm

This addresses the issues with the man page and an explanation of the exit() warning from the upstream author (see the Changelog). The man page will be included in newer releases.

Additionally, the upstream author believes that the final outstanding rpmlint warning associated with hidden files has to do with libtool and not JAGS.

This is an informal review, as I understand from a mail you are
seeking sponsership and should block this bug on FE-NEEDSPONSER accordingly.

Just looking at the .spec file for now 

1) The .la should be removed somehow.

See:
http://fedoraproject.org/wiki/PackagingGuidelines#Packaging_Static_Libraries

2) Your release tag is 2.1 which is odd, it typically starts at 1. :-)

See: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Version
In a nutshell it's the number of .spec file modifications or builds in fact
since the Version was last update.

3) Not important I would say but in principal packages take the name of 
the tar ball so JAGS but I would never not approve based on that.
http://fedoraproject.org/wiki/Packaging:NamingGuidelines

4) BuildRequires: gcc-c++ certainly is not needed, see
http://fedoraproject.org/wiki/PackagingGuidelines#Exceptions_2

5) I see you have a .tex file in your docs have you considered converting
   it to something more readable such as ps or pdf.

 Steve.

Hi Steve,
Thanks for for taking a quick look at this. Regarding:

1) I am really not sure how to deal with this one. I tried to configure it w/ --disable-static but those two *.la files still showed up. The two *.la files:

libjags.la
libjrmath.la

are JAGS specific I believe and looking here:
http://fedoraproject.org/wiki/PackagingGuidelines#Packaging_Static_Libraries

I can't determine exactly what I should do. Create a *-static package, contact upstream or something else. Help with this would be very useful!

2) I know. I didn't realize that until after I uploaded it. This should be fixed now.

3) and 4)  Fixed in JAGS-2.1.0-3.fc13.src.rpm[1]

5) I haven't really given this much consideration. It would be a good idea ... but what format? http://fedoraproject.org/wiki/Packaging:Guidelines doesn't seem to provide any real guidance.

[1] Updated Spec and Source URL
Spec URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS-2.1.0-3.fc13.src.rpm

(In reply to comment #5)
> Hi Steve,
> Thanks for for taking a quick look at this. Regarding:
> 
> 1) I am really not sure how to deal with this one. I tried to configure it w/
> --disable-static but those two *.la files still showed up. The two *.la files:
> 
> libjags.la
> libjrmath.la
> 
> are JAGS specific I believe and looking here:
> http://fedoraproject.org/wiki/PackagingGuidelines#Packaging_Static_Libraries
> 
> I can't determine exactly what I should do. Create a *-static package, contact
> upstream or something else. Help with this would be very useful!

If --disable-static does not work, just delete them
in the %build section after your build. i.e

rm %{_builddir}%{_libdir}/*.la

> 
> 2) I know. I didn't realize that until after I uploaded it. This should be
> fixed now.
> 
> 3) and 4)  Fixed in JAGS-2.1.0-3.fc13.src.rpm[1]
> 
> 5) I haven't really given this much consideration. It would be a good idea ...
> but what format? http://fedoraproject.org/wiki/Packaging:Guidelines doesn't
> seem to provide any real guidance.

Your choice even if it to do it or not , I would probably convert to
pdf , of course you will need to pull in pdflatex as build dependencies
if you choose to do this.
> 
> [1] Updated Spec and Source URL
> Spec URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS.spec
> Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS-2.1.0-3.fc13.src.rpm    

Will look after the next iteration.

Updated.
SPEC URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS-2.1.0-4.fc13.src.rpm

However, I still wasn't able to get:

rm %{_builddir}%{_libdir}/*.la


to work no matter where I put it. Also are all files that end in .la static? Are they all problematic? If so there are several files in JAGS that have .la in them.

[root@goddard x86_64]# rpm -ql JAGS | grep la
/usr/lib64/JAGS/modules-2.1.0/basemod.la
/usr/lib64/JAGS/modules-2.1.0/bugs.la
/usr/lib64/JAGS/modules-2.1.0/dic.la
/usr/lib64/JAGS/modules-2.1.0/glm.la
/usr/lib64/JAGS/modules-2.1.0/lecuyer.la
/usr/lib64/JAGS/modules-2.1.0/mix.la
/usr/lib64/JAGS/modules-2.1.0/msm.la
/usr/lib64/libjags.la
/usr/lib64/libjrmath.la

To be honest, I am a little stumped and unsure about how to deal with this. I looked at the Debian package and it had these .la files in there too. Any further help you can give me is greatly appreciated!

Oh the difference between -3 and -4 is that I now build the .tex files into PDF and some other subtle differences.

Hi Chris,

.la files must not be packaged in Fedora according to the packaging guidelines. I suggest to remove them in the %install section after "make install", e.g. with
find %{buildroot}%{_libdir} -name "*.la" -exec rm -f {} \;

Also, drop them in the %files section.

Here are some further comments:

- in the devel package, use a fully versioned dependency:
  Requires: %{name} = %{version}-%{release} 
  (rather than Requires: JAGS  = %{version})

- According to the source file headers, the license is GPLv2+ because 
  of the addition "or (at your option) any later version".

- add file COPYING to the base package (as %doc)

- the Group of the devel package should be 'Development/Libraries'

- as a minor cosmetic addition, I suggest to append a slash to directory 
  names, e.g. %{_includedir}/JAGS/
  This way it's easier to recognize that a folder is added rather than a 
  single file

Some more comments:
- %setup -q should be sufficient in %prep
- add %{_smp_mflags} to the first 'make'
- replace %defattr(-,root,root) with %defattr(-,root,root,-)
- add AUTHORS, ChangeLog, NEWS and TODO as %doc to the base package

Hi Martin,
Thank you very much for reviewing this package! I am really excited about hopefully getting this package into Fedora. 

I've updated my *.spec and *.src.rpm files to reflect all the changes that you've mentioned above. Please let me know what else I can do. 

Spec URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS-2.1.0-5.fc13.src.rpm

(In reply to comment #11)
> Thank you very much for reviewing this package! I am really excited about
> hopefully getting this package into Fedora. 

Hi Chris,

I'm not a sponsor, so I can't formally review this package. Nonetheless, polishing the spec file will certainly help to find one. In addition, you usually need to offer some more packages and comment on other packager's review requests to show that you're familiar with the packaging guidelines.

Here are a couple of further remarks on your package:

- In Source0, replace JAGS-2.1.0.tar.gz with JAGS-%{version}.tar.gz or
  %{name}-%{version}.tar.gz. It simplifies future updates of the package

- I think, you can also add %{_smp_mflags} to "make docs". It enables 
  parallel builds if possible, and thus speeds up the build process on 
  multi-processor systems.

- remove the trailing slash from %{_bindir}/jags/ because jags is not
  a directory here but a binary file

- you can drop jags_installation_manual.pdf as it's not of much use in 
  a binary package

- also adapt the %defattr parameters of the devel package as mentioned 
  in comment #10

- The tarball contains a copy of libltdl which should be removed in favor 
  of the package provided by Fedora (libtool-ltdl-devel).

Thanks. I will update the package once I get the libltdl situation figured out. AI am not entirely sure that removing the *.la files is correct and I have contacted the upstream author to verify this doesn't cripple the program. (It doesn't appear to). 

Also, I wanted to mention I am seeking sponsorship for these packages as well:

https://bugzilla.redhat.com/show_bug.cgi?id=600529
https://bugzilla.redhat.com/show_bug.cgi?id=600517

I would like to maintain several R packages in Fedora beyond these (including R-lme4 and R-MCMCglmm) but wanted to focus on these 3 first. Should I offer more packages too?

(In reply to comment #13)
> AI am not entirely sure that removing the *.la files is correct and I have
> contacted the upstream author to verify this doesn't cripple the program. (It
> doesn't appear to). 

Removing the libtool archives isn't harmful at all. They are only used when linking a library with libtool. Alternatively, you could also explicitly link against the corresponding .a or .so files. Once the binary is built, the .la files are not required any longer, especially since Fedora primarily distributes shared libs which are linked via .so files. And the latter are part of the devel package. 


> I would like to maintain several R packages in Fedora beyond these (including
> R-lme4 and R-MCMCglmm) but wanted to focus on these 3 first. Should I offer
> more packages too?    

OK, that's fine. As far as I know, three packages should be enough, but this decision is up to your sponsor.

Hi Martin,
Thanks for your speedy reply. Do you know how I would do this?

> - The tarball contains a copy of libltdl which should be removed in favor 
>  of the package provided by Fedora (libtool-ltdl-devel).  

In the .spec file? I am unclear how to do this. Thanks!

I haven't had a deeper look into the tarball and the autotools scripts, but you can probably do it this way:

- add BuildRequires: libtool-ltdl-devel autoconf automake

- in %prep: 
  * remove the libltdl directory
  * remove all references to this directory from configure.ac and Makefile.am 
    by patching these files accordingly 
   (don't modify the files in the tarball but add .patch/.diff files
    describing the changes to your package)
  * run 'autoreconf'

This is just the general idea. I haven't tried it, so there might be further things to be addressed.

I have addressed all of the outstanding comments (#12 and #16) in the latest version.

SPEC URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS-2.1.0-6.fc13.src.rpm

I have patched configure.ac and Makefile.am as per Martin's requests. Also, I removed ChangeLog as it is empty. The binary installs fine and works.

Updated:

SPEC URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS-2.1.0-7.fc13.src.rpm

The change from -6 to -7 is that I removed the patches for ltdl. The upstream author informed that while JAGS ships with its own copy of ltdl if it detects a system copy then it will automatically use that and not the version in the sources. He said that I need to only add libtoool-ltdl-devel in the BuildRequires. Therefore, I believed this rendered my patches unnecessary. This is reflected in the new .src.rpm and .spec file. Also he thinks it's probably fine to remove those two .la files.

(In reply to comment #18)
> He said that I need to only add libtoool-ltdl-devel in the
> BuildRequires. Therefore, I believed this rendered my patches unnecessary.

Hi Chris,

you should not rely on this. Since using bundled libraries is not permitted in Fedora, you have to ensure that these are not linked. You usually do this by removing the bundled libs. Thus, I suggest to revert to release 6.

OK reverting to release 6.

SPEC URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS.spec
Source URL: http://dl.dropbox.com/u/1501309/Fedora/JAGS/JAGS-2.1.0-6.fc13.src.rpm

Sorry I don't have time to do this anymore. Graduate school!!! So I have to close this bug.