diff --git a/ProjectDirector.Test/SimilarReposTests.cs b/ProjectDirector.Test/SimilarReposTests.cs index 34d6bc0..656c43e 100644 --- a/ProjectDirector.Test/SimilarReposTests.cs +++ b/ProjectDirector.Test/SimilarReposTests.cs @@ -364,6 +364,139 @@ public void RefreshingOneFileRejectsARepositoryThatIsNotOnGitHub() () => ProjectDirector.RefreshFileDiff(repoA, other, RelativeFilePath.Create("shared.txt"))); } + [TestMethod] + public async Task SiblingsSharingASubmodulePathCompareWithTheGitlinkLeftOut() + { + string a = CreateRepository([("shared.txt", "one\n")]); + string b = CreateRepository([("shared.txt", "two\n")]); + + try + { + // A submodule is tracked as a gitlink, and its path is a directory on disk even before + // the submodule is initialised. Reading it as a file threw out of the background task. + AddGitlink(a, "external"); + AddGitlink(b, "external"); + + GitHubRepository repoA = Repository(a, "A"); + ConcurrentQueue log = []; + + await ProjectDirector.CompareSiblingsAsync(repoA, [repoA, Repository(b, "B")], log.Enqueue).ConfigureAwait(false); + + Assert.IsFalse(repoA.SimilarReposPending, "The comparison should finish rather than stay pending."); + Assert.IsNotNull(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("shared.txt"))); + Assert.IsNull(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("external")), "A gitlink has no content to diff."); + Assert.Contains("Compared", log.Single()); + } + finally + { + TryDeleteDirectory(a); + TryDeleteDirectory(b); + } + } + + [TestMethod] + public async Task ASharedFileThatCannotBeReadIsLeftOutAndLogged() + { + string a = CreateRepository([("shared.txt", "one\n"), ("locked.txt", "a\n")]); + string b = CreateRepository([("shared.txt", "two\n"), ("locked.txt", "b\n")]); + + try + { + GitHubRepository repoA = Repository(a, "A"); + ConcurrentQueue log = []; + + // An exclusive handle stands in for a file another process holds open, and unlike a + // permission bit it is refused under root as well. + using (new FileStream(Path.Join(b, "locked.txt"), FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + await ProjectDirector.CompareSiblingsAsync(repoA, [repoA, Repository(b, "B")], log.Enqueue).ConfigureAwait(false); + } + + Assert.IsFalse(repoA.SimilarReposPending, "An unreadable file must not leave the comparison pending."); + Assert.IsNotNull(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("shared.txt")), "The readable files are still compared."); + Assert.IsNull(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("locked.txt"))); + Assert.Contains(line => line.Contains("Left locked.txt out", StringComparison.Ordinal), log, "The log should say which file was left out."); + } + finally + { + TryDeleteDirectory(a); + TryDeleteDirectory(b); + } + } + + [TestMethod] + public async Task ASharedFileMissingFromTheWorkingTreeIsComparedAsEmpty() + { + string a = CreateRepository([("gone.txt", "a\n")]); + string b = CreateRepository([("gone.txt", "b\n")]); + + try + { + AddNestedFile(a, "a\n"); + AddNestedFile(b, "b\n"); + + // A tracked file can be absent on disk, directly or because its whole folder is. + File.Delete(Path.Join(b, "gone.txt")); + Directory.Delete(Path.Join(b, "sub"), recursive: true); + + GitHubRepository repoA = Repository(a, "A"); + await ProjectDirector.CompareSiblingsAsync(repoA, [repoA, Repository(b, "B")], _ => { }).ConfigureAwait(false); + + Assert.IsFalse(repoA.SimilarReposPending); + Assert.IsNotEmpty(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("gone.txt"))!.DiffBlocks, "A missing file diffs against nothing."); + Assert.IsNotEmpty(ProjectDirector.FindDiff(repoA, Name("B"), RelativeFilePath.Create("sub/nested.txt"))!.DiffBlocks, "A file in a missing folder diffs against nothing."); + } + finally + { + TryDeleteDirectory(a); + TryDeleteDirectory(b); + } + } + + // Windows file systems cannot hold a file name containing '<' or '>'. + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows)] + public async Task AComparisonThatFailsStillEndsThePendingStateAndSaysWhy() + { + // A path that git tracks happily but the semantic path types refuse, which throws partway + // through building the comparison. + string a = CreateRepository([("a.txt", "one\n")]); + string b = CreateRepository([("a.txt", "two\n")]); + + try + { + GitHubRepository repoA = Repository(a, "A"); + ConcurrentQueue log = []; + + await ProjectDirector.CompareSiblingsAsync(repoA, [repoA, Repository(b, "B")], log.Enqueue).ConfigureAwait(false); + + Assert.IsFalse(repoA.SimilarReposPending, "A failed comparison must not leave the panels pending."); + Assert.IsEmpty(repoA.SimilarRepoDiffs); + Assert.Contains("failed", log.Single()); + } + finally + { + TryDeleteDirectory(a); + TryDeleteDirectory(b); + } + } + + private static void AddNestedFile(string root, string contents) + { + _ = Directory.CreateDirectory(Path.Join(root, "sub")); + File.WriteAllText(Path.Join(root, "sub", "nested.txt"), contents); + Assert.IsTrue(GitCli.RunIn(root, "add", "--all").Succeeded, "git add failed."); + Assert.IsTrue(GitCli.RunIn(root, "commit", "-m", "Add nested file").Succeeded, "git commit failed."); + } + + private static void AddGitlink(string root, string path) + { + string head = GitCli.RunIn(root, "rev-parse", "HEAD").OutputText; + Assert.IsTrue(GitCli.RunIn(root, "update-index", "--add", "--cacheinfo", $"160000,{head},{path}").Succeeded, "Adding the gitlink failed."); + Assert.IsTrue(GitCli.RunIn(root, "commit", "-m", "Add submodule").Succeeded, "Committing the gitlink failed."); + _ = Directory.CreateDirectory(Path.Join(root, path)); + } + private static string CreateRepository(IEnumerable<(string RelativePath, string Contents)> files) { string root = Path.Join(Path.GetTempPath(), $"ktsu_pd_similar_{Guid.NewGuid():N}"); diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 2702c42..bda1081 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1336,7 +1336,24 @@ internal static Task CompareSiblingsAsync(GitRepository repo, IEnumerable { - Dictionary> diffs = DiffAgainstAll(repo, others); + Dictionary> diffs; + try + { + diffs = DiffAgainstAll(repo, others, log); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException or InvalidOperationException or ArgumentException) + { + // Nothing observes this task, so a throw out of it would leave the panels on + // "Comparing repositories..." for good. Publishing an empty answer ends the pending + // state, and the log says why there is nothing to show. + if (repo.TryApplySimilarRepoDiffs(token, [])) + { + log($"[{DateTimeOffset.Now}] Comparing {repo.RemotePath} failed: {e.Message}"); + } + + return; + } + if (repo.TryApplySimilarRepoDiffs(token, diffs)) { log($"[{DateTimeOffset.Now}] Compared {repo.RemotePath} against {others.Count} other {(others.Count == 1 ? "repository" : "repositories")}"); @@ -1352,6 +1369,7 @@ internal static Task CompareSiblingsAsync(GitRepository repo, IEnumerable /// The repository every sibling is compared against. /// The siblings, paired with the names to key the result by. + /// Reports each shared file that could not be read and so was left out. /// The diffs for each sibling. /// /// The base repository's tracked-file list is read once for the whole run rather than once per @@ -1361,7 +1379,8 @@ internal static Task CompareSiblingsAsync(GitRepository repo, IEnumerable internal static Dictionary> DiffAgainstAll( GitRepository repo, - IEnumerable> others) + IEnumerable> others, + Action? log = null) { Collection trackedFiles = GitCli.IsRepository(repo.LocalPath) ? GitCli.ListTrackedFiles(repo.LocalPath) @@ -1370,13 +1389,13 @@ internal static Dictionary> diffs = []; foreach ((FullyQualifiedGitHubRepoName otherRepoName, GitRepository otherRepo) in others) { - diffs[otherRepoName] = DiffRepos(repo, trackedFiles, otherRepo); + diffs[otherRepoName] = DiffRepos(repo, trackedFiles, otherRepo, log); } return diffs; } - private static Dictionary DiffRepos(GitRepository repoA, IEnumerable trackedFilesA, GitRepository repoB) + private static Dictionary DiffRepos(GitRepository repoA, IEnumerable trackedFilesA, GitRepository repoB, Action? log) { Dictionary diffs = []; @@ -1389,12 +1408,22 @@ private static Dictionary DiffRepos(GitRepository .Intersect(GitCli.ListTrackedFiles(repoB.LocalPath)) .ToCollection(); - Dictionary fileContents = matches.ToDictionary(x => x, x => ReadFileOrEmpty(repoA.LocalPath, x)); - Dictionary otherFileContents = matches.ToDictionary(x => x, x => ReadFileOrEmpty(repoB.LocalPath, x)); - foreach (string match in matches) { - diffs[RelativeFilePath.Create(match)] = Differ.Instance.CreateLineDiffs(fileContents[match], otherFileContents[match], ignoreWhitespace: false, ignoreCase: false); + // A submodule's gitlink is tracked but is a directory on disk, as is a tracked link to + // one. Neither has content to diff, so the pair is left out rather than read. + if (!TryReadTrackedFile(repoA.LocalPath, match, out string fileContents, out string? reason) + || !TryReadTrackedFile(repoB.LocalPath, match, out string otherFileContents, out reason)) + { + if (reason is not null) + { + log?.Invoke($"[{DateTimeOffset.Now}] Left {match} out of the comparison with {repoB.RemotePath}: {reason}"); + } + + continue; + } + + diffs[RelativeFilePath.Create(match)] = Differ.Instance.CreateLineDiffs(fileContents, otherFileContents, ignoreWhitespace: false, ignoreCase: false); } return diffs; @@ -1404,22 +1433,55 @@ private static Dictionary DiffRepos(GitRepository /// Reads a tracked file from a working tree, treating anything missing on disk as empty. A file /// can be tracked and still be absent, and a diff against nothing is the useful answer. /// - private static string ReadFileOrEmpty(string repoPath, string relativePath) + /// The working tree. + /// The tracked path, relative to . + /// The file's text, or empty when it is missing or cannot be read. + /// + /// Why an existing file could not be read, or when there is nothing to + /// report: the file was read, was missing, or is a directory. + /// + /// Whether there is file content to diff, which a directory never has. + /// + /// Every failure is answered rather than thrown. The whole-repository comparison runs on a task + /// nobody observes, and the single-file re-diff runs on the render thread. + /// + private static bool TryReadTrackedFile(string repoPath, string relativePath, out string contents, out string? reason) { + contents = string.Empty; + reason = null; + + string fullPath = Path.Join(repoPath, relativePath); + if (Directory.Exists(fullPath)) + { + return false; + } + try { - return File.ReadAllText(Path.Combine(repoPath, relativePath)); + contents = File.ReadAllText(fullPath); + return true; } catch (FileNotFoundException) { - return string.Empty; + return true; } catch (DirectoryNotFoundException) { - return string.Empty; + return true; + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + reason = e.Message; + return false; } } + private static string ReadFileOrEmpty(string repoPath, string relativePath) + { + _ = TryReadTrackedFile(repoPath, relativePath, out string contents, out _); + return contents; + } + /// /// Looks up one file's diff against a sibling repository. ///