Remove cdash_unlink() helper (#2650)

The `cdash_unlink()` helper function is only used in a handful of locations, and most of those locations do not appear to benefit from its behavior. This PR removes the `cdash_unlink()` helper and replaces it with either the raw PHP `unlink()` command or Laravel's Storage facade's delete helper. In the future, I plan to create a dedicated Laravel temporary disk which will allow us to use the storage facade for all file I/O operations.
Kitware · Jan 6, 2025 · 4d3f796 · 4d3f796
1 parent 3e11a51
commit 4d3f796
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 35 deletions.
diff --git a/app/cdash/include/dailyupdates.php b/app/cdash/include/dailyupdates.php
@@ -1114,9 +1114,8 @@ function addDailyChanges(int $projectid): void
             $files = Storage::allFiles($dir_to_clean);
             foreach ($files as $filename) {
                 $filepath = Storage::path($filename);
-                if (file_exists($filepath) && is_file($filepath) &&
-                        time() - filemtime($filepath) > $timeframe * 3600) {
-                    cdash_unlink($filepath);
+                if (file_exists($filepath) && is_file($filepath) && time() - filemtime($filepath) > $timeframe * 3600) {
+                    Storage::delete($filepath);
                 }
             }
         }

diff --git a/app/cdash/include/log.php b/app/cdash/include/log.php
@@ -17,19 +17,6 @@
 use Illuminate\Support\Facades\Log;
 use \Psr\Log\LogLevel;
 
-if (!function_exists('cdash_unlink')) {
-    function cdash_unlink($filename)
-    {
-        unlink($filename);
-
-        if (file_exists($filename)) {
-            throw new Exception("file still exists after unlink: $filename");
-        }
-
-        return true;
-    }
-}
-
 if (!function_exists('to_psr3_level')) {
     function to_psr3_level($type)
     {

diff --git a/app/cdash/xml_handlers/upload_handler.php b/app/cdash/xml_handlers/upload_handler.php
@@ -185,8 +185,7 @@ public function endElement($parser, $name): void
             unset($whandle);
 
             // Delete base64 encoded file
-            $success = cdash_unlink($this->Base64TmpFilename);
-            if (!$success) {
+            if (!unlink($this->Base64TmpFilename)) {
                 Log::warning("Failed to delete file '{$this->Base64TmpFilename}'");
             }
 
@@ -195,7 +194,9 @@ public function endElement($parser, $name): void
             if ($upload_file_size > $this->GetProject()->UploadQuota) {
                 Log::error("Size of uploaded file {$this->TmpFilename} is {$upload_file_size} bytes, which is greater than the total upload quota for this project ({$this->GetProject()->UploadQuota} bytes)");
                 $this->UploadError = true;
-                cdash_unlink($this->TmpFilename);
+                if (!unlink($this->TmpFilename)) {
+                    Log::warning("Failed to delete file '{$this->TmpFilename}'");
+                }
                 return;
             }
 
@@ -227,7 +228,7 @@ public function endElement($parser, $name): void
                     $fp_to_upload = fopen($this->TmpFilename, 'r');
                     if ($fp_to_upload === false) {
                         Log::error("Failed to open temporary file {$this->TmpFilename} for upload");
-                        cdash_unlink($this->TmpFilename);
+                        unlink($this->TmpFilename);
                         $this->UploadError = true;
                         return;
                     }
@@ -239,7 +240,7 @@ public function endElement($parser, $name): void
                     fclose($fp_to_upload);
                     if (!$response->successful()) {
                         Log::error('Error uploading file via API: ' .  $response->status() . ' ' . $response->body());
-                        cdash_unlink($this->TmpFilename);
+                        unlink($this->TmpFilename);
                         $this->UploadError = true;
                         return;
                     }
@@ -251,13 +252,13 @@ public function endElement($parser, $name): void
                             $fileToUpload = new File($this->TmpFilename);
                         } catch (FileNotFoundException $e) {
                             Log::error("Could not find file {$this->TmpFilename} to upload");
-                            cdash_unlink($this->TmpFilename);
+                            unlink($this->TmpFilename);
                             $this->UploadError = true;
                             return;
                         }
                         if (Storage::putFileAs('upload', $fileToUpload, (string) $this->UploadFile->Sha1Sum) === false) {
                             Log::error("Failed to store {$this->TmpFilename} as {$uploadFilepath}");
-                            cdash_unlink($this->TmpFilename);
+                            unlink($this->TmpFilename);
                             $this->UploadError = true;
                             return;
                         }
@@ -266,8 +267,7 @@ public function endElement($parser, $name): void
             }
 
             // Delete decoded temporary file.
-            $success = cdash_unlink($this->TmpFilename);
-            if (!$success) {
+            if (!unlink($this->TmpFilename)) {
                 Log::error("Failed to delete file '{$this->TmpFilename}");
             }
 

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -15200,16 +15200,6 @@ parameters:
 			count: 1
 			path: app/cdash/include/log.php
 
-		-
-			message: "#^Function cdash_unlink\\(\\) has no return type specified\\.$#"
-			count: 1
-			path: app/cdash/include/log.php
-
-		-
-			message: "#^Function cdash_unlink\\(\\) has parameter \\$filename with no type specified\\.$#"
-			count: 1
-			path: app/cdash/include/log.php
-
 		-
 			message: "#^Function to_psr3_level\\(\\) has no return type specified\\.$#"
 			count: 1