Bug 483663

Summary: Review Request: tetgen - A tetrahedral mesh generator
Product: [Fedora] Fedora Reporter: Fabian Affolter <mail>
Component: Package ReviewAssignee: Dominik 'Rathann' Mierzejewski <dominik>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, i, kevin, manisandro, mycae, nonamedotc, rvokal, tcallawa
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: dominik: fedora-review+
gwync: fedora-cvs+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: tetgen-1.5.0-4.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-07-03 04:02:22 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 721112, 1111388    

Description Fabian Affolter 2009-02-02 20:56:54 UTC
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/tetgen.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/tetgen-1.4.2-1.fc10.src.rpm

Project URL: http://tetgen.berlios.de/

Description: 
TetGen generates the Delaunay tetrahedralization, Voronoi diagram,
constrained Delaunay tetrahedralizations and quality tetrahedral
meshes. The main goal of TetGen is to generate suitable meshes for
solving partial differential equations by finite element or finite
volume methods.

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1099998

rpmlint output:
[fab@laptop24 i386]$ rpmlint tet*
tetgen-devel.i386: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

[fab@laptop24 SRPMS]$ rpmlint tetgen-1.4.2-1.fc10.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Dominik 'Rathann' Mierzejewski 2009-02-03 00:57:00 UTC
Sorry, but this isn't eligible for Fedora. The licence is non-free. You're welcome to try and convince upstream to release it under a free licence.

Comment 2 Tom "spot" Callaway 2009-02-12 18:21:38 UTC
Indeed. This not MIT but a poorly worded custom license with a clear commercial use restriction. Closing this out as CANTFIX.

Comment 3 Jason Tibbitts 2009-02-12 19:08:47 UTC
Someone should let berlios know that this software should not be hosted there.

Comment 4 Fabian Affolter 2011-07-01 16:12:38 UTC
*** Bug 714336 has been marked as a duplicate of this bug. ***

Comment 5 Kevin Kofler 2011-07-01 22:43:38 UTC
> Someone should let berlios know that this software should not be hosted there.

+1. Over 2 years have passed and this software which is neither Free nor Open Source is still hosted on BerliOS, which claims to be about Open Source hosting.

Comment 6 Ankur Sinha (FranciscoD) 2011-07-02 03:42:03 UTC
Mailed them.

Comment 7 Tom "spot" Callaway 2011-07-05 14:19:17 UTC
I would note that Jörg Schilling is the BerliOS admin, so it is possible that his licensing interpretations differ from the rest of the universe.

Comment 8 mycae 2014-02-07 17:54:30 UTC
tetgen is now relicenced as AGPL [1].
[1] http://wias-berlin.de/software/tetgen/1.5/FAQ-license.html

Comment 9 Dominik 'Rathann' Mierzejewski 2014-02-11 23:34:29 UTC
Good news!

If anyone would like to resubmit the package, then I'll review it. It's actually an optional build dependency of one of my packages, so I'm still interested in this.

Comment 10 Fabian Affolter 2014-02-26 08:40:24 UTC
I sent a message to upstream. They placed the source tarball behind a registration form.

Comment 11 Kevin Kofler 2014-02-26 14:31:48 UTC
The AGPL (like any Free Software license) allows you to register once, download the tarball and redistribute it.

This silly "free registration" bullsh*t is unfortunately quite common in the academic world. ("We want to know who uses our software.") They don't realize that it just doesn't work for packaging, nor that they can't force registration for Free Software.

(By the way, I recommend that you download it NOW, because if they realize they can't force you to register under the new license, they might change it back to something more restrictive. :-( )

Comment 12 Sandro Mani 2014-06-14 01:00:12 UTC
I'd like to move on with this:

Spec URL: http://smani.fedorapeople.org/review/tetgen.spec
SRPM URL: http://smani.fedorapeople.org/review/tetgen-1.5.0-1.fc21.src.rpm

Comment 13 Mukundan Ragavan 2014-06-14 01:09:18 UTC
I can review this unless rathann (or someone else) will take care of it ....

Comment 14 Sandro Mani 2014-06-17 12:57:55 UTC
@Fabian: Mind if I take over?

Comment 15 Dominik 'Rathann' Mierzejewski 2014-06-17 13:33:34 UTC
Taking review.

Comment 16 Dominik 'Rathann' Mierzejewski 2014-06-18 21:35:15 UTC
Downloaded sources match SRPM:
$ sha256sum tetgen1.5.0.tar.gz 
4d114861d5ef2063afd06ef38885ec46822e90e7b4ea38c864f76493451f9cf3  tetgen1.5.0.tar.gz

rpmlint:
Checking: tetgen-1.5.0-1.fc20.x86_64.rpm
          tetgen-devel-1.5.0-1.fc20.x86_64.rpm
          tetgen-1.5.0-1.fc20.src.rpm
tetgen.x86_64: W: spelling-error %description -l en_US tetrahedralizations -> rationalizations, liberalizations
tetgen.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/tetgen/example.poly

Please convert.

tetgen.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libtet.so

It looks like it's getting caught by:
%{_libdir}/libtet.so*

Try using
%{_libdir}/libtet.so.*
instead.

tetgen.x86_64: W: no-manual-page-for-binary tetgen
tetgen-devel.x86_64: W: no-documentation
tetgen.src: W: spelling-error %description -l en_US tetrahedralizations -> rationalizations, liberalizations
tetgen.src: W: patch-not-applied Patch0: tetgen_cmake.patch
3 packages and 0 specfiles checked; 0 errors, 7 warnings.

The rest seem to be false positives.

Looking at the tetgen-cmake.patch, I notice that you arbitrarily set the SO version to 1. I suggest starting from 0. Is there a good reason to start from 1?

Also, I don't like it that (almost) the same code is compiled in /usr/bin/tetgen and in %{_libdir}/libtet.so.* . Is it not possible to patch the code so that /usr/bin/tetgen is linked against the library?

Comment 17 Sandro Mani 2014-06-19 07:00:10 UTC
Spec URL: http://smani.fedorapeople.org/review/tetgen.spec
SRPM URL: http://smani.fedorapeople.org/review/tetgen-1.5.0-2.fc21.src.rpm

%changelog
* Sat Jun 14 2014 Sandro Mani <manisandro> - 1.5.0-2
- Fix line endings of example.poly
- Fix libtet.so in main package
- Replace tetgen_cmake.patch with tetgen_build.patch

Comment 18 Sandro Mani 2014-06-19 08:15:08 UTC
(Fixed date)

Spec URL: http://smani.fedorapeople.org/review/tetgen.spec
SRPM URL: http://smani.fedorapeople.org/review/tetgen-1.5.0-2.fc21.src.rpm

%changelog
* Thu Jun 19 2014 Sandro Mani <manisandro> - 1.5.0-2
- Fix line endings of example.poly
- Fix libtet.so in main package
- Replace tetgen_cmake.patch with tetgen_build.patch

Comment 19 Dominik 'Rathann' Mierzejewski 2014-06-20 08:29:54 UTC
This looks much better, thanks.

One more thing. I see that a 2MB PDF manual file is available at
http://wias-berlin.de/software/tetgen/#Documentation

Don't you think it's a good idea to package that as a noarch -doc subpackage?

Comment 20 Sandro Mani 2014-06-20 08:37:44 UTC
Yes good idea!

Spec URL: http://smani.fedorapeople.org/review/tetgen.spec
SRPM URL: http://smani.fedorapeople.org/review/tetgen-1.5.0-3.fc21.src.rpm

%changelog
* Fri Jun 20 2014 Sandro Mani <manisandro> - 1.5.0-3
- Add doc subpackage

Comment 21 Christopher Meng 2014-06-20 08:43:26 UTC
My 2 cents:

%doc README example.poly LICENSE

Are you sure that example belongs to the main package?

Comment 22 Sandro Mani 2014-06-20 08:47:56 UTC
example.poly can indeed go into -doc, but I'd keep readme in the main package, given its contents.

Comment 23 Sandro Mani 2014-06-20 08:52:23 UTC
SPEC and SRPM updated (no release bump).

Comment 24 Dominik 'Rathann' Mierzejewski 2014-06-24 07:55:49 UTC
Two more nitpicks:

The -doc subpackage doesn't depend on the main package, so it could be installed without it and the LICENSE file would not be present then.

The -doc subpackage places its files in:
/usr/share/doc/tetgen-doc
this is not ideal and you could use _pkgdocdir macro to keep manual.pdf in /usr/share/doc/tetgen

For example, like this:

%install
...
cp -p %{SOURCE1} %{buildroot}%{_pkgdocdir}/

%files
...
%exclude %{_pkgdocdir}/manual.pdf


%files doc
%{_pkgdocdir}/manual.pdf

Comment 25 Michael Schwendt 2014-06-24 08:08:54 UTC
Documentation packages that pull in dependencies (just for a license file) are inconvenient. Please duplicate the license file instead of adding dependencies on base packages and lots of other stuff.

Comment 26 Sandro Mani 2014-06-24 08:38:58 UTC
Uhm, the -doc is not requiring the main package, and does ship a copy of the license file.

As for the pkgdocdir, /usr/share/doc/tetgen-doc would need to exist anyways for LICENSE, so I would end up having

%files
%doc README LICENSE
%dir %{_pkgdocdir}
%exclude %{_pkgdocdir}/manual.pdf
%exclude %{_pkgdocdir}/example.poly

%files doc
%doc LICENSE
%dir %{_pkgdocdir}
%{_pkgdocdir}/manual.pdf
%{_pkgdocdir}/example.poly


In my opinion, it is cleaner to just keep

%files
%doc README LICENSE

%files doc
%doc example.poly LICENSE manual.pdf

Comment 27 Dominik 'Rathann' Mierzejewski 2014-06-24 11:57:14 UTC
Well, the Guidelines don't mandate putting docs in %{_pkgdocdir} yet, so I won't consider this a review blocker.

You could expand the description of the -doc subpackage a bit (saying that it contains the manual and an example file), but that's not a blocker either.

Source checksums match:
----------------
http://www.tetgen.org/1.5/src/tetgen1.5.0.tar.gz :
  CHECKSUM(SHA256) this package     : 4d114861d5ef2063afd06ef38885ec46822e90e7b4ea38c864f76493451f9cf3
  CHECKSUM(SHA256) upstream package : 4d114861d5ef2063afd06ef38885ec46822e90e7b4ea38c864f76493451f9cf3
http://www.tetgen.org/1.5/doc/manual/manual.pdf :
  CHECKSUM(SHA256) this package     : ce71e755c33dc518b1a3bc376fb860c0659e7e14b18e4d9798edcbda05a24eca
  CHECKSUM(SHA256) upstream package : ce71e755c33dc518b1a3bc376fb860c0659e7e14b18e4d9798edcbda05a24eca
     
Package APPROVED.

Comment 28 Sandro Mani 2014-06-24 12:19:54 UTC
> Well, the Guidelines don't mandate putting docs in %{_pkgdocdir} yet, so I won't consider this a review blocker.

yet -> Do you know whether a corresponding discussion ongoing? Personally I'd also like to see the -doc situation cleaned up, since most of the time when packaged separately they should most likely either go into %{_datadir}/doc/%{name} or in %{_datadir}/doc/%{name}-devel. However, the issue that remains is what to do with the license file.

Anyhow, thanks for the review! Will adapt the -doc description when importing.

New Package SCM Request
=======================
Package Name: tetgen
Short Description: A tetrahedral mesh generator (
Owners: smani
Branches: f20 el6 epel7
InitialCC:

Comment 29 Sandro Mani 2014-06-24 12:20:33 UTC
(Copy-paste mistake fix...)

New Package SCM Request
=======================
Package Name: tetgen
Short Description: A tetrahedral mesh generator
Owners: smani
Branches: f20 el6 epel7
InitialCC:

Comment 30 Gwyn Ciesla 2014-06-24 12:47:47 UTC
Git done (by process-git-requests).

Comment 31 Fedora Update System 2014-06-24 15:07:15 UTC
tetgen-1.5.0-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/tetgen-1.5.0-3.fc20

Comment 32 Michael Schwendt 2014-06-24 21:42:42 UTC
Re: comment 28

Doubtful that there will ever be guidelines that suggest/mandate putting subpackage documentation into %_pkgdocdir. That would be a Herculean task for some packages [1], considering that you would have to %exclude lots of files and directories as not to duplicate files in multiple packages.

[1] Packages, which put many files directly into the top doc directory and/or which distinguish between user docs and developer docs.


Where all docs can be packaged in %_pkgdocdir without a lot effort, that's convenient for the package users, however, IMO.

Comment 33 Fedora Update System 2014-06-24 23:23:31 UTC
tetgen-1.5.0-3.fc20 has been pushed to the Fedora 20 testing repository.

Comment 34 Fedora Update System 2014-07-03 04:02:22 UTC
tetgen-1.5.0-3.fc20 has been pushed to the Fedora 20 stable repository.

Comment 35 Fedora Update System 2015-01-03 16:16:28 UTC
tetgen-1.5.0-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/tetgen-1.5.0-4.el6

Comment 36 Fedora Update System 2015-01-03 16:17:14 UTC
tetgen-1.5.0-4.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/tetgen-1.5.0-4.el7

Comment 37 Fedora Update System 2015-01-24 18:44:31 UTC
tetgen-1.5.0-4.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 38 Fedora Update System 2015-01-24 18:51:48 UTC
tetgen-1.5.0-4.el7 has been pushed to the Fedora EPEL 7 stable repository.