new-style Nauty package

[nathanncohen]

New style spkg for Nauty. The code has been updated to the latest upstream version available.

Renamed upstream tarball
http://www.lmona.de/files/sage/nauty-25r9.tar.gz

CC: @sagetrac-tmonteil @videlec @dcoudert @sagetrac-azi @sagetrac-borassi

Component: packages: optional

Author: François Bissey, Thierry Monteil

Branch/Commit: 42404ed

Reviewer: Thierry Monteil, Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/18425

[nathanncohen]

Branch: u/ncohen/18425

[nathanncohen]

Commit: c2d4c89

[nathanncohen]

New commits:

c2d4c89

trac #18425: new-style Nauty package

[sagetrac-tmonteil]

comment:2

I would prefer us to ship the unmodified upstream tarball which can be found at http://cs.anu.edu.au/~bdm/nauty/nauty25r9.tar.gz since the gain in the recompression is less than 100K, see for example #17529. I will upload a patch while ptestlong is finishing.

[sagetrac-tmonteil]

Changed branch from u/ncohen/18425 to u/tmonteil/18425

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

247b6a6	#18425 : cd into the correct source directory.
e23db00	#18425 tag doctests involving nauty.

[sagetrac-git]

Changed commit from c2d4c89 to e23db00

[sagetrac-tmonteil]

Reviewer: Thierry Monteil

[sagetrac-tmonteil]

comment:5

I modified the install script to allow building with an unmodified tarball. Also, the # optional - nauty flag should be applied on a whole doctest, to allow sage -t --optional=nauty work (my computer is currently very slow hence i can not run with sage -t --optional=sage,nauty right now, only test nauty-specific doctests).

Tell if it is OK like that.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

57c9922

#18425 : remove old gtools-h.in patch.

[sagetrac-git]

Changed commit from e23db00 to 57c9922

[sagetrac-tmonteil]

comment:7

According to the mercurial history of nauty-25.spkg, the "patch" (actually a file to overwrite the existing one) gtools-h.in was introduced in 2010 for version 2.4 of nauty in order "to compile on Ubuntu and other systems". Inbetween, the file gtools-h.in has evolved, so keeping it does not let us benefit from the evolution, hence i propose to simply remove it and test to see if some problem appears on some machines.

Note that nauty builds and passes tests without the patch.

[sagetrac-git]

Changed commit from 57c9922 to 34a829c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

34a829c

#18425 : add a self-check file since some self-tests are available.

[nathanncohen]

Changed branch from u/tmonteil/18425 to u/ncohen/18425

[nathanncohen]

Changed commit from 34a829c to c2d4c89

[nathanncohen]

comment:9

I disagree with the change you made to keep the original tarball. I initially did the same, then decided against it when I noticed the directory name problem that you avoided with:

+NAUTY_VERSION="$(cat ${SAGE_ROOT}/build/pkgs/nauty/package-version.txt)"
+SRC="nauty${NAUTY_VERSION}"

I do not think that it is a good idea to do this, as we could very well imagine that other scripts be written that apply to the packages, and those would expect a correctly named package.

Furthermore, I consider it very bad manners to go on somebody else's ticket, change the branch name and change the commits without even getting the ticket's author's agreement. Please do not do this again.

I set the ticket to its original content. If you have requests to make as a reviewer, please do.

Nathann

[sagetrac-tmonteil]

comment:10

Replying to @nathanncohen:

Furthermore, I consider it very bad manners to go on somebody else's ticket, change the branch name and change the commits without even getting the ticket's author's agreement.

What does "Tell if it is OK like that." mean ?

I don't understand your point at all. I did not remove your branch, i just propose some modifications for discussion. I guess in that case it is easier to propose changes that people could download to test (e.g. does nauty still work without the old patch), than asking the author of the previous commits to remove the patch themselves so that i can download their changes to see what happens. Same with requesting a spkg-check, then have to inspect it to filnally request some output parsing because it may fail silently if coded as the other similar files.

I understand that trac Branch field is not well designed since it does not allow to show various branches, but i do not see what can i do else than git trac push to show the proposed modifications on the ticket.

More generally, i do not understand why the people that opens a ticket owns it, Sage is a common goods that people try to improve collectively, not a staked territory. If you want to own a ticket, please write it explicitely by fillind the related field, i will not bother you on those.

[nathanncohen]

comment:11

I don't understand your point at all.

If you want to propose a commit as a reviewer, please push them on a new branch. Add a comment to tell the other guy what you did and where to find the branch, and that he can add them to the ticket's branch if he agrees.

More generally, i do not understand why the people that opens a ticket owns it,

Just picture several guys talking around a table. Guy '1' has an idea, and begins to explain it. Midway, you tell him to shut up and you explain to the others what you think that his idea is. That's how I see it.

If you want to own a ticket, please write it explicitely by fillind the related field, i will not bother you on those.

I did. The branch is named u/ncohen/18425, to which you have no write access. When I create branches to which anybody is welcome to contribute (e.g. reorganization of the doc, which cover all domains at once), I use a branch named public/something.

Nathann

[kiwifb]

comment:12

Can someone explain to me why all the installed programs have been prefixed with nauty-. What's the big idea? Does it clash with something? If you cannot justify it, I will remove it and patch every place that call a nauty-* program (and I already know where those are) as a review.

[nathanncohen]

comment:13

probably because the executables' names are not very meaningful if you don't know that they are nauty routines? I don't see any objection to the removal of that 'nauty-' in front of them, but then they should probably be in a nauty subfolder.

Nathann

[kiwifb]

comment:14

Very little is meaningful if you don't know what it is in advance on a unix system. I don't see why nauty routines should have a special treatment.

[nathanncohen]

comment:15

Very little is meaningful if you don't know what it is in advance on a unix system. I don't see why nauty routines should have a special treatment.

Sooo... your goal is to lose information for the sake of consistency?

[kiwifb]

comment:16

There are two points:

1. if you would like the shipped binaries to be prefixed like other nice software like sage, complain upstream
2. if the information you are worried about is the content of the nauty package, its binaries in particular, that's a defect of the sage packaging system that it doesn't have a database of the files installed by its various packages. This is a more advanced feature that has always been out of scope.

And about consistency, it is actually a good thing. Suppose that someone has some program or script that uses nauty and wants to use them in sage. I think they will be surprised when their scripts/programs don't work out of the box after installing sage's nauty package. It may actually take them time to figure out that they have to prefix everything.

Adding a prefix specifically in sage goes against the principle of least surprise because you modify what is expected from upstream. The only case where it is justified is a clash in name (in which case it is proper to involve both upstreams if possible).

[kiwifb]

comment:17

And unfortunately for my last argument because nauty has been in sage prefixed for so long, we may have old user expecting it prefixed. So now we need to have both prefixed and non prefixed binaries. At the minimum the prefixed one should link to the non-prefixed ones. Better would be to have the prefixed one link to a wrapper that would execute the appropriate binary and output a deprecation message. But we shouldn't use prefixed version in sage in the future.

[jdemeyer]

comment:39

And in spkg-check, this can be removed:

echo "Tests suite for nauty passed without errors."
exit 0

[jdemeyer]

comment:40

For installing, I would use cp -p instead of plain cp.

[kiwifb]

comment:41

@sagetrac-tmonteil I did some copying to rebase stuff on top of 6.7 quick and dirty and I completely missed that you touched other files. I am not quite sure how to see the details of that commit from the command line.

@jdemeyer I'll have a quick look to see if I can add an install target, that would care of the permissions.

[jdemeyer]

comment:42

That's all my comments for now. If you make the edits, I will do a final review.

And I don't care much about renaming/repacking/changing the tarball, just document what you did.

[jdemeyer]

comment:43

Replying to @kiwifb:

@jdemeyer I'll have a quick look to see if I can add an install target, that would care of the permissions.

I don't think that's needed, just copying with cp -p in spkg-install should work.

[kiwifb]

comment:44

And should I do something to put it in conformance with #18431?

[jdemeyer]

comment:45

Replying to @kiwifb:

And should I do something to put it in conformance with #18431?

Ah yes, I think you need to add a file build/pkgs/nauty/type containing optional (although this might not be strictly required)

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

0d4e420

Incorporated all of Jeroen's comment. Reverted to a renamed upstream tarball and documented packaging.

[sagetrac-git]

Changed commit from e8ead70 to 0d4e420

[jdemeyer]

comment:48

Replace the arcane cd $SRC stuff by a much simpler cd nauty*.

And $SAGE_LOCAL -> "$SAGE_LOCAL" in shell scripts.

[jdemeyer]

comment:49

And obviously, this comment needs updating:

# Unfortunately, runalltests script does not exit with a nonzero status if some
# tests failed, so we parse the last line of the log to decide whether all
# tests passed or not, in order to exit with the correct status.

[jdemeyer]

Changed reviewer from Thierry Monteil to Thierry Monteil, Jeroen Demeyer

[sagetrac-git]

Changed commit from 0d4e420 to 032648d

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

032648d

Further simplification and sliming

[jdemeyer]

comment:52

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1. New commits:

032648d

Further simplification and sliming

sliming???

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

42404ed

Put some comments back in spkg-check

[sagetrac-git]

Changed commit from 032648d to 42404ed

[kiwifb]

comment:54

I'd say that was too much sliming :) - OK I'll remember that should be slimming for next time (may be, I have always been terrible at spelling).

I didn't know you could do the cd nauty* thing.

---

New commits:

42404ed

Put some comments back in spkg-check

[vbraun]

Changed branch from u/fbissey/trac18425 to 42404ed