Opened 10 years ago
Closed 10 years ago
#9444 closed defect (fixed)
Fix "rm: Cannot remove any directory in the path of the current working directory" on t2
Reported by: | mpatel | Owned by: | drkirkby |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.5.3 |
Component: | porting: Solaris | Keywords: | |
Cc: | drkirkby | Merged in: | sage-4.5.3.alpha0 |
Authors: | Mitesh Patel | Reviewers: | David Kirkby, John Palmieri, Leif Leonhardy |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
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?
Attachments (1)
Change History (13)
Changed 10 years ago by
comment:2 Changed 10 years ago by
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
comment:3 Changed 10 years ago by
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.
comment:4 follow-up: ↓ 8 Changed 10 years ago by
- Status changed from new to needs_review
mpatel: I assume this is ready for review?
comment:5 Changed 10 years ago by
- Reviewers set to Leif Leonhardy
- Status changed from needs_review to positive_review
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
comment:6 Changed 10 years ago by
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
comment:7 follow-up: ↓ 10 Changed 10 years ago by
- Reviewers changed from Leif Leonhardy to David Kirkby, John Palmieri, Leif Leonhardy
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
).
comment:8 in reply to: ↑ 4 Changed 10 years ago by
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?
comment:9 Changed 10 years ago by
No, everything has worked fine.
comment:10 in reply to: ↑ 7 Changed 10 years ago by
comment:11 Changed 10 years ago by
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
comment:12 Changed 10 years ago by
- Merged in set to sage-4.5.3.alpha0
- Milestone changed from sage-5.0 to sage-4.5.3
- Resolution set to fixed
- Status changed from positive_review to closed
Step out of spkg/build/foo-x.y.z before deleting it.