This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 483663 - Review Request: tetgen - A tetrahedral mesh generator
Review Request: tetgen - A tetrahedral mesh generator
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dominik 'Rathann' Mierzejewski
Fedora Extras Quality Assurance
: Reopened
: tetgen (view as bug list)
Depends On:
Blocks: FE-SCITECH vmtk 1111388
  Show dependency treegraph
 
Reported: 2009-02-02 15:56 EST by Fabian Affolter
Modified: 2015-01-24 13:51 EST (History)
8 users (show)

See Also:
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 00:02:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
dominik: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Fabian Affolter 2009-02-02 15:56:54 EST
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-02 19:57:00 EST
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 13:21:38 EST
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 14:08:47 EST
Someone should let berlios know that this software should not be hosted there.
Comment 4 Fabian Affolter 2011-07-01 12:12:38 EDT
*** Bug 714336 has been marked as a duplicate of this bug. ***
Comment 5 Kevin Kofler 2011-07-01 18:43:38 EDT
> 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-01 23:42:03 EDT
Mailed them.
Comment 7 Tom "spot" Callaway 2011-07-05 10:19:17 EDT
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 12:54:30 EST
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 18:34:29 EST
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 03:40:24 EST
I sent a message to upstream. They placed the source tarball behind a registration form.
Comment 11 Kevin Kofler 2014-02-26 09:31:48 EST
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-13 21:00:12 EDT
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-13 21:09:18 EDT
I can review this unless rathann (or someone else) will take care of it ....
Comment 14 Sandro Mani 2014-06-17 08:57:55 EDT
@Fabian: Mind if I take over?
Comment 15 Dominik 'Rathann' Mierzejewski 2014-06-17 09:33:34 EDT
Taking review.
Comment 16 Dominik 'Rathann' Mierzejewski 2014-06-18 17:35:15 EDT
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 03:00:10 EDT
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@gmail.com> - 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 04:15:08 EDT
(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@gmail.com> - 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 04:29:54 EDT
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 04:37:44 EDT
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@gmail.com> - 1.5.0-3
- Add doc subpackage
Comment 21 Christopher Meng 2014-06-20 04:43:26 EDT
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 04:47:56 EDT
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 04:52:23 EDT
SPEC and SRPM updated (no release bump).
Comment 24 Dominik 'Rathann' Mierzejewski 2014-06-24 03:55:49 EDT
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 04:08:54 EDT
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 04:38:58 EDT
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 07:57:14 EDT
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 08:19:54 EDT
> 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 08:20:33 EDT
(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 Jon Ciesla 2014-06-24 08:47:47 EDT
Git done (by process-git-requests).
Comment 31 Fedora Update System 2014-06-24 11:07:15 EDT
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 17:42:42 EDT
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 19:23:31 EDT
tetgen-1.5.0-3.fc20 has been pushed to the Fedora 20 testing repository.
Comment 34 Fedora Update System 2014-07-03 00:02:22 EDT
tetgen-1.5.0-3.fc20 has been pushed to the Fedora 20 stable repository.
Comment 35 Fedora Update System 2015-01-03 11:16:28 EST
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 11:17:14 EST
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 13:44:31 EST
tetgen-1.5.0-4.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 38 Fedora Update System 2015-01-24 13:51:48 EST
tetgen-1.5.0-4.el7 has been pushed to the Fedora EPEL 7 stable repository.

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