DEV: Raise errors when cleaning the download cache, and fix for macOS (#8319)

DEV: Raise errors when cleaning the download cache, and fix for macOS (#8319)

POSIX’s head specification states: “The application shall ensure that the number option-argument is a positive decimal integer”

Negative values are supported on GNU head, so this works in the discourse docker image. However, in some environments (e.g. macOS), the system head version fails with a negative n parameter.

This commit does two things:

Checks the status at each stage of the pipe, so it cannot fail silently Flip the ls command to list in descending time order, and use tail -n +501 instead of head -n -500.

The visible result is that macOS users no longer see head: illegal line count – -500 printed throughout the test suite.

diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb
index 6e8c53442d..5e455ac1b5 100644
--- a/lib/file_store/base_store.rb
+++ b/lib/file_store/base_store.rb
@@ -139,7 +139,13 @@ module FileStore
       FileUtils.mkdir_p(dir) unless Dir.exist?(dir)
       FileUtils.cp(file.path, path)
       # keep latest 500 files
-      `ls -tr #{CACHE_DIR} | head -n -#{CACHE_MAXIMUM_SIZE} | awk '$0="#{CACHE_DIR}"$0' | xargs rm -f`
+      processes = Open3.pipeline(
+        "ls -t #{CACHE_DIR}",
+        "tail -n +#{CACHE_MAXIMUM_SIZE + 1}",
+        "awk '$0=\"#{CACHE_DIR}\"$0'",
+        "xargs rm -f"
+      )
+      raise "Error clearing old cache" if !processes.all?(&:success?)
     end
 
     private

GitHub sha: 1998be3b

2 Likes

:heart_eyes: This is awesome! Those illegal line count – -500 messages have been an eyesore forever!

2 Likes

Yeah, I don’t know why we put up with it for so long! This uninterrupted grid makes me very happy! :heart_eyes_cat:

3 Likes