cloning is broken on Solaris

[jhpalmieri]

On Solaris, cloning is broken. This is because the command "ln" interprets its options differently on Solaris as compared to linux or OS X. Suppose OLD1 and OLD2 are existing directories. On linux or OS X:

$ ln -snf OLD-1 NEW
$ ls -l
...  NEW@ -> OLD1
...  OLD1
...  OLD2
$ ln -snf OLD2 NEW
$ ls -l
...  NEW@ -> OLD2
...  OLD1
...  OLD2

On Solaris:

$ ln -snf OLD-1 NEW
$ ls -l
...  NEW@ -> OLD1
...  OLD1
...  OLD2
$ ln -snf OLD2 NEW
$ ls -l
...  NEW@ -> OLD1
...  OLD1
...  OLD2

NEW still points to OLD1. The second command creates a link inside NEW (which means, inside OLD1), linking to OLD2.

This means that the line

    ln -snf "sage-$1" sage

in sage-build doesn't do what we want it to on Solaris: unlink 'sage' and then relink it to "sage-$1". I think we need to delete 'sage' first, so that's what the attached patch does.

[leif]

In that case, you could omit the -f from ln.

I would add a comment that the rm is necessary because of the different semantics on Solaris. I'm pretty sure we have further instances of such... ;-)

Btw, sage-build needs a lot of quoting..., and calls(!) sage-env rather than sourcing it. (In principle I think we could remove that, since it should have been called by sage-sage, right? Redirecting stdout to /dev/null shouldn't be necessary anyway.)

devel/sage/spkg-install is clean regarding ln -snf, but needs other work...

I'm not sure about SageNB; I made some changes there that afair never got into Sage because they then were fixed(?) in a different way (originally caused by a . in PYTHONPATH), don't remember.

[leif]

Replying to leif:

In that case, you could omit the -f from ln.

... or make it explicit by

    if [ "$UNAME" = SunOS ]; then
        rm -f "$SAGE_ROOT/devel/sage"
    fi
    ln -snf "sage-$1" sage

We should also check that SAGE_ROOT or SAGE_LOCAL is defined in sage-build, in which case we can be sure it was called from sage-sage, or at least sage-env was sourced.

[jhpalmieri]

Here is a new version of the patch. I've added a comment about rm being necessary. I didn't want to make Solaris a special case, because other systems like HP UX may have the same problem, and we might as well have code that works as widely as possible.

[jhpalmieri]

Other comments: I'm not touch sage-sage beyond what's required for this ticket, regardless of the state of the rest of that file.

Any issues with devel/sage/spkg-install should be dealt with elsewhere.

Finally, this intentionally misses one instance of quoting at the end of sage-build, since the quotes were added by the patch at #10303.

[leif]

Replying to jhpalmieri:

Other comments: I'm not touch sage-sage beyond what's required for this ticket, regardless of the state of the rest of that file.

Well, you did more than would have been necessary to fix the ticket's issue. :)

Patch in principle looks good to me; just one question: Is the "blank" echo line at the beginning intentional?

(Dave prefers [ -z "$SAGE_LOCAL" ], i.e. not comparing against empty strings. I don't mind, though I also prefer -z and -n.)

The comment(s) regarding ln -snf is a bit misleading, since we never want to delete (or overwrite) the target, which is sage-<branch>, but the link to it, sage.

Any issues with devel/sage/spkg-install should be dealt with elsewhere.

I just mentioned it because it does similar (but is "Solaris-safe" in that way).

[leif]

The "sage-clone" in the commit message should read "sage -clone" (which refers to sage-sage's option; sage-clone is a Python script).

Applies cleanly on Sage 4.6.1.alpha2, positive review.

Feel free to change the comment etc. if you like, I'll then review again.

[jhpalmieri]

Replying to leif:

Replying to jhpalmieri:

Other comments: I'm not touch sage-sage beyond what's required for this ticket, regardless of the state of the rest of that file.

Well, you did more than would have been necessary to fix the ticket's issue. :)

Patch in principle looks good to me; just one question: Is the "blank" echo line at the beginning intentional?

Not really, I just copied the whole block of code from somewhere else, although if there is a problem like this with the script, having a blank line before the error message can make it stand out more, especially if it comes after many other lines of output.

(Dave prefers [ -z "$SAGE_LOCAL" ], i.e. not comparing against empty strings. I don't mind, though I also prefer -z and -n.)

I can change it to -z.

The comment(s) regarding ln -snf is a bit misleading, since we never want to delete (or overwrite) the target, which is sage-<branch>, but the link to it, sage.

Oh, I see, on some platforms (e.g. OS X and Solaris), the man page for ln refers to the link as the "target" and the existing file being linked as the "source", while on other platforms (e.g. at least some linux platforms), the existing file is called the "target" and the link is called "LINK NAME". Since I've been looking at the Solaris man page when dealing with this, that terminology must have stuck in my head. So the problem is that the terminology in the man pages wildly inconsistent, and so the word "target" should be avoided completely.

Any issues with devel/sage/spkg-install should be dealt with elsewhere.

I just mentioned it because it does similar (but is "Solaris-safe" in that way).

Right. I'd already searched through the rest of local/bin/sage-*, and also the scripts in spkg for other instances of "ln".

---

If you have time to review it again, I would appreciate it.

[jhpalmieri]

scripts repo

[leif]

Replying to jhpalmieri:

Replying to leif:

Patch in principle looks good to me; just one question: Is the "blank" echo line at the beginning intentional?

Not really, I just copied the whole block of code from somewhere else, although if there is a problem like this with the script, having a blank line before the error message can make it stand out more, especially if it comes after many other lines of output.

In that case, I would have also prepended "Error: ", and perhaps redirected the output to stderr (>&2). ;-)

(The "standard" phrase Maybe run sage -sh? is missing, but I appreciate that. :) )

The comment(s) regarding ln -snf is a bit misleading, since we never want to delete (or overwrite) the target, which is sage-<branch>, but the link to it, sage.

Oh, I see, on some platforms (e.g. OS X and Solaris), the man page for ln refers to the link as the "target" and the existing file being linked as the "source", while on other platforms (e.g. at least some linux platforms), the existing file is called the "target" and the link is called "LINK NAME". Since I've been looking at the Solaris man page when dealing with this, that terminology must have stuck in my head. So the problem is that the terminology in the man pages wildly inconsistent, and so the word "target" should be avoided completely.

POSIX talks about source file(s), a target file or target directory (like also GNU does), and destination paths.

(In addition, "unlinking" was ambiguous as in the context of UNIX, "unlink" is used synonymously for "remove", since that actually refers to directory entries, not the files themselves; a file only gets deleted if there are no more directory entries referring to it, i.e. its link count reaches zero.)

The current formulation should be clear to everyone, though here both arguments are [symbolic links to] directories, where (more) ambiguity occurs.