Bug 518049 - Update to 0.98.7, add -devel package
Summary: Update to 0.98.7, add -devel package
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: tachyon
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-18 15:18 UTC by Susi Lehtola
Modified: 2010-01-15 20:55 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-12-26 14:34:46 UTC


Attachments (Terms of Use)
Patch adding Open MPI and MPICH2 support (10.27 KB, patch)
2009-12-26 21:55 UTC, Susi Lehtola
no flags Details | Diff
Revised patch adding Open MPI and MPICH2 support (10.36 KB, patch)
2009-12-26 22:20 UTC, Susi Lehtola
no flags Details | Diff

Description Susi Lehtola 2009-08-18 15:18:30 UTC
Upstream has released 0.98.7, please update to it.

Please build and install also the library and the headers. By default tachyon has support for building a static library, Debian has added patches to support shared libraries as well, e.g. http://packages.debian.org/lenny/libtachyon-dev

Comment 1 Dominik 'Rathann' Mierzejewski 2009-09-21 00:39:50 UTC
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.

Comment 2 Itamar Reis Peixoto 2009-09-21 00:44:46 UTC
please send patche`s to upstream.

Comment 3 Bug Zapper 2009-11-16 11:32:10 UTC
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

Comment 4 Itamar Reis Peixoto 2009-11-19 07:03:46 UTC
any news about this ?

Comment 5 Dominik 'Rathann' Mierzejewski 2009-12-11 23:50:45 UTC
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.

Comment 6 Susi Lehtola 2009-12-14 11:23:39 UTC
OK, now is a good time to fix the package to conform to the MPI guidelines as well.

Comment 7 Susi Lehtola 2009-12-26 15:06:33 UTC
So you're not going to ship MPI enabled packages at all..?

Comment 8 Dominik 'Rathann' Mierzejewski 2009-12-26 18:20:06 UTC
Not now due to lack of time, but it's possible later. Patches welcome.

Comment 9 Susi Lehtola 2009-12-26 21:55:54 UTC
Created attachment 380469 [details]
Patch adding Open MPI and MPICH2 support

Comment 10 Susi Lehtola 2009-12-26 21:57:24 UTC
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 :)

Comment 11 Susi Lehtola 2009-12-26 22:20:47 UTC
Created attachment 380470 [details]
Revised patch adding Open MPI and MPICH2 support

Comment 12 Susi Lehtola 2009-12-26 22:27:58 UTC
Revised package builds fine in koji
http://koji.fedoraproject.org/koji/taskinfo?taskID=1892104

Comment 13 Dominik 'Rathann' Mierzejewski 2009-12-27 12:45:37 UTC
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@fedoraproject.org> 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.

Comment 14 Susi Lehtola 2009-12-27 15:20:58 UTC
(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.

Comment 15 Dominik 'Rathann' Mierzejewski 2010-01-15 18:55:30 UTC
After some more thought, the patch is OK, but without the cosmetics and "helpful comments". Feel free to apply directly to CVS.

Comment 16 Susi Lehtola 2010-01-15 19:57:09 UTC
Maybe it would be better if you applied the relevant bits yourself :)

Comment 17 Dominik 'Rathann' Mierzejewski 2010-01-15 20:55:21 UTC
Sure, I'll try to do that this weekend. Thanks for your work.


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