sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 462 forks source link

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

Closed e14f4152-4982-4ace-8c95-73a0599b109b closed 14 years ago

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago

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

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago

Attachment: trac_9444-spkg_build_dir_t2.patch.gz

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

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago

Author: Mitesh Patel

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago
comment:1

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

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
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 commented 14 years ago
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 commented 14 years ago
comment:4

mpatel: I assume this is ready for review?

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 14 years ago
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

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 14 years ago

Reviewer: Leif Leonhardy

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
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

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 14 years ago
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).

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 14 years ago

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

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago
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 commented 14 years ago
comment:9

No, everything has worked fine.

jhpalmieri commented 14 years ago
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.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
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

e14f4152-4982-4ace-8c95-73a0599b109b commented 14 years ago

Merged: sage-4.5.3.alpha0