Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: sconsUtils
-
Labels:
-
Team:Architecture
Description
If you run "scons -c" in a directory tree with a directory named "core", it will delete that directory. Found that out when inside "daf_butler", which has directory named python/lsst/daf/butler/core, and it deleted the directory and the code below it.
Attachments
Activity
| Field | Original Value | New Value |
|---|---|---|
| Risk Score | 0 |
| Team | Architecture [ 10304 ] |
| Priority | Undefined [ 10000 ] | Critical [ 2 ] |
| Watchers | John Parejko, Steve Pietrowicz, Tim Jenness [ John Parejko, Steve Pietrowicz, Tim Jenness ] | Jim Bosch, John Parejko, Steve Pietrowicz, Tim Jenness [ Jim Bosch, John Parejko, Steve Pietrowicz, Tim Jenness ] |
| Issue Type | Story [ 10001 ] | Bug [ 1 ] |
| Labels | SciencePipelines |
| Watchers | Jim Bosch, John Parejko, Steve Pietrowicz, Tim Jenness [ Jim Bosch, John Parejko, Steve Pietrowicz, Tim Jenness ] | Jim Bosch, John Parejko, Kian-Tat Lim, Steve Pietrowicz, Tim Jenness [ Jim Bosch, John Parejko, Kian-Tat Lim, Steve Pietrowicz, Tim Jenness ] |
On Slack the feeling was that at the very least cleaning of files should not be using rm -rf and that -type f and -type d should be used to make clear that directory and file confusion could never happen.
It's not elegant, but I think it gets the job done. Tested against a C++ package (geom) and the original problem package (daf_butler).
| Reviewers | Tim Jenness [ tjenness ] | |
| Status | To Do [ 10001 ] | In Review [ 10004 ] |
| Assignee | Kian-Tat Lim [ ktl ] |
Thanks. Looks good.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
Merged in 914fe58. I don't think Jenkins ever uses scons -c, so I took a chance and didn't run it.
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
It looks to me like the cleanExt option in sconsUtils is only ever None (I don't see it overridden anywhere), which means we only get the default file list in scripts.py:114. Given this, we could probably put a little logic about "core" in builders.CleanTree() (which is the only thing that takes cleanExt) to make the recurse option a bit smarter.
I may have been the cause of this with
DM-14124. Which suggests a possibly simple solution: two separate runs of CleanTree(), the first with the "standard" list of things to remove and without -prune, the second with any known directories (e.g. .cache) and with -prune.