Adjust tests, fix some bugs

Author: Perksey

sources/SilkTouch/SilkTouch/Caching/CacheUtils.cs

@@ -15,4 +15,7 @@ internal static void ThrowRequireHostDirectoryNeedsWrite() =>
             "CacheFlags.RequireHostDirectory needs FileAccess.Write due to the cache accesses being uncontrolled by "
                 + "the cache system."
         );
+
+    internal static string ToCacheEntryPath(this string s) =>
+        s.Trim('\r', '\n', '\\', '/').Replace('\\', '/');
 }

sources/SilkTouch/SilkTouch/Caching/FileSystemArchiveCacheDirectory.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.IO.Compression;
 using System.Text;
 using Microsoft.Extensions.Logging;
@@ -19,13 +20,18 @@ FileSystemCacheProvider parent
 ) : ICacheDirectory
 {
     private ZipArchive? _committed;
+    private bool _hasCommitted;
     private string? _newPath;
     private ZipArchive? _new;
+    private HashSet<string>? _newEntries;
     private SemaphoreSlim _sema = new(1, 1);
 
     private async ValueTask<ZipArchive?> GetOrCreateCommittedAsync()
     {
-        if (!File.Exists(committedPath) || (Flags & CacheFlags.RequireNew) == CacheFlags.RequireNew)
+        if (
+            !File.Exists(committedPath)
+            || (Flags & CacheFlags.RequireNew) == CacheFlags.RequireNew && !_hasCommitted
+        )
         {
             parent.Logger.LogDebug(
                 "Cache miss with key \"{0}\"! Expected ZIP archive at {1}",
@@ -48,17 +54,22 @@ FileSystemCacheProvider parent
         );
     }
 
+    // [MemberNotNull(nameof(_new))] <-- TODO WHY IS THE COMPILER NOT HAPPY?
+    [MemberNotNull(nameof(_newEntries))]
     private async ValueTask<ZipArchive> GetOrCreateNewAsync()
     {
         if (_new is not null)
         {
+            Debug.Assert(_newEntries is not null);
             return _new;
         }
 
         if ((Access & FileAccess.Write) == 0)
         {
             CacheUtils.ThrowAccessException();
+#pragma warning disable CS8774 // Member must have a non-null value when exiting.
             return null!;
+#pragma warning restore CS8774 // Member must have a non-null value when exiting.
         }
 
         _newPath = IOPath.GetTempFileName();
@@ -67,6 +78,7 @@ private async ValueTask<ZipArchive> GetOrCreateNewAsync()
             CacheKey,
             _newPath
         );
+        _newEntries = [];
         return _new = await ZipArchive.CreateAsync(
             File.OpenWrite(_newPath),
             ZipArchiveMode.Create,
@@ -93,7 +105,7 @@ var entry in (await GetOrCreateCommittedAsync())?.Entries
                 ?? Enumerable.Empty<ZipArchiveEntry>()
         )
         {
-            yield return entry.FullName;
+            yield return entry.FullName.ToCacheEntryPath();
         }
     }
 
@@ -105,12 +117,14 @@ public async ValueTask<Stream> GetCommittedFileAsync(string filePath)
         }
 
         parent.Logger.LogTrace("Cache hit (\"{0}\", ZIP): {1}", CacheKey, filePath);
-        var entry = _committed?.GetEntry(filePath) ?? throw new FileNotFoundException();
+        var entry =
+            _committed?.GetEntry(filePath.ToCacheEntryPath()) ?? throw new FileNotFoundException();
         return await entry.OpenAsync();
     }
 
     public async ValueTask AddFileAsync(string filePath, Stream stream)
     {
+        filePath = filePath.ToCacheEntryPath();
         if ((Access & FileAccess.Write) == 0)
         {
             CacheUtils.ThrowAccessException();
@@ -124,6 +138,7 @@ public async ValueTask AddFileAsync(string filePath, Stream stream)
                 .CreateEntry(filePath, CompressionLevel.SmallestSize)
                 .OpenAsync();
             await stream.CopyToAsync(dst);
+            _newEntries.Add(filePath);
         }
         finally
         {
@@ -153,7 +168,7 @@ public async ValueTask CommitAsync()
                 // If the user doesn't want this, they can use RequireNew.
                 foreach (var entry in _committed.Entries)
                 {
-                    if (@new.GetEntry(entry.FullName) is null)
+                    if (!_newEntries.Contains(entry.FullName.ToCacheEntryPath()))
                     {
                         parent.Logger.LogTrace(
                             "Cache unchanged (\"{0}\", ZIP): {1}",
@@ -181,6 +196,7 @@ public async ValueTask CommitAsync()
 
             await @new.DisposeAsync();
             _new = null;
+            _hasCommitted = true;
             Debug.Assert(_newPath is not null);
             File.Move(_newPath, committedPath, true);
         }

sources/SilkTouch/SilkTouch/Caching/FileSystemCacheProvider.cs

@@ -16,14 +16,20 @@ public class FileSystemCacheProvider(ILogger<FileSystemCacheProvider> logger) :
 
     static string GetCachePath(string cacheKey, CacheIntent intent)
     {
+        var outDir = Path.Combine(CommonDir, ".silktouch");
+        if (!Directory.Exists(outDir))
+        {
+            Directory.CreateDirectory(outDir);
+        }
+
         cacheKey = cacheKey.ToLower();
         var ext = intent switch
         {
             CacheIntent.ResolvedForeignInput => "stdownload",
             CacheIntent.StageIntermediateOutput => "stout",
             _ => throw new ArgumentOutOfRangeException(nameof(intent), intent, null),
         };
-        return Path.Combine(CommonDir, ".silktouch", $"{cacheKey}.{ext}");
+        return Path.Combine(outDir, $"{cacheKey}.{ext}");
     }
 
     /// <inheritdoc />

sources/SilkTouch/SilkTouch/Caching/FileSystemDiskCacheDirectory.cs

@@ -16,7 +16,7 @@ FileSystemCacheProvider parent
 ) : ICacheDirectory
 {
     private string? _newPath;
-    private bool _committed; // literally just for logging purposes
+    private bool _committed;
 
     public async ValueTask InitAsync()
     {
@@ -97,17 +97,26 @@ private string NewPath
             return _newPath;
         }
     }
-    public string? Path => (Flags & CacheFlags.RequireHostDirectory) != 0 ? _newPath : null;
+    public string? Path => (Flags & CacheFlags.RequireHostDirectory) != 0 ? NewPath : null;
 
     public IAsyncEnumerable<string> GetCommittedFilesAsync()
     {
         if ((Access & FileAccess.Read) == 0)
         {
             CacheUtils.ThrowAccessException();
         }
+
+        if (
+            !Directory.Exists(committedPath)
+            || ((Flags & CacheFlags.RequireNew) == CacheFlags.RequireNew && !_committed)
+        )
+        {
+            return AsyncEnumerable.Empty<string>();
+        }
+
         return Directory
             .GetFiles(committedPath, "*", SearchOption.AllDirectories)
-            .Select(x => IOPath.GetRelativePath(committedPath, x))
+            .Select(x => IOPath.GetRelativePath(committedPath, x).ToCacheEntryPath())
             .ToAsyncEnumerable();
     }
 
@@ -151,6 +160,10 @@ public async ValueTask CommitAsync()
         {
             parent.Logger.LogTrace("Erasing {0} as RequireNew is set", committedPath);
             Directory.Delete(committedPath, true);
+        }
+
+        if (!Directory.Exists(committedPath))
+        {
             Directory.CreateDirectory(committedPath);
         }
 
@@ -179,7 +192,8 @@ await Task.Run(
                         {
                             File.Copy(
                                 x,
-                                IOPath.Combine(committedPath, IOPath.GetRelativePath(_newPath, x))
+                                IOPath.Combine(committedPath, IOPath.GetRelativePath(_newPath, x)),
+                                true
                             );
                         }
                         else