811d6fef1fb93efa979d039f4effb7ebe7d98780
braney
  Wed Sep 2 10:21:29 2026 -0700
trackDbCacheCleaner: parse -n with getopts, and allow a cache at the top level, refs #37551 #38211

Code review asked why the option parsing compared $1 to "-n" by hand instead
of using getopts, and pointed out that the path depth rule refused a cache
directory a mirror put at the top level, such as /mirrorTrash.

Use getopts for -n, and move the usage text into a function so that an unknown
option prints it too.

Drop the depth rule.  It also did not do what its comment claimed, since
/dev/shm is two levels deep and passed it.  Refuse only the root directory,
after stripping any trailing slash so that "/" and "//" are both caught.  The
check on name.txt below it is what decides whether a directory really is a
trackDb cache.

diff --git src/product/scripts/trackDbCacheCleaner.sh src/product/scripts/trackDbCacheCleaner.sh
index c6780bccbdd..773447d7617 100755
--- src/product/scripts/trackDbCacheCleaner.sh
+++ src/product/scripts/trackDbCacheCleaner.sh
@@ -31,61 +31,76 @@
 # system mounted noatime the access time never advances, and taking the
 # newer of the two falls back to the write time, which only ever keeps
 # entries longer.
 #
 # Removing a cache file while a CGI is using it is safe.  An existing mmap
 # survives the unlink.  A CGI that loses the race finds the open fails and
 # builds trackDb from the database instead, so the worst case is one slow
 # request.
 #
 # For a weekly cron, for example:
 #   0 5 * * 0 /usr/local/apache/product/scripts/trackDbCacheCleaner.sh /data/trackDbCache
 
 # exit on any error at any time
 set -beEu -o pipefail
 
-export dryRun=0
-if [ "${1-}" = "-n" ]; then
-  dryRun=1
-  shift
-fi
-
-if [ $# -lt 1 ] || [ $# -gt 2 ]; then
+usage() {
   echo "usage: trackDbCacheCleaner.sh [-n] <cacheDir> [expireDays]" 1>&2
   echo "  expire trackDb cache directories unread for expireDays days" 1>&2
+  echo "  -n          list what would be removed, remove nothing" 1>&2
   echo "  cacheDir is the cacheTrackDbDir setting from hg.conf" 1>&2
   echo "  expireDays defaults to 30" 1>&2
   exit 255
+}
+
+export dryRun=0
+while getopts ":n" opt
+do
+  case "${opt}" in
+    n)  dryRun=1 ;;
+    \?) echo "ERROR: unknown option '-${OPTARG}'" 1>&2
+        usage ;;
+  esac
+done
+shift $((OPTIND - 1))
+
+if [ $# -lt 1 ] || [ $# -gt 2 ]; then
+  usage
 fi
 
 export cacheDir="${1}"
 export expireDays="${2-30}"
 
 ##########################################################################
 # refuse anything that does not look like a trackDb cache, since this
 # script removes directory trees
 
 case "${cacheDir}" in
   /*) ;;
   *)  echo "ERROR: cacheDir must be an absolute path: '${cacheDir}'" 1>&2
       exit 255 ;;
 esac
 
-# /data and /dev/shm are the parents of a cache directory, never the cache
-export pathDepth=`echo "${cacheDir}" | awk -F/ '{n=0;for(i=1;i<=NF;i++)if(length($i))n++;print n}'`
-if [ "${pathDepth}" -lt 2 ]; then
-  echo "ERROR: refusing to clean the top level directory '${cacheDir}'" 1>&2
+# drop any trailing slash, so that the root check below cannot be dodged by
+# writing it as '/' or '//'
+cacheDir=`echo "${cacheDir}" | sed -e 's#/*$##'`
+
+# the root directory is never a trackDb cache.  Every other absolute path can
+# be, including one a mirror made at the top level, so the check on name.txt
+# below is what decides the rest.
+if [ -z "${cacheDir}" ]; then
+  echo "ERROR: refusing to clean the root directory" 1>&2
   exit 255
 fi
 
 if [ ! -d "${cacheDir}" ]; then
   echo "ERROR: no such directory '${cacheDir}'" 1>&2
   exit 255
 fi
 
 if ! echo "${expireDays}" | grep -q -E '^[1-9][0-9]*$'; then
   echo "ERROR: expireDays must be a positive integer, not '${expireDays}'" 1>&2
   exit 255
 fi
 
 # every trackDb cache directory holds a name.txt.  If there are
 # subdirectories but not one name.txt among them, this is some other