Fix "rm: Cannot remove any directory in the path of the current working directory" on t2

[qed777]

In /rootpool2/local/kirkby/sage-4.5.alpha1 on t2:

$ tail spkg/logs/rubiks-20070912.p11.log
real    2m30.575s
user    2m20.699s
sys     0m5.083s
Successfully installed rubiks-20070912.p11
Now cleaning up tmp files.
rm: Cannot remove any directory in the path of the current working directory
/rootpool2/local/kirkby/sage-4.5.alpha1/spkg/build/rubiks-20070912.p11
Making Sage/Python scripts relocatable...
Making script relocatable
Finished installing rubiks-20070912.p11.spkg

This leaves an empty directory SAGE_ROOT/spkg/build/rubiks-20070912.p11.

It seems the problem is

rm -rf "$SAGE_PACKAGES/build/$PKG_NAME"

near the end of SAGE_LOCAL/bin/sage-spkg. What if we precede this with cd .., say?

CC: @sagetrac-drkirkby

Component: porting: Solaris

Author: Mitesh Patel

Reviewer: David Kirkby, John Palmieri, Leif Leonhardy

Merged: sage-4.5.3.alpha0

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

[qed777]

Attachment: trac_9444-spkg_build_dir_t2.patch.gz

Step out of spkg/build/foo-x.y.z before deleting it.

[qed777]

Author: Mitesh Patel

[qed777]

comment:1

I've attached a trial patch that uses cd "$SAGE_PACKAGES/build/".

[sagetrac-drkirkby]

comment:2

This looks good and I'm confident it would fix this rather annoying bug.

My only concern is whether fixing this bug will cause a problem for any code that might have relied on the old behavior. That will need testing.

Dave

[jhpalmieri]

comment:3

I think that this message only appears on Solaris; at least, that's been my experience. On sage.math, for example, I can do this:

$ cd /scratch/palmieri
$ mkdir TEMP
$ cd TEMP
$ rm -rf /scratch/palmieri/TEMP

but doing this on t2.math results in an error:

rm: Cannot remove any directory in the path of the current working directory

So if it ever relied on this behavior, it didn't rely on it on sage.math. (All of the linux machines I've used, and also Mac OS X, behave like sage.math in this regard.)

I'm trying a build on sage.math with this patch, and I'll try it on t2 later.

[jhpalmieri]

comment:4

mpatel: I assume this is ready for review?

[nexttime]

comment:5

I'm ok with the patch, though we could replicate

   # Make triply sure that we are in the build directory before doing 
    # a scary "rm -rf".
    cd "$SAGE_PACKAGES/build"
    if [ $? -ne 0 ]; then
        echo "Unable to find build directory."
    else
        rm -rf "$PKG_BASE-"*
    fi

which is what is done some lines above.

There are many other things to fix or improve in sage-spkg, but I'll leave those for further tickets (something like work in progress) since hopefully this one gets merged soon.

If anyone feels Mitesh's solution is not sufficient, feel free to revert it to "needs review" or "needs work".

-Leif

[nexttime]

Reviewer: Leif Leonhardy

[sagetrac-drkirkby]

comment:6

I'm happy too. My initial concern was that the directory was not getting deleted on any operating system, in which case deleting it could be dangerous. But now I realise it gets deleted on Linux, then clearly there is no risk.

I'm 100% happy with this.

Dave

[nexttime]

comment:7

Just for the record: Besides other things, I'd like to have something like $SAGE_SPKG_KEEP_SRC (or SAGE_KEEP_BUILT_SPKGS) for developers, because -s is not available for whole builds (with make).

[nexttime]

Changed reviewer from Leif Leonhardy to David Kirkby, John Palmieri, Leif Leonhardy

[qed777]

comment:8

Replying to @jhpalmieri:

mpatel: I assume this is ready for review?

Yes. I didn't change the status field because I had tested the patch only with sage -f on sage.math and t2.

Were there any problems with your builds?

[jhpalmieri]

comment:9

No, everything has worked fine.

[jhpalmieri]

comment:10

Replying to @nexttime:

Just for the record: Besides other things, I'd like to have something like $SAGE_SPKG_KEEP_SRC (or SAGE_KEEP_BUILT_SPKGS) for developers, because -s is not available for whole builds (with make).

See #4949.

[sagetrac-drkirkby]

comment:11

Why is the milestone for this set to 5.0? Do we really need to wait that long to fix this bug, given it already has positive review and is one of the simplest possible bug fixes.

Dave

[qed777]

Merged: sage-4.5.3.alpha0