Ensure package plugin sandbox contents is deterministic (#7876)

Previously, nondeterministic item replacement directory paths could be
included in the sandbox content. This can lead to spurious incremental
rebuilds of plugin outputs.

Because `itemReplacementDirectories` returns a unique temporary
directory on each call, I don't believe including it in the sandbox
profile had the intended effect, so removing it should not break
existing plugins.

Author: owenv

Sources/Basics/Sandbox.swift

@@ -217,9 +217,19 @@ fileprivate func macOSSandboxProfile(
     // Emit rules for paths under which writing is allowed, even if they are descendants directories that are otherwise read-only.
     if writableDirectories.count > 0 {
         contents += "(allow file-write*\n"
-        // For any explicit writable directories, also include the relevant item replacement directories so that Foundation APIs using atomic writes are not blocked by the sandbox.
-        for path in writableDirectories + Set(writableDirectories.compactMap { try? fileSystem.itemReplacementDirectories(for: $0) }.flatMap { $0}) {
+        for path in writableDirectories {
             contents += "    (subpath \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
+            
+            // `itemReplacementDirectories` may return a combination of stable directory paths, and subdirectories which are unique on every call. Avoid including unnecessary subdirectories in the Sandbox profile which may lead to nondeterminism in its construction.
+            if let itemReplacementDirectories = try? fileSystem.itemReplacementDirectories(for: path).sorted(by: { $0.pathString.count < $1.pathString.count }) {
+                var stableItemReplacementDirectories: [AbsolutePath] = []
+                for directory in itemReplacementDirectories {
+                    if !stableItemReplacementDirectories.contains(where: { $0.isAncestorOfOrEqual(to: directory) }) {
+                        stableItemReplacementDirectories.append(directory)
+                        contents += "    (subpath \(try resolveSymlinks(directory).quotedAsSubpathForSandboxProfile))\n"
+                    }
+                }
+            }
         }
         contents += ")\n"
     }

Tests/BasicsTests/SandboxTests.swift

@@ -217,6 +217,32 @@ final class SandboxTest: XCTestCase {
              XCTAssertNoThrow(try AsyncProcess.checkNonZeroExit(arguments: allowedCommand))
          }
      }
+    
+    func testDeterministicOrdering() throws {
+        #if !os(macOS)
+        try XCTSkipIf(true, "test is only supported on macOS")
+        #endif
+        try withTemporaryDirectory { path in
+            // Ensure the contents of the produced sandbox directory is deterministic, in order
+            // to avoid spurious incremental rebuilds. This test is not guaranteed to catch rare
+            // cases of nondeterminism, but in practice guards well against past issues in this area.
+            let writeable1 = path.appending(component: "w1")
+            let writeable2 = path.appending(component: "w2")
+            let writeable3 = path.appending(component: "w3")
+            let writeable4 = path.appending(component: "w4")
+            
+            let readable1 = path.appending(component: "r1")
+            let readable2 = path.appending(component: "r2")
+            let readable3 = path.appending(component: "r3")
+            let readable4 = path.appending(component: "r4")
+            
+            let command = try Sandbox.apply(command: ["echo", "hello"], strictness: .default, writableDirectories: [writeable1, writeable2, writeable3, writeable4], readOnlyDirectories: [readable1, readable2, readable3, readable4], fileSystem: localFileSystem)
+            for _ in 0..<10 {
+                let newCommand = try Sandbox.apply(command: ["echo", "hello"], strictness: .default, writableDirectories: [writeable1, writeable2, writeable3, writeable4], readOnlyDirectories: [readable1, readable2, readable3, readable4],fileSystem: localFileSystem)
+                XCTAssertEqual(command, newCommand)
+            }
+        }
+    }
 }
 
 extension Sandbox {
@@ -225,11 +251,12 @@ extension Sandbox {
         strictness: Strictness = .default,
         writableDirectories: [AbsolutePath] = [],
         readOnlyDirectories: [AbsolutePath] = [],
-        allowNetworkConnections: [SandboxNetworkPermission] = []
+        allowNetworkConnections: [SandboxNetworkPermission] = [],
+        fileSystem: FileSystem = InMemoryFileSystem()
     ) throws -> [String] {
         return try self.apply(
             command: command,
-            fileSystem: InMemoryFileSystem(),
+            fileSystem: fileSystem,
             strictness: strictness,
             writableDirectories: writableDirectories,
             readOnlyDirectories: readOnlyDirectories,