www: create script to free space by removing old builds #3199
Conversation
Signed-off-by: Stewart X Addison <sxa@redhat.com>
|
I think a regular cron job would likely be better versus adding something that might affect/delay releases. |
|
Most releasers don't have the permissions to change old files anyway. |
|
@mhdawson @targos (and @richardlau just because he's been involved) are you ok with the logic in the script? I don't want to make it live with automation until it's been eyeballed (approved) by others for potential issues. |
|
I'm sorry I'm not familiar enough with shell scripts to review this with confidence. |
tools/wwwspace/prune.sh
Outdated
| [ $# -lt 2 ] && echo Usage: $0 '[nightly|v8-canary]' '[show|delete]' && exit 1 | ||
| PRODUCT=$1 | ||
| [ "$1" != "nightly" -a "$1" != "v8-canary" ] && echo Parameter mist be one of: nightly v8-canary && exit 1 | ||
| if [ "$2" = "show" ]; then |
There was a problem hiding this comment.
👍 nice feature, although this would normally be "dry-run" in most commands that do this
There was a problem hiding this comment.
Was very useful when I was being paranoid during testing ;-) Yep happy to rename it
|
|
||
| # Versions from the last two calendar years except last two months | ||
| # Keep every date number ending in 1 (So 01 11 21 31) | ||
| SELECTED=`ls -1d v${VERSION}.* | egrep "${PRODUCT}$THISYEAR|${PRODUCT}$LASTYEAR" | egrep -v "${PRODUCT}${THISMONTH}|${PRODUCT}${LASTMONTH}" | grep -v ${PRODUCT}20.....1 | grep -v "${LATEST}"` |
There was a problem hiding this comment.
these regexes are fairly lose, is it worth being specific with ^ and/or $ to bound them properly?
There was a problem hiding this comment.
The issue is that with names like v20.0.0-nightly20230222a3211e18b4 it means regexping the whole thing which I'd suggest makes it more complex than it needs to be. $PRODUCT should be an adequate start position (I could use ^v${VERSION}.* at the start and [0-9a-e]$ at the end though I guess although I'm not sure we'll hit a situation where that would make much of a difference.
Maybe if someone renamed a file to have a -KEEPME suffix or similar that they wanted to retain?
There was a problem hiding this comment.
Functionally this seems fine to me, my critique is mostly just personal preference; so that that as you want. I would have leaned on:
- Heavier use of quoting
- No backticks,
$()only - More use of
findwith its ability to do date stuff; but what you've got here seems perfectly reasonable since we're doing nice date things in file/dir names.
|
I'm not sure if some the Shellcheck rules will catch those https://github.com/nodejs/build/blob/main/.shellcheckrc. Never took another pass at cleaning any more of them up |
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
|
Verified that this is still behaving as it should after all the changes from the comments. I feel it's a bit safer to have a script like this manually updated, but the cron job could pull it down directly from the nodejs/build repository. I suspect it won't need to be updated too often though. It's currently in ~dist but owned by |
It looks like this is passing the github shellcheck check - I assume that means it's in line with that configuration? |
|
@sxa I would assume that. |
|
Merging since there have been no objections |
Script as described in #3125
This is the script which will clear up the nightlies and v8-canary directories so they do not constantly increase the disk space. If we are happy with this logic we can determine how to run it - either