From 6112b38ef7ebcf0a1a6371b02e824b329e3e7b45 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 13:47:35 -0600 Subject: [PATCH 1/6] fix(recovery): never discard local work, and deliver the commits it reports "Resolve" and --sanitize-paths were built for a clone that was fetched but never checked out. They are also offered when a pull into an ordinary, checked-out clone finds Windows-invalid paths in the incoming commits, and there the apply wrote the tree of the commit already checked out, with force: uncommitted edits were overwritten, the incoming commits were not delivered although the row said Done, and the next pull moved the branch to origin's tip without checking that local commits were contained in it. - Applying a recovery to an ordinary clone now goes to origin's tip, after proving that loses nothing: uncommitted changes to tracked files, or commits origin does not have, stop it with the advice the ordinary pull gives, and nothing is changed. Files that left the tree are removed. - A recovery-managed repository is only ever fast-forwarded. - The pending-checkout marker is set for the whole apply, so an apply that fails part-way is completed or re-reported by the next pull instead of being called up to date. - The index is written from the commit, so git reports only the renamed and skipped paths as changed instead of every file as deleted. - Attribute files are materialized first and apply to the whole tree. - When recoveries are merged, the newer choice for a path wins whatever kind it is: a rename replaces a stored skip, a skip a stored rename. Closes #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 17 +- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 381 ++++++++++++++++++++ gclo.Engine/LibGit2GitClient.cs | 157 ++++++-- 4 files changed, 530 insertions(+), 27 deletions(-) create mode 100644 gclo.Engine.Tests/RecoveryLocalWorkTests.cs diff --git a/README.md b/README.md index 154feb3..a33108c 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca 8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, **Retry failed** re-runs exactly the failed rows, and **Open folder** opens the target folder in Explorer. -**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). +**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). The same applies when an update to a repository you already have brings the first such path: gclo fast-forwards it and writes the renamed tree, unless that clone has uncommitted changes or commits of its own, in which case it says so and changes nothing. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). **Settings** (File > Settings…) let you set a default GitHub token (stored in the Windows Credential Manager), the default target folder (with a folder picker), the default parallelism, and the theme (System, Light, or Dark); an Advanced section toggles the startup splash and its duration. diff --git a/docs/CLI.md b/docs/CLI.md index 7f8639f..584d1cb 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -158,12 +158,21 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: upstream tip has not moved. A later sync that brings *new* invalid paths not covered by the stored mapping fails with those paths listed — sync again with `--sanitize-paths` (or use the GUI's **Resolve…** dialog) and the new renames - and skips are merged into the stored mapping. Use `--include ` to - target just that repository. + and skips are merged into the stored mapping; a newer choice for a path + replaces the stored one. Use `--include ` to target just that + repository. +- A repository that was already checked out normally, and whose update brings + the first invalid path, is fast-forwarded to origin's tip and then written + with the renames. gclo never discards local work: if that clone has + uncommitted changes to tracked files, or commits origin does not have, it + fails with a message saying so, and nothing in it is changed. Only the working tree is renamed — the repository's history and index still hold -the original paths, so `git status` reports the renamed and skipped files as -local changes. Treat a sanitized repository as a read-only mirror. +the original paths, so `git status` reports the renamed and skipped files (and +only those) as local changes. Later syncs rewrite the whole tree, so treat a +sanitized repository as a read-only mirror: edits made there are overwritten, +though a commit made there is never dropped (the sync fails instead, as for any +clone with local commits). #### `--json` diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs new file mode 100644 index 0000000..cd79720 --- /dev/null +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -0,0 +1,381 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +using System.Text; +using LibGit2Sharp; +using static gclo.Engine.Tests.GitTestHelpers; + + +namespace gclo.Engine.Tests; + + +/// +/// Path recovery never discards local work, delivers the commits it reports, and +/// leaves a repository git itself can make sense of (#147). Real repositories on the +/// local file system, like . +/// +public sealed class RecoveryLocalWorkTests : IDisposable +{ + private const string Token = "test-token"; + + private const string PendingKey = "gclo.checkoutpending"; + + private readonly string _root = + Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + + private readonly LibGit2GitClient _client = new(); + + + + public RecoveryLocalWorkTests() + { + Directory.CreateDirectory(_root); + } + + + + public void Dispose() => TryDeleteDirectory(_root); + + + + // ---------------------------------------------------------------- an ordinary clone + + [Fact] + public async Task OrdinaryClone_CleanAndBehind_IsFastForwardedToOriginsTip_ThenMaterialized() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("gone.txt", "going"), ("keep.txt", "keep")); + // Upstream moves on: one file changes, one goes away, and a path Windows cannot + // create arrives. The pull refuses, with the tree untouched. + ReplaceEntry(source, remove: "gone.txt", add: ("readme.txt", "v2")); + string tip = AppendForgedCommit(source, ("bad:file.txt", "bad")); + await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + // Done means the incoming commits are on disk: HEAD is origin's tip, not the + // commit that was already checked out. + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("v2", File.ReadAllText(Path.Combine(target, "readme.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "gone.txt")), "a file that left the tree is removed, as a checkout would"); + Assert.False(IsCheckoutPending(target)); + + // And the next pull has nothing to do. + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(tip, HeadSha(target)); + } + + + + [Fact] + public async Task OrdinaryClone_WithUncommittedChanges_IsRefused_AndTheChangesSurvive() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + string before = HeadSha(target); + File.WriteAllText(Path.Combine(target, "readme.txt"), "my local edit"); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Contains("has uncommitted changes", ex.Message); + Assert.Contains("gclo never discards local work", ex.Message); + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "readme.txt"))); + Assert.Equal(before, HeadSha(target)); + Assert.False(IsCheckoutPending(target)); + Assert.False(File.Exists(RecoveryFilePath(target)), "a refused apply leaves the repository an ordinary clone"); + } + + + + [Fact] + public async Task OrdinaryClone_WithLocalCommits_IsRefused_AndTheCommitsSurvive() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + string local = CommitInWorkingTree(target, "mine.txt", "my work"); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Equal(local, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + Assert.False(File.Exists(RecoveryFilePath(target))); + } + + + + [Fact] + public async Task OrdinaryClone_WhoseBranchHasNoUpstream_HasNothingToResolve() + { + (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + using(var repo = new Repository(target)) + { + repo.Branches.Update(repo.Head, b => b.TrackedBranch = null); + } + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["anything"]), CancellationToken.None)); + + Assert.Contains("nothing to update from on origin", ex.Message); + Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); + } + + + + [Fact] + public async Task OrdinaryClone_AnApplyThatFailsPartWay_LeavesTheMarker_SoTheNextPullDoesNotSayUpToDate() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("vendor/lib.txt", "lib")); + await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + // The folder the new tree needs already exists, as a link to somewhere else: + // the write into it is refused after the branch has moved. + string elsewhere = Path.Combine(_root, "elsewhere"); + Directory.CreateDirectory(elsewhere); + string link = Path.Combine(target, "vendor"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + + Assert.True(IsCheckoutPending(target), "an interrupted apply is recorded"); + // The marker makes the next pull re-examine the tree instead of trusting it. + await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + } + + + + // ---------------------------------------------------------------- a recovery-managed repository + + [Fact] + public async Task RecoveryManaged_WithALocalCommit_IsNotMovedOffIt_ByTheNextPull() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "managed-local-commit"); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + // Someone commits in the recovered clone; upstream moves on as well. + string local = CommitOnTopOfHead(target, "mine.txt", "my work"); + AppendForgedCommit(source, ("other.txt", "o")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Equal(local, HeadSha(target)); + } + + + + [Fact] + public async Task RecoveryManaged_StillFollowsOrigin_WhenNothingLocalIsInTheWay() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "managed-follow"); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + } + + + + // ---------------------------------------------------------------- what git sees afterwards + + [Fact] + public async Task AfterAnApply_TheIndexDescribesTheCommit_SoOnlyRenamedAndSkippedPathsDiffer() + { + string source = CreateForgedRepo( + _root, ("bad:file.txt", "bad"), ("skip:me.txt", "skipped"), ("keep.txt", "keep"), ("src/app.cs", "code")); + string target = await CloneExpectingInvalidPathsAsync(source, "index"); + + await _client.ApplyRecoveryAsync( + target, Recovery(renames: [("bad:file.txt", "bad_file.txt")], skips: ["skip:me.txt"]), CancellationToken.None); + + using var repo = new Repository(target); + Assert.Equal( + new[] { "bad:file.txt", "keep.txt", "skip:me.txt", "src/app.cs" }, + repo.Index.Select(e => e.Path).Order(StringComparer.Ordinal), + StringComparer.Ordinal); + RepositoryStatus status = repo.RetrieveStatus(new StatusOptions { IncludeUntracked = true }); + // The files that kept their names are clean; nothing reads as "everything deleted". + Assert.Equal(FileStatus.Unaltered, repo.RetrieveStatus("keep.txt")); + Assert.Equal(FileStatus.Unaltered, repo.RetrieveStatus("src/app.cs")); + Assert.Equal( + new[] { "bad:file.txt", "skip:me.txt" }, + status.Missing.Select(e => e.FilePath).Order(StringComparer.Ordinal), + StringComparer.Ordinal); + Assert.Equal("bad_file.txt", Assert.Single(status.Untracked).FilePath); + } + + + + [Fact] + public async Task AttributeFiles_ApplyToEveryFile_WhateverTheirPlaceInTheTree() + { + // "!early.txt" sorts before ".gitattributes" and "late.txt" after it; both must + // come out with the line endings the attributes ask for. + string source = CreateForgedRepo( + _root, + ("!early.txt", "one\ntwo\n"), + (".gitattributes", "*.txt text eol=lf\n"), + ("late.txt", "one\ntwo\n"), + ("bad:file.txt", "one\ntwo\n")); + string target = await CloneExpectingInvalidPathsAsync(source, "attributes"); + using(var repo = new Repository(target)) + { + // Without the attributes this setting would write CRLF. + repo.Config.Set("core.autocrlf", "true", ConfigurationLevel.Local); + } + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "!early.txt"))); + Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "late.txt"))); + Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + } + + + + // ---------------------------------------------------------------- the newer choice wins + + [Fact] + public async Task AStoredSkip_IsReplacedByALaterRename_OfTheSamePath() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "skip-then-rename"); + await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + string json = File.ReadAllText(RecoveryFilePath(target)); + Assert.Contains("bad_file.txt", json); + } + + + + [Fact] + public async Task AStoredRename_IsReplacedByALaterSkip_OfTheSamePath() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "rename-then-skip"); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + Assert.True(File.Exists(Path.Combine(target, "bad_file.txt"))); + + await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); + + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt")), "the file written under the old choice is removed"); + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.DoesNotContain("bad_file.txt", File.ReadAllText(RecoveryFilePath(target))); + } + + + + // ---------------------------------------------------------------- helpers + + /// A forged source with valid paths, and an ordinary (fully checked-out) clone of it. + private async Task<(string Source, string Target)> CloneOrdinaryAsync(params (string Name, string Content)[] entries) + { + string source = CreateForgedRepo(_root, entries); + string target = Path.Combine(_root, "clone-" + Guid.NewGuid().ToString("N")[..8]); + await _client.CloneAsync(source, target, Token, null, CancellationToken.None); + return (source, target); + } + + + + private async Task CloneExpectingInvalidPathsAsync(string source, string name) + { + string target = Path.Combine(_root, name); + await Assert.ThrowsAsync( + () => _client.CloneAsync(source, target, Token, null, CancellationToken.None)); + return target; + } + + + + private static PathRecovery Recovery((string From, string To)[]? renames = null, string[]? skips = null) + => new( + (renames ?? []).ToDictionary(r => r.From, r => r.To, StringComparer.Ordinal), + new HashSet(skips ?? [], StringComparer.Ordinal)); + + + + /// Adds a commit that removes one entry and writes (or overwrites) another. + private static void ReplaceEntry(string workdir, string remove, (string Name, string Content) add) + { + using var repo = new Repository(workdir); + Commit head = repo.Head.Tip; + TreeDefinition definition = TreeDefinition.From(head.Tree).Remove(remove); + Blob blob = repo.ObjectDatabase.CreateBlob(new MemoryStream(Encoding.UTF8.GetBytes(add.Content))); + definition.Add(add.Name, blob, Mode.NonExecutableFile); + Signature signature = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit( + signature, signature, "forged change", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + } + + + + /// An ordinary commit made in the working tree, the way a person would. + private static string CommitInWorkingTree(string workdir, string fileName, string content) + { + using var repo = new Repository(workdir); + File.WriteAllText(Path.Combine(workdir, fileName), content); + Commands.Stage(repo, fileName); + Signature signature = MakeSignature(); + return repo.Commit("local work", signature, signature).Sha; + } + + + + /// + /// A commit on top of HEAD that adds one file, made through the object database: + /// a recovery-managed tree holds paths the index tooling cannot stage on Windows. + /// + private static string CommitOnTopOfHead(string workdir, string fileName, string content) + { + using var repo = new Repository(workdir); + Commit head = repo.Head.Tip; + var definition = TreeDefinition.From(head.Tree); + definition.Add(fileName, repo.ObjectDatabase.CreateBlob(new MemoryStream(Encoding.UTF8.GetBytes(content))), Mode.NonExecutableFile); + Signature signature = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit( + signature, signature, "local work", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + return commit.Sha; + } + + + + private static bool IsCheckoutPending(string repoPath) + { + using var repo = new Repository(repoPath); + return repo.Config.Get(PendingKey)?.Value == true; + } + + + + private static string RecoveryFilePath(string repoPath) + { + using var repo = new Repository(repoPath); + return Path.Combine(repo.Info.Path, "gclo-recovery.json"); + } +} diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index da5e047..2463271 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -52,6 +52,10 @@ public sealed class LibGit2GitClient : IGitClient private static readonly JsonSerializerOptions s_recoveryJsonOptions = new() { WriteIndented = true }; + /// No renames, no skips: every entry of a tree under its own path. + private static readonly PathRecovery s_noRecovery = new( + new Dictionary(StringComparer.Ordinal), new HashSet(StringComparer.Ordinal)); + /// /// Name of the remote that exists only for the duration of one fetch, when a /// folder's origin names the listed repository but spells its URL differently @@ -268,7 +272,9 @@ public sealed class LibGit2GitClient : IGitClient // A recovery-managed repo stays recovery-managed: re-materialize the new // tip through the stored mapping. Freshly-invalid paths the mapping does // not cover surface as a typed failure listing the effective paths. - AdvanceHeadToTrackedTip(repo); + // Its working tree is written by gclo, but its branch is still the user's: + // a commit made there must not be dropped by moving the branch (#147). + AdvanceHeadToTrackedTip(repo, fastForwardOnly: true); RecoveryDocument stored = LoadRecoveryDocument(recoveryFile, WorkingDirectoryOf(repo)); if(stored.MaterializedTip is not null && string.Equals(stored.MaterializedTip, repo.Head.Tip?.Sha, StringComparison.Ordinal)) { @@ -662,13 +668,25 @@ public sealed class LibGit2GitClient : IGitClient /// - /// Materializes HEAD's tree into the working directory with the recovery's renames - /// and skips applied, after validating that the effective path set is actually - /// creatable on Windows. Any recovery persisted from an earlier run is merged in - /// first (the incoming recovery wins on conflicts), so a recovery-managed repo - /// that gains new invalid paths can be fixed incrementally. Persists the merged - /// recovery so later pulls re-apply it, then clears the pending-checkout marker. + /// Materializes a tree into the working directory with the recovery's renames and + /// skips applied, after validating that the effective path set is actually creatable + /// on Windows. Any recovery persisted from an earlier run is merged in first (the + /// incoming recovery wins on conflicts), so a recovery-managed repo that gains new + /// invalid paths can be fixed incrementally. Persists the merged recovery so later + /// pulls re-apply it, then clears the pending-checkout marker. /// + /// + /// Which tree depends on where the repository stands (#147): + /// + /// Fetched but never checked out (the marker is set), or already + /// recovery-managed: HEAD's tree. Nothing of the user's is on disk, or what is there + /// was written by an earlier apply. + /// An ordinary, checked-out clone whose pull found invalid paths in the + /// incoming commits: origin's tip, after proving that nothing local is lost by going + /// there (see ). Materializing HEAD + /// would rewrite the commit that is already checked out and deliver nothing. + /// + /// private static void ApplyRecoveryCore(Repository repo, PathRecovery recovery, CancellationToken ct) { if(OperatingSystem.IsWindows()) @@ -684,7 +702,9 @@ public sealed class LibGit2GitClient : IGitClient recovery = MergeRecoveries(stored: ToRecovery(previous), incoming: recovery); } - Commit tip = repo.Head.Tip + Commit? fastForwardTo = previous is null && !IsCheckoutPending(repo) ? FastForwardTargetOfOrdinaryClone(repo) : null; + Commit tip = fastForwardTo + ?? repo.Head.Tip ?? throw new InvalidOperationException("Repository has no commits; nothing to materialize."); List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(tip.Tree, recovery); @@ -698,12 +718,36 @@ public sealed class LibGit2GitClient : IGitClient throw new InvalidRepositoryPathsException(stillInvalid); } + ///////////////////////////////////////////////////////////////////////////// + // A real checkout removes what left the tree; this materialization must too, // + // or the working tree accumulates every file upstream ever deleted or // + // renamed (#31). What "was there" is the previous apply's manifest, or, for // + // an ordinary clone taking this path for the first time, the files of the // + // commit that is checked out now. // + ///////////////////////////////////////////////////////////////////////////// + IEnumerable onDiskBefore = previous?.MaterializedPaths + ?? (fastForwardTo is null ? [] : CollectEffectiveEntries(repo.Head.Tip.Tree, s_noRecovery).Select(e => e.EffectivePath).ToList()); + + ///////////////////////////////////////////////////////////////////////////// + // From here until the end the tree on disk is in between two states. The // + // marker says so durably: if this is interrupted (a failure part-way, the // + // process dying), the next pull completes or re-reports instead of calling // + // a partial tree up to date (#147). // + ///////////////////////////////////////////////////////////////////////////// + repo.Config.Set(CheckoutPendingKey, true, ConfigurationLevel.Local); + if(fastForwardTo is not null) + { + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), fastForwardTo.Id); + } + + // The index describes the commit, as after any checkout; without it git reports + // every file as deleted and untracked. Only renamed and skipped paths differ + // from it on disk. Written first so attribute lookups can already see the tree. + repo.Index.Replace(tip.Tree); + repo.Index.Write(); - // A real checkout removes what left the tree; this materialization must too, - // or a recovery-managed working tree accumulates every file upstream ever - // deleted or renamed (#31). The previous apply's manifest says what it wrote. var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); - foreach(string stale in previous?.MaterializedPaths ?? []) + foreach(string stale in onDiskBefore) { if(!effectiveNow.Contains(stale)) { @@ -712,8 +756,10 @@ public sealed class LibGit2GitClient : IGitClient } // Parent folders are shared by many files; each is checked for a link once. + // Attribute files go first: they decide how the files after them are written + // (line endings), exactly as a checkout applies them to the whole tree. var verifiedFolders = new HashSet(StringComparer.Ordinal); - foreach((string? originalPath, string? effectivePath, Blob? blob) in entries) + foreach((string? originalPath, string? effectivePath, Blob? blob) in entries.OrderBy(e => IsAttributesFile(e.OriginalPath) ? 0 : 1)) { ct.ThrowIfCancellationRequested(); @@ -731,19 +777,79 @@ public sealed class LibGit2GitClient : IGitClient + private static bool IsAttributesFile(string repoPath) + => repoPath.EndsWith(".gitattributes", StringComparison.Ordinal) + && (repoPath.Length == ".gitattributes".Length || repoPath[^(".gitattributes".Length + 1)] == '/'); + + + /// - /// Overlays onto : renames - /// union with the incoming value winning on a shared original path; skips union. + /// For an ordinary, checked-out clone that is about to become recovery-managed: + /// the commit to go to (origin's tip for the current branch), once it is certain + /// that going there loses nothing. gclo never discards local work, on this path + /// either: uncommitted changes to tracked files, or commits origin does not have, + /// stop the apply with the same advice the ordinary pull gives (#147). + /// + private static Commit FastForwardTargetOfOrdinaryClone(Repository repo) + { + Commit? head = repo.Head.Tip; + Commit? upstream = repo.Head.TrackedBranch?.Tip; + if(head is null || upstream is null) + { + throw new InvalidOperationException( + "This repository's current branch has nothing to update from on origin, so there are no paths to resolve. " + + "Sync it again; if it keeps failing, delete the folder to re-clone."); + } + + if(repo.RetrieveStatus(new StatusOptions { IncludeUntracked = false, IncludeIgnored = false }).IsDirty) + { + throw new InvalidOperationException( + $"'{repo.Head.FriendlyName}' has uncommitted changes, and resolving paths rewrites the working tree; " + + "gclo never discards local work. Commit or discard the changes, then resolve again."); + } + + EnsureFastForward(repo, head, upstream); + return upstream; + } + + + + /// Throws the "local commits" refusal unless is contained in . + private static void EnsureFastForward(Repository repo, Commit from, Commit to) + { + if(!string.Equals(repo.ObjectDatabase.FindMergeBase(from, to)?.Sha, from.Sha, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"'{repo.Head.FriendlyName}' has local commits that origin does not have; gclo only fast-forwards and never merges. " + + "Push or reset the local commits, or delete the folder to re-clone."); + } + } + + + + /// + /// Overlays onto . The newer + /// choice for a path wins whatever kind it is: an incoming rename replaces a stored + /// rename or a stored skip of that path, and an incoming skip replaces a stored + /// rename. Everything else is kept from both. /// private static PathRecovery MergeRecoveries(PathRecovery stored, PathRecovery incoming) { - var renames = new Dictionary(stored.SegmentRenames, StringComparer.Ordinal); + var renames = new Dictionary(StringComparer.Ordinal); + foreach((string? originalPath, string? replacement) in stored.SegmentRenames) + { + if(!incoming.SkippedPaths.Contains(originalPath)) + { + renames[originalPath] = replacement; + } + } foreach((string? originalPath, string? replacement) in incoming.SegmentRenames) { renames[originalPath] = replacement; } - var skips = new HashSet(stored.SkippedPaths, StringComparer.Ordinal); + var skips = new HashSet( + stored.SkippedPaths.Where(path => !incoming.SegmentRenames.ContainsKey(path)), StringComparer.Ordinal); skips.UnionWith(incoming.SkippedPaths); return new PathRecovery(renames, skips); @@ -819,7 +925,7 @@ public sealed class LibGit2GitClient : IGitClient /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { - AdvanceHeadToTrackedTip(repo); + AdvanceHeadToTrackedTip(repo, fastForwardOnly: false); Commit? tip = repo.Head.Tip; if(tip is not null) @@ -856,17 +962,24 @@ public sealed class LibGit2GitClient : IGitClient /// /// Moves the current branch's ref to its upstream tip without touching the working - /// tree. Only used for never-checked-out (marker) and recovery-managed repos, whose - /// local branch can hold no local work — so a plain ref move is safe where the - /// normal pull path would insist on a fast-forward merge. + /// tree. A repository whose clone was never checked out holds no local work, so + /// its branch simply follows origin ( false). A + /// recovery-managed repository has a working tree and an index, and someone may + /// have committed there: its branch only moves when that loses nothing. /// - private static void AdvanceHeadToTrackedTip(Repository repo) + private static void AdvanceHeadToTrackedTip(Repository repo, bool fastForwardOnly) { Branch tracked = repo.Head.TrackedBranch; if(tracked?.Tip is null || string.Equals(repo.Head.Tip?.Sha, tracked.Tip.Sha, StringComparison.Ordinal)) { return; } + + if(fastForwardOnly && repo.Head.Tip is { } head) + { + EnsureFastForward(repo, head, tracked.Tip); + } + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), tracked.Tip.Id); } -- 2.54.0 From 02817e7acc0fa9df7e1e3bbde38f96417322c911 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 14:12:20 -0600 Subject: [PATCH 2/6] fix(recovery): redesign the apply so a checked-out clone keeps its work The independent review of this branch found that converting an ordinary clone into a recovery-managed one opened new ways to lose local work: an untracked file where a file was about to be written was overwritten, and an apply that failed part-way reused the pending-checkout marker, whose handler treats the repository as never checked out and force-checks-out over it. The apply now follows one set of rules for every repository: - Which commit it writes is decided in one place. The branch only moves to origin's tip when that drops no commit; a managed repository still at the commit gclo last wrote follows a rewritten origin; local commits on top of origin are left alone; diverged histories are refused. The move happens after validation, so an uncovered invalid path leaves the branch where it was. - Before anything changes, an ordinary clone and a managed repository whose last apply finished are checked for local work: uncommitted changes to tracked files, and files gclo did not write and git does not track sitting where a file is about to be written (an ignored file is replaced, as git does). A managed repository gets this on every later pull too. - The pending marker is no longer set on a repository that was checked out. Before the first destructive step the recovery file is saved without a materialized tip, listing everything that was and will be on disk; an interrupted apply is then completed by the next pull through the same code, and what left the tree is still cleaned up. - A config flag records that a repository is recovery-managed, so deleting a corrupt recovery file (as its message advises) no longer makes the renamed tree look like uncommitted changes. - An empty clone whose origin's first commit has invalid paths can be resolved; resolving a clone that has nothing new on origin is refused instead of converting it for nothing. - A folder without files where a file belongs is replaced; one with files is refused before anything changes. Refs #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 14 +- gclo.Engine.Tests/PathRecoveryTests.cs | 10 +- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 594 +++++++++++++++++--- gclo.Engine.Tests/WorkingTreeTests.cs | 32 ++ gclo.Engine/LibGit2GitClient.cs | 411 ++++++++++---- gclo.Engine/WorkingTree.cs | 23 +- 7 files changed, 892 insertions(+), 194 deletions(-) diff --git a/README.md b/README.md index a33108c..2ed92e3 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca 8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, **Retry failed** re-runs exactly the failed rows, and **Open folder** opens the target folder in Explorer. -**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). The same applies when an update to a repository you already have brings the first such path: gclo fast-forwards it and writes the renamed tree, unless that clone has uncommitted changes or commits of its own, in which case it says so and changes nothing. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). +**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). The same applies when an update to a repository you already have brings the first such path: gclo fast-forwards it and writes the renamed tree, unless that clone has uncommitted changes, commits of its own, or an untracked file in the way, in which case it says so and changes nothing. A repository resolved this way keeps that protection on later syncs; only the renamed files themselves, which git cannot track under their new names, are overwritten when origin changes them. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). **Settings** (File > Settings…) let you set a default GitHub token (stored in the Windows Credential Manager), the default target folder (with a folder picker), the default parallelism, and the theme (System, Light, or Dark); an Advanced section toggles the startup splash and its duration. diff --git a/docs/CLI.md b/docs/CLI.md index 584d1cb..a076f8e 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -164,15 +164,19 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: - A repository that was already checked out normally, and whose update brings the first invalid path, is fast-forwarded to origin's tip and then written with the renames. gclo never discards local work: if that clone has - uncommitted changes to tracked files, or commits origin does not have, it + uncommitted changes to tracked files, commits origin does not have, or an + untracked file sitting where a file of the repository would be written, it fails with a message saying so, and nothing in it is changed. Only the working tree is renamed — the repository's history and index still hold the original paths, so `git status` reports the renamed and skipped files (and -only those) as local changes. Later syncs rewrite the whole tree, so treat a -sanitized repository as a read-only mirror: edits made there are overwritten, -though a commit made there is never dropped (the sync fails instead, as for any -clone with local commits). +only those) as local changes. Later syncs rewrite the whole tree when origin has +new commits. They protect local work the way an ordinary update does: a commit +made there is never dropped, and an uncommitted change to a tracked file or an +untracked file in the way makes the sync fail with a message instead of being +overwritten. The one exception is a file that was renamed: it sits under a name +git does not track, so an edit to it cannot be seen and is overwritten. Treat +the renamed files as read-only. #### `--json` diff --git a/gclo.Engine.Tests/PathRecoveryTests.cs b/gclo.Engine.Tests/PathRecoveryTests.cs index 4b41e48..19f6e44 100644 --- a/gclo.Engine.Tests/PathRecoveryTests.cs +++ b/gclo.Engine.Tests/PathRecoveryTests.cs @@ -397,8 +397,16 @@ public sealed class PathRecoveryTests : IDisposable await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal("locally edited", File.ReadAllText(Path.Combine(target, "keep.txt"))); - // A new upstream commit re-materializes through the mapping as before. + // A new upstream commit would rewrite the tree, and with it the local edit: + // that is refused, as an ordinary pull refuses it (#147). ReplaceForgedEntry(source, remove: "keep.txt", add: ("keep.txt", "upstream edit")); + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Contains("has uncommitted changes", ex.Message); + Assert.Equal("locally edited", File.ReadAllText(Path.Combine(target, "keep.txt"))); + + // With the edit gone, the new commit re-materializes through the mapping. + File.WriteAllText(Path.Combine(target, "keep.txt"), "keep"); await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal("upstream edit", File.ReadAllText(Path.Combine(target, "keep.txt"))); } diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs index cd79720..0eaef0f 100644 --- a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -4,6 +4,7 @@ */ using System.Text; +using System.Text.Json.Nodes; using LibGit2Sharp; using static gclo.Engine.Tests.GitTestHelpers; @@ -40,7 +41,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable - // ---------------------------------------------------------------- an ordinary clone + // ---------------------------------------------------------------- an ordinary clone: what gets delivered [Fact] public async Task OrdinaryClone_CleanAndBehind_IsFastForwardedToOriginsTip_ThenMaterialized() @@ -50,7 +51,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable // create arrives. The pull refuses, with the tree untouched. ReplaceEntry(source, remove: "gone.txt", add: ("readme.txt", "v2")); string tip = AppendForgedCommit(source, ("bad:file.txt", "bad")); - await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + await PullExpectingInvalidPathsAsync(target); Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); @@ -63,6 +64,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); Assert.False(File.Exists(Path.Combine(target, "gone.txt")), "a file that left the tree is removed, as a checkout would"); Assert.False(IsCheckoutPending(target)); + Assert.Equal(tip, MaterializedTip(target)); // And the next pull has nothing to do. await _client.PullAsync(target, Token, CancellationToken.None); @@ -72,41 +74,16 @@ public sealed class RecoveryLocalWorkTests : IDisposable [Fact] - public async Task OrdinaryClone_WithUncommittedChanges_IsRefused_AndTheChangesSurvive() + public async Task OrdinaryClone_WithNothingNewOnOrigin_HasNothingToResolve_AndStaysAnOrdinaryClone() { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - string before = HeadSha(target); - File.WriteAllText(Path.Combine(target, "readme.txt"), "my local edit"); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["anything"]), CancellationToken.None)); - Assert.Contains("has uncommitted changes", ex.Message); - Assert.Contains("gclo never discards local work", ex.Message); - Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "readme.txt"))); - Assert.Equal(before, HeadSha(target)); - Assert.False(IsCheckoutPending(target)); - Assert.False(File.Exists(RecoveryFilePath(target)), "a refused apply leaves the repository an ordinary clone"); - } - - - - [Fact] - public async Task OrdinaryClone_WithLocalCommits_IsRefused_AndTheCommitsSurvive() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - string local = CommitInWorkingTree(target, "mine.txt", "my work"); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - - Assert.Contains("has local commits that origin does not have", ex.Message); - Assert.Equal(local, HeadSha(target)); - Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + Assert.Contains("Origin has nothing new", ex.Message); Assert.False(File.Exists(RecoveryFilePath(target))); + Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); } @@ -123,37 +100,270 @@ public sealed class RecoveryLocalWorkTests : IDisposable InvalidOperationException ex = await Assert.ThrowsAsync( () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["anything"]), CancellationToken.None)); - Assert.Contains("nothing to update from on origin", ex.Message); + Assert.Contains("Origin has nothing new", ex.Message); Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); } [Fact] - public async Task OrdinaryClone_AnApplyThatFailsPartWay_LeavesTheMarker_SoTheNextPullDoesNotSayUpToDate() + public async Task EmptyClone_WhoseOriginGetsItsFirstCommitWithAnInvalidPath_CanBeResolved() + { + string source = Path.Combine(_root, "empty-source"); + Repository.Init(source); + string target = Path.Combine(_root, "empty-clone"); + await _client.CloneAsync(source, target, Token, null, CancellationToken.None); + string tip = FirstForgedCommit(source, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + await PullExpectingInvalidPathsAsync(target); + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + // The branch tracks origin, so later pulls follow it. + string next = AppendForgedCommit(source, ("more.txt", "m")); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(next, HeadSha(target)); + } + + + + // ---------------------------------------------------------------- an ordinary clone: what is refused + + [Fact] + public async Task OrdinaryClone_WithUncommittedChanges_IsRefused_AndTheChangesSurvive() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + string before = HeadSha(target); + File.WriteAllText(Path.Combine(target, "readme.txt"), "my local edit"); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Contains("has uncommitted changes", ex.Message); + Assert.Contains("readme.txt", ex.Message); + Assert.Contains("gclo never discards local work", ex.Message); + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "readme.txt"))); + Assert.Equal(before, HeadSha(target)); + Assert.False(File.Exists(RecoveryFilePath(target)), "a refused apply leaves the repository an ordinary clone"); + } + + + + [Fact] + public async Task OrdinaryClone_WithLocalCommits_IsRefused_AndTheCommitsSurvive() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + string local = CommitInWorkingTree(target, "mine.txt", "my work"); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Equal(local, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + Assert.False(File.Exists(RecoveryFilePath(target))); + } + + + + [Theory] + [InlineData("notes.txt", "notes.txt")] // where a new file of the repository goes + [InlineData("bad_file.txt", "bad_file.txt")] // where a renamed file goes + public async Task OrdinaryClone_AnUntrackedFileWhereOneWouldBeWritten_IsRefused_AndSurvives(string untracked, string named) + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + string before = HeadSha(target); + File.WriteAllText(Path.Combine(target, untracked), "mine, never committed"); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("notes.txt", "upstream notes")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains($"'{named}' is in this folder but is not part of the repository", ex.Message); + Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, untracked))); + Assert.Equal(before, HeadSha(target)); + Assert.False(File.Exists(RecoveryFilePath(target))); + } + + + + [Fact] + public async Task OrdinaryClone_AnUntrackedFileUnderANameDifferingOnlyByCase_IsRefusedToo_OnWindows() + { + if(!OperatingSystem.IsWindows()) + { + return; // elsewhere NOTES.TXT and notes.txt are two files, and neither is in the other's way + } + + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + File.WriteAllText(Path.Combine(target, "NOTES.TXT"), "mine, never committed"); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("notes.txt", "upstream notes")); + await PullExpectingInvalidPathsAsync(target); + + await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, "NOTES.TXT"))); + } + + + + [Fact] + public async Task OrdinaryClone_AFolderWithUntrackedFilesWhereAFileWouldBeWritten_IsRefused_AndSurvives() + { + (string source, string target) = await CloneOrdinaryAsync(("docs/guide.txt", "guide"), ("readme.txt", "v1")); + string mine = Path.Combine(target, "docs", "mine.txt"); + File.WriteAllText(mine, "mine, never committed"); + // Upstream turns the folder into a file. + ReplaceEntry(source, remove: "docs/guide.txt", add: ("docs", "now a file")); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + + Assert.Contains("'docs/mine.txt' is in this folder but is not part of the repository", ex.Message); + Assert.Equal("mine, never committed", File.ReadAllText(mine)); + Assert.Equal("guide", File.ReadAllText(Path.Combine(target, "docs", "guide.txt"))); + } + + + + [Fact] + public async Task OrdinaryClone_AFolderOfTheOldTreeThatBecomesAFile_IsReplaced_EvenWithEmptyFoldersLeftInIt() + { + (string source, string target) = await CloneOrdinaryAsync(("docs/guide.txt", "guide"), ("readme.txt", "v1")); + Directory.CreateDirectory(Path.Combine(target, "docs", "empty", "deeper")); + ReplaceEntry(source, remove: "docs/guide.txt", add: ("docs", "now a file")); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + + await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); + + Assert.Equal("now a file", File.ReadAllText(Path.Combine(target, "docs"))); + } + + + + [Fact] + public async Task OrdinaryClone_AnIgnoredFileInTheWay_IsOverwritten_AsGitDoes() + { + (string source, string target) = await CloneOrdinaryAsync((".gitignore", "build.log\n"), ("readme.txt", "v1")); + File.WriteAllText(Path.Combine(target, "build.log"), "local build output"); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("build.log", "now tracked upstream")); + await PullExpectingInvalidPathsAsync(target); + + await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); + + Assert.Equal("now tracked upstream", File.ReadAllText(Path.Combine(target, "build.log"))); + } + + + + // ---------------------------------------------------------------- an apply that does not finish + + [Fact] + public async Task OrdinaryClone_AnApplyThatFailsPartWay_IsNotMistakenForANeverCheckedOutClone_AndTheNextPullCompletesIt() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("gone.txt", "going")); + ReplaceEntry(source, remove: "gone.txt", add: ("vendor/lib.txt", "lib")); + string tip = AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + await FailAnApplyPartWayAsync(target, linkAt: "vendor"); + + // The pending marker means "nothing here is the user's" and its handler force- + // checks-out; it must never be set on a clone that was checked out. + Assert.False(IsCheckoutPending(target)); + // What is recorded instead: this repository is being rewritten, not finished. + Assert.True(File.Exists(RecoveryFilePath(target))); + Assert.Null(MaterializedTip(target)); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("lib", File.ReadAllText(Path.Combine(target, "vendor", "lib.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + Assert.False(File.Exists(Path.Combine(target, "gone.txt")), "what left the tree is still removed after an interrupted apply"); + } + + + + [Fact] + public async Task OrdinaryClone_AfterAnApplyThatFailedPartWay_ACommitMadeThere_IsNeverDropped() { (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); AppendForgedCommit(source, ("bad:file.txt", "bad"), ("vendor/lib.txt", "lib")); - await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); - // The folder the new tree needs already exists, as a link to somewhere else: - // the write into it is refused after the branch has moved. - string elsewhere = Path.Combine(_root, "elsewhere"); - Directory.CreateDirectory(elsewhere); - string link = Path.Combine(target, "vendor"); - WorkingTreeTests.CreateDirectoryLink(link, elsewhere); - try - { - await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); - } - finally - { - WorkingTreeTests.RemoveLink(link); - } + await PullExpectingInvalidPathsAsync(target); + await FailAnApplyPartWayAsync(target, linkAt: "vendor"); + // The user carries on working in the clone, and upstream fixes the path. + string local = CommitOnTopOfHead(target, "mine.txt", "my work"); + ReplaceEntry(source, remove: "bad:file.txt", add: ("good_file.txt", "fixed")); - Assert.True(IsCheckoutPending(target), "an interrupted apply is recorded"); - // The marker makes the next pull re-examine the tree instead of trusting it. - await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Equal(local, HeadSha(target)); + // Resolving again is refused for the same reason, not allowed because of some marker. + await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); + Assert.Equal(local, HeadSha(target)); + } + + + + [Fact] + public async Task OrdinaryClone_AfterAnApplyThatFailedPartWay_UpstreamFixingThePath_LeavesNoLeftovers() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("zz/last.txt", "last")); + await PullExpectingInvalidPathsAsync(target); + // "bad_file.txt" is written, then the apply stops at the folder "zz". + await FailAnApplyPartWayAsync(target, linkAt: "zz"); + Assert.True(File.Exists(Path.Combine(target, "bad_file.txt"))); + ReplaceEntry(source, remove: "bad:file.txt", add: ("good_file.txt", "fixed")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("fixed", File.ReadAllText(Path.Combine(target, "good_file.txt"))); + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zz", "last.txt"))); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt")), "the file written under the rename is gone with the path it stood for"); + } + + + + [Fact] + public async Task Managed_ReResolvingAtTheSameCommit_ThatFailsPartWay_IsCompletedByTheNextPull() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "managed-interrupted"); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + string tip = HeadSha(target); + // A different name for the same path; "keep.txt" is held open by a reader that + // lets others read it (so the checks before the apply pass) but not replace it: + // the apply fails after the old name was removed, before everything is written. + using(new FileStream(Path.Combine(target, "keep.txt"), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + await Assert.ThrowsAnyAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "renamed.txt")]), CancellationToken.None)); + } + Assert.Null(MaterializedTip(target)); + + // The tip did not move, but the tree is unfinished: "already at the tip" does not apply. + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "renamed.txt"))); + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); } @@ -161,13 +371,24 @@ public sealed class RecoveryLocalWorkTests : IDisposable // ---------------------------------------------------------------- a recovery-managed repository [Fact] - public async Task RecoveryManaged_WithALocalCommit_IsNotMovedOffIt_ByTheNextPull() + public async Task Managed_StillFollowsOrigin_WhenNothingLocalIsInTheWay() { - string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); - string target = await CloneExpectingInvalidPathsAsync(source, "managed-local-commit"); - await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); - // Someone commits in the recovered clone; upstream moves on as well. - string local = CommitOnTopOfHead(target, "mine.txt", "my work"); + (string source, string target) = await CreateManagedAsync("managed-follow"); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + } + + + + [Fact] + public async Task Managed_WithALocalCommitAndOriginMovedOn_IsNotMovedOffIt() + { + (string source, string target) = await CreateManagedAsync("managed-diverged"); + string local = CommitInWorkingTree(target, "mine.txt", "my work"); AppendForgedCommit(source, ("other.txt", "o")); InvalidOperationException ex = await Assert.ThrowsAsync( @@ -175,21 +396,128 @@ public sealed class RecoveryLocalWorkTests : IDisposable Assert.Contains("has local commits that origin does not have", ex.Message); Assert.Equal(local, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); } [Fact] - public async Task RecoveryManaged_StillFollowsOrigin_WhenNothingLocalIsInTheWay() + public async Task Managed_WithALocalCommitAndOriginUnchanged_SyncsWithoutComplaint_LikeAnOrdinaryCloneAheadOfOrigin() { - string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); - string target = await CloneExpectingInvalidPathsAsync(source, "managed-follow"); + (_, string target) = await CreateManagedAsync("managed-ahead"); + string local = CommitInWorkingTree(target, "mine.txt", "my work"); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(local, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + } + + + + [Fact] + public async Task Managed_WhoseOriginWasRewritten_FollowsIt_WhenNobodyCommittedThere() + { + (string source, string target) = await CreateManagedAsync("managed-rewritten"); + // Upstream replaces its history: a new root commit, unrelated to the old one. + string rewritten = RewriteHistory(source, ("bad:file.txt", "bad again"), ("fresh.txt", "fresh")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(rewritten, HeadSha(target)); + Assert.Equal("fresh", File.ReadAllText(Path.Combine(target, "fresh.txt"))); + Assert.Equal("bad again", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + Assert.False(File.Exists(Path.Combine(target, "keep.txt"))); + } + + + + [Fact] + public async Task Managed_AnUncoveredInvalidPath_FailsBeforeTheBranchMoves() + { + (string source, string target) = await CreateManagedAsync("managed-uncovered"); + string before = HeadSha(target); + AppendForgedCommit(source, ("worse:new.txt", "w")); + + await PullExpectingInvalidPathsAsync(target); + + // Nothing was delivered, so nothing moved: the branch still names the commit on disk. + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + } + + + + [Fact] + public async Task Managed_WithAnUncommittedChangeToATrackedFile_IsNotOverwrittenByTheNextCommit() + { + (string source, string target) = await CreateManagedAsync("managed-dirty"); + File.WriteAllText(Path.Combine(target, "keep.txt"), "my local edit"); + AppendForgedCommit(source, ("other.txt", "o")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("has uncommitted changes", ex.Message); + Assert.Contains("keep.txt", ex.Message); + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "other.txt"))); + } + + + + [Fact] + public async Task Managed_AnUntrackedFileWhereANewFileOfTheRepositoryGoes_IsRefused_AndSurvives() + { + (string source, string target) = await CreateManagedAsync("managed-untracked"); + File.WriteAllText(Path.Combine(target, "other.txt"), "mine, never committed"); + AppendForgedCommit(source, ("other.txt", "upstream")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("'other.txt' is in this folder but is not part of the repository", ex.Message); + Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, "other.txt"))); + } + + + + [Fact] + public async Task Managed_WhoseRecoveryFileWasDeleted_AsTheMessageForACorruptOneAdvises_CanBeResolvedAgain() + { + (string source, string target) = await CreateManagedAsync("managed-file-deleted"); + File.Delete(RecoveryFilePath(target)); + + // It is still known to be recovery-managed, so the pull reports the invalid + // path again instead of treating the renamed tree as a clean ordinary clone... + await PullExpectingInvalidPathsAsync(target); + // ...and resolving rebuilds the file instead of calling the tree "uncommitted changes". await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal(HeadSha(target), MaterializedTip(target)); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(tip, HeadSha(target)); + } + + + + [Fact] + public async Task Managed_ARecoveryFileFromABuildThatRecordedLess_IsCompleted_NotRefused() + { + (string source, string target) = await CreateManagedAsync("managed-old-file"); + // Files written before the manifest existed have neither list nor tip. + JsonNode document = JsonNode.Parse(File.ReadAllText(RecoveryFilePath(target)))!; + document.AsObject().Remove("MaterializedPaths"); + document.AsObject().Remove("MaterializedTip"); + File.WriteAllText(RecoveryFilePath(target), document.ToJsonString()); string tip = AppendForgedCommit(source, ("other.txt", "o")); await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); } @@ -237,11 +565,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable ("late.txt", "one\ntwo\n"), ("bad:file.txt", "one\ntwo\n")); string target = await CloneExpectingInvalidPathsAsync(source, "attributes"); - using(var repo = new Repository(target)) - { - // Without the attributes this setting would write CRLF. - repo.Config.Set("core.autocrlf", "true", ConfigurationLevel.Local); - } + UseCrLfByDefault(target); await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); @@ -252,6 +576,30 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task AttributeFiles_ThatChangeUpstream_ApplyToFilesWrittenBeforeThem_NotTheStaleOneOnDisk() + { + string source = CreateForgedRepo( + _root, + ("!early.txt", "one\ntwo\n"), + (".gitattributes", "*.txt text eol=crlf\n"), + ("bad:file.txt", "one\ntwo\n")); + string target = await CloneExpectingInvalidPathsAsync(source, "attributes-change"); + UseCrLfByDefault(target); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + Assert.Equal("one\r\ntwo\r\n", File.ReadAllText(Path.Combine(target, "!early.txt"))); + // Upstream switches the line endings; the old attributes file is still on disk + // when "!early.txt", which sorts before it, is rewritten. + ReplaceEntry(source, remove: ".gitattributes", add: (".gitattributes", "*.txt text eol=lf\n")); + AppendForgedCommit(source, ("!early.txt", "one\ntwo\nthree\n")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("one\ntwo\nthree\n", File.ReadAllText(Path.Combine(target, "!early.txt"))); + } + + + // ---------------------------------------------------------------- the newer choice wins [Fact] @@ -265,8 +613,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); - string json = File.ReadAllText(RecoveryFilePath(target)); - Assert.Contains("bad_file.txt", json); + Assert.Contains("bad_file.txt", File.ReadAllText(RecoveryFilePath(target))); } @@ -274,9 +621,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable [Fact] public async Task AStoredRename_IsReplacedByALaterSkip_OfTheSamePath() { - string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); - string target = await CloneExpectingInvalidPathsAsync(source, "rename-then-skip"); - await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + (_, string target) = await CreateManagedAsync("rename-then-skip"); Assert.True(File.Exists(Path.Combine(target, "bad_file.txt"))); await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); @@ -301,6 +646,17 @@ public sealed class RecoveryLocalWorkTests : IDisposable + /// A source with one invalid path and "keep.txt", and a clone of it resolved by renaming that path. + private async Task<(string Source, string Target)> CreateManagedAsync(string name) + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, name); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + return (source, target); + } + + + private async Task CloneExpectingInvalidPathsAsync(string source, string name) { string target = Path.Combine(_root, name); @@ -311,6 +667,36 @@ public sealed class RecoveryLocalWorkTests : IDisposable + private Task PullExpectingInvalidPathsAsync(string target) + => Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + + + + /// + /// Renames "bad:file.txt" in a way that fails after the apply's point of no return: + /// the folder of the new tree already exists as a link to + /// somewhere else, which the write refuses. The link is removed again. + /// + private async Task FailAnApplyPartWayAsync(string target, string linkAt) + { + string elsewhere = Path.Combine(_root, "elsewhere-" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(elsewhere); + string link = Path.Combine(target, linkAt); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + Assert.Contains("is a link to another location", ex.Message); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + private static PathRecovery Recovery((string From, string To)[]? renames = null, string[]? skips = null) => new( (renames ?? []).ToDictionary(r => r.From, r => r.To, StringComparer.Ordinal), @@ -324,8 +710,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable using var repo = new Repository(workdir); Commit head = repo.Head.Tip; TreeDefinition definition = TreeDefinition.From(head.Tree).Remove(remove); - Blob blob = repo.ObjectDatabase.CreateBlob(new MemoryStream(Encoding.UTF8.GetBytes(add.Content))); - definition.Add(add.Name, blob, Mode.NonExecutableFile); + definition.Add(add.Name, NewBlob(repo, add.Content), Mode.NonExecutableFile); Signature signature = MakeSignature(); Commit commit = repo.ObjectDatabase.CreateCommit( signature, signature, "forged change", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); @@ -334,6 +719,47 @@ public sealed class RecoveryLocalWorkTests : IDisposable + /// The first commit of a repository that has none yet; returns its SHA. + private static string FirstForgedCommit(string workdir, params (string Name, string Content)[] entries) + { + using var repo = new Repository(workdir); + Commit commit = NewRootCommit(repo, entries); + repo.Refs.Add(repo.Refs.Head.TargetIdentifier, commit.Id); + return commit.Sha; + } + + + + /// Replaces the branch's history with a single new root commit, as a force-push does; returns its SHA. + private static string RewriteHistory(string workdir, params (string Name, string Content)[] entries) + { + using var repo = new Repository(workdir); + Commit commit = NewRootCommit(repo, entries); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + return commit.Sha; + } + + + + private static Commit NewRootCommit(Repository repo, (string Name, string Content)[] entries) + { + var definition = new TreeDefinition(); + foreach((string name, string content) in entries) + { + definition.Add(name, NewBlob(repo, content), Mode.NonExecutableFile); + } + Signature signature = MakeSignature(); + return repo.ObjectDatabase.CreateCommit( + signature, signature, "forged root", repo.ObjectDatabase.CreateTree(definition), [], prettifyMessage: false); + } + + + + private static Blob NewBlob(Repository repo, string content) + => repo.ObjectDatabase.CreateBlob(new MemoryStream(Encoding.UTF8.GetBytes(content))); + + + /// An ordinary commit made in the working tree, the way a person would. private static string CommitInWorkingTree(string workdir, string fileName, string content) { @@ -348,14 +774,15 @@ public sealed class RecoveryLocalWorkTests : IDisposable /// /// A commit on top of HEAD that adds one file, made through the object database: - /// a recovery-managed tree holds paths the index tooling cannot stage on Windows. + /// for a tree that an interrupted apply left half written, where staging would + /// pick up the mess. /// private static string CommitOnTopOfHead(string workdir, string fileName, string content) { using var repo = new Repository(workdir); Commit head = repo.Head.Tip; var definition = TreeDefinition.From(head.Tree); - definition.Add(fileName, repo.ObjectDatabase.CreateBlob(new MemoryStream(Encoding.UTF8.GetBytes(content))), Mode.NonExecutableFile); + definition.Add(fileName, NewBlob(repo, content), Mode.NonExecutableFile); Signature signature = MakeSignature(); Commit commit = repo.ObjectDatabase.CreateCommit( signature, signature, "local work", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); @@ -365,6 +792,15 @@ public sealed class RecoveryLocalWorkTests : IDisposable + /// Without attributes, this setting writes text files with CRLF line endings. + private static void UseCrLfByDefault(string repoPath) + { + using var repo = new Repository(repoPath); + repo.Config.Set("core.autocrlf", "true", ConfigurationLevel.Local); + } + + + private static bool IsCheckoutPending(string repoPath) { using var repo = new Repository(repoPath); @@ -378,4 +814,10 @@ public sealed class RecoveryLocalWorkTests : IDisposable using var repo = new Repository(repoPath); return Path.Combine(repo.Info.Path, "gclo-recovery.json"); } + + + + /// The commit the recovery file says is on disk; null while an apply is unfinished. + private static string? MaterializedTip(string repoPath) + => JsonNode.Parse(File.ReadAllText(RecoveryFilePath(repoPath)))!["MaterializedTip"]?.GetValue(); } diff --git a/gclo.Engine.Tests/WorkingTreeTests.cs b/gclo.Engine.Tests/WorkingTreeTests.cs index fd141b7..f320382 100644 --- a/gclo.Engine.Tests/WorkingTreeTests.cs +++ b/gclo.Engine.Tests/WorkingTreeTests.cs @@ -329,6 +329,38 @@ public sealed class WorkingTreeTests : IDisposable + [Fact] + public void Create_WhereAFolderWithoutFilesSits_ReplacesItWithTheFile() + { + // Upstream turned a folder into a file; its files left with the old tree, and + // empty folders are nobody's. + Directory.CreateDirectory(Path.Combine(_workdir, "docs", "empty", "deeper")); + + using(FileStream file = WorkingTree.Create(_workdir, "docs")) + { + file.Write("now a file"u8); + } + + Assert.Equal("now a file", File.ReadAllText(Path.Combine(_workdir, "docs"))); + } + + + + [Fact] + public void Create_WhereAFolderWithFilesSits_IsRefused_AndTheFilesSurvive() + { + string mine = Path.Combine(_workdir, "docs", "deeper", "mine.txt"); + Directory.CreateDirectory(Path.GetDirectoryName(mine)!); + File.WriteAllText(mine, "not gclo's to remove"); + + InvalidOperationException ex = Assert.Throws(() => WorkingTree.Create(_workdir, "docs")); + + Assert.Contains("is a folder with files in it", ex.Message); + Assert.Equal("not gclo's to remove", File.ReadAllText(mine)); + } + + + [Fact] public void Create_OutsideOrInGit_IsRefused_BeforeAnythingIsTouched() { diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 2463271..cd0458a 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -47,6 +47,14 @@ public sealed class LibGit2GitClient : IGitClient /// private const string CheckoutPendingKey = "gclo.checkoutpending"; + /// + /// Local git config flag meaning "gclo writes this repository's working tree + /// itself, through a path recovery". It outlives the recovery file, so a + /// repository whose file was deleted (as the message for a corrupt one advises) + /// is still known to be recovery-managed and can be resolved again (#147). + /// + private const string RecoveryManagedKey = "gclo.recoverymanaged"; + /// File name (under .git) of the persisted . private const string RecoveryFileName = "gclo-recovery.json"; @@ -266,24 +274,16 @@ public sealed class LibGit2GitClient : IGitClient // recovery). Both take a dedicated path — their working tree is materialized // from the tip, never by a merge checkout that trusts the existing tree. // Every other repository takes the normal fetch + fast-forward pull below. - string recoveryFile = GetRecoveryFilePath(repo); - if(File.Exists(recoveryFile)) + if(File.Exists(GetRecoveryFilePath(repo)) || IsRecoveryManaged(repo)) { - // A recovery-managed repo stays recovery-managed: re-materialize the new - // tip through the stored mapping. Freshly-invalid paths the mapping does - // not cover surface as a typed failure listing the effective paths. - // Its working tree is written by gclo, but its branch is still the user's: - // a commit made there must not be dropped by moving the branch (#147). - AdvanceHeadToTrackedTip(repo, fastForwardOnly: true); - RecoveryDocument stored = LoadRecoveryDocument(recoveryFile, WorkingDirectoryOf(repo)); - if(stored.MaterializedTip is not null && string.Equals(stored.MaterializedTip, repo.Head.Tip?.Sha, StringComparison.Ordinal)) - { - // The tree on disk already reflects this tip: nothing to rewrite. A - // recovery-managed repo used to re-materialize every blob on every - // sync, touching gigabytes and every mtime for zero change (#31). - return; - } - ApplyRecoveryCore(repo, ToRecovery(stored), ct); + // A recovery-managed repo stays recovery-managed: the new tip is written + // through the stored mapping, or nothing is touched when the tree on disk + // already reflects it (a managed repo used to re-materialize every blob on + // every sync, touching gigabytes and every mtime for zero change, #31). + // Freshly-invalid paths the mapping does not cover surface as a typed + // failure listing the effective paths. + repo.Config.Set(RecoveryManagedKey, true, ConfigurationLevel.Local); + ApplyRecoveryCore(repo, incoming: null, ct); return; } @@ -668,26 +668,82 @@ public sealed class LibGit2GitClient : IGitClient /// - /// Materializes a tree into the working directory with the recovery's renames and - /// skips applied, after validating that the effective path set is actually creatable - /// on Windows. Any recovery persisted from an earlier run is merged in first (the - /// incoming recovery wins on conflicts), so a recovery-managed repo that gains new - /// invalid paths can be fixed incrementally. Persists the merged recovery so later - /// pulls re-apply it, then clears the pending-checkout marker. + /// Where a repository stands with respect to path recovery. It decides which commit + /// an apply writes and how much of what is on disk may be the user's (#147). + /// + private enum RecoveryStanding + { + /// Fetched but never checked out: the marker is set and nothing is recorded. Nothing on disk is the user's. + NeverCheckedOut, + + /// An ordinary, checked-out clone. Everything on disk may be the user's work. + Ordinary, + + /// Recovery-managed, and the last apply finished: the manifest says exactly what gclo wrote, for which commit. + Managed, + + /// + /// Recovery-managed, but the manifest is missing, from a build that recorded + /// less, or left by an apply that did not finish. What is on disk cannot be + /// told apart from the user's, so the apply simply completes. + /// + Unsettled, + } + + + + private static bool IsRecoveryManaged(Repository repo) + => repo.Config.Get(RecoveryManagedKey)?.Value == true; + + + + private static RecoveryStanding StandingOf(Repository repo, RecoveryDocument? stored) + { + if(stored is not null) + { + return stored.MaterializedTip is null || stored.MaterializedPaths is null + ? RecoveryStanding.Unsettled + : RecoveryStanding.Managed; + } + + if(IsRecoveryManaged(repo)) + { + return RecoveryStanding.Unsettled; + } + + return IsCheckoutPending(repo) ? RecoveryStanding.NeverCheckedOut : RecoveryStanding.Ordinary; + } + + + + /// + /// Materializes a commit's tree into the working directory with renames and skips + /// applied, after validating that the effective path set is actually creatable on + /// Windows. is what the user just chose (it wins over + /// stored choices path by path), or null when a pull re-applies what is stored. /// /// - /// Which tree depends on where the repository stands (#147): + /// + /// gclo never discards local work, and this is the one place that writes a working + /// tree by hand, so it is where that promise is easiest to break. The rules (#147): + /// /// - /// Fetched but never checked out (the marker is set), or already - /// recovery-managed: HEAD's tree. Nothing of the user's is on disk, or what is there - /// was written by an earlier apply. - /// An ordinary, checked-out clone whose pull found invalid paths in the - /// incoming commits: origin's tip, after proving that nothing local is lost by going - /// there (see ). Materializing HEAD - /// would rewrite the commit that is already checked out and deliver nothing. + /// The commit written is chosen by : origin's tip when + /// going there loses no commit, the current commit otherwise, a refusal when the + /// histories diverged. + /// For an ordinary clone and for a managed repository whose last apply + /// finished, stops the apply before + /// anything changes when tracked files were modified or when a file gclo did not + /// write sits where one is about to be written. + /// Before the first destructive step the recovery file is saved without a + /// materialized tip, listing everything that was and will be on disk. If the apply + /// is interrupted, the next pull finds that, knows the tree is unfinished, and + /// completes it through this same method. The pending-checkout marker is never set + /// on a repository that was checked out: that marker means "nothing here is the + /// user's", and its handler acts accordingly. /// /// - private static void ApplyRecoveryCore(Repository repo, PathRecovery recovery, CancellationToken ct) + private static void ApplyRecoveryCore(Repository repo, PathRecovery? incoming, CancellationToken ct) { if(OperatingSystem.IsWindows()) { @@ -696,58 +752,54 @@ public sealed class LibGit2GitClient : IGitClient string workdir = WorkingDirectoryOf(repo); string recoveryFile = GetRecoveryFilePath(repo); - RecoveryDocument? previous = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null; - if(previous is not null) - { - recovery = MergeRecoveries(stored: ToRecovery(previous), incoming: recovery); - } + RecoveryDocument? stored = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null; + RecoveryStanding standing = StandingOf(repo, stored); + PathRecovery storedRecovery = stored is null ? s_noRecovery : ToRecovery(stored); + PathRecovery recovery = MergeRecoveries(storedRecovery, incoming ?? s_noRecovery); - Commit? fastForwardTo = previous is null && !IsCheckoutPending(repo) ? FastForwardTargetOfOrdinaryClone(repo) : null; - Commit tip = fastForwardTo - ?? repo.Head.Tip - ?? throw new InvalidOperationException("Repository has no commits; nothing to materialize."); + Commit? head = repo.Head.Tip; + Commit tip = TargetOf(repo, standing, stored); + if(incoming is null && string.Equals(stored?.MaterializedTip, tip.Sha, StringComparison.Ordinal)) + { + return; // the tree on disk already is this commit, written through these choices + } List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(tip.Tree, recovery); // Validate BEFORE writing anything: a bad mapping (still-invalid segment, or // two originals landing on one destination) must fail cleanly with the - // effective paths, leaving the working tree untouched. + // effective paths, leaving the working tree and the branch untouched. IReadOnlyList stillInvalid = WindowsPathValidator.ValidatePaths(entries.Select(e => e.EffectivePath)); if(stillInvalid.Count > 0) { throw new InvalidRepositoryPathsException(stillInvalid); } - ///////////////////////////////////////////////////////////////////////////// - // A real checkout removes what left the tree; this materialization must too, // - // or the working tree accumulates every file upstream ever deleted or // - // renamed (#31). What "was there" is the previous apply's manifest, or, for // - // an ordinary clone taking this path for the first time, the files of the // - // commit that is checked out now. // - ///////////////////////////////////////////////////////////////////////////// - IEnumerable onDiskBefore = previous?.MaterializedPaths - ?? (fastForwardTo is null ? [] : CollectEffectiveEntries(repo.Head.Tip.Tree, s_noRecovery).Select(e => e.EffectivePath).ToList()); - - ///////////////////////////////////////////////////////////////////////////// - // From here until the end the tree on disk is in between two states. The // - // marker says so durably: if this is interrupted (a failure part-way, the // - // process dying), the next pull completes or re-reports instead of calling // - // a partial tree up to date (#147). // - ///////////////////////////////////////////////////////////////////////////// - repo.Config.Set(CheckoutPendingKey, true, ConfigurationLevel.Local); - if(fastForwardTo is not null) + HashSet? ours = OursOnDisk(standing, stored, head); + if(ours is not null && standing is RecoveryStanding.Ordinary or RecoveryStanding.Managed) { - repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), fastForwardTo.Id); + RefuseToOverwriteLocalWork(repo, workdir, standing, storedRecovery, entries, ours!); } - // The index describes the commit, as after any checkout; without it git reports - // every file as deleted and untracked. Only renamed and skipped paths differ - // from it on disk. Written first so attribute lookups can already see the tree. + var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); + + ///////////////////////////////////////////////////////////////////////////// + // The point of no return. Record first that this tree is being rewritten, // + // and everything that was and will be on disk by gclo's doing; then move the // + // branch, describe the commit in the index (as after any checkout: without // + // it git reports every file as deleted, and attribute lookups can already // + // see the new tree), remove what left the tree, and write the files. // + ///////////////////////////////////////////////////////////////////////////// + SaveRecovery(repo, recovery, (ours ?? []).Union(effectiveNow, StringComparer.Ordinal), materializedTip: null); + repo.Config.Set(RecoveryManagedKey, true, ConfigurationLevel.Local); + MoveHeadTo(repo, tip); repo.Index.Replace(tip.Tree); repo.Index.Write(); - var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); - foreach(string stale in onDiskBefore) + // A real checkout removes what left the tree; this materialization must too, + // or the working tree accumulates every file upstream ever deleted or renamed + // (#31). + foreach(string stale in ours ?? []) { if(!effectiveNow.Contains(stale)) { @@ -757,21 +809,20 @@ public sealed class LibGit2GitClient : IGitClient // Parent folders are shared by many files; each is checked for a link once. // Attribute files go first: they decide how the files after them are written - // (line endings), exactly as a checkout applies them to the whole tree. + // (line endings), and a stale one on disk would otherwise outrank the index. var verifiedFolders = new HashSet(StringComparer.Ordinal); foreach((string? originalPath, string? effectivePath, Blob? blob) in entries.OrderBy(e => IsAttributesFile(e.OriginalPath) ? 0 : 1)) { ct.ThrowIfCancellationRequested(); // Filter through .gitattributes (CRLF etc.) under the ORIGINAL repo path, - // exactly as a checkout of that entry would. Force semantics: whatever is - // at the destination is replaced, read-only or not (see WorkingTree). + // exactly as a checkout of that entry would. using Stream content = blob.GetContentStream(new FilteringOptions(originalPath)); using FileStream file = WorkingTree.Create(workdir, effectivePath, verifiedFolders); content.CopyTo(file); } - SaveRecovery(repo, recovery, entries.Select(e => e.EffectivePath), tip.Sha); + SaveRecovery(repo, recovery, effectiveNow, tip.Sha); repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); } @@ -784,45 +835,196 @@ public sealed class LibGit2GitClient : IGitClient /// - /// For an ordinary, checked-out clone that is about to become recovery-managed: - /// the commit to go to (origin's tip for the current branch), once it is certain - /// that going there loses nothing. gclo never discards local work, on this path - /// either: uncommitted changes to tracked files, or commits origin does not have, - /// stop the apply with the same advice the ordinary pull gives (#147). + /// The commit an apply writes. gclo only ever moves a branch forward to what origin + /// has, and only when that drops no commit (#147): + /// + /// Never checked out: the current commit, which the clone or the last pull put + /// there; nothing local exists. + /// Origin has nothing the current commit lacks (no upstream, the same commit, + /// or local commits on top of it): the current commit. For an ordinary clone that + /// means there is nothing to resolve, and the apply says so instead of turning a + /// working clone into a managed one for no reason. + /// The current commit is contained in origin's tip: origin's tip. + /// The histories diverged: origin's tip only for a managed repository still at + /// the commit gclo last wrote (nobody committed; origin was rewritten). Otherwise + /// the refusal the ordinary pull gives. + /// /// - private static Commit FastForwardTargetOfOrdinaryClone(Repository repo) + private static Commit TargetOf(Repository repo, RecoveryStanding standing, RecoveryDocument? stored) { Commit? head = repo.Head.Tip; - Commit? upstream = repo.Head.TrackedBranch?.Tip; - if(head is null || upstream is null) + if(standing == RecoveryStanding.NeverCheckedOut) { - throw new InvalidOperationException( - "This repository's current branch has nothing to update from on origin, so there are no paths to resolve. " - + "Sync it again; if it keeps failing, delete the folder to re-clone."); + return head ?? throw new InvalidOperationException("Repository has no commits; nothing to materialize."); } - if(repo.RetrieveStatus(new StatusOptions { IncludeUntracked = false, IncludeIgnored = false }).IsDirty) + if(head is null) { - throw new InvalidOperationException( - $"'{repo.Head.FriendlyName}' has uncommitted changes, and resolving paths rewrites the working tree; " - + "gclo never discards local work. Commit or discard the changes, then resolve again."); + // An empty clone whose origin has commits by now: its first checkout. + string branchName = repo.Refs.Head.TargetIdentifier["refs/heads/".Length..]; + return repo.Branches["origin/" + branchName]?.Tip + ?? throw new InvalidOperationException("This repository is empty, and so is its branch on origin; there is nothing to resolve."); } - EnsureFastForward(repo, head, upstream); - return upstream; + Commit? upstream = repo.Info.IsHeadDetached ? null : repo.Head.TrackedBranch?.Tip; + string? mergeBase = upstream is null ? null : repo.ObjectDatabase.FindMergeBase(head, upstream)?.Sha; + if(upstream is null || string.Equals(mergeBase, upstream.Sha, StringComparison.Ordinal)) + { + if(standing == RecoveryStanding.Ordinary) + { + throw new InvalidOperationException( + "Origin has nothing new for the branch that is checked out here, so there are no paths to resolve. " + + "Sync this repository again; if it keeps failing, delete the folder to re-clone."); + } + + return head; + } + + if(string.Equals(mergeBase, head.Sha, StringComparison.Ordinal)) + { + return upstream; + } + + if(standing == RecoveryStanding.Managed && string.Equals(stored!.MaterializedTip, head.Sha, StringComparison.Ordinal)) + { + return upstream; + } + + throw new InvalidOperationException( + $"'{repo.Head.FriendlyName}' has local commits that origin does not have; gclo only fast-forwards and never merges. " + + "Push or reset the local commits, or delete the folder to re-clone."); } - /// Throws the "local commits" refusal unless is contained in . - private static void EnsureFastForward(Repository repo, Commit from, Commit to) + /// Points the current branch at , creating it (with its upstream) for an empty clone. + private static void MoveHeadTo(Repository repo, Commit tip) { - if(!string.Equals(repo.ObjectDatabase.FindMergeBase(from, to)?.Sha, from.Sha, StringComparison.Ordinal)) + if(repo.Info.IsHeadUnborn) { - throw new InvalidOperationException( - $"'{repo.Head.FriendlyName}' has local commits that origin does not have; gclo only fast-forwards and never merges. " - + "Push or reset the local commits, or delete the folder to re-clone."); + string targetRef = repo.Refs.Head.TargetIdentifier; + repo.Refs.Add(targetRef, tip.Id); + Branch local = repo.Branches[targetRef["refs/heads/".Length..]]; + repo.Branches.Update(local, b => + { + b.Remote = "origin"; + b.UpstreamBranch = targetRef; + }); + return; } + + if(!string.Equals(repo.Head.Tip.Sha, tip.Sha, StringComparison.Ordinal)) + { + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), tip.Id); + } + } + + + + private static StringComparer PathNameComparer + => OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; + + + + /// + /// The files on disk that are there by gclo's doing (or by an ordinary checkout of + /// the current commit), as repository paths: they may be replaced, and removed when + /// they leave the tree. Null when that cannot be known. + /// + private static HashSet? OursOnDisk(RecoveryStanding standing, RecoveryDocument? stored, Commit? head) + { + if(standing == RecoveryStanding.Ordinary) + { + return new HashSet( + head is null ? [] : CollectEffectiveEntries(head.Tree, s_noRecovery).Select(e => e.EffectivePath), PathNameComparer); + } + + return stored?.MaterializedPaths is null ? null : new HashSet(stored.MaterializedPaths, PathNameComparer); + } + + + + /// + /// Stops an apply, before anything has changed, when it would destroy something + /// that is the user's: a tracked file with uncommitted changes, or a file gclo did + /// not write sitting where one is about to be written. An ordinary fast-forward + /// refuses in the same two cases (#147). + /// + /// + /// In a managed repository the files a recovery renamed or skipped are missing + /// under their original names by design, and are not changes. A file that was + /// renamed is on disk under a name git does not track, so an edit made to it + /// cannot be seen here and is overwritten: the price of a tree git cannot + /// represent. + /// + private static void RefuseToOverwriteLocalWork( + Repository repo, + string workdir, + RecoveryStanding standing, + PathRecovery storedRecovery, + List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, + HashSet ours) + { + foreach(StatusEntry entry in repo.RetrieveStatus(new StatusOptions { IncludeUntracked = false, IncludeIgnored = false })) + { + if(standing == RecoveryStanding.Managed && IsMovedBy(storedRecovery, entry.FilePath)) + { + continue; + } + + throw new InvalidOperationException( + $"'{repo.Head.FriendlyName}' has uncommitted changes (for example to '{entry.FilePath}'), and this would rewrite the working tree; " + + "gclo never discards local work. Commit or discard the changes, then try again."); + } + + foreach((_, string? effectivePath, _) in entries) + { + // Replaceable: what gclo wrote, and what git tracks here (the loop above + // has just shown that tracked files carry no uncommitted changes). + if(ours.Contains(effectivePath) || repo.Index[effectivePath] is not null) + { + continue; + } + + string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); + string? inTheWay = null; + if(Directory.Exists(full)) + { + // A folder where a file is about to go. It leaves with the old tree when + // everything in it is ours; anything else in it is the user's. + inTheWay = Directory.EnumerateFiles(full, "*", SearchOption.AllDirectories) + .Select(f => Path.GetRelativePath(workdir, f).Replace(Path.DirectorySeparatorChar, '/')) + .FirstOrDefault(p => !ours.Contains(p)); + } + else if(File.Exists(full) && !repo.Ignore.IsPathIgnored(effectivePath)) + { + inTheWay = effectivePath; + } + + if(inTheWay is not null) + { + throw new InvalidOperationException( + $"'{inTheWay}' is in this folder but is not part of the repository (an untracked file), and a file of the repository " + + "is about to be written there; gclo never discards local work. Move or delete it, then try again."); + } + } + } + + + + /// True when , or a folder above it, is renamed or skipped by . + private static bool IsMovedBy(PathRecovery recovery, string repoPath) + { + for(int end = repoPath.Length; end > 0; end = repoPath.LastIndexOf('/', end - 1)) + { + string prefix = repoPath[..end]; + if(recovery.SkippedPaths.Contains(prefix) || recovery.SegmentRenames.ContainsKey(prefix)) + { + return true; + } + } + + return false; } @@ -925,7 +1127,7 @@ public sealed class LibGit2GitClient : IGitClient /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { - AdvanceHeadToTrackedTip(repo, fastForwardOnly: false); + AdvanceHeadToTrackedTip(repo); Commit? tip = repo.Head.Tip; if(tip is not null) @@ -962,24 +1164,17 @@ public sealed class LibGit2GitClient : IGitClient /// /// Moves the current branch's ref to its upstream tip without touching the working - /// tree. A repository whose clone was never checked out holds no local work, so - /// its branch simply follows origin ( false). A - /// recovery-managed repository has a working tree and an index, and someone may - /// have committed there: its branch only moves when that loses nothing. + /// tree. Only for a repository whose clone was never checked out: it holds no local + /// work, so its branch simply follows origin. A repository that has a working tree + /// goes through instead. /// - private static void AdvanceHeadToTrackedTip(Repository repo, bool fastForwardOnly) + private static void AdvanceHeadToTrackedTip(Repository repo) { Branch tracked = repo.Head.TrackedBranch; if(tracked?.Tip is null || string.Equals(repo.Head.Tip?.Sha, tracked.Tip.Sha, StringComparison.Ordinal)) { return; } - - if(fastForwardOnly && repo.Head.Tip is { } head) - { - EnsureFastForward(repo, head, tracked.Tip); - } - repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), tracked.Tip.Id); } @@ -999,7 +1194,7 @@ public sealed class LibGit2GitClient : IGitClient /// Original path to replacement path map; null when absent from the file. /// Paths omitted from the working tree; null when absent from the file. /// Effective paths the last apply wrote, so the next one can delete what left the tree; null in files from before this field existed. - /// SHA of the commit the last apply materialized; null in older files. + /// SHA of the commit the last apply materialized; null in older files, and while an apply is under way (so a tree left unfinished is never taken for done). private sealed record RecoveryDocument( Dictionary? SegmentRenames, List? SkippedPaths, @@ -1009,13 +1204,13 @@ public sealed class LibGit2GitClient : IGitClient private static void SaveRecovery( - Repository repo, PathRecovery recovery, IEnumerable materializedPaths, string tipSha) + Repository repo, PathRecovery recovery, IEnumerable materializedPaths, string? materializedTip) { var document = new RecoveryDocument( new Dictionary(recovery.SegmentRenames, StringComparer.Ordinal), recovery.SkippedPaths.Order(StringComparer.Ordinal).ToList(), materializedPaths.Order(StringComparer.Ordinal).ToList(), - tipSha); + materializedTip); // Atomic replace: a crash mid-write must not leave a truncated document that // fails every later pull of this repository (#31). diff --git a/gclo.Engine/WorkingTree.cs b/gclo.Engine/WorkingTree.cs index f7b991c..a9d7fc0 100644 --- a/gclo.Engine/WorkingTree.cs +++ b/gclo.Engine/WorkingTree.cs @@ -213,9 +213,10 @@ internal static class WorkingTree /// - /// Removes what is at when it is a file or a link, read-only - /// or not: a link is removed itself (even one whose target is gone, even one to a - /// directory), never what it points at. + /// Removes what is at when it is a file, a link, or a folder + /// that holds no files, read-only or not: a link is removed itself (even one whose + /// target is gone, even one to a directory), never what it points at. A folder that + /// still holds files is refused. /// private static void RemoveFile(string full) { @@ -226,6 +227,22 @@ internal static class WorkingTree return; } + if(directory.Exists) + { + // A real folder where a file belongs (upstream turned a folder into a file). + // Whatever the tree had in it is gone by now; if files remain they are not + // gclo's to remove, and empty folders are nobody's. + if(directory.EnumerateFiles("*", SearchOption.AllDirectories).Any()) + { + throw new InvalidOperationException( + $"'{full}' is a folder with files in it, where a file of the repository belongs. " + + "Move or delete the folder, then try again."); + } + + DeleteDirectory(full); + return; + } + var file = new FileInfo(full); bool isLink = file.LinkTarget is not null; if(!file.Exists && !isLink) -- 2.54.0 From af43cc95fc076f4f6b4d4377a456b1765fc71e93 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 14:46:55 -0600 Subject: [PATCH 3/6] fix(recovery): never rewrite a checked-out clone; keep managed trees simple Two independent reviews of this branch found that rewriting a clone someone checked out cannot be made safe by checks: untracked files, files git was told to ignore changes in, an apply interrupted and edited since, and git's own view of a renamed tree each defeated one attempt, and the checks added for managed repositories stopped repositories managed by the released build from updating. So the rule is the simple one: - A clone that was checked out normally is never rewritten. Resolve and --sanitize-paths refuse it with a message that says how to get a fresh clone with the renames; nothing in it changes. This replaces the behaviour this branch set out to fix (the tree of the commit already checked out was rewritten with force, and later pulls moved the branch off local commits). - A repository gclo manages, or one that was never checked out, keeps the semantics it has in the release: gclo writes its tree, and the documentation says to treat it as a read-only mirror. With these fixes for managed repositories, each with a regression test: - The branch never moves off a commit made in the repository. It follows a rewritten origin only when the current commit is one origin's branch itself once pointed at, shown by the remote-tracking branch's reflog; a local commit that was synced once is not mistaken for that. Local commits on top of origin are left alone instead of being moved back. - The branch moves after validation, so an uncovered invalid path leaves it where it was. - What would fail half-way (an unusable destination, a link on the way, a file where a folder is needed) is found before anything changes. - An interrupted apply is completed by the next pull, through a recovery file saved without a materialized tip before the first destructive step. The pending-checkout marker is not used for a repository that has a tree. - A config flag, set once a recovery has been read or written, keeps a repository known as managed when its recovery file is deleted as the message for a corrupt one advises. An unreadable file in an ordinary clone sets nothing. - The index is written, attribute files are materialized first, the newer choice wins when recoveries merge, and an empty clone whose origin's first commit has invalid paths can be resolved. Refs #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 26 +- gclo.Engine.Tests/PathRecoveryTests.cs | 10 +- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 666 +++++++++----------- gclo.Engine.Tests/WorkingTreeTests.cs | 39 ++ gclo.Engine/IGitClient.cs | 26 +- gclo.Engine/LibGit2GitClient.cs | 317 ++++------ gclo.Engine/WorkingTree.cs | 50 +- 8 files changed, 506 insertions(+), 630 deletions(-) diff --git a/README.md b/README.md index 2ed92e3..a716b63 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca 8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, **Retry failed** re-runs exactly the failed rows, and **Open folder** opens the target folder in Explorer. -**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). The same applies when an update to a repository you already have brings the first such path: gclo fast-forwards it and writes the renamed tree, unless that clone has uncommitted changes, commits of its own, or an untracked file in the way, in which case it says so and changes nothing. A repository resolved this way keeps that protection on later syncs; only the renamed files themselves, which git cannot track under their new names, are overwritten when origin changes them. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). +**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). This applies to a repository gclo could not check out when it cloned it. A repository that was already checked out normally is never rewritten: when an update brings it such a path, it fails with a message that says how to get a fresh clone with the renames, and nothing in it is changed. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). **Settings** (File > Settings…) let you set a default GitHub token (stored in the Windows Credential Manager), the default target folder (with a folder picker), the default parallelism, and the theme (System, Light, or Dark); an Advanced section toggles the startup splash and its duration. diff --git a/docs/CLI.md b/docs/CLI.md index a076f8e..89fe7e1 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -161,22 +161,20 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: and skips are merged into the stored mapping; a newer choice for a path replaces the stored one. Use `--include ` to target just that repository. -- A repository that was already checked out normally, and whose update brings - the first invalid path, is fast-forwarded to origin's tip and then written - with the renames. gclo never discards local work: if that clone has - uncommitted changes to tracked files, commits origin does not have, or an - untracked file sitting where a file of the repository would be written, it - fails with a message saying so, and nothing in it is changed. +- A repository that was already checked out normally is never rewritten this + way. When an update brings such a repository its first invalid path, it + fails with a message saying so, and nothing in it is changed: its working + tree may hold work, and writing it by hand could destroy that. To get it + with the renames, move or delete its folder and sync again (the fresh clone + is then sanitized), or use the desktop app's **Clone in WSL**. Only the working tree is renamed — the repository's history and index still hold -the original paths, so `git status` reports the renamed and skipped files (and -only those) as local changes. Later syncs rewrite the whole tree when origin has -new commits. They protect local work the way an ordinary update does: a commit -made there is never dropped, and an uncommitted change to a tracked file or an -untracked file in the way makes the sync fail with a message instead of being -overwritten. The one exception is a file that was renamed: it sits under a name -git does not track, so an edit to it cannot be seen and is overwritten. Treat -the renamed files as read-only. +the original paths, so `git status` reports the renamed and skipped files (and, +where two names differ only by case, their twins) as local changes. Treat a +sanitized repository as a read-only mirror: later syncs rewrite its working +tree, so edits made there are overwritten. A commit made there is never +dropped: if origin has moved on as well, the sync fails for that repository, +as it does for any clone with local commits. #### `--json` diff --git a/gclo.Engine.Tests/PathRecoveryTests.cs b/gclo.Engine.Tests/PathRecoveryTests.cs index 19f6e44..4b41e48 100644 --- a/gclo.Engine.Tests/PathRecoveryTests.cs +++ b/gclo.Engine.Tests/PathRecoveryTests.cs @@ -397,16 +397,8 @@ public sealed class PathRecoveryTests : IDisposable await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal("locally edited", File.ReadAllText(Path.Combine(target, "keep.txt"))); - // A new upstream commit would rewrite the tree, and with it the local edit: - // that is refused, as an ordinary pull refuses it (#147). + // A new upstream commit re-materializes through the mapping as before. ReplaceForgedEntry(source, remove: "keep.txt", add: ("keep.txt", "upstream edit")); - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.PullAsync(target, Token, CancellationToken.None)); - Assert.Contains("has uncommitted changes", ex.Message); - Assert.Equal("locally edited", File.ReadAllText(Path.Combine(target, "keep.txt"))); - - // With the edit gone, the new commit re-materializes through the mapping. - File.WriteAllText(Path.Combine(target, "keep.txt"), "keep"); await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal("upstream edit", File.ReadAllText(Path.Combine(target, "keep.txt"))); } diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs index 0eaef0f..f492801 100644 --- a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -13,9 +13,10 @@ namespace gclo.Engine.Tests; /// -/// Path recovery never discards local work, delivers the commits it reports, and -/// leaves a repository git itself can make sense of (#147). Real repositories on the -/// local file system, like . +/// Path recovery writes a working tree by hand, so it is only done to a tree that is +/// gclo's to write, never moves a branch off a commit made in the repository, and +/// finishes what an interrupted apply left (#147). Real repositories on the local +/// file system, like . /// public sealed class RecoveryLocalWorkTests : IDisposable { @@ -23,6 +24,8 @@ public sealed class RecoveryLocalWorkTests : IDisposable private const string PendingKey = "gclo.checkoutpending"; + private const string ManagedKey = "gclo.recoverymanaged"; + private readonly string _root = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); @@ -41,67 +44,73 @@ public sealed class RecoveryLocalWorkTests : IDisposable - // ---------------------------------------------------------------- an ordinary clone: what gets delivered + // ---------------------------------------------------------------- a clone that was checked out is never rewritten - [Fact] - public async Task OrdinaryClone_CleanAndBehind_IsFastForwardedToOriginsTip_ThenMaterialized() + [Theory] + [InlineData("clean")] + [InlineData("uncommitted edit")] + [InlineData("untracked file in the way")] + [InlineData("local commit")] + public async Task OrdinaryClone_IsRefused_WhateverItsState_AndNothingInItChanges(string state) { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("gone.txt", "going"), ("keep.txt", "keep")); - // Upstream moves on: one file changes, one goes away, and a path Windows cannot - // create arrives. The pull refuses, with the tree untouched. - ReplaceEntry(source, remove: "gone.txt", add: ("readme.txt", "v2")); - string tip = AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); - - await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); - - // Done means the incoming commits are on disk: HEAD is origin's tip, not the - // commit that was already checked out. - Assert.Equal(tip, HeadSha(target)); - Assert.Equal("v2", File.ReadAllText(Path.Combine(target, "readme.txt"))); - Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); - Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); - Assert.False(File.Exists(Path.Combine(target, "gone.txt")), "a file that left the tree is removed, as a checkout would"); - Assert.False(IsCheckoutPending(target)); - Assert.Equal(tip, MaterializedTip(target)); - - // And the next pull has nothing to do. - await _client.PullAsync(target, Token, CancellationToken.None); - Assert.Equal(tip, HeadSha(target)); - } - - - - [Fact] - public async Task OrdinaryClone_WithNothingNewOnOrigin_HasNothingToResolve_AndStaysAnOrdinaryClone() - { - (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["anything"]), CancellationToken.None)); - - Assert.Contains("Origin has nothing new", ex.Message); - Assert.False(File.Exists(RecoveryFilePath(target))); - Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); - } - - - - [Fact] - public async Task OrdinaryClone_WhoseBranchHasNoUpstream_HasNothingToResolve() - { - (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - using(var repo = new Repository(target)) + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + switch(state) { - repo.Branches.Update(repo.Head, b => b.TrackedBranch = null); + case "uncommitted edit": + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + break; + case "untracked file in the way": + File.WriteAllText(Path.Combine(target, "bad_file.txt"), "mine, never committed"); + break; + case "local commit": + CommitInWorkingTree(target, "mine.txt", "my work"); + break; + default: + break; } + string before = HeadSha(target); + Dictionary filesBefore = ReadAllFiles(target); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("readme.txt", "v2")); + await PullExpectingInvalidPathsAsync(target); InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["anything"]), CancellationToken.None)); + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); - Assert.Contains("Origin has nothing new", ex.Message); - Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); + Assert.Contains("is checked out normally", ex.Message); + Assert.Contains("move or delete the folder", ex.Message); + Assert.Contains(target, ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(filesBefore, ReadAllFiles(target)); + Assert.False(File.Exists(RecoveryFilePath(target))); + Assert.False(IsFlagSet(target, ManagedKey), "a refused apply leaves the repository an ordinary clone"); + Assert.False(IsFlagSet(target, PendingKey)); + + // And it stays an ordinary clone to the next pull, which reports the same paths. + await PullExpectingInvalidPathsAsync(target); + Assert.Equal(filesBefore, ReadAllFiles(target)); + } + + + + [Fact] + public async Task OrdinaryClone_WithAnUnreadableRecoveryFileInIt_IsNotTurnedIntoAManagedRepository() + { + (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + File.WriteAllText(RecoveryFilePath(target), "{ this is not a recovery file"); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Contains("is corrupt", ex.Message); + Assert.False(IsFlagSet(target, ManagedKey), "a file that could not be read proves nothing about the repository"); + + // The message says to delete it. After that the clone is an ordinary clone again, + // and a pull does what a pull does: it leaves local edits alone. + File.Delete(RecoveryFilePath(target)); + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "work.txt"))); + Assert.False(IsFlagSet(target, ManagedKey)); } @@ -116,6 +125,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable string tip = FirstForgedCommit(source, ("bad:file.txt", "bad"), ("keep.txt", "keep")); await PullExpectingInvalidPathsAsync(target); + // Nothing was ever checked out here, so the tree is gclo's to write. await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); Assert.Equal(tip, HeadSha(target)); @@ -125,253 +135,15 @@ public sealed class RecoveryLocalWorkTests : IDisposable string next = AppendForgedCommit(source, ("more.txt", "m")); await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal(next, HeadSha(target)); + Assert.Equal("m", File.ReadAllText(Path.Combine(target, "more.txt"))); } - // ---------------------------------------------------------------- an ordinary clone: what is refused + // ---------------------------------------------------------------- a managed repository: its branch [Fact] - public async Task OrdinaryClone_WithUncommittedChanges_IsRefused_AndTheChangesSurvive() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - string before = HeadSha(target); - File.WriteAllText(Path.Combine(target, "readme.txt"), "my local edit"); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - - Assert.Contains("has uncommitted changes", ex.Message); - Assert.Contains("readme.txt", ex.Message); - Assert.Contains("gclo never discards local work", ex.Message); - Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "readme.txt"))); - Assert.Equal(before, HeadSha(target)); - Assert.False(File.Exists(RecoveryFilePath(target)), "a refused apply leaves the repository an ordinary clone"); - } - - - - [Fact] - public async Task OrdinaryClone_WithLocalCommits_IsRefused_AndTheCommitsSurvive() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - string local = CommitInWorkingTree(target, "mine.txt", "my work"); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - - Assert.Contains("has local commits that origin does not have", ex.Message); - Assert.Equal(local, HeadSha(target)); - Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); - Assert.False(File.Exists(RecoveryFilePath(target))); - } - - - - [Theory] - [InlineData("notes.txt", "notes.txt")] // where a new file of the repository goes - [InlineData("bad_file.txt", "bad_file.txt")] // where a renamed file goes - public async Task OrdinaryClone_AnUntrackedFileWhereOneWouldBeWritten_IsRefused_AndSurvives(string untracked, string named) - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - string before = HeadSha(target); - File.WriteAllText(Path.Combine(target, untracked), "mine, never committed"); - AppendForgedCommit(source, ("bad:file.txt", "bad"), ("notes.txt", "upstream notes")); - await PullExpectingInvalidPathsAsync(target); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); - - Assert.Contains($"'{named}' is in this folder but is not part of the repository", ex.Message); - Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, untracked))); - Assert.Equal(before, HeadSha(target)); - Assert.False(File.Exists(RecoveryFilePath(target))); - } - - - - [Fact] - public async Task OrdinaryClone_AnUntrackedFileUnderANameDifferingOnlyByCase_IsRefusedToo_OnWindows() - { - if(!OperatingSystem.IsWindows()) - { - return; // elsewhere NOTES.TXT and notes.txt are two files, and neither is in the other's way - } - - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - File.WriteAllText(Path.Combine(target, "NOTES.TXT"), "mine, never committed"); - AppendForgedCommit(source, ("bad:file.txt", "bad"), ("notes.txt", "upstream notes")); - await PullExpectingInvalidPathsAsync(target); - - await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - - Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, "NOTES.TXT"))); - } - - - - [Fact] - public async Task OrdinaryClone_AFolderWithUntrackedFilesWhereAFileWouldBeWritten_IsRefused_AndSurvives() - { - (string source, string target) = await CloneOrdinaryAsync(("docs/guide.txt", "guide"), ("readme.txt", "v1")); - string mine = Path.Combine(target, "docs", "mine.txt"); - File.WriteAllText(mine, "mine, never committed"); - // Upstream turns the folder into a file. - ReplaceEntry(source, remove: "docs/guide.txt", add: ("docs", "now a file")); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - - Assert.Contains("'docs/mine.txt' is in this folder but is not part of the repository", ex.Message); - Assert.Equal("mine, never committed", File.ReadAllText(mine)); - Assert.Equal("guide", File.ReadAllText(Path.Combine(target, "docs", "guide.txt"))); - } - - - - [Fact] - public async Task OrdinaryClone_AFolderOfTheOldTreeThatBecomesAFile_IsReplaced_EvenWithEmptyFoldersLeftInIt() - { - (string source, string target) = await CloneOrdinaryAsync(("docs/guide.txt", "guide"), ("readme.txt", "v1")); - Directory.CreateDirectory(Path.Combine(target, "docs", "empty", "deeper")); - ReplaceEntry(source, remove: "docs/guide.txt", add: ("docs", "now a file")); - AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - - await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); - - Assert.Equal("now a file", File.ReadAllText(Path.Combine(target, "docs"))); - } - - - - [Fact] - public async Task OrdinaryClone_AnIgnoredFileInTheWay_IsOverwritten_AsGitDoes() - { - (string source, string target) = await CloneOrdinaryAsync((".gitignore", "build.log\n"), ("readme.txt", "v1")); - File.WriteAllText(Path.Combine(target, "build.log"), "local build output"); - AppendForgedCommit(source, ("bad:file.txt", "bad"), ("build.log", "now tracked upstream")); - await PullExpectingInvalidPathsAsync(target); - - await _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None); - - Assert.Equal("now tracked upstream", File.ReadAllText(Path.Combine(target, "build.log"))); - } - - - - // ---------------------------------------------------------------- an apply that does not finish - - [Fact] - public async Task OrdinaryClone_AnApplyThatFailsPartWay_IsNotMistakenForANeverCheckedOutClone_AndTheNextPullCompletesIt() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("gone.txt", "going")); - ReplaceEntry(source, remove: "gone.txt", add: ("vendor/lib.txt", "lib")); - string tip = AppendForgedCommit(source, ("bad:file.txt", "bad")); - await PullExpectingInvalidPathsAsync(target); - await FailAnApplyPartWayAsync(target, linkAt: "vendor"); - - // The pending marker means "nothing here is the user's" and its handler force- - // checks-out; it must never be set on a clone that was checked out. - Assert.False(IsCheckoutPending(target)); - // What is recorded instead: this repository is being rewritten, not finished. - Assert.True(File.Exists(RecoveryFilePath(target))); - Assert.Null(MaterializedTip(target)); - - await _client.PullAsync(target, Token, CancellationToken.None); - - Assert.Equal(tip, HeadSha(target)); - Assert.Equal(tip, MaterializedTip(target)); - Assert.Equal("lib", File.ReadAllText(Path.Combine(target, "vendor", "lib.txt"))); - Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); - Assert.False(File.Exists(Path.Combine(target, "gone.txt")), "what left the tree is still removed after an interrupted apply"); - } - - - - [Fact] - public async Task OrdinaryClone_AfterAnApplyThatFailedPartWay_ACommitMadeThere_IsNeverDropped() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - AppendForgedCommit(source, ("bad:file.txt", "bad"), ("vendor/lib.txt", "lib")); - await PullExpectingInvalidPathsAsync(target); - await FailAnApplyPartWayAsync(target, linkAt: "vendor"); - // The user carries on working in the clone, and upstream fixes the path. - string local = CommitOnTopOfHead(target, "mine.txt", "my work"); - ReplaceEntry(source, remove: "bad:file.txt", add: ("good_file.txt", "fixed")); - - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.PullAsync(target, Token, CancellationToken.None)); - - Assert.Contains("has local commits that origin does not have", ex.Message); - Assert.Equal(local, HeadSha(target)); - // Resolving again is refused for the same reason, not allowed because of some marker. - await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(skips: ["bad:file.txt"]), CancellationToken.None)); - Assert.Equal(local, HeadSha(target)); - } - - - - [Fact] - public async Task OrdinaryClone_AfterAnApplyThatFailedPartWay_UpstreamFixingThePath_LeavesNoLeftovers() - { - (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); - AppendForgedCommit(source, ("bad:file.txt", "bad"), ("zz/last.txt", "last")); - await PullExpectingInvalidPathsAsync(target); - // "bad_file.txt" is written, then the apply stops at the folder "zz". - await FailAnApplyPartWayAsync(target, linkAt: "zz"); - Assert.True(File.Exists(Path.Combine(target, "bad_file.txt"))); - ReplaceEntry(source, remove: "bad:file.txt", add: ("good_file.txt", "fixed")); - - await _client.PullAsync(target, Token, CancellationToken.None); - - Assert.Equal("fixed", File.ReadAllText(Path.Combine(target, "good_file.txt"))); - Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zz", "last.txt"))); - Assert.False(File.Exists(Path.Combine(target, "bad_file.txt")), "the file written under the rename is gone with the path it stood for"); - } - - - - [Fact] - public async Task Managed_ReResolvingAtTheSameCommit_ThatFailsPartWay_IsCompletedByTheNextPull() - { - string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); - string target = await CloneExpectingInvalidPathsAsync(source, "managed-interrupted"); - await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); - string tip = HeadSha(target); - // A different name for the same path; "keep.txt" is held open by a reader that - // lets others read it (so the checks before the apply pass) but not replace it: - // the apply fails after the old name was removed, before everything is written. - using(new FileStream(Path.Combine(target, "keep.txt"), FileMode.Open, FileAccess.Read, FileShare.Read)) - { - await Assert.ThrowsAnyAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "renamed.txt")]), CancellationToken.None)); - } - Assert.Null(MaterializedTip(target)); - - // The tip did not move, but the tree is unfinished: "already at the tip" does not apply. - await _client.PullAsync(target, Token, CancellationToken.None); - - Assert.Equal(tip, MaterializedTip(target)); - Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "renamed.txt"))); - Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); - Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); - } - - - - // ---------------------------------------------------------------- a recovery-managed repository - - [Fact] - public async Task Managed_StillFollowsOrigin_WhenNothingLocalIsInTheWay() + public async Task Managed_FollowsOrigin_WhenItsCommitIsContainedInOriginsTip() { (string source, string target) = await CreateManagedAsync("managed-follow"); string tip = AppendForgedCommit(source, ("other.txt", "o")); @@ -379,6 +151,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); } @@ -402,7 +175,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable [Fact] - public async Task Managed_WithALocalCommitAndOriginUnchanged_SyncsWithoutComplaint_LikeAnOrdinaryCloneAheadOfOrigin() + public async Task Managed_WithALocalCommitAndOriginUnchanged_SyncsWithoutMovingTheBranch() { (_, string target) = await CreateManagedAsync("managed-ahead"); string local = CommitInWorkingTree(target, "mine.txt", "my work"); @@ -416,6 +189,28 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task Managed_ALocalCommitThatWasSyncedOnce_IsStillNotDropped_WhenOriginMovesOn() + { + // After the first sync the local commit is the one written to disk. That must + // not be taken for "origin's commit": what counts is whether origin's branch + // ever pointed at it. + (string source, string target) = await CreateManagedAsync("managed-synced-local-commit"); + string local = CommitInWorkingTree(target, "mine.txt", "my work"); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(local, MaterializedTip(target)); + AppendForgedCommit(source, ("other.txt", "from a colleague")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Equal(local, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + } + + + [Fact] public async Task Managed_WhoseOriginWasRewritten_FollowsIt_WhenNobodyCommittedThere() { @@ -433,6 +228,28 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task Managed_OriginRewrittenAfterAnInterruptedPull_IsStillFollowed() + { + (string source, string target) = await CreateManagedAsync("managed-interrupted-then-rewritten"); + string moved = AppendForgedCommit(source, ("other.txt", "o")); + await InterruptAsync(target, "keep.txt", () => _client.PullAsync(target, Token, CancellationToken.None)); + // The branch moved before the files were written; nobody committed anything. + Assert.Equal(moved, HeadSha(target)); + Assert.Null(MaterializedTip(target)); + string rewritten = RewriteHistory(source, ("bad:file.txt", "bad again"), ("fresh.txt", "fresh")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(rewritten, HeadSha(target)); + Assert.Equal(rewritten, MaterializedTip(target)); + Assert.Equal("fresh", File.ReadAllText(Path.Combine(target, "fresh.txt"))); + Assert.False(File.Exists(Path.Combine(target, "other.txt")), "what the interrupted pull wrote and origin no longer has is gone"); + Assert.False(File.Exists(Path.Combine(target, "keep.txt"))); + } + + + [Fact] public async Task Managed_AnUncoveredInvalidPath_FailsBeforeTheBranchMoves() { @@ -445,44 +262,113 @@ public sealed class RecoveryLocalWorkTests : IDisposable // Nothing was delivered, so nothing moved: the branch still names the commit on disk. Assert.Equal(before, HeadSha(target)); Assert.Equal(before, MaterializedTip(target)); + + // Resolving the new path then delivers the commit. + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("worse:new.txt", "worse_new.txt")]), CancellationToken.None); + Assert.False(string.Equals(before, HeadSha(target), StringComparison.Ordinal), "the branch moved to origin's commit"); + Assert.Equal("w", File.ReadAllText(Path.Combine(target, "worse_new.txt"))); + } + + + + // ---------------------------------------------------------------- a managed repository: failures + + [Fact] + public async Task Managed_WhatWouldFailHalfWay_IsFoundBeforeAnythingChanges_AFileWhereAFolderIsNeeded() + { + (string source, string target) = await CreateManagedAsync("managed-file-for-folder"); + string before = HeadSha(target); + File.WriteAllText(Path.Combine(target, "docs"), "a file of the user's"); + AppendForgedCommit(source, ("docs/a.txt", "a")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("'docs' is a file in this folder", ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + Assert.Equal("a file of the user's", File.ReadAllText(Path.Combine(target, "docs"))); } [Fact] - public async Task Managed_WithAnUncommittedChangeToATrackedFile_IsNotOverwrittenByTheNextCommit() + public async Task Managed_WhatWouldFailHalfWay_IsFoundBeforeAnythingChanges_ALinkOnTheWay() { - (string source, string target) = await CreateManagedAsync("managed-dirty"); - File.WriteAllText(Path.Combine(target, "keep.txt"), "my local edit"); - AppendForgedCommit(source, ("other.txt", "o")); + (string source, string target) = await CreateManagedAsync("managed-link"); + string before = HeadSha(target); + string elsewhere = Path.Combine(_root, "elsewhere"); + Directory.CreateDirectory(elsewhere); + string link = Path.Combine(target, "vendor"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + AppendForgedCommit(source, ("vendor/lib.txt", "lib")); - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.PullAsync(target, Token, CancellationToken.None)); + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); - Assert.Contains("has uncommitted changes", ex.Message); - Assert.Contains("keep.txt", ex.Message); - Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "keep.txt"))); - Assert.False(File.Exists(Path.Combine(target, "other.txt"))); + Assert.Contains("is a link to another location", ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + Assert.Empty(Directory.EnumerateFileSystemEntries(elsewhere)); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } } [Fact] - public async Task Managed_AnUntrackedFileWhereANewFileOfTheRepositoryGoes_IsRefused_AndSurvives() + public async Task Managed_APullInterruptedHalfWay_IsCompletedByTheNext_AndWhatLeftTheTreeIsStillRemoved() { - (string source, string target) = await CreateManagedAsync("managed-untracked"); - File.WriteAllText(Path.Combine(target, "other.txt"), "mine, never committed"); - AppendForgedCommit(source, ("other.txt", "upstream")); + (string source, string target) = await CreateManagedAsync("managed-interrupted-pull"); + ReplaceEntry(source, remove: "keep.txt", add: ("zz-new.txt", "new")); + string tip = AppendForgedCommit(source, ("aa-new.txt", "new")); + // "bad_file.txt" is held open: the pull fails after the branch moved and + // "keep.txt" was removed. + await InterruptAsync(target, "bad_file.txt", () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Null(MaterializedTip(target)); + Assert.False(IsFlagSet(target, PendingKey), "the never-checked-out marker is not for a repository that has a tree"); - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.PullAsync(target, Token, CancellationToken.None)); + await _client.PullAsync(target, Token, CancellationToken.None); - Assert.Contains("'other.txt' is in this folder but is not part of the repository", ex.Message); - Assert.Equal("mine, never committed", File.ReadAllText(Path.Combine(target, "other.txt"))); + Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("new", File.ReadAllText(Path.Combine(target, "aa-new.txt"))); + Assert.Equal("new", File.ReadAllText(Path.Combine(target, "zz-new.txt"))); + Assert.False(File.Exists(Path.Combine(target, "keep.txt"))); } + [Fact] + public async Task Managed_ReResolvingAtTheSameCommit_ThatFailsHalfWay_IsCompletedByTheNextPull() + { + (_, string target) = await CreateManagedAsync("managed-interrupted-resolve"); + string tip = HeadSha(target); + // A different name for the same path; the apply fails after the old name was + // removed, before everything is written. + await InterruptAsync( + target, "keep.txt", + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "renamed.txt")]), CancellationToken.None)); + Assert.Null(MaterializedTip(target)); + + // The tip did not move, but the tree is unfinished: "already at the tip" does not apply. + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "renamed.txt"))); + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); + } + + + + // ---------------------------------------------------------------- a managed repository: its recovery file + [Fact] public async Task Managed_WhoseRecoveryFileWasDeleted_AsTheMessageForACorruptOneAdvises_CanBeResolvedAgain() { @@ -490,9 +376,9 @@ public sealed class RecoveryLocalWorkTests : IDisposable File.Delete(RecoveryFilePath(target)); // It is still known to be recovery-managed, so the pull reports the invalid - // path again instead of treating the renamed tree as a clean ordinary clone... + // path again instead of treating the renamed tree as an ordinary clone... await PullExpectingInvalidPathsAsync(target); - // ...and resolving rebuilds the file instead of calling the tree "uncommitted changes". + // ...and resolving rebuilds the file instead of refusing a "checked-out clone". await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); Assert.Equal(HeadSha(target), MaterializedTip(target)); @@ -504,7 +390,39 @@ public sealed class RecoveryLocalWorkTests : IDisposable [Fact] - public async Task Managed_ARecoveryFileFromABuildThatRecordedLess_IsCompleted_NotRefused() + public async Task Managed_ByTheReleasedBuild_WhichWroteNoIndexAndNoFlag_KeepsUpdatingAfterTheUpgrade() + { + (string source, string target) = await CreateManagedAsync("managed-released-build"); + // What 1.1.0 leaves: the recovery file with its manifest and tip, but no index + // and no managed flag. + File.Delete(Path.Combine(target, ".git", "index")); + UnsetFlag(target, ManagedKey); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + Assert.True(IsFlagSet(target, ManagedKey), "once its file has been read, the repository is known to be managed"); + } + + + + [Fact] + public async Task Managed_ByTheReleasedBuild_IsFlaggedOnAPullThatHasNothingToDo_SoLosingTheFileLaterIsNotADeadEnd() + { + (_, string target) = await CreateManagedAsync("managed-released-build-idle"); + UnsetFlag(target, ManagedKey); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.True(IsFlagSet(target, ManagedKey)); + } + + + + [Fact] + public async Task Managed_ARecoveryFileFromABuildThatRecordedLess_IsCompleted() { (string source, string target) = await CreateManagedAsync("managed-old-file"); // Files written before the manifest existed have neither list nor tip. @@ -523,6 +441,30 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task Managed_WithATwinThatDiffersOnlyByCaseSkipped_KeepsUpdating() + { + // Skipping one of two names that differ only by case leaves a tree git itself + // reads as changed; that must not stop later updates. + string source = CreateForgedRepo(_root, ("readme.md", "lower"), ("README.md", "upper"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "managed-twins"); + using(var repo = new Repository(source)) + { + Assert.NotNull(repo.Head.Tip["README.md"]); + } + await _client.ApplyRecoveryAsync(target, Recovery(skips: ["README.md"]), CancellationToken.None); + Assert.Equal("lower", File.ReadAllText(Path.Combine(target, "readme.md"))); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + Assert.Equal("lower", File.ReadAllText(Path.Combine(target, "readme.md"))); + } + + + // ---------------------------------------------------------------- what git sees afterwards [Fact] @@ -553,29 +495,6 @@ public sealed class RecoveryLocalWorkTests : IDisposable - [Fact] - public async Task AttributeFiles_ApplyToEveryFile_WhateverTheirPlaceInTheTree() - { - // "!early.txt" sorts before ".gitattributes" and "late.txt" after it; both must - // come out with the line endings the attributes ask for. - string source = CreateForgedRepo( - _root, - ("!early.txt", "one\ntwo\n"), - (".gitattributes", "*.txt text eol=lf\n"), - ("late.txt", "one\ntwo\n"), - ("bad:file.txt", "one\ntwo\n")); - string target = await CloneExpectingInvalidPathsAsync(source, "attributes"); - UseCrLfByDefault(target); - - await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); - - Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "!early.txt"))); - Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "late.txt"))); - Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); - } - - - [Fact] public async Task AttributeFiles_ThatChangeUpstream_ApplyToFilesWrittenBeforeThem_NotTheStaleOneOnDisk() { @@ -596,6 +515,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal("one\ntwo\nthree\n", File.ReadAllText(Path.Combine(target, "!early.txt"))); + Assert.Equal("one\ntwo\n", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); } @@ -673,26 +593,15 @@ public sealed class RecoveryLocalWorkTests : IDisposable /// - /// Renames "bad:file.txt" in a way that fails after the apply's point of no return: - /// the folder of the new tree already exists as a link to - /// somewhere else, which the write refuses. The link is removed again. + /// Runs while is held open + /// by a reader that lets others read it but not replace it: everything that is + /// checked before the tree is rewritten passes, and the rewrite itself fails when + /// it reaches that file, after the point of no return. /// - private async Task FailAnApplyPartWayAsync(string target, string linkAt) + private static async Task InterruptAsync(string target, string fileName, Func operation) { - string elsewhere = Path.Combine(_root, "elsewhere-" + Guid.NewGuid().ToString("N")[..8]); - Directory.CreateDirectory(elsewhere); - string link = Path.Combine(target, linkAt); - WorkingTreeTests.CreateDirectoryLink(link, elsewhere); - try - { - InvalidOperationException ex = await Assert.ThrowsAsync( - () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); - Assert.Contains("is a link to another location", ex.Message); - } - finally - { - WorkingTreeTests.RemoveLink(link); - } + using var held = new FileStream(Path.Combine(target, fileName), FileMode.Open, FileAccess.Read, FileShare.Read); + await Assert.ThrowsAnyAsync(operation); } @@ -772,23 +681,12 @@ public sealed class RecoveryLocalWorkTests : IDisposable - /// - /// A commit on top of HEAD that adds one file, made through the object database: - /// for a tree that an interrupted apply left half written, where staging would - /// pick up the mess. - /// - private static string CommitOnTopOfHead(string workdir, string fileName, string content) - { - using var repo = new Repository(workdir); - Commit head = repo.Head.Tip; - var definition = TreeDefinition.From(head.Tree); - definition.Add(fileName, NewBlob(repo, content), Mode.NonExecutableFile); - Signature signature = MakeSignature(); - Commit commit = repo.ObjectDatabase.CreateCommit( - signature, signature, "local work", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); - repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); - return commit.Sha; - } + /// Every file of the working tree (not of .git) with its content. + private static Dictionary ReadAllFiles(string workdir) + => Directory.EnumerateFiles(workdir, "*", SearchOption.AllDirectories) + .Select(f => Path.GetRelativePath(workdir, f)) + .Where(f => !f.StartsWith(".git" + Path.DirectorySeparatorChar, StringComparison.Ordinal)) + .ToDictionary(f => f, f => File.ReadAllText(Path.Combine(workdir, f)), StringComparer.Ordinal); @@ -801,10 +699,18 @@ public sealed class RecoveryLocalWorkTests : IDisposable - private static bool IsCheckoutPending(string repoPath) + private static bool IsFlagSet(string repoPath, string key) { using var repo = new Repository(repoPath); - return repo.Config.Get(PendingKey)?.Value == true; + return repo.Config.Get(key, ConfigurationLevel.Local)?.Value == true; + } + + + + private static void UnsetFlag(string repoPath, string key) + { + using var repo = new Repository(repoPath); + repo.Config.Unset(key, ConfigurationLevel.Local); } diff --git a/gclo.Engine.Tests/WorkingTreeTests.cs b/gclo.Engine.Tests/WorkingTreeTests.cs index f320382..7c51e18 100644 --- a/gclo.Engine.Tests/WorkingTreeTests.cs +++ b/gclo.Engine.Tests/WorkingTreeTests.cs @@ -346,6 +346,45 @@ public sealed class WorkingTreeTests : IDisposable + [Fact] + public void Create_WhereAFolderHoldingOnlyALinkSits_ReplacesIt_AndLeavesTheLinksTargetAlone() + { + File.WriteAllText(Path.Combine(_outside, "kept.txt"), "kept"); + Directory.CreateDirectory(Path.Combine(_workdir, "docs", "inner")); + LinkDirectory(Path.Combine(_workdir, "docs", "inner", "linked"), _outside); + + using(FileStream file = WorkingTree.Create(_workdir, "docs")) + { + file.Write("now a file"u8); + } + + Assert.Equal("now a file", File.ReadAllText(Path.Combine(_workdir, "docs"))); + Assert.Equal("kept", File.ReadAllText(Path.Combine(_outside, "kept.txt"))); + } + + + + [Fact] + public void Verify_ChecksWhatCreateChecks_WithoutTouchingTheDisk() + { + var verified = new HashSet(StringComparer.Ordinal); + Directory.CreateDirectory(Path.Combine(_workdir, "src")); + LinkDirectory(Path.Combine(_workdir, "vendor"), _outside); + + WorkingTree.Verify(_workdir, "src/app/main.cs", verified); + WorkingTree.Verify(_workdir, "readme.txt"); + Assert.Throws(() => WorkingTree.Verify(_workdir, "../outside/x.txt", verified)); + Assert.Throws(() => WorkingTree.Verify(_workdir, ".git/config", verified)); + Assert.Throws(() => WorkingTree.Verify(_workdir, "vendor/lib.txt", verified)); + + Assert.False(Directory.Exists(Path.Combine(_workdir, "src", "app")), "nothing is created by a check"); + Assert.False(File.Exists(Path.Combine(_workdir, "readme.txt"))); + Assert.Contains(Path.Combine(_workdir, "src"), verified); + Assert.Empty(Directory.EnumerateFileSystemEntries(_outside)); + } + + + [Fact] public void Create_WhereAFolderWithFilesSits_IsRefused_AndTheFilesSurvive() { diff --git a/gclo.Engine/IGitClient.cs b/gclo.Engine/IGitClient.cs index cd00349..78880b8 100644 --- a/gclo.Engine/IGitClient.cs +++ b/gclo.Engine/IGitClient.cs @@ -41,15 +41,23 @@ public interface IGitClient /// - /// Materializes the working tree of a fetched-but-never-checked-out repository - /// (one whose clone failed with ) - /// by writing HEAD's blobs to disk with 's renames - /// and skips applied. Any previously persisted recovery is merged in first, - /// with winning on conflicts, so a repository that - /// gains new invalid paths can be fixed without restating earlier choices. - /// Throws when the effective - /// (post-mapping) path set is still invalid or collides; on success the merged - /// recovery is persisted so later pulls re-apply it to new commits. + /// Writes the working tree of a repository whose paths Windows cannot create, with + /// 's renames and skips applied. It applies to a + /// repository that was fetched but never checked out (its clone failed with + /// , or it was empty) and to one an + /// earlier call already wrote this way, for which it also brings in origin's new + /// commits. Any previously persisted recovery is merged in first, the newer choice + /// for a path winning, so a repository that gains new invalid paths can be fixed + /// without restating earlier choices. /// + /// + /// The tree is written by hand, with force, so this refuses + /// () a clone that was checked out + /// normally: that tree may hold someone's work. It never moves a branch off a + /// commit made in the repository. Throws + /// when the effective path set is + /// still invalid or collides, before anything is changed; on success the merged + /// recovery is persisted so later pulls re-apply it to new commits. + /// Task ApplyRecoveryAsync(string path, PathRecovery recovery, CancellationToken cancellationToken); } diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index cd0458a..5026fad 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -51,7 +51,9 @@ public sealed class LibGit2GitClient : IGitClient /// Local git config flag meaning "gclo writes this repository's working tree /// itself, through a path recovery". It outlives the recovery file, so a /// repository whose file was deleted (as the message for a corrupt one advises) - /// is still known to be recovery-managed and can be resolved again (#147). + /// is still known to be recovery-managed and can be resolved again, instead of + /// being taken for a clone someone checked out (#147). Set only once a recovery + /// has been read or written successfully, never because a file merely exists. /// private const string RecoveryManagedKey = "gclo.recoverymanaged"; @@ -282,7 +284,6 @@ public sealed class LibGit2GitClient : IGitClient // every sync, touching gigabytes and every mtime for zero change, #31). // Freshly-invalid paths the mapping does not cover surface as a typed // failure listing the effective paths. - repo.Config.Set(RecoveryManagedKey, true, ConfigurationLevel.Local); ApplyRecoveryCore(repo, incoming: null, ct); return; } @@ -667,55 +668,6 @@ public sealed class LibGit2GitClient : IGitClient - /// - /// Where a repository stands with respect to path recovery. It decides which commit - /// an apply writes and how much of what is on disk may be the user's (#147). - /// - private enum RecoveryStanding - { - /// Fetched but never checked out: the marker is set and nothing is recorded. Nothing on disk is the user's. - NeverCheckedOut, - - /// An ordinary, checked-out clone. Everything on disk may be the user's work. - Ordinary, - - /// Recovery-managed, and the last apply finished: the manifest says exactly what gclo wrote, for which commit. - Managed, - - /// - /// Recovery-managed, but the manifest is missing, from a build that recorded - /// less, or left by an apply that did not finish. What is on disk cannot be - /// told apart from the user's, so the apply simply completes. - /// - Unsettled, - } - - - - private static bool IsRecoveryManaged(Repository repo) - => repo.Config.Get(RecoveryManagedKey)?.Value == true; - - - - private static RecoveryStanding StandingOf(Repository repo, RecoveryDocument? stored) - { - if(stored is not null) - { - return stored.MaterializedTip is null || stored.MaterializedPaths is null - ? RecoveryStanding.Unsettled - : RecoveryStanding.Managed; - } - - if(IsRecoveryManaged(repo)) - { - return RecoveryStanding.Unsettled; - } - - return IsCheckoutPending(repo) ? RecoveryStanding.NeverCheckedOut : RecoveryStanding.Ordinary; - } - - - /// /// Materializes a commit's tree into the working directory with renames and skips /// applied, after validating that the effective path set is actually creatable on @@ -724,24 +676,28 @@ public sealed class LibGit2GitClient : IGitClient /// /// /// - /// gclo never discards local work, and this is the one place that writes a working - /// tree by hand, so it is where that promise is easiest to break. The rules (#147): + /// This writes a working tree by hand, with force. It is therefore only ever done + /// to a tree that is gclo's to write (#147): /// /// - /// The commit written is chosen by : origin's tip when - /// going there loses no commit, the current commit otherwise, a refusal when the - /// histories diverged. - /// For an ordinary clone and for a managed repository whose last apply - /// finished, stops the apply before - /// anything changes when tracked files were modified or when a file gclo did not - /// write sits where one is about to be written. - /// Before the first destructive step the recovery file is saved without a - /// materialized tip, listing everything that was and will be on disk. If the apply - /// is interrupted, the next pull finds that, knows the tree is unfinished, and - /// completes it through this same method. The pending-checkout marker is never set - /// on a repository that was checked out: that marker means "nothing here is the - /// user's", and its handler acts accordingly. + /// A repository whose clone was fetched but never checked out (the pending + /// marker), or that is empty: nothing was ever written there. + /// A recovery-managed repository: an earlier apply wrote its tree, and the + /// documentation says to treat it as a read-only mirror. /// + /// + /// A clone that was checked out normally is refused, whatever its state. Proving + /// that such a clone holds nothing of the user's is not possible in general + /// (untracked files, files git was told to ignore changes in, an apply interrupted + /// half-way and edited since), and a wrong guess destroys work. The way to get such + /// a repository with renamed paths is a fresh clone. + /// + /// + /// Commits are protected in every case: the branch only moves where + /// allows. And an apply that is interrupted is completed by + /// the next pull: before the first destructive step the recovery file is saved + /// without a materialized tip, listing everything that was and will be on disk. + /// /// private static void ApplyRecoveryCore(Repository repo, PathRecovery? incoming, CancellationToken ct) { @@ -753,15 +709,25 @@ public sealed class LibGit2GitClient : IGitClient string workdir = WorkingDirectoryOf(repo); string recoveryFile = GetRecoveryFilePath(repo); RecoveryDocument? stored = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null; - RecoveryStanding standing = StandingOf(repo, stored); - PathRecovery storedRecovery = stored is null ? s_noRecovery : ToRecovery(stored); - PathRecovery recovery = MergeRecoveries(storedRecovery, incoming ?? s_noRecovery); + bool managed = stored is not null || IsRecoveryManaged(repo); + if(!managed && !IsCheckoutPending(repo) && !repo.Info.IsHeadUnborn) + { + throw new InvalidOperationException( + "This repository is checked out normally. Resolving its paths would replace the working tree with one that gclo " + + "writes itself, which could overwrite work in it, so gclo does not do that to a clone that was checked out. " + + $"To get it with renamed paths, move or delete the folder '{Path.TrimEndingDirectorySeparator(workdir)}' and sync again" + + (OperatingSystem.IsWindows() ? ", or clone it in WSL." : ".")); + } - Commit? head = repo.Head.Tip; - Commit tip = TargetOf(repo, standing, stored); + PathRecovery recovery = MergeRecoveries(stored is null ? s_noRecovery : ToRecovery(stored), incoming ?? s_noRecovery); + Commit tip = TargetOf(repo, managed); if(incoming is null && string.Equals(stored?.MaterializedTip, tip.Sha, StringComparison.Ordinal)) { - return; // the tree on disk already is this commit, written through these choices + // The tree on disk already is this commit, written through these choices. + // (The file was read without complaint, so the repository is known to be + // managed from here on even if the file is lost.) + repo.Config.Set(RecoveryManagedKey, true, ConfigurationLevel.Local); + return; } List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(tip.Tree, recovery); @@ -775,11 +741,8 @@ public sealed class LibGit2GitClient : IGitClient throw new InvalidRepositoryPathsException(stillInvalid); } - HashSet? ours = OursOnDisk(standing, stored, head); - if(ours is not null && standing is RecoveryStanding.Ordinary or RecoveryStanding.Managed) - { - RefuseToOverwriteLocalWork(repo, workdir, standing, storedRecovery, entries, ours!); - } + HashSet? ours = stored?.MaterializedPaths is null ? null : new HashSet(stored.MaterializedPaths, StringComparer.Ordinal); + RefuseWhatCannotBeWritten(workdir, entries, ours); var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); @@ -816,7 +779,8 @@ public sealed class LibGit2GitClient : IGitClient ct.ThrowIfCancellationRequested(); // Filter through .gitattributes (CRLF etc.) under the ORIGINAL repo path, - // exactly as a checkout of that entry would. + // exactly as a checkout of that entry would. Force semantics: whatever is + // at the destination is replaced, read-only or not (see WorkingTree). using Stream content = blob.GetContentStream(new FilteringOptions(originalPath)); using FileStream file = WorkingTree.Create(workdir, effectivePath, verifiedFolders); content.CopyTo(file); @@ -828,6 +792,11 @@ public sealed class LibGit2GitClient : IGitClient + private static bool IsRecoveryManaged(Repository repo) + => repo.Config.Get(RecoveryManagedKey)?.Value == true; + + + private static bool IsAttributesFile(string repoPath) => repoPath.EndsWith(".gitattributes", StringComparison.Ordinal) && (repoPath.Length == ".gitattributes".Length || repoPath[^(".gitattributes".Length + 1)] == '/'); @@ -835,57 +804,52 @@ public sealed class LibGit2GitClient : IGitClient /// - /// The commit an apply writes. gclo only ever moves a branch forward to what origin - /// has, and only when that drops no commit (#147): + /// The commit an apply writes. gclo moves a branch only to what origin has, and only + /// when that drops no commit (#147): /// - /// Never checked out: the current commit, which the clone or the last pull put - /// there; nothing local exists. - /// Origin has nothing the current commit lacks (no upstream, the same commit, - /// or local commits on top of it): the current commit. For an ordinary clone that - /// means there is nothing to resolve, and the apply says so instead of turning a - /// working clone into a managed one for no reason. - /// The current commit is contained in origin's tip: origin's tip. - /// The histories diverged: origin's tip only for a managed repository still at - /// the commit gclo last wrote (nobody committed; origin was rewritten). Otherwise - /// the refusal the ordinary pull gives. + /// An empty repository (unborn HEAD): origin's tip for that branch, its first + /// checkout. + /// Fetched but never checked out: the current commit, which the clone or the + /// last pull put there. + /// Managed, and origin has nothing the current commit lacks (no upstream, a + /// detached HEAD, the same commit, or commits made here on top of origin's): the + /// current commit. + /// Managed, and the current commit is contained in origin's tip: origin's + /// tip. + /// Managed, and the histories diverged: origin's tip only when the current + /// commit is one origin's branch itself once pointed at, which the reflog of the + /// remote-tracking branch shows. Then nobody committed here and origin rewrote its + /// history. Otherwise the refusal the ordinary pull gives. /// /// - private static Commit TargetOf(Repository repo, RecoveryStanding standing, RecoveryDocument? stored) + private static Commit TargetOf(Repository repo, bool managed) { Commit? head = repo.Head.Tip; - if(standing == RecoveryStanding.NeverCheckedOut) - { - return head ?? throw new InvalidOperationException("Repository has no commits; nothing to materialize."); - } - if(head is null) { - // An empty clone whose origin has commits by now: its first checkout. - string branchName = repo.Refs.Head.TargetIdentifier["refs/heads/".Length..]; - return repo.Branches["origin/" + branchName]?.Tip - ?? throw new InvalidOperationException("This repository is empty, and so is its branch on origin; there is nothing to resolve."); + const string Prefix = "refs/heads/"; + string targetRef = repo.Refs.Head.TargetIdentifier; + Commit? first = targetRef.StartsWith(Prefix, StringComparison.Ordinal) + ? repo.Branches["origin/" + targetRef[Prefix.Length..]]?.Tip + : null; + return first ?? throw new InvalidOperationException( + "This repository has no commits, and neither has its branch on origin; there is nothing to materialize."); } - Commit? upstream = repo.Info.IsHeadDetached ? null : repo.Head.TrackedBranch?.Tip; - string? mergeBase = upstream is null ? null : repo.ObjectDatabase.FindMergeBase(head, upstream)?.Sha; - if(upstream is null || string.Equals(mergeBase, upstream.Sha, StringComparison.Ordinal)) + Branch? tracked = managed && !repo.Info.IsHeadDetached ? repo.Head.TrackedBranch : null; + Commit? upstream = tracked?.Tip; + if(upstream is null) { - if(standing == RecoveryStanding.Ordinary) - { - throw new InvalidOperationException( - "Origin has nothing new for the branch that is checked out here, so there are no paths to resolve. " - + "Sync this repository again; if it keeps failing, delete the folder to re-clone."); - } - return head; } - if(string.Equals(mergeBase, head.Sha, StringComparison.Ordinal)) + string? mergeBase = repo.ObjectDatabase.FindMergeBase(head, upstream)?.Sha; + if(string.Equals(mergeBase, upstream.Sha, StringComparison.Ordinal)) { - return upstream; + return head; } - if(standing == RecoveryStanding.Managed && string.Equals(stored!.MaterializedTip, head.Sha, StringComparison.Ordinal)) + if(string.Equals(mergeBase, head.Sha, StringComparison.Ordinal) || WasOnceTheTipOf(repo, tracked!, head)) { return upstream; } @@ -897,7 +861,17 @@ public sealed class LibGit2GitClient : IGitClient - /// Points the current branch at , creating it (with its upstream) for an empty clone. + /// + /// True when the remote-tracking branch itself pointed at + /// at some time, going by its reflog (every fetch that + /// moves the branch adds an entry). A commit made in this clone is never in there. + /// + private static bool WasOnceTheTipOf(Repository repo, Branch tracked, Commit commit) + => repo.Refs.Log(tracked.CanonicalName).Any(entry => entry.To == commit.Id || entry.From == commit.Id); + + + + /// Points the current branch at , creating it (with its upstream) for an empty repository. private static void MoveHeadTo(Repository repo, Commit tip) { if(repo.Info.IsHeadUnborn) @@ -921,114 +895,39 @@ public sealed class LibGit2GitClient : IGitClient - private static StringComparer PathNameComparer - => OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; - - - /// - /// The files on disk that are there by gclo's doing (or by an ordinary checkout of - /// the current commit), as repository paths: they may be replaced, and removed when - /// they leave the tree. Null when that cannot be known. + /// Finds, before anything has changed, what would make the apply fail half-way: a + /// destination that is not a usable path in the working tree, a link on the way to + /// it, or a file sitting where a folder is needed (unless that file is one gclo + /// wrote and is about to remove). Failing here leaves the branch, the index and + /// every file as they were (#147). /// - private static HashSet? OursOnDisk(RecoveryStanding standing, RecoveryDocument? stored, Commit? head) + private static void RefuseWhatCannotBeWritten( + string workdir, List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, HashSet? ours) { - if(standing == RecoveryStanding.Ordinary) - { - return new HashSet( - head is null ? [] : CollectEffectiveEntries(head.Tree, s_noRecovery).Select(e => e.EffectivePath), PathNameComparer); - } - - return stored?.MaterializedPaths is null ? null : new HashSet(stored.MaterializedPaths, PathNameComparer); - } - - - - /// - /// Stops an apply, before anything has changed, when it would destroy something - /// that is the user's: a tracked file with uncommitted changes, or a file gclo did - /// not write sitting where one is about to be written. An ordinary fast-forward - /// refuses in the same two cases (#147). - /// - /// - /// In a managed repository the files a recovery renamed or skipped are missing - /// under their original names by design, and are not changes. A file that was - /// renamed is on disk under a name git does not track, so an edit made to it - /// cannot be seen here and is overwritten: the price of a tree git cannot - /// represent. - /// - private static void RefuseToOverwriteLocalWork( - Repository repo, - string workdir, - RecoveryStanding standing, - PathRecovery storedRecovery, - List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, - HashSet ours) - { - foreach(StatusEntry entry in repo.RetrieveStatus(new StatusOptions { IncludeUntracked = false, IncludeIgnored = false })) - { - if(standing == RecoveryStanding.Managed && IsMovedBy(storedRecovery, entry.FilePath)) - { - continue; - } - - throw new InvalidOperationException( - $"'{repo.Head.FriendlyName}' has uncommitted changes (for example to '{entry.FilePath}'), and this would rewrite the working tree; " - + "gclo never discards local work. Commit or discard the changes, then try again."); - } - + var verifiedFolders = new HashSet(StringComparer.Ordinal); + var checkedFolders = new HashSet(StringComparer.Ordinal); foreach((_, string? effectivePath, _) in entries) { - // Replaceable: what gclo wrote, and what git tracks here (the loop above - // has just shown that tracked files carry no uncommitted changes). - if(ours.Contains(effectivePath) || repo.Index[effectivePath] is not null) - { - continue; - } + WorkingTree.Verify(workdir, effectivePath, verifiedFolders); - string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); - string? inTheWay = null; - if(Directory.Exists(full)) + for(int slash = effectivePath.IndexOf('/', StringComparison.Ordinal); slash > 0; slash = effectivePath.IndexOf('/', slash + 1)) { - // A folder where a file is about to go. It leaves with the old tree when - // everything in it is ours; anything else in it is the user's. - inTheWay = Directory.EnumerateFiles(full, "*", SearchOption.AllDirectories) - .Select(f => Path.GetRelativePath(workdir, f).Replace(Path.DirectorySeparatorChar, '/')) - .FirstOrDefault(p => !ours.Contains(p)); - } - else if(File.Exists(full) && !repo.Ignore.IsPathIgnored(effectivePath)) - { - inTheWay = effectivePath; - } - - if(inTheWay is not null) - { - throw new InvalidOperationException( - $"'{inTheWay}' is in this folder but is not part of the repository (an untracked file), and a file of the repository " - + "is about to be written there; gclo never discards local work. Move or delete it, then try again."); + string folder = effectivePath[..slash]; + if(checkedFolders.Add(folder) + && ours?.Contains(folder) != true + && File.Exists(Path.Combine(workdir, folder.Replace('/', Path.DirectorySeparatorChar)))) + { + throw new InvalidOperationException( + $"'{folder}' is a file in this folder, and the repository has a folder of that name. " + + "Move or delete the file, then try again."); + } } } } - /// True when , or a folder above it, is renamed or skipped by . - private static bool IsMovedBy(PathRecovery recovery, string repoPath) - { - for(int end = repoPath.Length; end > 0; end = repoPath.LastIndexOf('/', end - 1)) - { - string prefix = repoPath[..end]; - if(recovery.SkippedPaths.Contains(prefix) || recovery.SegmentRenames.ContainsKey(prefix)) - { - return true; - } - } - - return false; - } - - - /// /// Overlays onto . The newer /// choice for a path wins whatever kind it is: an incoming rename replaces a stored diff --git a/gclo.Engine/WorkingTree.cs b/gclo.Engine/WorkingTree.cs index a9d7fc0..7a44e7b 100644 --- a/gclo.Engine/WorkingTree.cs +++ b/gclo.Engine/WorkingTree.cs @@ -85,6 +85,17 @@ internal static class WorkingTree + /// + /// Everything checks before it touches the disk, without + /// touching it: the path resolves inside the working directory, and no folder on + /// the way is a link. For finding out up front, before an operation that cannot be + /// undone begins, whether its writes can succeed. + /// + public static void Verify(string workdir, string repoPath, ISet? verifiedFolders = null) + => RefuseLinkOnTheWay(workdir, Resolve(workdir, repoPath), verifiedFolders); + + + /// /// Opens for writing as a new, empty file, creating its /// folders. Whatever file is already there is removed first, read-only or not, so @@ -212,6 +223,36 @@ internal static class WorkingTree + /// + /// Removes a folder that holds nothing but empty folders, bottom up, and never a + /// file: if one is in there (it is not gclo's, or it would have left with the old + /// tree) the folder is refused instead. + /// + private static void RemoveEmptyFolders(DirectoryInfo folder) + { + foreach(DirectoryInfo child in folder.EnumerateDirectories()) + { + if(child.LinkTarget is not null) + { + child.Delete(); // a link is nobody's content; remove the link itself + continue; + } + RemoveEmptyFolders(child); + } + + if(folder.EnumerateFiles().Any()) + { + throw new InvalidOperationException( + $"'{folder.FullName}' is a folder with files in it, where the repository has a file. " + + "Move or delete the folder, then try again."); + } + + folder.Attributes = FileAttributes.Directory; + folder.Delete(); + } + + + /// /// Removes what is at when it is a file, a link, or a folder /// that holds no files, read-only or not: a link is removed itself (even one whose @@ -232,14 +273,7 @@ internal static class WorkingTree // A real folder where a file belongs (upstream turned a folder into a file). // Whatever the tree had in it is gone by now; if files remain they are not // gclo's to remove, and empty folders are nobody's. - if(directory.EnumerateFiles("*", SearchOption.AllDirectories).Any()) - { - throw new InvalidOperationException( - $"'{full}' is a folder with files in it, where a file of the repository belongs. " - + "Move or delete the folder, then try again."); - } - - DeleteDirectory(full); + RemoveEmptyFolders(directory); return; } -- 2.54.0 From a114d10a32b1a01b29bb24a363f2277ab9b49c8e Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 15:56:26 -0600 Subject: [PATCH 4/6] fix(recovery): address the third independent review - An empty repository counts as never checked out only while its index is empty and no file sits where the repository has one; a branch started with 'checkout --orphan' in a checked-out clone is refused like any ordinary clone. - What counts as origin's commit: reflog entries a push wrote do not; a branch that tracks another local branch has no record of origin. - A pull that has nothing to write writes nothing: commits made on top with origin unchanged leave every other file alone; a branch moved back by hand is brought to origin's tip again. - A repository written by the released build (commits, no index) whose recovery file was lost can be resolved again, as the message advises. - A clone that was never checked out follows origin by the same rule as everything else: a commit made in it by hand is not dropped. - Found before anything changes: another git process at work, a folder with someone's files where a file is needed, a link on the way to a file that is to be removed. A file of gclo's own is recognised whatever its case on a file system that ignores case. - A file that left the tree, where a folder of the user's now sits, leaves the folder; a refused folder removal has touched nothing in it. - The managed marker is read from the repository's own configuration only; a refused apply leaves the configuration as it was. Threat model updated. - The CLI keeps the list of invalid paths on a repository whose sanitization failed. Refs #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 29 +- docs/security/threat-model.md | 2 +- gclo.Cli.Tests/SyncCommandTests.cs | 25 +- gclo.Cli/SyncCommand.cs | 5 +- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 510 +++++++++++++++++++- gclo.Engine.Tests/WorkingTreeTests.cs | 68 +++ gclo.Engine/IGitClient.cs | 5 +- gclo.Engine/LibGit2GitClient.cs | 212 ++++++-- gclo.Engine/WorkingTree.cs | 67 ++- 10 files changed, 842 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index a716b63..eea832e 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca 8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, **Retry failed** re-runs exactly the failed rows, and **Open folder** opens the target folder in Explorer. -**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). This applies to a repository gclo could not check out when it cloned it. A repository that was already checked out normally is never rewritten: when an update brings it such a path, it fails with a message that says how to get a fresh clone with the renames, and nothing in it is changed. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). +**When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). This applies to a repository gclo could not check out when it cloned it. A repository that was already checked out normally is never rewritten: when an update brings it such a path, it fails and nothing in it is changed, and **Resolve…** (or `--sanitize-paths`) then says how to get a fresh clone with the renames instead of touching it. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). **Settings** (File > Settings…) let you set a default GitHub token (stored in the Windows Credential Manager), the default target folder (with a folder picker), the default parallelism, and the theme (System, Light, or Dark); an Advanced section toggles the startup splash and its duration. diff --git a/docs/CLI.md b/docs/CLI.md index 89fe7e1..6951386 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -163,18 +163,24 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: repository. - A repository that was already checked out normally is never rewritten this way. When an update brings such a repository its first invalid path, it - fails with a message saying so, and nothing in it is changed: its working - tree may hold work, and writing it by hand could destroy that. To get it - with the renames, move or delete its folder and sync again (the fresh clone - is then sanitized), or use the desktop app's **Clone in WSL**. + fails with those paths listed; with `--sanitize-paths` the failure says + that the repository is checked out normally, and nothing in it is changed: + its working tree may hold work, and writing it by hand could destroy that. + To get it with the renames, move or delete its folder and sync again (the + fresh clone is then sanitized), or use the desktop app's **Clone in WSL**. + The same holds for an empty clone into whose folder files were put by hand: + a file that is already where the repository has one is never replaced. Only the working tree is renamed — the repository's history and index still hold -the original paths, so `git status` reports the renamed and skipped files (and, -where two names differ only by case, their twins) as local changes. Treat a -sanitized repository as a read-only mirror: later syncs rewrite its working -tree, so edits made there are overwritten. A commit made there is never -dropped: if origin has moved on as well, the sync fails for that repository, -as it does for any clone with local commits. +the original paths, so `git status` reports renamed and skipped paths as local +changes. Treat a sanitized repository as a read-only mirror: later syncs +rewrite its working tree, so edits made there are overwritten. A commit made +there is not dropped: while origin has nothing new, a sync leaves the tree as +it is, and once origin has moved on as well the sync fails for that +repository, as it does for any clone with local commits. The branch is only +ever moved to a commit of origin's: forward, or, when origin's history was +rewritten, off a commit that this clone's record of earlier fetches shows was +origin's own. #### `--json` @@ -192,7 +198,8 @@ run ends (also on cancellation): - `type` is always `"summary"`. - `failures[].invalidPaths` is the **full** list of offending paths when the failure was Windows path validation (the message itself only quotes the - first three); `null` for any other failure. + first three), also when `--sanitize-paths` then could not fix the repository + (`error` says why); `null` for any other failure. - `sanitized[]` lists the repositories `--sanitize-paths` checked out with renames and skips; they are counted under `cloned`, but `skippedPaths` are not on disk. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 4400533..78e4f9f 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -63,7 +63,7 @@ flowchart LR | Credential Manager | out/in | Tokens by `gclo[:]:account:` target | Windows user session | `CredentialManagerVault`; scope derived from the data root | | `settings.json`, `accounts.json` (with `accounts.json.lock`) | out/in | Non-secret settings and account metadata | File ACL (user profile) | Written beside the file and put in place in one step; sanitized on load; an accounts file whose content is not valid is moved aside (`.corrupt-*`), one that cannot be read is left in place; the previous accounts file is kept as `.bak`. Changes to the accounts are serialized across gclo processes by the lock file: a process of the same user that holds it open can delay account changes (they fail after a few seconds, nothing is written), not read or alter them | | Activity logs | out | Actions and errors; token *lengths* only | File ACL | `IActivityLog` callers never pass tokens | -| `.git/gclo-recovery.json`, `gclo.checkoutpending` config key | out/in | Path-recovery plan; checkout-pending marker | File ACL | Written atomically; read only for the repository it belongs to | +| `.git/gclo-recovery.json`, `gclo.checkoutpending` and `gclo.recoverymanaged` config keys | out/in | Path-recovery plan; checkout-pending marker; "gclo wrote this working tree" marker | File ACL | Written atomically; read only for the repository it belongs to (the markers from the repository's own configuration, never the user's or the machine's). A repository marked as written by gclo has its working tree rewritten on sync; one that is not, and was checked out by git, is never rewritten | | Command line (CLI) | in | Options, org, target, filters | n/a | `OptionReader` / `RepoFilterSpec`; parallelism bounded 1..64; token options refused on argv | | `GCLO_DATA_DIR`, `GCLO_UITEST_FIXTURE` env | in | Data root override; offline test seam | n/a | Fixture only answers for the exact fixture token (every other token, and the question whether a folder is a repository, go to the real services); ignored by an installed build; logged when enabled; never set in production (#145) | | `wsl.exe -e sh -s` | out/in | POSIX script on stdin, org/repo/url as positional args, `GCLO_GIT_TOKEN` via `WSLENV` | PAT via env | Nothing is re-parsed by wsl.exe's command line; token never on argv or disk; UTF-16 wsl.exe messages decoded | diff --git a/gclo.Cli.Tests/SyncCommandTests.cs b/gclo.Cli.Tests/SyncCommandTests.cs index 22fe283..e1c8415 100644 --- a/gclo.Cli.Tests/SyncCommandTests.cs +++ b/gclo.Cli.Tests/SyncCommandTests.cs @@ -468,7 +468,9 @@ public sealed class SyncCommandTests : IDisposable Assert.Contains("\"sanitized\":[{\"repo\":\"fixable\",\"renamed\":1,\"skipped\":1,\"skippedPaths\":[\"dup\"]}]", json); Assert.Contains("\"repo\":\"stuck\"", json); Assert.Contains("path sanitization failed: disk full", json); - Assert.Contains("\"invalidPaths\":null", json); // a non-path failure has no list + // The paths that made it fail in the first place are still what is wrong with it. + Assert.Contains("\"path\":\"bad:name.txt\"", json); + Assert.DoesNotContain("\"invalidPaths\":null", json); } @@ -720,6 +722,27 @@ public sealed class SyncCommandTests : IDisposable + [Fact] + public async Task SanitizePaths_RecoveryRefused_StillNamesThePathsInTheDocument() + { + using var console = new ConsoleCapture(); + var lister = new FakeRepoLister { Result = [Repo("a")] }; + var git = new FakeGit + { + OnClone = (_, _, _, _, _) => Task.FromException(InvalidPaths()), + OnApplyRecovery = (_, _, _) => Task.FromException(new InvalidOperationException("This repository is checked out normally.")), + }; + + int code = await Run(lister, git, "--org", "acme", "--target", Target, "--token-env", EnvVar, "--sanitize-paths", "--json"); + + Assert.Equal(1, code); + Assert.Contains("path sanitization failed: This repository is checked out normally.", console.Out); + Assert.Contains("\"path\":\"bad:name.txt\"", console.Out); + Assert.DoesNotContain("\"invalidPaths\":null", console.Out); + } + + + [Fact] public async Task InvalidPathFailure_WithoutSanitize_PrintsOffendingPaths() { diff --git a/gclo.Cli/SyncCommand.cs b/gclo.Cli/SyncCommand.cs index 569fd34..c1fd604 100644 --- a/gclo.Cli/SyncCommand.cs +++ b/gclo.Cli/SyncCommand.cs @@ -670,9 +670,12 @@ internal static class SyncCommand } catch(Exception ex) { + // The paths that made the repository fail are still what is wrong with it, + // whatever stopped the sanitization (for one, a clone that was checked out + // normally is never rewritten): the report keeps them. progress.Report(new RepoProgress( pending.Repo, SyncStatus.Failed, $"path sanitization failed: {ex.Message}", - InvalidPaths: (ex as InvalidRepositoryPathsException)?.Paths)); + InvalidPaths: (ex as InvalidRepositoryPathsException)?.Paths ?? pending.Paths)); return false; } } diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs index f492801..b6b6065 100644 --- a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -132,6 +132,10 @@ public sealed class RecoveryLocalWorkTests : IDisposable Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); // The branch tracks origin, so later pulls follow it. + using(var repo = new Repository(target)) + { + Assert.Equal("refs/remotes/origin/" + repo.Head.FriendlyName, repo.Head.TrackedBranch?.CanonicalName); + } string next = AppendForgedCommit(source, ("more.txt", "m")); await _client.PullAsync(target, Token, CancellationToken.None); Assert.Equal(next, HeadSha(target)); @@ -167,7 +171,7 @@ public sealed class RecoveryLocalWorkTests : IDisposable InvalidOperationException ex = await Assert.ThrowsAsync( () => _client.PullAsync(target, Token, CancellationToken.None)); - Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Contains("have diverged", ex.Message); Assert.Equal(local, HeadSha(target)); Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); } @@ -192,19 +196,19 @@ public sealed class RecoveryLocalWorkTests : IDisposable [Fact] public async Task Managed_ALocalCommitThatWasSyncedOnce_IsStillNotDropped_WhenOriginMovesOn() { - // After the first sync the local commit is the one written to disk. That must - // not be taken for "origin's commit": what counts is whether origin's branch - // ever pointed at it. + // A sync with the local commit in place goes through without complaint. That + // must not turn it into "origin's commit": what counts is whether origin's + // branch ever pointed at it. (string source, string target) = await CreateManagedAsync("managed-synced-local-commit"); string local = CommitInWorkingTree(target, "mine.txt", "my work"); await _client.PullAsync(target, Token, CancellationToken.None); - Assert.Equal(local, MaterializedTip(target)); + Assert.Equal(local, HeadSha(target)); AppendForgedCommit(source, ("other.txt", "from a colleague")); InvalidOperationException ex = await Assert.ThrowsAsync( () => _client.PullAsync(target, Token, CancellationToken.None)); - Assert.Contains("has local commits that origin does not have", ex.Message); + Assert.Contains("have diverged", ex.Message); Assert.Equal(local, HeadSha(target)); Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); } @@ -553,8 +557,502 @@ public sealed class RecoveryLocalWorkTests : IDisposable + // ---------------------------------------------------------------- an empty repository is only empty while nothing is in it + + [Fact] + public async Task EmptyClone_WithAFileOfTheUsersWhereTheRepositoryHasOne_IsRefused_AndTheFileIsKept() + { + string source = Path.Combine(_root, "empty-source-2"); + Repository.Init(source); + string target = Path.Combine(_root, "empty-clone-2"); + await _client.CloneAsync(source, target, Token, null, CancellationToken.None); + File.WriteAllText(Path.Combine(target, "keep.txt"), "mine, put here by hand"); + FirstForgedCommit(source, ("bad:file.txt", "bad"), ("keep.txt", "origin's")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains("'keep.txt' is already in this folder", ex.Message); + Assert.Equal("mine, put here by hand", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); + Assert.False(IsFlagSet(target, ManagedKey)); + Assert.False(File.Exists(RecoveryFilePath(target))); + + // With the file out of the way the empty clone resolves as before. + File.Delete(Path.Combine(target, "keep.txt")); + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + Assert.Equal("origin's", File.ReadAllText(Path.Combine(target, "keep.txt"))); + } + + + + [Fact] + public async Task ABranchWithoutCommitsInACheckedOutClone_IsStillACheckedOutClone_AndIsRefused() + { + // What "git checkout --orphan topic" leaves: no commit on the branch, the index + // and the working tree of the branch it was started from. + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + using(var repo = new Repository(target)) + { + repo.Refs.UpdateTarget("HEAD", "refs/heads/topic"); + } + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + using(var repo = new Repository(source)) + { + Commit onTopic = NewRootCommit(repo, [("bad:file.txt", "bad"), ("work.txt", "origin's")]); + repo.Refs.Add("refs/heads/topic", onTopic.Id); + } + Dictionary filesBefore = ReadAllFiles(target); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains("is checked out normally", ex.Message); + Assert.Equal(filesBefore, ReadAllFiles(target)); + Assert.False(IsFlagSet(target, ManagedKey)); + } + + + + // ---------------------------------------------------------------- what counts as origin's commit + + [Fact] + public async Task Managed_ACommitThatWasPushed_IsNotTakenForOneOfOrigins_WhenOriginIsRewrittenWithoutIt() + { + (string source, string target) = await CreateManagedAsync("managed-pushed"); + string mine = CommitInWorkingTree(target, "mine.txt", "my work"); + // What "git push" records in this clone: the remote-tracking branch now points + // at the pushed commit, and its reflog says a push put it there. + using(var repo = new Repository(target)) + { + repo.Refs.UpdateTarget(repo.Refs[repo.Head.TrackedBranch.CanonicalName], new ObjectId(mine), "update by push"); + } + // A colleague force-pushes a history that does not have the commit. + RewriteHistory(source, ("bad:file.txt", "bad again"), ("keep.txt", "keep"), ("fresh.txt", "fresh")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("have diverged", ex.Message); + Assert.Equal(mine, HeadSha(target)); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + } + + + + [Fact] + public async Task Managed_ABranchThatTracksAnotherLocalBranch_IsNeverMovedOffItsCommits() + { + (_, string target) = await CreateManagedAsync("managed-local-upstream"); + string start = HeadSha(target); + string mine = CommitInWorkingTree(target, "mine.txt", "my work"); + using(var repo = new Repository(target)) + { + // "I committed on the wrong branch": the commit goes to a branch that tracks + // the local one, which is put back and later moves on without it. + string main = repo.Head.FriendlyName; + Branch feature = repo.CreateBranch("feature", repo.Head.Tip); + repo.Branches.Update(feature, b => + { + b.Remote = "."; + b.UpstreamBranch = "refs/heads/" + main; + }); + Signature signature = MakeSignature(); + Commit startCommit = repo.Lookup(start); + Commit elsewhere = repo.ObjectDatabase.CreateCommit( + signature, signature, "main moves on", startCommit.Tree, [startCommit], prettifyMessage: false); + repo.Refs.UpdateTarget(repo.Refs["refs/heads/" + main], elsewhere.Id); + repo.Refs.UpdateTarget("HEAD", "refs/heads/feature"); + } + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("have diverged", ex.Message); + Assert.Equal(mine, HeadSha(target)); + } + + + + [Fact] + public async Task Managed_OriginRewritten_IsFollowed_GoingByAnEarlierFetch_NotOnlyByTheLatest() + { + (string source, string target) = await CreateManagedAsync("managed-rewritten-after-two"); + AppendForgedCommit(source, ("second.txt", "2")); + await _client.PullAsync(target, Token, CancellationToken.None); // the branch is where a fetch put it + string rewritten = RewriteHistory(source, ("bad:file.txt", "bad again"), ("keep.txt", "keep")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(rewritten, HeadSha(target)); + Assert.False(File.Exists(Path.Combine(target, "second.txt"))); + } + + + + [Fact] + public async Task Managed_OriginRewritten_IsFollowed_WhenTheOlderRecordsOfWhatWasFetchedHaveExpired() + { + (string source, string target) = await CreateManagedAsync("managed-expired-reflog"); + DeleteReflogOfTheTrackedBranch(target); + string rewritten = RewriteHistory(source, ("bad:file.txt", "bad again"), ("keep.txt", "keep")); + + // The fetch itself records where the branch was before it moved it. + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(rewritten, HeadSha(target)); + } + + + + [Fact] + public async Task Managed_WithNoRecordOfWhatWasFetched_StillFastForwards_AndRefusesARewriteWithoutClaimingLocalCommits() + { + (string source, string target) = await CreateManagedAsync("managed-no-reflog"); + using(var repo = new Repository(target)) + { + repo.Config.Set("core.logAllRefUpdates", false, ConfigurationLevel.Local); + } + DeleteReflogOfTheTrackedBranch(target); + + string next = AppendForgedCommit(source, ("more.txt", "m")); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(next, HeadSha(target)); // contained in origin's tip: no record is needed + + RewriteHistory(source, ("bad:file.txt", "bad again"), ("keep.txt", "keep")); + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("have diverged", ex.Message); + Assert.Contains("the record of what was fetched is gone", ex.Message); + Assert.DoesNotContain("has local commits", ex.Message); + Assert.Equal(next, HeadSha(target)); + } + + + + [Fact] + public async Task NeverCheckedOut_WithACommitMadeByHand_IsNotMovedOffIt_ByAPull() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "pending-with-commit"); + string mine; + using(var repo = new Repository(target)) + { + Signature signature = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit( + signature, signature, "made by hand", repo.Head.Tip.Tree, [repo.Head.Tip], prettifyMessage: false); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + mine = commit.Sha; + } + + await PullExpectingInvalidPathsAsync(target); + + Assert.Equal(mine, HeadSha(target)); + } + + + + // ---------------------------------------------------------------- a pull that has nothing to write, writes nothing + + [Fact] + public async Task Managed_ACommitMadeOnTop_WithOriginUnchanged_LeavesEveryOtherFileAsItIs() + { + (_, string target) = await CreateManagedAsync("managed-commit-and-edit"); + File.WriteAllText(Path.Combine(target, "keep.txt"), "edited, not committed"); + string mine = CommitInWorkingTree(target, "mine.txt", "my work"); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(mine, HeadSha(target)); + Assert.Equal("edited, not committed", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + Assert.Equal("my work", File.ReadAllText(Path.Combine(target, "mine.txt"))); + } + + + + [Fact] + public async Task Managed_WhoseBranchWasMovedBackByHand_IsBroughtToOriginsTipAgain() + { + (string source, string target) = await CreateManagedAsync("managed-moved-back"); + string first = HeadSha(target); + string tip = AppendForgedCommit(source, ("second.txt", "2")); + await _client.PullAsync(target, Token, CancellationToken.None); + using(var repo = new Repository(target)) + { + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), new ObjectId(first)); // "git reset --soft HEAD~1" + } + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); + } + + + + [Fact] + public async Task Managed_ARecordedTipThatIsNotACommit_IsNotTrusted_AndTheTreeIsWrittenAgain() + { + (_, string target) = await CreateManagedAsync("managed-odd-tip"); + File.WriteAllText(Path.Combine(target, "keep.txt"), "changed on disk"); + string file = RecoveryFilePath(target); + JsonNode document = JsonNode.Parse(File.ReadAllText(file))!; + document["MaterializedTip"] = "not a commit id"; + File.WriteAllText(file, document.ToJsonString()); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.Equal(HeadSha(target), MaterializedTip(target)); + } + + + + // ---------------------------------------------------------------- found before anything changes + + [Fact] + public async Task Managed_AnotherGitProcessAtWork_IsFoundBeforeAnythingChanges() + { + (string source, string target) = await CreateManagedAsync("managed-index-lock"); + string before = HeadSha(target); + string tip = AppendForgedCommit(source, ("more.txt", "m")); + string lockFile = Path.Combine(target, ".git", "index.lock"); + File.WriteAllText(lockFile, ""); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("its index is locked", ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + + File.Delete(lockFile); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(tip, HeadSha(target)); + } + + + + [Fact] + public async Task Managed_AFolderWithSomeonesFilesWhereTheRepositoryNowHasAFile_IsFoundBeforeAnythingChanges() + { + (string source, string target) = await CreateManagedAsync("managed-folder-in-the-way"); + string before = HeadSha(target); + Directory.CreateDirectory(Path.Combine(target, "notes", "deeper")); + File.WriteAllText(Path.Combine(target, "notes", "deeper", "mine.txt"), "mine"); + AppendForgedCommit(source, ("notes", "origin's file")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("'notes' is a folder with files in it ('notes/deeper/mine.txt' is one)", ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + Assert.Equal("mine", File.ReadAllText(Path.Combine(target, "notes", "deeper", "mine.txt"))); + } + + + + [Fact] + public async Task Managed_AFolderOfItsOwnThatUpstreamTurnedIntoAFile_IsReplaced() + { + (string source, string target) = await CreateManagedAsync("managed-own-folder-to-file"); + AppendForgedCommit(source, ("notes/a.txt", "a"), ("notes/deeper/b.txt", "b")); + await _client.PullAsync(target, Token, CancellationToken.None); + using(var repo = new Repository(source)) + { + Commit head = repo.Head.Tip; + TreeDefinition definition = TreeDefinition.From(head.Tree).Remove("notes"); + definition.Add("notes", NewBlob(repo, "now a file"), Mode.NonExecutableFile); + Signature signature = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit( + signature, signature, "folder becomes file", repo.ObjectDatabase.CreateTree(definition), [head], prettifyMessage: false); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + } + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("now a file", File.ReadAllText(Path.Combine(target, "notes"))); + } + + + + [Fact] + public async Task Managed_ALinkOnTheWayToAFileThatLeftTheTree_IsFoundBeforeAnythingChanges() + { + (string source, string target) = await CreateManagedAsync("managed-link-to-stale"); + AppendForgedCommit(source, ("sub/old.txt", "old")); + await _client.PullAsync(target, Token, CancellationToken.None); + string before = HeadSha(target); + // The folder gclo wrote into is replaced by a link to a folder elsewhere that + // happens to have a file of the same name. + string elsewhere = Path.Combine(_root, "elsewhere-stale"); + Directory.CreateDirectory(elsewhere); + File.WriteAllText(Path.Combine(elsewhere, "old.txt"), "someone else's"); + Directory.Delete(Path.Combine(target, "sub"), recursive: true); + string link = Path.Combine(target, "sub"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + ReplaceEntry(source, remove: "sub/old.txt", add: ("other.txt", "o")); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("is a link to another location", ex.Message); + Assert.Equal(before, HeadSha(target)); + Assert.Equal(before, MaterializedTip(target)); + Assert.Equal("someone else's", File.ReadAllText(Path.Combine(elsewhere, "old.txt"))); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + [Theory] + [InlineData("docs", "docs/a.txt")] + [InlineData("Docs", "docs/a.txt")] + public async Task Managed_AFileOfItsOwnThatUpstreamTurnedIntoAFolder_IsNotTakenForSomeoneElses(string file, string inFolder) + { + if(!string.Equals(file, "docs", StringComparison.Ordinal) && !OperatingSystem.IsWindows()) + { + return; // only a file system that ignores case sees 'Docs' in the way of 'docs' + } + (string source, string target) = await CreateManagedAsync("managed-own-file-to-folder-" + file); + AppendForgedCommit(source, (file, "a file")); + await _client.PullAsync(target, Token, CancellationToken.None); + ReplaceEntry(source, remove: file, add: (inFolder, "in a folder")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("in a folder", File.ReadAllText(Path.Combine(target, "docs", "a.txt"))); + } + + + + [Fact] + public async Task Managed_AFileThatLeftTheTree_WhereAFolderOfTheUsersNowSits_LeavesTheFolder_AndTheSyncSucceeds() + { + (string source, string target) = await CreateManagedAsync("managed-stale-is-a-folder"); + AppendForgedCommit(source, ("notes", "a file")); + await _client.PullAsync(target, Token, CancellationToken.None); + File.Delete(Path.Combine(target, "notes")); + Directory.CreateDirectory(Path.Combine(target, "notes")); + File.WriteAllText(Path.Combine(target, "notes", "mine.txt"), "mine"); + ReplaceEntry(source, remove: "notes", add: ("other.txt", "o")); + string tip = HeadSha(source); + + await _client.PullAsync(target, Token, CancellationToken.None); + await _client.PullAsync(target, Token, CancellationToken.None); // and it stays fine + + Assert.Equal(tip, HeadSha(target)); + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("mine", File.ReadAllText(Path.Combine(target, "notes", "mine.txt"))); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + } + + + + // ---------------------------------------------------------------- what an interrupted apply had and was about to have + + [Fact] + public async Task Managed_AFileThatCouldNotBeRemovedWhenThePullWasInterrupted_IsRemovedByTheNext() + { + (string source, string target) = await CreateManagedAsync("managed-stale-held"); + string tip = HeadSha(source); + ReplaceEntry(source, remove: "keep.txt", add: ("zz-new.txt", "new")); + tip = HeadSha(source); + // "keep.txt" left the tree and is held open: removing it is what fails. + await InterruptAsync(target, "keep.txt", () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Null(MaterializedTip(target)); + Assert.True(File.Exists(Path.Combine(target, "keep.txt"))); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, MaterializedTip(target)); + Assert.False(File.Exists(Path.Combine(target, "keep.txt")), "it is still known to be gclo's, and it left the tree"); + } + + + + [Fact] + public async Task Managed_AFileWrittenBeforeThePullWasInterrupted_IsRemovedByTheNext_WhenItHasLeftTheTreeByThen() + { + (string source, string target) = await CreateManagedAsync("managed-new-then-gone"); + AppendForgedCommit(source, ("aa-new.txt", "new"), ("keep.txt", "changed")); + // "aa-new.txt" is written, then "keep.txt" (held open) stops the pull. + await InterruptAsync(target, "keep.txt", () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.True(File.Exists(Path.Combine(target, "aa-new.txt"))); + ReplaceEntry(source, remove: "aa-new.txt", add: ("other.txt", "o")); + + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.False(File.Exists(Path.Combine(target, "aa-new.txt")), "written by the interrupted pull, so gclo's to remove"); + Assert.Equal("changed", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.Equal("o", File.ReadAllText(Path.Combine(target, "other.txt"))); + } + + + + // ---------------------------------------------------------------- written by the released build, recovery file lost + + [Fact] + public async Task ResolvedByTheReleasedBuild_WhoseRecoveryFileWasLost_CanBeResolvedAgain_AsTheMessageAdvises() + { + (string source, string target) = await CreateManagedAsync("released-build-file-lost"); + // What 1.1.0 leaves (no index, no managed flag), with its recovery file gone + // before this build ever read it. + File.Delete(Path.Combine(target, ".git", "index")); + UnsetFlag(target, ManagedKey); + File.Delete(RecoveryFilePath(target)); + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal(HeadSha(target), MaterializedTip(target)); + Assert.True(IsFlagSet(target, ManagedKey)); + string tip = AppendForgedCommit(source, ("other.txt", "o")); + await _client.PullAsync(target, Token, CancellationToken.None); + Assert.Equal(tip, HeadSha(target)); + } + + + + [Fact] + public async Task ARefusedApply_LeavesTheRepositorysConfigurationAsItWas() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + string config = Path.Combine(target, ".git", "config"); + string before = File.ReadAllText(config); + + await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Equal(before, File.ReadAllText(config)); + } + + + // ---------------------------------------------------------------- helpers + /// Removes what this clone remembers of where origin's branch was, as expiry does. + private static void DeleteReflogOfTheTrackedBranch(string workdir) + { + using var repo = new Repository(workdir); + string log = Path.Combine(repo.Info.Path, "logs", repo.Head.TrackedBranch.CanonicalName.Replace('/', Path.DirectorySeparatorChar)); + File.Delete(log); + } + + + /// A forged source with valid paths, and an ordinary (fully checked-out) clone of it. private async Task<(string Source, string Target)> CloneOrdinaryAsync(params (string Name, string Content)[] entries) { diff --git a/gclo.Engine.Tests/WorkingTreeTests.cs b/gclo.Engine.Tests/WorkingTreeTests.cs index 7c51e18..4ea2a44 100644 --- a/gclo.Engine.Tests/WorkingTreeTests.cs +++ b/gclo.Engine.Tests/WorkingTreeTests.cs @@ -449,6 +449,74 @@ public sealed class WorkingTreeTests : IDisposable + [Fact] + public void Delete_WhereAFolderNowSits_LeavesIt_WithEverythingInIt() + { + Directory.CreateDirectory(Path.Combine(_workdir, "notes", "empty")); + File.WriteAllText(Path.Combine(_workdir, "notes", "mine.txt"), "mine"); + + WorkingTree.Delete(_workdir, "notes"); + + Assert.Equal("mine", File.ReadAllText(Path.Combine(_workdir, "notes", "mine.txt"))); + Assert.True(Directory.Exists(Path.Combine(_workdir, "notes", "empty"))); + } + + + + [Fact] + public void Create_WhereAFolderWithFilesSits_IsRefused_BeforeAnythingInItIsTouched() + { + Directory.CreateDirectory(Path.Combine(_workdir, "docs", "aaa-empty")); + Directory.CreateDirectory(Path.Combine(_workdir, "docs", "zzz")); + File.WriteAllText(Path.Combine(_workdir, "docs", "zzz", "mine.txt"), "mine"); + LinkDirectory(Path.Combine(_workdir, "docs", "bbb-link"), _outside); + + Assert.Throws(() => WorkingTree.Create(_workdir, "docs").Dispose()); + + // Not even the empty folder or the link that come before the file is found. + Assert.True(Directory.Exists(Path.Combine(_workdir, "docs", "aaa-empty"))); + Assert.True(Directory.Exists(Path.Combine(_workdir, "docs", "bbb-link"))); + Assert.Equal("mine", File.ReadAllText(Path.Combine(_workdir, "docs", "zzz", "mine.txt"))); + } + + + + [Fact] + public void Create_WhereAReadOnlyEmptyFolderSits_ReplacesIt() + { + var folder = new DirectoryInfo(Path.Combine(_workdir, "docs")); + folder.Create(); + folder.Attributes |= FileAttributes.ReadOnly; + + using(FileStream file = WorkingTree.Create(_workdir, "docs")) + { + file.Write("now a file"u8); + } + + Assert.Equal("now a file", File.ReadAllText(Path.Combine(_workdir, "docs"))); + } + + + + [Fact] + public void FilesUnder_ListsTheFilesOfARealFolder_NotThoseBehindALink() + { + Directory.CreateDirectory(Path.Combine(_workdir, "docs", "deeper")); + File.WriteAllText(Path.Combine(_workdir, "docs", "a.txt"), "a"); + File.WriteAllText(Path.Combine(_workdir, "docs", "deeper", "b.txt"), "b"); + File.WriteAllText(Path.Combine(_outside, "outside.txt"), "o"); + LinkDirectory(Path.Combine(_workdir, "docs", "linked"), _outside); + LinkDirectory(Path.Combine(_workdir, "whole-link"), _outside); + File.WriteAllText(Path.Combine(_workdir, "plain.txt"), "p"); + + Assert.Equal(["docs/a.txt", "docs/deeper/b.txt"], WorkingTree.FilesUnder(_workdir, "docs").Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Empty(WorkingTree.FilesUnder(_workdir, "whole-link")); + Assert.Empty(WorkingTree.FilesUnder(_workdir, "plain.txt")); + Assert.Empty(WorkingTree.FilesUnder(_workdir, "not/there")); + } + + + [Fact] public void Delete_MissingFile_IsFine() { diff --git a/gclo.Engine/IGitClient.cs b/gclo.Engine/IGitClient.cs index 78880b8..47e226a 100644 --- a/gclo.Engine/IGitClient.cs +++ b/gclo.Engine/IGitClient.cs @@ -53,8 +53,9 @@ public interface IGitClient /// /// The tree is written by hand, with force, so this refuses /// () a clone that was checked out - /// normally: that tree may hold someone's work. It never moves a branch off a - /// commit made in the repository. Throws + /// normally: that tree may hold someone's work. It moves a branch only to a + /// commit of origin's, and off the current one only when that is contained in + /// origin's or was itself once origin's tip. Throws /// when the effective path set is /// still invalid or collides, before anything is changed; on success the merged /// recovery is persisted so later pulls re-apply it to new commits. diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 5026fad..a32d662 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -701,29 +701,36 @@ public sealed class LibGit2GitClient : IGitClient /// private static void ApplyRecoveryCore(Repository repo, PathRecovery? incoming, CancellationToken ct) { - if(OperatingSystem.IsWindows()) - { - repo.Config.Set("core.longpaths", true, ConfigurationLevel.Local); - } - string workdir = WorkingDirectoryOf(repo); string recoveryFile = GetRecoveryFilePath(repo); RecoveryDocument? stored = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null; - bool managed = stored is not null || IsRecoveryManaged(repo); - if(!managed && !IsCheckoutPending(repo) && !repo.Info.IsHeadUnborn) + bool managed = stored is not null || IsRecoveryManaged(repo) || WasWrittenWithoutAnIndex(repo); + + /////////////////////////////////////////////////////////////////////////////// + // An empty repository counts as never checked out only while git has staged // + // nothing in it either: a branch started with "checkout --orphan" has no // + // commit yet, and a working tree full of someone's files. // + /////////////////////////////////////////////////////////////////////////////// + bool emptyAndUntouched = repo.Info.IsHeadUnborn && repo.Index.Count == 0; + if(!managed && !IsCheckoutPending(repo) && !emptyAndUntouched) { throw new InvalidOperationException( "This repository is checked out normally. Resolving its paths would replace the working tree with one that gclo " + "writes itself, which could overwrite work in it, so gclo does not do that to a clone that was checked out. " + $"To get it with renamed paths, move or delete the folder '{Path.TrimEndingDirectorySeparator(workdir)}' and sync again" - + (OperatingSystem.IsWindows() ? ", or clone it in WSL." : ".")); + + (OperatingSystem.IsWindows() ? ", or use Clone in WSL in the desktop app." : ".")); + } + + // Only now: a refused apply leaves the repository's configuration as it was too. + if(OperatingSystem.IsWindows()) + { + repo.Config.Set("core.longpaths", true, ConfigurationLevel.Local); } PathRecovery recovery = MergeRecoveries(stored is null ? s_noRecovery : ToRecovery(stored), incoming ?? s_noRecovery); - Commit tip = TargetOf(repo, managed); - if(incoming is null && string.Equals(stored?.MaterializedTip, tip.Sha, StringComparison.Ordinal)) + Commit tip = TargetOf(repo); + if(incoming is null && IsAlreadyOnDisk(repo, stored, tip)) { - // The tree on disk already is this commit, written through these choices. // (The file was read without complaint, so the repository is known to be // managed from here on even if the file is lost.) repo.Config.Set(RecoveryManagedKey, true, ConfigurationLevel.Local); @@ -742,9 +749,8 @@ public sealed class LibGit2GitClient : IGitClient } HashSet? ours = stored?.MaterializedPaths is null ? null : new HashSet(stored.MaterializedPaths, StringComparer.Ordinal); - RefuseWhatCannotBeWritten(workdir, entries, ours); - var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); + RefuseWhatCannotBeWritten(repo, workdir, entries, ours, effectiveNow, nothingHereIsOurs: !managed && !IsCheckoutPending(repo)); ///////////////////////////////////////////////////////////////////////////// // The point of no return. Record first that this tree is being rewritten, // @@ -792,8 +798,50 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// The repository's own word that gclo wrote its working tree. Read from the + /// repository's configuration only: a value set for the user or the machine says + /// nothing about this folder. + /// private static bool IsRecoveryManaged(Repository repo) - => repo.Config.Get(RecoveryManagedKey)?.Value == true; + => repo.Config.Get(RecoveryManagedKey, ConfigurationLevel.Local)?.Value == true; + + + + /// + /// True for a repository that has commits and no index: git never checked it out, + /// so whatever tree is there was written by gclo. That is how a repository looks + /// that the released build resolved (it wrote files and no index), also after its + /// recovery file was lost, and a clone that was fetched and never checked out. + /// + private static bool WasWrittenWithoutAnIndex(Repository repo) + => !repo.Info.IsHeadUnborn && !File.Exists(Path.Combine(repo.Info.Path, "index")); + + + + /// + /// True when a pull has nothing to write: the branch already is at the commit to + /// write, and what gclo last wrote is that commit or one it builds on. In the + /// second case the commits on top were made here and origin brought nothing; + /// rewriting the tree to what was committed would throw away everything else in it. + /// + private static bool IsAlreadyOnDisk(Repository repo, RecoveryDocument? stored, Commit tip) + { + if(stored?.MaterializedTip is not { } written + || !string.Equals(repo.Head.Tip?.Sha, tip.Sha, StringComparison.Ordinal)) + { + return false; // an apply is unfinished, or the branch still has to move + } + + if(string.Equals(written, tip.Sha, StringComparison.Ordinal)) + { + return true; + } + + Commit? writtenCommit = ObjectId.TryParse(written, out ObjectId writtenId) ? repo.Lookup(writtenId) : null; + return writtenCommit is not null + && string.Equals(repo.ObjectDatabase.FindMergeBase(writtenCommit, tip)?.Sha, written, StringComparison.Ordinal); + } @@ -809,20 +857,16 @@ public sealed class LibGit2GitClient : IGitClient /// /// An empty repository (unborn HEAD): origin's tip for that branch, its first /// checkout. - /// Fetched but never checked out: the current commit, which the clone or the - /// last pull put there. - /// Managed, and origin has nothing the current commit lacks (no upstream, a - /// detached HEAD, the same commit, or commits made here on top of origin's): the - /// current commit. - /// Managed, and the current commit is contained in origin's tip: origin's - /// tip. - /// Managed, and the histories diverged: origin's tip only when the current - /// commit is one origin's branch itself once pointed at, which the reflog of the - /// remote-tracking branch shows. Then nobody committed here and origin rewrote its - /// history. Otherwise the refusal the ordinary pull gives. + /// Origin has nothing the current commit lacks (no upstream, a detached HEAD, + /// the same commit, or commits made here on top of origin's): the current + /// commit. + /// The current commit is contained in origin's tip: origin's tip. + /// The histories diverged: origin's tip only when the current commit is one + /// origin's branch itself once pointed at (). Then + /// nobody committed here and origin rewrote its history. Otherwise a refusal. /// /// - private static Commit TargetOf(Repository repo, bool managed) + private static Commit TargetOf(Repository repo) { Commit? head = repo.Head.Tip; if(head is null) @@ -836,7 +880,7 @@ public sealed class LibGit2GitClient : IGitClient "This repository has no commits, and neither has its branch on origin; there is nothing to materialize."); } - Branch? tracked = managed && !repo.Info.IsHeadDetached ? repo.Head.TrackedBranch : null; + Branch? tracked = repo.Info.IsHeadDetached ? null : repo.Head.TrackedBranch; Commit? upstream = tracked?.Tip; if(upstream is null) { @@ -855,19 +899,46 @@ public sealed class LibGit2GitClient : IGitClient } throw new InvalidOperationException( - $"'{repo.Head.FriendlyName}' has local commits that origin does not have; gclo only fast-forwards and never merges. " - + "Push or reset the local commits, or delete the folder to re-clone."); + $"'{repo.Head.FriendlyName}' and its branch on origin have diverged, and nothing here shows that the commits " + + "in this folder came from origin: either commits were made here, or origin's history was rewritten and the " + + "record of what was fetched is gone. gclo only fast-forwards and never merges. " + + "Push or reset commits made here, or delete the folder to re-clone."); } /// - /// True when the remote-tracking branch itself pointed at - /// at some time, going by its reflog (every fetch that - /// moves the branch adds an entry). A commit made in this clone is never in there. + /// True when origin's branch itself pointed at at some + /// time, going by the reflog of the remote-tracking branch : + /// every fetch that moves the branch adds an entry saying where it went. A push + /// adds one too, with the commit that was made here, so entries written by a push + /// do not count. Where the branch was before the oldest entry that is left (older + /// ones expire) counts as well. A branch that tracks another local branch has no + /// such record of origin, and nothing is taken from its reflog. /// private static bool WasOnceTheTipOf(Repository repo, Branch tracked, Commit commit) - => repo.Refs.Log(tracked.CanonicalName).Any(entry => entry.To == commit.Id || entry.From == commit.Id); + { + if(!tracked.IsRemote) + { + return false; + } + + ReflogEntry? oldest = null; + foreach(ReflogEntry entry in repo.Refs.Log(tracked.CanonicalName)) // newest first + { + if(entry.To == commit.Id && !IsWrittenByAPush(entry)) + { + return true; + } + oldest = entry; + } + return oldest is not null && oldest.From == commit.Id; + } + + + + private static bool IsWrittenByAPush(ReflogEntry entry) + => entry.Message?.StartsWith("update by push", StringComparison.Ordinal) == true; @@ -896,26 +967,60 @@ public sealed class LibGit2GitClient : IGitClient /// - /// Finds, before anything has changed, what would make the apply fail half-way: a - /// destination that is not a usable path in the working tree, a link on the way to - /// it, or a file sitting where a folder is needed (unless that file is one gclo - /// wrote and is about to remove). Failing here leaves the branch, the index and - /// every file as they were (#147). + /// Finds, before anything has changed, what it can of what would make the apply + /// fail half-way or write over something that is not gclo's: another git process at + /// work, a destination that is not a usable path in the working tree, a link on the + /// way to it or to a file that is to be removed, a file sitting where a folder is + /// needed, a folder holding someone's files where a file is needed, and, in a + /// folder gclo has not written to yet, any file already at a destination. Failing + /// here leaves the branch, the index and every file as they were (#147). What only + /// shows while writing (a file held open) still fails later; the next pull then + /// completes the apply. /// private static void RefuseWhatCannotBeWritten( - string workdir, List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, HashSet? ours) + Repository repo, + string workdir, + List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, + HashSet? ours, + HashSet effectiveNow, + bool nothingHereIsOurs) { + if(File.Exists(Path.Combine(repo.Info.Path, "index.lock"))) + { + throw new InvalidOperationException( + "Another git process is working in this repository (its index is locked). Try again when it has finished."); + } + + // The file system decides whether two names are one file; on Windows it does + // not tell 'Docs' from 'docs', so neither does the question "is this ours". + var oursAsOnDisk = new HashSet( + ours ?? [], OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); var verifiedFolders = new HashSet(StringComparer.Ordinal); var checkedFolders = new HashSet(StringComparer.Ordinal); foreach((_, string? effectivePath, _) in entries) { WorkingTree.Verify(workdir, effectivePath, verifiedFolders); + if(nothingHereIsOurs && File.Exists(Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)))) + { + throw new InvalidOperationException( + $"'{effectivePath}' is already in this folder. gclo has not written anything here yet, so that file is not " + + "its to replace. Move or delete it, then try again."); + } + + string? foreign = WorkingTree.FilesUnder(workdir, effectivePath).FirstOrDefault(file => !oursAsOnDisk.Contains(file)); + if(foreign is not null) + { + throw new InvalidOperationException( + $"'{effectivePath}' is a folder with files in it ('{foreign}' is one), where the repository has a file. " + + "Move or delete the folder, then try again."); + } + for(int slash = effectivePath.IndexOf('/', StringComparison.Ordinal); slash > 0; slash = effectivePath.IndexOf('/', slash + 1)) { string folder = effectivePath[..slash]; if(checkedFolders.Add(folder) - && ours?.Contains(folder) != true + && !oursAsOnDisk.Contains(folder) && File.Exists(Path.Combine(workdir, folder.Replace('/', Path.DirectorySeparatorChar)))) { throw new InvalidOperationException( @@ -924,6 +1029,15 @@ public sealed class LibGit2GitClient : IGitClient } } } + + // What left the tree is removed through real folders only, like what is written. + foreach(string stale in ours ?? []) + { + if(!effectiveNow.Contains(stale)) + { + WorkingTree.Verify(workdir, stale, verifiedFolders); + } + } } @@ -1062,19 +1176,23 @@ public sealed class LibGit2GitClient : IGitClient /// - /// Moves the current branch's ref to its upstream tip without touching the working - /// tree. Only for a repository whose clone was never checked out: it holds no local - /// work, so its branch simply follows origin. A repository that has a working tree - /// goes through instead. + /// Moves the current branch to where origin's is, without touching the working + /// tree, for a repository whose clone was never checked out. By the rule every + /// branch move here follows (): a commit somebody made in + /// it by hand is not dropped. /// private static void AdvanceHeadToTrackedTip(Repository repo) { - Branch tracked = repo.Head.TrackedBranch; - if(tracked?.Tip is null || string.Equals(repo.Head.Tip?.Sha, tracked.Tip.Sha, StringComparison.Ordinal)) + if(repo.Info.IsHeadUnborn) { return; } - repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), tracked.Tip.Id); + + Commit target = TargetOf(repo); + if(!string.Equals(repo.Head.Tip.Sha, target.Sha, StringComparison.Ordinal)) + { + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), target.Id); + } } diff --git a/gclo.Engine/WorkingTree.cs b/gclo.Engine/WorkingTree.cs index 7a44e7b..1a9d411 100644 --- a/gclo.Engine/WorkingTree.cs +++ b/gclo.Engine/WorkingTree.cs @@ -120,13 +120,20 @@ internal static class WorkingTree /// - /// Deletes (a missing file is fine) and then the folders - /// it leaves empty, stopping at the working directory itself. + /// Deletes the file (a missing one is fine) and then + /// the folders it leaves empty, stopping at the working directory itself. A folder + /// that sits there now is left alone with everything in it: the file gclo wrote + /// is gone already, and whoever put the folder there owns it. /// public static void Delete(string workdir, string repoPath) { string full = Resolve(workdir, repoPath); RefuseLinkOnTheWay(workdir, full, verifiedFolders: null); + var folder = new DirectoryInfo(full); + if(folder.Exists && folder.LinkTarget is null) + { + return; + } RemoveFile(full); string root = Root(workdir); @@ -224,9 +231,43 @@ internal static class WorkingTree /// - /// Removes a folder that holds nothing but empty folders, bottom up, and never a - /// file: if one is in there (it is not gclo's, or it would have left with the old - /// tree) the folder is refused instead. + /// The files inside the real folder at , as paths + /// relative to the working directory; nothing when no folder is there, or a link + /// is. Links inside it are not entered and not listed. + /// + public static IEnumerable FilesUnder(string workdir, string repoPath) + { + var folder = new DirectoryInfo(Resolve(workdir, repoPath)); + return folder.Exists && folder.LinkTarget is null ? FilesIn(folder, repoPath) : []; + } + + + + private static IEnumerable FilesIn(DirectoryInfo folder, string repoPath) + { + foreach(FileInfo file in folder.EnumerateFiles()) + { + yield return repoPath + "/" + file.Name; + } + + foreach(DirectoryInfo child in folder.EnumerateDirectories()) + { + if(child.LinkTarget is not null) + { + continue; + } + foreach(string inner in FilesIn(child, repoPath + "/" + child.Name)) + { + yield return inner; + } + } + } + + + + /// + /// Removes a folder that holds nothing but empty folders and links, bottom up. + /// Called only for a folder already found to hold no file. /// private static void RemoveEmptyFolders(DirectoryInfo folder) { @@ -240,13 +281,6 @@ internal static class WorkingTree RemoveEmptyFolders(child); } - if(folder.EnumerateFiles().Any()) - { - throw new InvalidOperationException( - $"'{folder.FullName}' is a folder with files in it, where the repository has a file. " - + "Move or delete the folder, then try again."); - } - folder.Attributes = FileAttributes.Directory; folder.Delete(); } @@ -272,7 +306,14 @@ internal static class WorkingTree { // A real folder where a file belongs (upstream turned a folder into a file). // Whatever the tree had in it is gone by now; if files remain they are not - // gclo's to remove, and empty folders are nobody's. + // gclo's to remove, and empty folders are nobody's. Looked at as a whole + // first: a folder that is refused loses nothing, not even an empty subfolder. + if(FilesIn(directory, directory.Name).Any()) + { + throw new InvalidOperationException( + $"'{directory.FullName}' is a folder with files in it, where the repository has a file. " + + "Move or delete the folder, then try again."); + } RemoveEmptyFolders(directory); return; } -- 2.54.0 From 52adc1a450ef9232a65988aaa7e85a7badef9246 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 16:23:44 -0600 Subject: [PATCH 5/6] fix(recovery): address the re-check of the review fixes - A branch ahead of what was written counts as 'commits made here' only when origin has nothing beyond what was written. A pull of the released build that failed after it moved the branch leaves the branch at origin's commit and the tree behind it; that tree is now written, and a new invalid path is reported again instead of 'nothing to do'. - A tree is taken for one gclo wrote without an index only in a repository gclo cloned (it carries gclo's checkout marker). A clone made by git without a checkout is not. - Where nothing records that gclo wrote the tree (never checked out, empty, or recognised only by its shape), a file already at a destination is replaced only when it is, byte for byte, what the current commit has there. A file a person put there or changed stops the apply. - The pull recognises a tree written without an index, so a repository of the released build whose recovery file was lost reports its paths again and can be resolved from the app and with --sanitize-paths. - A pull does not check out over a branch that has no commits yet when files are staged in the folder ('checkout --orphan' in a checked-out clone). - What counts as origin's commit is what a clone, fetch, or pull recorded; any other entry on the remote-tracking branch does not. - The checkout marker is read from the repository's own configuration only. Messages: a stale index.lock, a branch that tracks a local branch. Refs #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/CLI.md | 12 +- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 190 +++++++++++++++++++- gclo.Engine/IGitClient.cs | 6 +- gclo.Engine/LibGit2GitClient.cs | 158 ++++++++++++---- 4 files changed, 325 insertions(+), 41 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index 6951386..784755a 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -168,8 +168,10 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: its working tree may hold work, and writing it by hand could destroy that. To get it with the renames, move or delete its folder and sync again (the fresh clone is then sanitized), or use the desktop app's **Clone in WSL**. - The same holds for an empty clone into whose folder files were put by hand: - a file that is already where the repository has one is never replaced. + The same holds for a clone that was never checked out, or an empty one, into + whose folder files were put by hand: a file that is already where the + repository has one, and is not what an unfinished checkout would have left + there, is never replaced. Only the working tree is renamed — the repository's history and index still hold the original paths, so `git status` reports renamed and skipped paths as local @@ -178,9 +180,9 @@ rewrite its working tree, so edits made there are overwritten. A commit made there is not dropped: while origin has nothing new, a sync leaves the tree as it is, and once origin has moved on as well the sync fails for that repository, as it does for any clone with local commits. The branch is only -ever moved to a commit of origin's: forward, or, when origin's history was -rewritten, off a commit that this clone's record of earlier fetches shows was -origin's own. +ever moved to the tip of the branch it tracks: forward, or, when origin's +history was rewritten, off a commit that this clone's record of earlier +fetches shows was origin's own. #### `--json` diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs index b6b6065..dddd4f5 100644 --- a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -604,8 +604,13 @@ public sealed class RecoveryLocalWorkTests : IDisposable repo.Refs.Add("refs/heads/topic", onTopic.Id); } Dictionary filesBefore = ReadAllFiles(target); - await PullExpectingInvalidPathsAsync(target); + // The pull does not take "no commit yet" for "nothing checked out"... + InvalidOperationException pull = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + Assert.Contains("has no commits here yet, but files are staged", pull.Message); + + // ...and neither does resolving. InvalidOperationException ex = await Assert.ThrowsAsync( () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); @@ -616,6 +621,30 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task ABranchWithoutCommitsInACheckedOutClone_IsNotCheckedOutOver_ByAPull_EvenWithValidPaths() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + using(var repo = new Repository(target)) + { + repo.Refs.UpdateTarget("HEAD", "refs/heads/topic"); + } + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + using(var repo = new Repository(source)) + { + Commit onTopic = NewRootCommit(repo, [("work.txt", "origin's")]); + repo.Refs.Add("refs/heads/topic", onTopic.Id); + } + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains("gclo does not check out over them", ex.Message); + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "work.txt"))); + } + + + // ---------------------------------------------------------------- what counts as origin's commit [Fact] @@ -1013,6 +1042,10 @@ public sealed class RecoveryLocalWorkTests : IDisposable UnsetFlag(target, ManagedKey); File.Delete(RecoveryFilePath(target)); + // The sync reports the paths again, which is what offers Resolve in the app + // and what --sanitize-paths acts on... + await PullExpectingInvalidPathsAsync(target); + // ...and resolving rebuilds the file. await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); Assert.Equal(HeadSha(target), MaterializedTip(target)); @@ -1030,6 +1063,10 @@ public sealed class RecoveryLocalWorkTests : IDisposable (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1")); AppendForgedCommit(source, ("bad:file.txt", "bad")); await PullExpectingInvalidPathsAsync(target); + using(var repo = new Repository(target)) + { + repo.Config.Unset("core.longpaths", ConfigurationLevel.Local); // the pull set it; the refused apply must not + } string config = Path.Combine(target, ".git", "config"); string before = File.ReadAllText(config); @@ -1041,6 +1078,157 @@ public sealed class RecoveryLocalWorkTests : IDisposable + // ---------------------------------------------------------------- what the released build can leave behind + + /// + /// Puts a managed repository into the state a pull of the released build leaves + /// when it fails after its first step: the branch is at origin's new commit, the + /// tree and the recovery file are still at the previous one, and there is neither + /// an index nor the managed flag. + /// + private static void LeaveAsAFailedPullOfTheReleasedBuild(string target) + { + using var repo = new Repository(target); + Commands.Fetch(repo, "origin", [], null, null); + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), repo.Head.TrackedBranch.Tip.Id); + repo.Config.Unset(ManagedKey, ConfigurationLevel.Local); + File.Delete(Path.Combine(repo.Info.Path, "index")); + } + + + + [Fact] + public async Task AfterAPullOfTheReleasedBuildThatWasInterrupted_TheNextPullWritesTheTree() + { + (string source, string target) = await CreateManagedAsync("released-build-interrupted"); + string first = HeadSha(target); + string tip = AppendForgedCommit(source, ("new.txt", "new"), ("keep.txt", "changed")); + LeaveAsAFailedPullOfTheReleasedBuild(target); + Assert.Equal(tip, HeadSha(target)); + Assert.Equal(first, MaterializedTip(target)); + + // The branch being ahead of what was written is origin's doing here, not a + // commit made in the folder: the tree still has to be written. + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal(tip, MaterializedTip(target)); + Assert.Equal("new", File.ReadAllText(Path.Combine(target, "new.txt"))); + Assert.Equal("changed", File.ReadAllText(Path.Combine(target, "keep.txt"))); + } + + + + [Fact] + public async Task AfterAPullOfTheReleasedBuildThatFailedOnANewInvalidPath_TheNextPullReportsItAgain() + { + (string source, string target) = await CreateManagedAsync("released-build-new-invalid"); + AppendForgedCommit(source, ("another:bad.txt", "bad too")); + LeaveAsAFailedPullOfTheReleasedBuild(target); + + // Reporting nothing would take the Resolve link away and leave the tree a commit behind. + InvalidRepositoryPathsException ex = await Assert.ThrowsAsync( + () => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Contains(ex.Paths, p => string.Equals(p.RepoPath, "another:bad.txt", StringComparison.Ordinal)); + } + + + + // ---------------------------------------------------------------- a tree that only looks like one gclo wrote + + [Fact] + public async Task AClonedRepositoryWhoseIndexWasDeleted_KeepsItsEditedFile_WhenResolved() + { + (string source, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + File.WriteAllText(Path.Combine(target, "untracked.txt"), "mine"); + File.Delete(Path.Combine(target, ".git", "index")); + AppendForgedCommit(source, ("bad:file.txt", "bad"), ("readme.txt", "v2")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains("'work.txt' is already in this folder", ex.Message); + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "work.txt"))); + Assert.Equal("mine", File.ReadAllText(Path.Combine(target, "untracked.txt"))); + Assert.Equal("v1", File.ReadAllText(Path.Combine(target, "readme.txt"))); + Assert.False(IsFlagSet(target, ManagedKey)); + } + + + + [Fact] + public async Task ARepositoryClonedByGitWithoutACheckout_IsNotTakenForOneGcloWrote() + { + string source = CreateForgedRepo(_root, ("readme.txt", "v1")); + string target = Path.Combine(_root, "cloned-by-git"); + Repository.Clone(source, target, new CloneOptions { Checkout = false }); + File.WriteAllText(Path.Combine(target, "readme.txt"), "put here by hand"); + AppendForgedCommit(source, ("bad:file.txt", "bad")); + await PullExpectingInvalidPathsAsync(target); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains("is checked out normally", ex.Message); + Assert.Equal("put here by hand", File.ReadAllText(Path.Combine(target, "readme.txt"))); + } + + + + [Fact] + public async Task NeverCheckedOut_WithAFileOfTheUsersWhereTheRepositoryHasOne_IsRefused_AndTheFileIsKept() + { + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "pending-with-a-file"); + File.WriteAllText(Path.Combine(target, "keep.txt"), "mine, put here by hand"); + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None)); + + Assert.Contains("'keep.txt' is already in this folder", ex.Message); + Assert.Equal("mine, put here by hand", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.False(File.Exists(Path.Combine(target, "bad_file.txt"))); + } + + + + [Fact] + public async Task NeverCheckedOut_WithAFileAnUnfinishedCheckoutLeft_IsResolved() + { + // What a checkout that was cut short leaves: a file with exactly the commit's content. + string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep")); + string target = await CloneExpectingInvalidPathsAsync(source, "pending-with-its-own-file"); + File.WriteAllText(Path.Combine(target, "keep.txt"), "keep"); + + await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None); + + Assert.Equal("keep", File.ReadAllText(Path.Combine(target, "keep.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(target, "bad_file.txt"))); + } + + + + [Fact] + public async Task Managed_ACommitPutOnTheTrackingBranchByHand_IsNotTakenForOneOfOrigins() + { + (string source, string target) = await CreateManagedAsync("managed-hand-updated-ref"); + string mine = CommitInWorkingTree(target, "mine.txt", "my work"); + using(var repo = new Repository(target)) + { + // Not a fetch: whatever wrote this entry, it does not say origin had the commit. + repo.Refs.UpdateTarget(repo.Refs[repo.Head.TrackedBranch.CanonicalName], new ObjectId(mine), ""); + } + RewriteHistory(source, ("bad:file.txt", "bad again"), ("keep.txt", "keep")); + + await Assert.ThrowsAsync(() => _client.PullAsync(target, Token, CancellationToken.None)); + + Assert.Equal(mine, HeadSha(target)); + } + + + // ---------------------------------------------------------------- helpers /// Removes what this clone remembers of where origin's branch was, as expiry does. diff --git a/gclo.Engine/IGitClient.cs b/gclo.Engine/IGitClient.cs index 47e226a..4a687d2 100644 --- a/gclo.Engine/IGitClient.cs +++ b/gclo.Engine/IGitClient.cs @@ -53,9 +53,9 @@ public interface IGitClient /// /// The tree is written by hand, with force, so this refuses /// () a clone that was checked out - /// normally: that tree may hold someone's work. It moves a branch only to a - /// commit of origin's, and off the current one only when that is contained in - /// origin's or was itself once origin's tip. Throws + /// normally: that tree may hold someone's work. It moves a branch only to the + /// tip of the branch it tracks, and off the current commit only when that is + /// contained in that tip or was itself once origin's tip. Throws /// when the effective path set is /// still invalid or collides, before anything is changed; on success the merged /// recovery is persisted so later pulls re-apply it to new commits. diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index a32d662..0507a78 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -276,7 +276,8 @@ public sealed class LibGit2GitClient : IGitClient // recovery). Both take a dedicated path — their working tree is materialized // from the tip, never by a merge checkout that trusts the existing tree. // Every other repository takes the normal fetch + fast-forward pull below. - if(File.Exists(GetRecoveryFilePath(repo)) || IsRecoveryManaged(repo)) + if(File.Exists(GetRecoveryFilePath(repo)) || IsRecoveryManaged(repo) + || (!IsCheckoutPending(repo) && WasWrittenWithoutAnIndex(repo))) { // A recovery-managed repo stays recovery-managed: the new tip is written // through the stored mapping, or nothing is touched when the tree on disk @@ -620,6 +621,20 @@ public sealed class LibGit2GitClient : IGitClient return; // the remote is still empty; nothing to pull } + /////////////////////////////////////////////////////////////////////////////// + // No commit on the branch does not mean nothing was checked out: a branch // + // started with "checkout --orphan" keeps the index and the working tree of // + // the branch it was started from. Checking out over that, with force, would // + // replace someone's files. // + /////////////////////////////////////////////////////////////////////////////// + if(repo.Index.Count > 0) + { + throw new InvalidOperationException( + $"'{branchName}' has no commits here yet, but files are staged in this folder (a branch started with " + + "'git checkout --orphan'?). gclo does not check out over them; fetched, but nothing was checked out. " + + "Commit or remove them, or check out another branch."); + } + if(OperatingSystem.IsWindows()) { IReadOnlyList invalidPaths = WindowsPathValidator.Validate(remoteBranch.Tip.Tree); @@ -704,7 +719,10 @@ public sealed class LibGit2GitClient : IGitClient string workdir = WorkingDirectoryOf(repo); string recoveryFile = GetRecoveryFilePath(repo); RecoveryDocument? stored = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null; - bool managed = stored is not null || IsRecoveryManaged(repo) || WasWrittenWithoutAnIndex(repo); + // "Managed" by gclo's own word (its file, its flag), or only by what the + // repository looks like; the second is trusted less (see below). + bool managedByRecord = stored is not null || IsRecoveryManaged(repo); + bool managed = managedByRecord || WasWrittenWithoutAnIndex(repo); /////////////////////////////////////////////////////////////////////////////// // An empty repository counts as never checked out only while git has staged // @@ -750,7 +768,7 @@ public sealed class LibGit2GitClient : IGitClient HashSet? ours = stored?.MaterializedPaths is null ? null : new HashSet(stored.MaterializedPaths, StringComparer.Ordinal); var effectiveNow = new HashSet(entries.Select(e => e.EffectivePath), StringComparer.Ordinal); - RefuseWhatCannotBeWritten(repo, workdir, entries, ours, effectiveNow, nothingHereIsOurs: !managed && !IsCheckoutPending(repo)); + RefuseWhatCannotBeWritten(repo, workdir, entries, ours, effectiveNow, replaceOnlyWhatTheCommitHas: !managedByRecord); ///////////////////////////////////////////////////////////////////////////// // The point of no return. Record first that this tree is being rewritten, // @@ -809,13 +827,20 @@ public sealed class LibGit2GitClient : IGitClient /// - /// True for a repository that has commits and no index: git never checked it out, + /// True for a repository that gclo cloned (it carries gclo's checkout marker, + /// whatever its value), has commits, and has no index: git never checked it out, /// so whatever tree is there was written by gclo. That is how a repository looks /// that the released build resolved (it wrote files and no index), also after its - /// recovery file was lost, and a clone that was fetched and never checked out. + /// recovery file was lost. A clone made by git, with or without a checkout, has + /// no such marker and is not taken for one. The inference is still only an + /// inference (an index can be deleted by hand), so a tree recognised this way is + /// rewritten only where its files are what the current commit has + /// (). /// private static bool WasWrittenWithoutAnIndex(Repository repo) - => !repo.Info.IsHeadUnborn && !File.Exists(Path.Combine(repo.Info.Path, "index")); + => !repo.Info.IsHeadUnborn + && repo.Config.Get(CheckoutPendingKey, ConfigurationLevel.Local) is not null + && !File.Exists(Path.Combine(repo.Info.Path, "index")); @@ -838,8 +863,23 @@ public sealed class LibGit2GitClient : IGitClient return true; } + /////////////////////////////////////////////////////////////////////////////// + // The branch is ahead of what was written. That is "commits made here" only // + // when origin has nothing beyond what was written. If origin does, the // + // branch was moved to origin's commit and the tree was never written for it: // + // the released build moved the branch first, so a pull of its that failed // + // half-way (a new invalid path, a cancel, a locked file) leaves exactly that. // + /////////////////////////////////////////////////////////////////////////////// Commit? writtenCommit = ObjectId.TryParse(written, out ObjectId writtenId) ? repo.Lookup(writtenId) : null; - return writtenCommit is not null + if(writtenCommit is null) + { + return false; + } + + Commit? upstream = repo.Info.IsHeadDetached ? null : repo.Head.TrackedBranch?.Tip; + bool originHasNothingNew = upstream is null + || string.Equals(repo.ObjectDatabase.FindMergeBase(upstream, writtenCommit)?.Sha, upstream.Sha, StringComparison.Ordinal); + return originHasNothingNew && string.Equals(repo.ObjectDatabase.FindMergeBase(writtenCommit, tip)?.Sha, written, StringComparison.Ordinal); } @@ -898,11 +938,13 @@ public sealed class LibGit2GitClient : IGitClient return upstream; } - throw new InvalidOperationException( - $"'{repo.Head.FriendlyName}' and its branch on origin have diverged, and nothing here shows that the commits " - + "in this folder came from origin: either commits were made here, or origin's history was rewritten and the " - + "record of what was fetched is gone. gclo only fast-forwards and never merges. " - + "Push or reset commits made here, or delete the folder to re-clone."); + throw new InvalidOperationException(tracked!.IsRemote + ? $"'{repo.Head.FriendlyName}' and its branch on origin have diverged, and nothing here shows that the commits " + + "in this folder came from origin: either commits were made here, or origin's history was rewritten and the " + + "record of what was fetched is gone. gclo only fast-forwards and never merges. " + + "Push or reset commits made here, or delete the folder to re-clone." + : $"'{repo.Head.FriendlyName}' and the local branch it tracks ('{tracked.FriendlyName}') have diverged. " + + "gclo only fast-forwards and never merges. Merge or reset by hand, or check out a branch that tracks origin."); } @@ -910,11 +952,12 @@ public sealed class LibGit2GitClient : IGitClient /// /// True when origin's branch itself pointed at at some /// time, going by the reflog of the remote-tracking branch : - /// every fetch that moves the branch adds an entry saying where it went. A push - /// adds one too, with the commit that was made here, so entries written by a push - /// do not count. Where the branch was before the oldest entry that is left (older - /// ones expire) counts as well. A branch that tracks another local branch has no - /// such record of origin, and nothing is taken from its reflog. + /// every clone, fetch, and pull that moves the branch adds an entry saying where + /// it went, and only those entries count. Other things write there too (a push + /// records the commit that was made here, a hand-made update records anything), + /// and they do not. Where the branch was before the oldest entry that is left + /// (older ones expire) counts as well. A branch that tracks another local branch + /// has no such record of origin, and nothing is taken from its reflog. /// private static bool WasOnceTheTipOf(Repository repo, Branch tracked, Commit commit) { @@ -926,7 +969,7 @@ public sealed class LibGit2GitClient : IGitClient ReflogEntry? oldest = null; foreach(ReflogEntry entry in repo.Refs.Log(tracked.CanonicalName)) // newest first { - if(entry.To == commit.Id && !IsWrittenByAPush(entry)) + if(entry.To == commit.Id && IsWrittenByAFetch(entry)) { return true; } @@ -937,8 +980,14 @@ public sealed class LibGit2GitClient : IGitClient - private static bool IsWrittenByAPush(ReflogEntry entry) - => entry.Message?.StartsWith("update by push", StringComparison.Ordinal) == true; + /// True for a reflog entry whose message says a clone, a fetch, or a pull wrote it. + private static bool IsWrittenByAFetch(ReflogEntry entry) + { + string message = entry.Message ?? ""; + return message.StartsWith("fetch", StringComparison.Ordinal) + || message.StartsWith("pull", StringComparison.Ordinal) + || message.StartsWith("clone", StringComparison.Ordinal); + } @@ -971,41 +1020,52 @@ public sealed class LibGit2GitClient : IGitClient /// fail half-way or write over something that is not gclo's: another git process at /// work, a destination that is not a usable path in the working tree, a link on the /// way to it or to a file that is to be removed, a file sitting where a folder is - /// needed, a folder holding someone's files where a file is needed, and, in a - /// folder gclo has not written to yet, any file already at a destination. Failing - /// here leaves the branch, the index and every file as they were (#147). What only - /// shows while writing (a file held open) still fails later; the next pull then - /// completes the apply. + /// needed, a folder holding someone's files where a file is needed, and, where + /// gclo has no record of having written the tree, a file at a destination that is + /// not what the current commit has there. Failing here leaves the branch, the + /// index and every file as they were (#147). What only shows while writing (a file + /// held open) still fails later; the next pull then completes the apply. /// + /// + /// replaceOnlyWhatTheCommitHas is true when nothing records that gclo wrote + /// this tree (a clone that was never checked out, an empty one, a tree recognised + /// only by its shape). A file that is already at a destination is then replaced + /// only when it is, byte for byte, what the current commit has for that path: an + /// unfinished checkout's file, not something a person put there or changed. + /// private static void RefuseWhatCannotBeWritten( Repository repo, string workdir, List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, HashSet? ours, HashSet effectiveNow, - bool nothingHereIsOurs) + bool replaceOnlyWhatTheCommitHas) { if(File.Exists(Path.Combine(repo.Info.Path, "index.lock"))) { throw new InvalidOperationException( - "Another git process is working in this repository (its index is locked). Try again when it has finished."); + "Another git process is working in this repository (its index is locked). Try again when it has finished; " + + $"if no git is running, one that crashed left '{Path.Combine(repo.Info.Path, "index.lock")}' behind, and deleting that file clears it."); } + Tree? current = replaceOnlyWhatTheCommitHas ? repo.Head.Tip?.Tree : null; + // The file system decides whether two names are one file; on Windows it does // not tell 'Docs' from 'docs', so neither does the question "is this ours". var oursAsOnDisk = new HashSet( ours ?? [], OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); var verifiedFolders = new HashSet(StringComparer.Ordinal); var checkedFolders = new HashSet(StringComparer.Ordinal); - foreach((_, string? effectivePath, _) in entries) + foreach((string? originalPath, string? effectivePath, _) in entries) { WorkingTree.Verify(workdir, effectivePath, verifiedFolders); - if(nothingHereIsOurs && File.Exists(Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)))) + string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); + if(replaceOnlyWhatTheCommitHas && File.Exists(full) && !IsWhatTheCommitHas(full, originalPath, current)) { throw new InvalidOperationException( - $"'{effectivePath}' is already in this folder. gclo has not written anything here yet, so that file is not " - + "its to replace. Move or delete it, then try again."); + $"'{effectivePath}' is already in this folder, and it is not a file gclo has a record of writing " + + "or one the current commit has in that form, so it is not gclo's to replace. Move or delete it, then try again."); } string? foreign = WorkingTree.FilesUnder(workdir, effectivePath).FirstOrDefault(file => !oursAsOnDisk.Contains(file)); @@ -1042,6 +1102,39 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// True when the file at holds exactly what a checkout of + /// writes for + /// (line-ending conversion included). False when the commit has no such file. + /// + private static bool IsWhatTheCommitHas(string full, string originalPath, Tree? commitTree) + { + if(commitTree?[originalPath]?.Target is not Blob blob) + { + return false; + } + + using Stream expected = blob.GetContentStream(new FilteringOptions(originalPath)); + using var actual = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + byte[] expectedBuffer = new byte[81920]; + byte[] actualBuffer = new byte[81920]; + while(true) + { + int expectedRead = expected.ReadAtLeast(expectedBuffer, expectedBuffer.Length, throwOnEndOfStream: false); + int actualRead = actual.ReadAtLeast(actualBuffer, actualBuffer.Length, throwOnEndOfStream: false); + if(expectedRead != actualRead || !expectedBuffer.AsSpan(0, expectedRead).SequenceEqual(actualBuffer.AsSpan(0, actualRead))) + { + return false; + } + if(expectedRead == 0) + { + return true; + } + } + } + + + /// /// Overlays onto . The newer /// choice for a path wins whatever kind it is: an incoming rename replaces a stored @@ -1197,8 +1290,9 @@ public sealed class LibGit2GitClient : IGitClient + /// The never-checked-out marker, from the repository's own configuration only. private static bool IsCheckoutPending(Repository repo) - => repo.Config.Get(CheckoutPendingKey)?.Value == true; + => repo.Config.Get(CheckoutPendingKey, ConfigurationLevel.Local)?.Value == true; -- 2.54.0 From 0324e58575d8681f80d0deb841f9c5c00bc4fdf2 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 16:46:56 -0600 Subject: [PATCH 6/6] fix(recovery): a tree with nothing to rename is an ordinary clone A repository without an index was taken for one path recovery wrote whenever gclo had cloned it. An ordinary clone whose index was deleted and that has no path to rename then went through the hand-written apply on a pull, was marked as managed for good, and had later edits overwritten. The pull now takes that route only when the current commit holds a path that cannot be created here. Anything else takes the ordinary pull, which leaves local changes alone and marks nothing. Refs #147 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/CLI.md | 8 +++---- gclo.Engine.Tests/RecoveryLocalWorkTests.cs | 23 +++++++++++++++++++++ gclo.Engine/LibGit2GitClient.cs | 16 ++++++++++++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index 784755a..c32d146 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -168,10 +168,10 @@ With `--sanitize-paths`, gclo checks such a repository out anyway: its working tree may hold work, and writing it by hand could destroy that. To get it with the renames, move or delete its folder and sync again (the fresh clone is then sanitized), or use the desktop app's **Clone in WSL**. - The same holds for a clone that was never checked out, or an empty one, into - whose folder files were put by hand: a file that is already where the - repository has one, and is not what an unfinished checkout would have left - there, is never replaced. + The same holds when the paths of a clone that was never checked out, or of + an empty one, are resolved and files were put into its folder by hand: a file + that is already where the repository has one, and is not what an unfinished + checkout would have left there, is not replaced by the renaming. Only the working tree is renamed — the repository's history and index still hold the original paths, so `git status` reports renamed and skipped paths as local diff --git a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs index dddd4f5..cba458b 100644 --- a/gclo.Engine.Tests/RecoveryLocalWorkTests.cs +++ b/gclo.Engine.Tests/RecoveryLocalWorkTests.cs @@ -1158,6 +1158,29 @@ public sealed class RecoveryLocalWorkTests : IDisposable + [Fact] + public async Task AClonedRepositoryWhoseIndexWasDeleted_AndThatHasNothingToRename_StaysAnOrdinaryClone() + { + (_, string target) = await CloneOrdinaryAsync(("readme.txt", "v1"), ("work.txt", "work")); + File.Delete(Path.Combine(target, ".git", "index")); + + await _client.PullAsync(target, Token, CancellationToken.None); // nothing new on origin + + Assert.False(IsFlagSet(target, ManagedKey), "a tree with nothing to rename is not one path recovery wrote"); + Assert.False(File.Exists(RecoveryFilePath(target))); + + // And it is pulled like any clone: with an edit in it, and nothing new on + // origin, the sync goes through and the edit is still there. + File.WriteAllText(Path.Combine(target, "work.txt"), "my local edit"); + await _client.PullAsync(target, Token, CancellationToken.None); + + Assert.Equal("my local edit", File.ReadAllText(Path.Combine(target, "work.txt"))); + Assert.False(IsFlagSet(target, ManagedKey)); + Assert.False(File.Exists(RecoveryFilePath(target))); + } + + + [Fact] public async Task ARepositoryClonedByGitWithoutACheckout_IsNotTakenForOneGcloWrote() { diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 0507a78..adac830 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -277,7 +277,7 @@ public sealed class LibGit2GitClient : IGitClient // from the tip, never by a merge checkout that trusts the existing tree. // Every other repository takes the normal fetch + fast-forward pull below. if(File.Exists(GetRecoveryFilePath(repo)) || IsRecoveryManaged(repo) - || (!IsCheckoutPending(repo) && WasWrittenWithoutAnIndex(repo))) + || (!IsCheckoutPending(repo) && WasWrittenWithoutAnIndex(repo) && HasPathsWindowsCannotCreate(repo))) { // A recovery-managed repo stays recovery-managed: the new tip is written // through the stored mapping, or nothing is touched when the tree on disk @@ -844,6 +844,17 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// True when the current commit holds a path that cannot be created here. Only + /// then can a tree without an index be one that path recovery wrote: a repository + /// with nothing to rename is an ordinary clone whatever became of its index, and + /// takes the ordinary pull, which leaves local changes alone and marks nothing. + /// + private static bool HasPathsWindowsCannotCreate(Repository repo) + => OperatingSystem.IsWindows() && WindowsPathValidator.Validate(repo.Head.Tip.Tree).Count > 0; + + + /// /// True when a pull has nothing to write: the branch already is at the commit to /// write, and what gclo last wrote is that commit or one it builds on. In the @@ -1065,7 +1076,8 @@ public sealed class LibGit2GitClient : IGitClient { throw new InvalidOperationException( $"'{effectivePath}' is already in this folder, and it is not a file gclo has a record of writing " - + "or one the current commit has in that form, so it is not gclo's to replace. Move or delete it, then try again."); + + "or one the current commit has in that form, so it is not gclo's to replace. Move or delete it, then try again; " + + "if that goes for many files, move or delete the whole folder and sync again."); } string? foreign = WorkingTree.FilesUnder(workdir, effectivePath).FirstOrDefault(file => !oursAsOnDisk.Contains(file)); -- 2.54.0