Bug 518049

Debian is building just one variant of tachyon, but I'm building four and two of them end up with a different libtachyon (pthreads vs. LAM), so it's not so trivial. I've updated tachyon to 0.98.7 without any big changes for now and I'll work on the other stuff when time permits.

please send patche`s to upstream.

This bug appears to have been reported against 'rawhide' during the Fedora 12 development cycle.
Changing version to '12'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

any news about this ?

Well, since LAM has just been retired from rawhide, I guess I'll just drop the -lam subpackage. This makes it possible to implement what Debian has.

OK, now is a good time to fix the package to conform to the MPI guidelines as well.

So you're not going to ship MPI enabled packages at all..?

Not now due to lack of time, but it's possible later. Patches welcome.

Created attachment 380469 [details]
Patch adding Open MPI and MPICH2 support

There you go. Took me a while, but this should work. Check the changelog.

I also took the liberty of "modernizing" the spec file to my pleasure :)

Created attachment 380470 [details]
Revised patch adding Open MPI and MPICH2 support

Revised package builds fine in koji
http://koji.fedoraproject.org/koji/taskinfo?taskID=1892104

Patch review:

--- tachyon.spec	2009-12-26 16:27:14.000000000 +0200
+++ tachyon.spec.new	2009-12-27 00:18:58.568777805 +0200
+# To make sure the docs are removed when executables are removed
+Provides: %{name}-executable = %{version}-%{release}

Explain.

+# Due to unison of -gl package
+Obsoletes: %{name}-gl < %{version}-%{release}

Rejected. What if someone wants to use tachyon to render on a machine without OpenGL packages installed?

+%package headers 
+Summary: Development headers for tachyon
+Group: Development/Libraries
+BuildArch: noarch

What's wrong with having headers in -devel?

+%description headers
+This package contains common files for tachyon executables.

Wrong description anyway.

+
+%package man
+Summary: Manual pages for tachyon
+Group: Documentation
+BuildArch: noarch
+
+%description man
+This package contains man pages for tachyon executables.

There's just one man page.

@@ -76,7 +174,7 @@ with tachyon.
 %setup -q -n %{name}
 %patch0 -p1 -b .r
 %patch1 -p1 -b .shared
-find . -name CVS | xargs %{__rm} -r
+find . -name CVS | xargs rm -rf {} \;

Less efficient and I omitted the 'f' option intentionally. I want it to fail when the CVS dirs disappear from the tarball.

 # executable sources
 chmod 644 src/hash.{c,h}
 chmod 644 src/pngfile.h
@@ -85,60 +183,143 @@ chmod 644 demosrc/trackball.{c,h}
 chmod 644 scenes/imaps/*
 
 %build
+# Build binaries

Unnecessary comment. Most of the others are redundant, too.

+# Install serial versions

They're not serial, but parallel, too. They just don't use MPI.

+# Create a saner html doc dir
 mkdir docs/html
-%{__cp} -pr docs/tachyon/*.{css,html,png} docs/html
-%{__cp} -pr scenes $RPM_BUILD_ROOT%{_datadir}/tachyon/
-install -pm644 %{SOURCE1} $RPM_BUILD_ROOT%{_mandir}/man1/
-echo ".so tachyon.1" > $RPM_BUILD_ROOT%{_mandir}/man1/tachyon-ogl.1
-%{__cp} -a compile/%{target}-thr/libtachyon*.so $RPM_BUILD_ROOT%{_libdir}/
-install -pm644 src/{hash,rtcommon,tachyon,util}.h $RPM_BUILD_ROOT%{_includedir}/tachyon/
+cp -pr docs/tachyon/*.{css,html,png} docs/html
+
+# Install data, headers and man pages
+cp -pr scenes %{buildroot}%{_datadir}/tachyon/
+install -p -m 644 src/{hash,rtcommon,tachyon,util}.h %{buildroot}%{_includedir}/tachyon/
+install -p -m 644 %{SOURCE1} %{buildroot}%{_mandir}/man1/
+echo ".so tachyon.1" > %{buildroot}%{_mandir}/man1/tachyon-ogl.1

Unnecessary cosmetic changes, rejected. All similar are rejected, too.
I'd consider them if they were in a separate patch.
 
 %files docs
-%defattr(644,root,root,755)
+%defattr(-,root,root,-)
 %doc Changes docs/tachyon.pdf docs/tachyon.ps docs/html
-%{_datadir}/tachyon
+%{_datadir}/tachyon/

Why?

 %changelog
+* Sat Dec 26 2009 Jussi Lehtola <jussilehtola> 0.98.7-3
+- Branch out man pages and headers into their own packages

OK.

+- Add support for Open MPI and MPICH2 with a correct upgrade path from the LAM
+  package

OK.

+- Due to the small size of the binary packages and the explosion of the
+  package number caused by the MPI support, unite -gl packages with main
+  binary packages

Rejected.

+- Add helpful comments to spec file

Unnecessary, rejected.

+- Sanitate the Requires: for the -docs package. Made -docs noarch.

Sanit_ize_. Explain the Requires changes, please.

(In reply to comment #13)
> Patch review:
> 
> --- tachyon.spec 2009-12-26 16:27:14.000000000 +0200
> +++ tachyon.spec.new 2009-12-27 00:18:58.568777805 +0200
> +# To make sure the docs are removed when executables are removed
> +Provides: %{name}-executable = %{version}-%{release}
> 
> Explain.

Currently -gl
 Provides: %{name} = %{version}-%{release}
and -docs
 Requires: %{name} = %{version}-%{release}

I don't like %{name} being used this way, IMHO it's better to have a metaprovide for the same purpose.


> +# Due to unison of -gl package
> +Obsoletes: %{name}-gl < %{version}-%{release}
> 
> Rejected. What if someone wants to use tachyon to render on a machine without
> OpenGL packages installed?

As you can see, the patch unites -gl with the main package. Thus rendering works anyway. I don't see the point of having two separate 32kb packages. With the unison you get 12 binary rpms with Open MPI and MPICH2, without it you get 3 more.

> +%package headers 
> +Summary: Development headers for tachyon
> +Group: Development/Libraries
> +BuildArch: noarch
> 
> What's wrong with having headers in -devel?

Because both mpi devel packages also need the headers, and they're not dependent on the serial devel package.

> +
> +%package man
> +Summary: Manual pages for tachyon
> +Group: Documentation
> +BuildArch: noarch
> +
> +%description man
> +This package contains man pages for tachyon executables.
> 
> There's just one man page.

No, two: the one that was in tachyon and the one that was in tachyon-gl. If these are identical, then you can use a common package anyway (symlink?).

> -find . -name CVS | xargs %{__rm} -r
> +find . -name CVS | xargs rm -rf {} \;
> 
> Less efficient and I omitted the 'f' option intentionally. I want it to fail
> when the CVS dirs disappear from the tarball.

ok.

> +# Install serial versions
> 
> They're not serial, but parallel, too. They just don't use MPI.

Well, yeah. But that's a bit beside the point. 

> Unnecessary cosmetic changes, rejected. All similar are rejected, too.
> I'd consider them if they were in a separate patch.

Cut'n'paste? :)

>  %files docs
> -%defattr(644,root,root,755)
> +%defattr(-,root,root,-)
>  %doc Changes docs/tachyon.pdf docs/tachyon.ps docs/html
> -%{_datadir}/tachyon
> +%{_datadir}/tachyon/
> 
> Why?

Cosmetic changes.

> +- Sanitate the Requires: for the -docs package. Made -docs noarch.
> 
> Sanit_ize_. Explain the Requires changes, please.  

See top of comment, refer to current spec file.

After some more thought, the patch is OK, but without the cosmetics and "helpful comments". Feel free to apply directly to CVS.

Maybe it would be better if you applied the relevant bits yourself :)

Sure, I'll try to do that this weekend. Thanks for your work.