From ddd93e36fee84c8bafd1a579e895ff9afc5f1b64 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 20:12:06 -0600 Subject: [PATCH 1/5] fix(clone): clones that were empty, cut off, or still ending are handled safely by the next sync - The first checkout of a clone that was empty refuses, before anything changes, when something is already where the repository has a file or needs a folder: a forced checkout would have replaced it. - The folder is marked unfinished before its branch is created, so a checkout that is cut off is completed by the next sync instead of being taken for up to date. - An empty clone follows origin's default branch when origin does not have the branch the local HEAD names. - One git operation per folder at a time in a process. A canceled sync stops waiting for its operations without stopping them; a second operation in the same folder is refused until the first has ended. - A clone without .git/index says so and how to rebuild it, instead of git's count of conflicts. - Clone in WSL clones under a temporary name and moves the folder into place when it is complete; leftovers of killed runs are removed once a day old. Closes #174 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/security/threat-model.md | 1 + gclo.Engine.Tests/InterruptedCloneTests.cs | 305 +++++++++++++++++++++ gclo.Engine.Tests/WslClonerTests.cs | 20 +- gclo.Engine/LibGit2GitClient.cs | 187 +++++++++++-- gclo.Engine/WslCloner.cs | 20 +- 5 files changed, 512 insertions(+), 21 deletions(-) create mode 100644 gclo.Engine.Tests/InterruptedCloneTests.cs diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 49be21e..1c99e6f 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -121,3 +121,4 @@ flowchart LR | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated: path recovery writes and deletes only through `WorkingTree` (inside the working directory, never in `.git`, never through a link); `.git` and its Windows aliases are invalid path segments; the stored recovery file's paths are validated on load; cleanup of a failed clone does not follow links (#141) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T15 added: text from repositories and servers is shown with control, separator and format characters made visible, in the activity log and in everything the CLI prints as text, including characters a legacy console code page would turn into control bytes; the desktop app's surfaces are named as not covered (#143) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | The project's home moved to `git.kof22.com`: the Help menu, the About dialog, the CLI's error text and the documentation link there, and the Scoop bucket is added from there (the manifest that pins the download URL and its hash now comes from that server over HTTPS; T2 is unchanged in kind). Help > Share hands a fixed text (what gclo is, where to download it, the sender's gclo version) to the system's mail handler as a `mailto:` link with no recipient, or shows it and copies it to the clipboard; nothing about the user, their accounts, or their repositories is in it, and the address it is built from must be a plain http(s) host and path (#163, #165). The update feed and the release downloads named in package manifests are still GitHub Releases at this revision. | +| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated (#174): the first checkout of a clone that was empty refuses, before anything changes, to replace a file or folder that is already in the target folder; the folder is marked unfinished before its branch is created, so a checkout that is cut off is completed by the next sync instead of being taken for up to date; one git operation per folder at a time within a process; the WSL clone is made under a temporary name and moved into place when complete. | diff --git a/gclo.Engine.Tests/InterruptedCloneTests.cs b/gclo.Engine.Tests/InterruptedCloneTests.cs new file mode 100644 index 0000000..65eb5fa --- /dev/null +++ b/gclo.Engine.Tests/InterruptedCloneTests.cs @@ -0,0 +1,305 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +using LibGit2Sharp; +using static gclo.Engine.Tests.GitTestHelpers; + + +namespace gclo.Engine.Tests; + + +/// +/// Clones that did not get to their first checkout (an origin that was empty, a clone +/// that was cut off) and operations that are still ending when the next one starts: +/// the next sync completes what is missing, replaces nothing that is not gclo's, and +/// never works in a folder that another operation still holds. +/// +public sealed class InterruptedCloneTests : IDisposable +{ + private const string Token = "test-token"; + + private readonly string _root = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + + private readonly LibGit2GitClient _client = new(); + + + + public InterruptedCloneTests() => Directory.CreateDirectory(_root); + + + + public void Dispose() => TryDeleteDirectory(_root); + + + + private string NewPath(string name) => Path.Combine(_root, name); + + + + /// An empty bare origin and a clone of it, whose HEAD names a branch without commits. + private async Task<(string Origin, string Clone)> CloneAnEmptyOriginAsync() + { + string origin = NewPath("origin.git"); + Repository.Init(origin, isBare: true); + string clone = NewPath("clone"); + await _client.CloneAsync(origin, clone, Token, null, CancellationToken.None); + using var repo = new Repository(clone); + Assert.True(repo.Info.IsHeadUnborn); + return (origin, clone); + } + + + + /// + /// Gives the origin its first commit through another working copy, on + /// (the branch a new clone starts with when null), and + /// returns the commit's id. + /// + private async Task SeedAsync(string origin, string? branch, params (string Path, string Content)[] files) + { + string seeder = NewPath("seeder-" + Guid.NewGuid().ToString("N")[..8]); + await _client.CloneAsync(origin, seeder, Token, null, CancellationToken.None); + using var repo = new Repository(seeder); + if(branch is not null) + { + repo.Refs.UpdateTarget("HEAD", "refs/heads/" + branch); + } + foreach((string path, string content) in files) + { + string full = Path.Combine(seeder, path.Replace('/', Path.DirectorySeparatorChar)); + Directory.CreateDirectory(Path.GetDirectoryName(full)!); + File.WriteAllText(full, content); + Commands.Stage(repo, path); + } + Signature who = MakeSignature(); + Commit commit = repo.Commit("first commit", who, who); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Head.CanonicalName, new PushOptions()); + if(branch is not null) + { + using var bare = new Repository(origin); + bare.Refs.UpdateTarget("HEAD", "refs/heads/" + branch); // origin's default branch + } + return commit.Sha; + } + + + + private static bool IsCheckoutPending(string workdir) + { + using var repo = new Repository(workdir); + return repo.Config.Get("gclo.checkoutpending", ConfigurationLevel.Local)?.Value == true; + } + + + + // ---------------------------------------------------------------- the first checkout of an empty clone + + [Fact] + public async Task AnEmptyClone_WhoseOriginStartedOnAnotherBranch_GetsOriginsDefaultBranch() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string sha = await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + using var repo = new Repository(clone); + Assert.False(repo.Info.IsHeadUnborn); + Assert.Equal("trunk", repo.Head.FriendlyName); + Assert.Equal(sha, repo.Head.Tip.Sha); + Assert.Equal("origin/trunk", repo.Head.TrackedBranch?.FriendlyName); + Assert.Equal("from trunk", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Theory] + [InlineData("readme.txt", false, "readme.txt")] // a file where the repository has a file + [InlineData("readme.txt", true, "readme.txt")] // a folder where the repository has a file + [InlineData("docs", false, "docs")] // a file where the repository needs a folder + public async Task WhatIsAlreadyInTheFolder_WhereTheRepositoryHasSomething_IsNotReplaced(string inTheWay, bool asFolder, string named) + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string mine = Path.Combine(clone, inTheWay); + if(asFolder) + { + Directory.CreateDirectory(mine); + File.WriteAllText(Path.Combine(mine, "kept.txt"), "mine"); + } + else + { + File.WriteAllText(mine, "mine"); + } + await SeedAsync(origin, null, ("readme.txt", "theirs"), ("docs/guide.txt", "theirs")); + + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains($"'{named}' is already in this folder", refused.Message, StringComparison.Ordinal); + Assert.Contains("nothing was checked out", refused.Message, StringComparison.Ordinal); + Assert.Equal("mine", File.ReadAllText(asFolder ? Path.Combine(mine, "kept.txt") : mine)); + using var repo = new Repository(clone); + Assert.True(repo.Info.IsHeadUnborn); // nothing was started: no branch, no marker + Assert.False(IsCheckoutPending(clone)); + Assert.False(File.Exists(Path.Combine(clone, "docs", "guide.txt"))); + } + + + + [Fact] + public async Task AFileOfTheUsers_ThatTheRepositoryDoesNotHave_StaysBesideTheCheckout() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + File.WriteAllText(Path.Combine(clone, "notes.txt"), "mine"); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "notes.txt"))); + } + + + + [Fact] + public async Task AFirstCheckoutThatWasCutOff_IsCompletedByTheNextSync_NotReportedAsUpToDate() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string sha = await SeedAsync(origin, null, ("readme.txt", "theirs")); + + // Cut off once the branch exists and before a file is written. + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + using(var cutOff = new Repository(clone)) + { + Assert.False(cutOff.Info.IsHeadUnborn); // it looks like any clone now + } + + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + Assert.True(IsCheckoutPending(clone)); // the folder says of itself that it is not finished + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + using var repo = new Repository(clone); + Assert.Equal(sha, repo.Head.Tip.Sha); + } + + + + [Fact] + public async Task ABranchStartedByHand_WithFilesStaged_IsLeftAlone_WhenOriginDoesNotHaveThatBranch() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + File.WriteAllText(Path.Combine(clone, "draft.txt"), "mine"); + using(var repo = new Repository(clone)) + { + Commands.Stage(repo, "draft.txt"); + } + await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + + await _client.PullAsync(clone, Token, CancellationToken.None); // fetches; checks nothing out + + using var after = new Repository(clone); + Assert.True(after.Info.IsHeadUnborn); + Assert.False(string.Equals("trunk", after.Head.FriendlyName, StringComparison.Ordinal)); + Assert.Equal(1, after.Index.Count); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "draft.txt"))); + } + + + + [Fact] + public async Task AnOriginThatNamesNoDefaultBranch_LeavesTheEmptyCloneAsItIs() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + using(var bare = new Repository(origin)) + { + bare.Refs.UpdateTarget("HEAD", "refs/heads/does-not-exist"); // HEAD names a branch that is not there + } + + await _client.PullAsync(clone, Token, CancellationToken.None); + + using var after = new Repository(clone); + Assert.True(after.Info.IsHeadUnborn); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + } + + + + // ---------------------------------------------------------------- one operation per folder + + [Fact] + public async Task WhileAnOperationStillWorksInAFolder_AnotherOneThere_IsRefused_AndWorksOnceTheFirstHasEnded() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + string target = NewPath("busy"); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var letGo = new ManualResetEventSlim(); + _client.AfterFetchForTesting = () => + { + entered.SetResult(); + letGo.Wait(TimeSpan.FromSeconds(30)); + }; + + Task clone = _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(30)); + + // Another client, as another workspace of the same process would be; and the + // same folder however it is written. + var other = new LibGit2GitClient(); + InvalidOperationException pull = await Assert.ThrowsAsync( + () => other.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + InvalidOperationException again = await Assert.ThrowsAsync( + () => other.CloneAsync(origin, target + Path.DirectorySeparatorChar, Token, null, CancellationToken.None)); + Assert.Contains("An earlier operation on this folder is still stopping", pull.Message, StringComparison.Ordinal); + Assert.Contains("Sync again in a moment", again.Message, StringComparison.Ordinal); + + letGo.Set(); + await clone; + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(target, "readme.txt"))); // the first one was not disturbed + await other.FetchAndPullAsync(origin, target, Token, CancellationToken.None); // and the folder is free again + } + + + + // ---------------------------------------------------------------- a clone without an index + + [Fact] + public async Task ACloneWhoseIndexIsGone_SaysSo_AndHowToRepairIt_InsteadOfCountingConflicts() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "one")); + string target = NewPath("no-index"); + await _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + File.Delete(Path.Combine(target, ".git", "index")); + + // Origin moves on: the next sync has a file to update. + string second = NewPath("second"); + await _client.CloneAsync(origin, second, Token, null, CancellationToken.None); + using(var repo = new Repository(second)) + { + File.WriteAllText(Path.Combine(second, "readme.txt"), "two"); + Commands.Stage(repo, "readme.txt"); + Signature who = MakeSignature(); + repo.Commit("second commit", who, who); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Head.CanonicalName, new PushOptions()); + } + + InvalidOperationException failure = await Assert.ThrowsAsync( + () => _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + + Assert.Contains("has no index", failure.Message, StringComparison.Ordinal); + Assert.Contains("reset", failure.Message, StringComparison.Ordinal); + Assert.Equal("one", File.ReadAllText(Path.Combine(target, "readme.txt"))); // nothing was changed + } +} diff --git a/gclo.Engine.Tests/WslClonerTests.cs b/gclo.Engine.Tests/WslClonerTests.cs index ddc75e4..80a8e17 100644 --- a/gclo.Engine.Tests/WslClonerTests.cs +++ b/gclo.Engine.Tests/WslClonerTests.cs @@ -278,8 +278,24 @@ public sealed class WslClonerTests Assert.Equal(2, CountOf(script, "-c credential.helper= -c \"credential.https://$host.helper=$helper\"")); Assert.Equal(0, CountOf(script, "\"credential.helper=$helper\"")); Assert.Contains("host=\"$4\"", script); - // "--" ends option parsing, so a URL can never be read as an option. - Assert.Contains("clone --quiet -- \"$url\" \"$dest\"", script); + // "--" ends option parsing, so a URL can never be read as an option. The + // clone is made beside the destination and moved into place when it is + // complete: a clone that is cut off leaves no folder that looks like one. + Assert.Contains("clone --quiet -- \"$url\" \"$tmp\"", script); + Assert.Contains("tmp=\"$dest.gclo-clone.$$\"", script); + Assert.Contains("trap 'rm -rf \"$tmp\"' EXIT", script); + Assert.True( + script.IndexOf("clone --quiet", StringComparison.Ordinal) < script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal), + "the folder is moved into place after the clone"); + Assert.True( + script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal) < script.IndexOf("trap - EXIT", StringComparison.Ordinal), + "the cleanup stays armed until the folder is in place"); + // What a killed run left is removed only when it is old: a folder of that name + // that is young may be another gclo's clone in progress. + Assert.Contains("-name \"$repo.gclo-clone.*\" -mmin +1440 -exec rm -rf {} +", script); + Assert.DoesNotContain("rm -rf \"$dest\"", script); + // Something that is in the way and is not a repository is not cloned into. + Assert.Contains("exists but is not a repository", script); // An existing destination is only fast-forwarded when it is a clone of this URL. // The stored URL is compared, not "remote get-url": that one applies the user's // url.*.insteadOf rewrites and would refuse a destination gclo itself cloned. diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index adac830..b3f08b9 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -3,6 +3,7 @@ * SPDX-License-Identifier: MIT */ +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Text.Json; using LibGit2Sharp; @@ -94,6 +95,13 @@ public sealed class LibGit2GitClient : IGitClient /// internal Action? AfterFetchForTesting { get; set; } + /// + /// Test seam: runs in the first checkout of an empty clone, after its branch was + /// created and before any file is written. A test cancels there to leave exactly + /// what a cut-off checkout leaves. + /// + internal Action? BeforeFirstCheckoutForTesting { get; set; } + /// @@ -107,7 +115,7 @@ public sealed class LibGit2GitClient : IGitClient /// public Task CloneAsync(string url, string path, string token, Action? onProgress, CancellationToken cancellationToken) => Task.Factory.StartNew( - () => Clone(url, path, token, onProgress, cancellationToken), + () => OneAtATime(path, () => Clone(url, path, token, onProgress, cancellationToken)), cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default); @@ -115,7 +123,7 @@ public sealed class LibGit2GitClient : IGitClient /// public Task FetchAndPullAsync(string url, string path, string token, CancellationToken cancellationToken) => Task.Factory.StartNew( - () => FetchAndPull(url, path, token, cancellationToken), + () => OneAtATime(path, () => FetchAndPull(url, path, token, cancellationToken)), cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default); @@ -123,11 +131,49 @@ public sealed class LibGit2GitClient : IGitClient /// public Task ApplyRecoveryAsync(string path, PathRecovery recovery, CancellationToken cancellationToken) => Task.Factory.StartNew( - () => ApplyRecovery(path, recovery, cancellationToken), + () => OneAtATime(path, () => ApplyRecovery(path, recovery, cancellationToken)), cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default); + /// + /// The folders a git operation of this process is working in right now. A caller + /// that stops waiting for an operation (a canceled sync does) does not stop the + /// operation: it goes on until git notices, and then cleans up after itself, which + /// for a clone means deleting the folder. Starting the next operation on that + /// folder before then would have the two working against each other. + /// + private static readonly ConcurrentDictionary s_inFlight = + new(OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); + + + + /// + /// Runs as the only one in ; + /// refuses at once when an earlier one there has not ended yet. + /// + private static void OneAtATime(string path, Action operation) + { + string folder = Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)); + if(!s_inFlight.TryAdd(folder, 0)) + { + throw new InvalidOperationException( + "An earlier operation on this folder is still stopping (a canceled clone or update ends on its own, a moment " + + "after it was canceled). Sync again in a moment."); + } + + try + { + operation(); + } + finally + { + s_inFlight.TryRemove(folder, out _); + } + } + + + private void Clone(string url, string path, string token, Action? onProgress, CancellationToken ct) { bool existedBefore = Directory.Exists(path); @@ -202,9 +248,12 @@ public sealed class LibGit2GitClient : IGitClient ct.ThrowIfCancellationRequested(); Commands.Checkout(repo, repo.Head, new CheckoutOptions { - CheckoutModifiers = CheckoutModifiers.Force, // materialize the fresh working tree - // The only checkout that lacked the hook: a canceled clone used to - // write its whole tree before the token was ever observed (#31). + // Force: materialize the fresh working tree. The hook lets a + // cancellation stop the checkout before it has written much; once + // libgit2 is writing, it may run to the end, and the cancellation is + // then the caller's to see (the engine stops waiting; the folder stays + // marked as busy until this returns). + CheckoutModifiers = CheckoutModifiers.Force, CheckoutNotifyFlags = CheckoutNotifyFlags.Updated, OnCheckoutNotify = (_, _) => !ct.IsCancellationRequested, }); @@ -307,7 +356,7 @@ public sealed class LibGit2GitClient : IGitClient // Empty clone — or a clone that was killed before its first checkout ran. // If the remote branch exists now, materialize it instead of silently // reporting an empty working tree as up to date forever. - MaterializeUnbornHead(repo, ct); + MaterializeUnbornHead(repo, url, token, ct); return; } @@ -370,6 +419,18 @@ public sealed class LibGit2GitClient : IGitClient $"'{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.", ex); } + catch(CheckoutConflictException ex) when(!File.Exists(Path.Combine(repo.Info.Path, "index"))) + { + /////////////////////////////////////////////////////////////////////////// + // Without an index git cannot tell which files are unchanged, takes // + // every one of them for a conflict, and says only how many. Say what // + // is wrong and what clears it; nothing was changed. // + /////////////////////////////////////////////////////////////////////////// + throw new InvalidOperationException( + "This clone has no index ('.git/index' is missing), so git cannot tell which files are unchanged and " + + $"refuses to update any of them ({ex.Message}). Run 'git -C \"{Path.TrimEndingDirectorySeparator(repo.Info.WorkingDirectory)}\" reset' " + + "to rebuild the index, or delete the folder to re-clone.", ex); + } } @@ -542,9 +603,8 @@ public sealed class LibGit2GitClient : IGitClient const string Prefix = "refs/heads/"; string oldName = repo.Head.FriendlyName; - string? defaultRef = repo.Network.ListReferences(url, credentials) - .FirstOrDefault(r => string.Equals(r.CanonicalName, "HEAD", StringComparison.Ordinal))?.TargetIdentifier; - if(defaultRef is null || !defaultRef.StartsWith(Prefix, StringComparison.Ordinal)) + string? defaultRef = RemoteDefaultBranch(repo, url, credentials); + if(defaultRef is null) { throw new InvalidOperationException( $"'{oldName}' no longer exists on origin, and origin has no default branch to follow. " @@ -601,11 +661,33 @@ public sealed class LibGit2GitClient : IGitClient /// - /// HEAD symbolically points at a branch with no commits. If the fetch brought that - /// branch's remote counterpart, create the local branch there, wire its upstream, - /// and check it out to populate the working tree. + /// The branch origin says is its default ("refs/heads/..."), asked of origin itself; + /// null when origin does not name one (an empty repository). /// - private static void MaterializeUnbornHead(Repository repo, CancellationToken ct) + private static string? RemoteDefaultBranch(Repository repo, string url, CredentialsHandler credentials) + { + string? head = repo.Network.ListReferences(url, credentials) + .FirstOrDefault(r => string.Equals(r.CanonicalName, "HEAD", StringComparison.Ordinal))?.TargetIdentifier; + return head is not null && head.StartsWith("refs/heads/", StringComparison.Ordinal) ? head : null; + } + + + + /// + /// HEAD symbolically points at a branch with no commits: an empty clone, or a clone + /// that was cut off before its first checkout. If the fetch brought a branch to + /// check out, create the local branch there, wire its upstream, and check it out + /// to populate the working tree. + /// + /// + /// Which branch: the one HEAD names when origin has it. When it does not, HEAD + /// still names the branch a new repository starts with here, which need not be + /// origin's (a repository that was empty when it was cloned and got its first + /// commit on "main"; a clone that was killed before it had learned origin's + /// default). Origin is then asked, and HEAD is pointed at its default branch. + /// Nothing is lost by that: a branch without commits holds nothing. + /// + private void MaterializeUnbornHead(Repository repo, string url, string token, CancellationToken ct) { const string Prefix = "refs/heads/"; string targetRef = repo.Refs.Head.TargetIdentifier; @@ -618,7 +700,26 @@ public sealed class LibGit2GitClient : IGitClient Branch remoteBranch = repo.Branches["origin/" + branchName]; if(remoteBranch?.Tip is null) { - return; // the remote is still empty; nothing to pull + if(repo.Index.Count > 0 || !repo.Branches.Any(b => b.IsRemote)) + { + // A branch somebody started here by hand (files are staged), or origin + // is still empty: nothing to pull, and nothing to ask origin about. + return; + } + + // A provider of its own: the fetch already spent one of the two requests + // the replay guard allows. + string? defaultRef = RemoteDefaultBranch(repo, url, MakeCredentialsProvider(token, url)); + Branch? defaultBranch = defaultRef is null ? null : repo.Branches["origin/" + defaultRef[Prefix.Length..]]; + if(defaultBranch?.Tip is null) + { + return; // origin names no default branch, or it was not fetched + } + + repo.Refs.UpdateTarget("HEAD", defaultRef!); + targetRef = defaultRef!; + branchName = targetRef[Prefix.Length..]; + remoteBranch = defaultBranch; } /////////////////////////////////////////////////////////////////////////////// @@ -644,6 +745,15 @@ public sealed class LibGit2GitClient : IGitClient } } + RefuseToCheckOutOverWhatIsThere(repo, remoteBranch.Tip.Tree); + + /////////////////////////////////////////////////////////////////////////////// + // From the moment the branch exists the repository looks like any clone, so // + // the marker goes first: if the checkout below fails or the process dies, // + // the next pull completes it instead of reporting an empty or half-written // + // tree as up to date. // + /////////////////////////////////////////////////////////////////////////////// + repo.Config.Set(CheckoutPendingKey, true, ConfigurationLevel.Local); repo.Refs.Add(targetRef, remoteBranch.Tip.Id); Branch local = repo.Branches[branchName]; repo.Branches.Update(local, b => @@ -651,12 +761,13 @@ public sealed class LibGit2GitClient : IGitClient b.Remote = "origin"; b.UpstreamBranch = targetRef; }); + BeforeFirstCheckoutForTesting?.Invoke(); ct.ThrowIfCancellationRequested(); try { - // Force: the working tree of a killed half-clone may hold partial files; - // recovering means making it match the branch tip exactly. + // Force: nothing in the folder is in the way (checked above), and a + // checkout that is being completed after a failure must write every file. Commands.Checkout(repo, local, new CheckoutOptions { CheckoutModifiers = CheckoutModifiers.Force, @@ -668,10 +779,52 @@ public sealed class LibGit2GitClient : IGitClient { throw new OperationCanceledException(ct); } + + repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); } + /// + /// The first checkout into a folder gclo has written nothing to yet: whatever is + /// already where the repository has a file (or a folder where it has a file, or a + /// file where it has a folder) was put there by someone, and a forced checkout + /// would replace it. Refused before the branch is created, so nothing has changed. + /// + private static void RefuseToCheckOutOverWhatIsThere(Repository repo, Tree tree) + { + string workdir = WorkingDirectoryOf(repo); + var checkedFolders = new HashSet(StringComparer.Ordinal); + foreach((string? path, _, _) in CollectEffectiveEntries(tree, s_noRecovery)) + { + string? inTheWay = File.Exists(InWorkingDirectory(workdir, path)) || Directory.Exists(InWorkingDirectory(workdir, path)) + ? path + : null; + for(int slash = path.IndexOf('/', StringComparison.Ordinal); inTheWay is null && slash > 0; slash = path.IndexOf('/', slash + 1)) + { + string folder = path[..slash]; + if(checkedFolders.Add(folder) && File.Exists(InWorkingDirectory(workdir, folder))) + { + inTheWay = folder; + } + } + + if(inTheWay is not null) + { + throw new InvalidOperationException( + $"'{inTheWay}' is already in this folder, which gclo has not checked out yet, so it is not gclo's to replace. " + + "Move or delete it (or the whole folder), then sync again; fetched, but nothing was checked out."); + } + } + } + + + + private static string InWorkingDirectory(string workdir, string repoPath) + => Path.Combine(workdir, repoPath.Replace('/', Path.DirectorySeparatorChar)); + + + // ---------------------------------------------------------------- path recovery private static void ApplyRecovery(string path, PathRecovery recovery, CancellationToken ct) diff --git a/gclo.Engine/WslCloner.cs b/gclo.Engine/WslCloner.cs index 311173f..c992a99 100644 --- a/gclo.Engine/WslCloner.cs +++ b/gclo.Engine/WslCloner.cs @@ -24,7 +24,14 @@ public sealed class WslCloner : IWslCloner /// host (with a non-default port). Prints the distribution name and the destination /// as its last two lines on success. A folder that already is a clone of the same /// URL is fast-forwarded instead of failing, so the verb is safe to use twice; a - /// folder that is a clone of anything else is refused. The credential helper is + /// folder that is a clone of anything else is refused. A new clone is made under a + /// temporary name beside the destination and moved into place when it is complete, + /// so one that is cut off never leaves a folder that the next attempt would take + /// for a clone and try to update. The temporary folder is removed when the script + /// ends without having moved it; one that a killed script left behind is removed + /// by a later run once it is a day old (not sooner: another gclo may be cloning the + /// same repository right now under such a name). + /// The credential helper is /// scoped to $4, so git (and anything it starts, such as an LFS filter pointed at /// another server by the repository's own configuration) gets the token for that /// host only (#139). @@ -41,8 +48,17 @@ public sealed class WslCloner : IWslCloner + " fi\n" + " git -C \"$dest\" -c credential.helper= -c \"credential.https://$host.helper=$helper\" pull --ff-only --quiet\n" + "else\n" + + " if [ -e \"$dest\" ]; then\n" + + " echo \"$dest exists but is not a repository; move or delete it, then try again\" >&2\n" + + " exit 1\n" + + " fi\n" + " mkdir -p \"$HOME/gclo/$org\"\n" - + " git -c credential.helper= -c \"credential.https://$host.helper=$helper\" clone --quiet -- \"$url\" \"$dest\"\n" + + " tmp=\"$dest.gclo-clone.$$\"\n" + + " find \"$HOME/gclo/$org\" -maxdepth 1 -name \"$repo.gclo-clone.*\" -mmin +1440 -exec rm -rf {} + 2>/dev/null || true\n" + + " trap 'rm -rf \"$tmp\"' EXIT\n" + + " git -c credential.helper= -c \"credential.https://$host.helper=$helper\" clone --quiet -- \"$url\" \"$tmp\"\n" + + " mv \"$tmp\" \"$dest\"\n" + + " trap - EXIT\n" + "fi\n" + "printf '%s\\n%s\\n' \"$WSL_DISTRO_NAME\" \"$dest\"\n"; -- 2.54.0 From 9c53d985e2d5070f7e9bb7a75acdde1bb11716d0 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 20:40:44 -0600 Subject: [PATCH 2/5] fix(clone): review of #175: nothing that is not gclo's is written over, by the first checkout or by its retry The first review reproduced two ways in which this change itself replaced a user's files, and several holes in what it claimed. - A repository that has local branches is left alone when its HEAD names a branch without commits: that is a branch somebody started, not an empty clone. Origin's default branch is followed only by a repository that has no branch of its own, and nothing changes before every check has passed. - The first checkout and the completion of a cut-off checkout go through the refusals of the path recovery before anything changes: a link on the way, a file where a folder is needed, a folder with files where a file is needed, a file that is not what the unfinished checkout wrote, and anything but an empty folder where the repository has a submodule. The file a cut-off checkout was writing counts as gclo's and is finished. - A branch's upstream is configured before the branch exists, and a marker without a branch starts the first checkout over instead of reporting an empty folder as done. - The WSL clone uses a temporary folder of gclo's own, removes it on signals too, does not move into a destination that appeared meanwhile, accepts an empty folder at the destination, and removes old leftovers only from its own folder. The script is now also run for real by the tests. - One operation per folder: the long-path spelling of a folder is the same folder, and the message fits every holder. Refs #174 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 4 + docs/security/threat-model.md | 2 +- gclo.Engine.Tests/InterruptedCloneTests.cs | 349 +++++++++++++++++++- gclo.Engine.Tests/WslCloneScriptRunTests.cs | 249 ++++++++++++++ gclo.Engine.Tests/WslClonerTests.cs | 8 +- gclo.Engine/LibGit2GitClient.cs | 184 ++++++++--- gclo.Engine/WslCloner.cs | 34 +- 8 files changed, 751 insertions(+), 81 deletions(-) create mode 100644 gclo.Engine.Tests/WslCloneScriptRunTests.cs diff --git a/README.md b/README.md index 3efc2b6..ea38584 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, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. -**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`). +**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. What it completes is its own: a file that is in the folder and is not what the unfinished checkout wrote is never replaced. The row fails and names the file; move it and sync again. The same holds for a repository that was empty when it was cloned and gets its first files later (it then follows the origin's default branch), and a branch you started yourself in a clone is left alone. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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 01c67c1..bcbe0db 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -49,6 +49,10 @@ Options may be written as `--name value` or `--name=value`. Clones every repository of `--org` into `\`. Repositories that are already valid git repositories locally are fetched and fast-forwarded instead. +A file in a target folder that gclo did not write is never replaced: when one is where +the repository has a file (a repository that was empty when it was cloned, or a clone +whose first checkout was interrupted), that repository fails and the message names the +file. Repositories fail independently — one failure never stops the rest. Up to `--parallel` git operations run at once (default 8, maximum 64 — the same ceiling the desktop app applies, because more would trip GitHub's secondary diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 1c99e6f..a29b097 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -121,4 +121,4 @@ flowchart LR | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated: path recovery writes and deletes only through `WorkingTree` (inside the working directory, never in `.git`, never through a link); `.git` and its Windows aliases are invalid path segments; the stored recovery file's paths are validated on load; cleanup of a failed clone does not follow links (#141) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T15 added: text from repositories and servers is shown with control, separator and format characters made visible, in the activity log and in everything the CLI prints as text, including characters a legacy console code page would turn into control bytes; the desktop app's surfaces are named as not covered (#143) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | The project's home moved to `git.kof22.com`: the Help menu, the About dialog, the CLI's error text and the documentation link there, and the Scoop bucket is added from there (the manifest that pins the download URL and its hash now comes from that server over HTTPS; T2 is unchanged in kind). Help > Share hands a fixed text (what gclo is, where to download it, the sender's gclo version) to the system's mail handler as a `mailto:` link with no recipient, or shows it and copies it to the clipboard; nothing about the user, their accounts, or their repositories is in it, and the address it is built from must be a plain http(s) host and path (#163, #165). The update feed and the release downloads named in package manifests are still GitHub Releases at this revision. | -| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated (#174): the first checkout of a clone that was empty refuses, before anything changes, to replace a file or folder that is already in the target folder; the folder is marked unfinished before its branch is created, so a checkout that is cut off is completed by the next sync instead of being taken for up to date; one git operation per folder at a time within a process; the WSL clone is made under a temporary name and moved into place when complete. | +| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on): a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) goes through the same refusals as the path recovery before anything changes: a link on the way, a file where a folder is needed, a folder with files where a file is needed, a file at a destination that is not what the unfinished checkout wrote, and anything but an empty folder where the repository has a submodule. A repository with local branches whose HEAD names a branch without commits is left alone. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes). The WSL clone is made in gclo's own temporary folder and moved into place when complete; it is not moved into a destination that appeared meanwhile. Not covered: a file created in the folder between the check and the checkout that follows it. | diff --git a/gclo.Engine.Tests/InterruptedCloneTests.cs b/gclo.Engine.Tests/InterruptedCloneTests.cs index 65eb5fa..7b8a094 100644 --- a/gclo.Engine.Tests/InterruptedCloneTests.cs +++ b/gclo.Engine.Tests/InterruptedCloneTests.cs @@ -116,10 +116,11 @@ public sealed class InterruptedCloneTests : IDisposable [Theory] - [InlineData("readme.txt", false, "readme.txt")] // a file where the repository has a file - [InlineData("readme.txt", true, "readme.txt")] // a folder where the repository has a file - [InlineData("docs", false, "docs")] // a file where the repository needs a folder - public async Task WhatIsAlreadyInTheFolder_WhereTheRepositoryHasSomething_IsNotReplaced(string inTheWay, bool asFolder, string named) + [InlineData("readme.txt", false, "'readme.txt' is already in this folder")] // a file where the repository has a file + [InlineData("README.TXT", false, "'readme.txt' is already in this folder")] // the same file to Windows, whatever its case + [InlineData("readme.txt", true, "'readme.txt' is a folder with files in it")] // a folder where the repository has a file + [InlineData("docs", false, "'docs' is a file in this folder, and the repository has a folder of that name")] // a file where the repository needs a folder + public async Task WhatIsAlreadyInTheFolder_WhereTheRepositoryHasSomething_IsNotReplaced(string inTheWay, bool asFolder, string said) { (string origin, string clone) = await CloneAnEmptyOriginAsync(); string mine = Path.Combine(clone, inTheWay); @@ -137,8 +138,7 @@ public sealed class InterruptedCloneTests : IDisposable InvalidOperationException refused = await Assert.ThrowsAsync( () => _client.PullAsync(clone, Token, CancellationToken.None)); - Assert.Contains($"'{named}' is already in this folder", refused.Message, StringComparison.Ordinal); - Assert.Contains("nothing was checked out", refused.Message, StringComparison.Ordinal); + Assert.Contains(said, refused.Message, StringComparison.Ordinal); Assert.Equal("mine", File.ReadAllText(asFolder ? Path.Combine(mine, "kept.txt") : mine)); using var repo = new Repository(clone); Assert.True(repo.Info.IsHeadUnborn); // nothing was started: no branch, no marker @@ -181,6 +181,11 @@ public sealed class InterruptedCloneTests : IDisposable Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); Assert.True(IsCheckoutPending(clone)); // the folder says of itself that it is not finished + using(var cutOffAgain = new Repository(clone)) + { + // Its upstream was configured before the branch existed: no cut-off leaves a branch without one. + Assert.Equal("origin/" + cutOffAgain.Head.FriendlyName, cutOffAgain.Head.TrackedBranch?.FriendlyName); + } await _client.PullAsync(clone, Token, CancellationToken.None); @@ -260,8 +265,8 @@ public sealed class InterruptedCloneTests : IDisposable () => other.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); InvalidOperationException again = await Assert.ThrowsAsync( () => other.CloneAsync(origin, target + Path.DirectorySeparatorChar, Token, null, CancellationToken.None)); - Assert.Contains("An earlier operation on this folder is still stopping", pull.Message, StringComparison.Ordinal); - Assert.Contains("Sync again in a moment", again.Message, StringComparison.Ordinal); + Assert.Contains("Another git operation of gclo is still working in this folder", pull.Message, StringComparison.Ordinal); + Assert.Contains("Sync again when it has finished", again.Message, StringComparison.Ordinal); letGo.Set(); await clone; @@ -302,4 +307,332 @@ public sealed class InterruptedCloneTests : IDisposable Assert.Contains("reset", failure.Message, StringComparison.Ordinal); Assert.Equal("one", File.ReadAllText(Path.Combine(target, "readme.txt"))); // nothing was changed } + + + + // ---------------------------------------------------------------- what is not gclo's stays (review of #175) + + /// An empty clone whose first checkout was cut off once its branch existed and before a file was written. + private async Task<(string Origin, string Clone)> ACloneWhoseFirstCheckoutWasCutOffAsync(params (string Path, string Content)[] files) + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, files); + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + Assert.True(IsCheckoutPending(clone)); + return (origin, clone); + } + + + + [Fact] + public async Task AFileTheUserAdds_AfterACheckoutWasCutOff_IsNotReplacedByTheRetry_WhichStaysUnfinishedUntilItIsMoved() + { + (_, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("readme.txt", "theirs"), ("docs/guide.txt", "theirs")); + File.WriteAllText(Path.Combine(clone, "readme.txt"), "MINE"); + + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("'readme.txt' is already in this folder", refused.Message, StringComparison.Ordinal); + Assert.Equal("MINE", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(File.Exists(Path.Combine(clone, "docs", "guide.txt"))); // nothing was written beside it either + Assert.True(IsCheckoutPending(clone)); // and the folder is still not taken for finished + + File.Move(Path.Combine(clone, "readme.txt"), Path.Combine(clone, "mine.txt")); + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.Equal("MINE", File.ReadAllText(Path.Combine(clone, "mine.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task AFileOfTheUsers_ThatOriginStartsToHave_AfterACheckoutWasCutOff_IsNotReplaced() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + File.WriteAllText(Path.Combine(clone, "notes.txt"), "mine"); // there from the start; the first commit does not have it + await SeedAsync(origin, null, ("readme.txt", "theirs")); + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + + // Origin moves on and now has a file of that name. + string second = NewPath("second"); + await _client.CloneAsync(origin, second, Token, null, CancellationToken.None); + using(var repo = new Repository(second)) + { + File.WriteAllText(Path.Combine(second, "notes.txt"), "theirs"); + Commands.Stage(repo, "notes.txt"); + Signature who = MakeSignature(); + repo.Commit("second commit", who, who); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Head.CanonicalName, new PushOptions()); + } + + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("'notes.txt' is already in this folder", refused.Message, StringComparison.Ordinal); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "notes.txt"))); + Assert.True(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task TheFileACutOffCheckoutWasWriting_IsFinishedByTheRetry() + { + (_, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("readme.txt", "theirs, all of it"), ("done.txt", "written")); + File.WriteAllText(Path.Combine(clone, "done.txt"), "written"); // one file had been written in full + File.WriteAllText(Path.Combine(clone, "readme.txt"), "theirs, a"); // and this one was cut off in the middle + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs, all of it", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.Equal("written", File.ReadAllText(Path.Combine(clone, "done.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task ACutOffThatLeftTheMarkerButNoBranchYet_StartsOver_InsteadOfReportingAnEmptyFolderAsDone() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + using(var repo = new Repository(clone)) + { + repo.Config.Set("gclo.checkoutpending", true, ConfigurationLevel.Local); // the process died right after this + } + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task TheFolderIsMarkedUnfinished_BeforeItsBranchIsCreated() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + string branch; + using(var repo = new Repository(clone)) + { + branch = repo.Refs.Head.TargetIdentifier["refs/heads/".Length..]; + } + + // Something that makes creating the branch fail: a reference below its name. + string below = Path.Combine(clone, ".git", "refs", "heads", branch); + Directory.CreateDirectory(below); + File.WriteAllText(Path.Combine(below, "x"), "0000000000000000000000000000000000000000" + (char)10); + + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + // Whatever stopped it, the folder already says that it is not finished. + Assert.True(IsCheckoutPending(clone)); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + + Directory.Delete(below, recursive: true); + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task ABranchSomebodyStarted_InACloneThatHasBranches_IsTheirs_HoweverOftenItIsSynced() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs"), ("index.html", "theirs")); + string clone = NewPath("ordinary"); + await _client.CloneAsync(origin, clone, Token, null, CancellationToken.None); + + // 'git switch --orphan pages': HEAD names a branch without commits, the index + // and the files of the branch that was left are gone. + string before; + using(var repo = new Repository(clone)) + { + before = repo.Head.FriendlyName; + repo.Refs.UpdateTarget("HEAD", "refs/heads/pages"); + repo.Index.Clear(); + repo.Index.Write(); + } + File.Delete(Path.Combine(clone, "readme.txt")); + File.Delete(Path.Combine(clone, "index.html")); + File.WriteAllText(Path.Combine(clone, "page.html"), "mine"); + + await _client.PullAsync(clone, Token, CancellationToken.None); + File.WriteAllText(Path.Combine(clone, "readme.txt"), "mine too"); + await _client.PullAsync(clone, Token, CancellationToken.None); + + using var after = new Repository(clone); + Assert.True(after.Info.IsHeadUnborn); + Assert.Equal("pages", after.Head.FriendlyName); + Assert.NotNull(after.Branches[before]); // the branch that was left is still there, untouched + Assert.Equal(0, after.Index.Count); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "page.html"))); + Assert.Equal("mine too", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(File.Exists(Path.Combine(clone, "index.html"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Theory] + [InlineData(true)] // a folder with something in it + [InlineData(false)] // a file + public async Task WhatIsWhereTheRepositoryHasASubmodule_IsNotReplaced_AndAnEmptyFolderThereIsFine(bool asFolder) + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + + // Origin's first commit: one file and a submodule entry 'sub'. + string seeder = NewPath("seeder-with-submodule"); + await _client.CloneAsync(origin, seeder, Token, null, CancellationToken.None); + using(var repo = new Repository(seeder)) + { + Blob readme = repo.ObjectDatabase.CreateBlob(new MemoryStream("theirs"u8.ToArray())); + var definition = new TreeDefinition(); + definition.Add("readme.txt", readme, Mode.NonExecutableFile); + definition.AddGitLink("sub", new ObjectId("1234567890123456789012345678901234567890")); + Tree tree = repo.ObjectDatabase.CreateTree(definition); + Signature who = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit(who, who, "first commit", tree, [], prettifyMessage: false); + repo.Refs.Add(repo.Refs.Head.TargetIdentifier, commit.Id); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Refs.Head.TargetIdentifier, new PushOptions()); + } + + string mine = Path.Combine(clone, "sub"); + if(asFolder) + { + Directory.CreateDirectory(mine); + File.WriteAllText(Path.Combine(mine, "kept.txt"), "mine"); + } + else + { + File.WriteAllText(mine, "mine"); + } + + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("'sub' is already in this folder, where the repository has a submodule", refused.Message, StringComparison.Ordinal); + Assert.Equal("mine", File.ReadAllText(asFolder ? Path.Combine(mine, "kept.txt") : mine)); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + + // Out of the way, or an empty folder in its place: the checkout goes ahead. + if(asFolder) + { + File.Delete(Path.Combine(mine, "kept.txt")); + } + else + { + File.Delete(mine); + } + await _client.PullAsync(clone, Token, CancellationToken.None); + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + } + + + + [Fact] + public async Task ALinkWhereTheRepositoryNeedsAFolder_IsNotReplacedByARealFolder() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("docs/guide.txt", "theirs")); + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; + string link = Path.Combine(clone, "docs"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.NotNull(new DirectoryInfo(link).LinkTarget); // still the link it was + Assert.Empty(Directory.GetFileSystemEntries(elsewhere)); // and nothing was written through it + using var repo = new Repository(clone); + Assert.True(repo.Info.IsHeadUnborn); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + // ---------------------------------------------------------------- one operation per folder, however it is written + + [Fact] + public async Task AFolderThatIsBusy_IsBusyForARecoveryToo_AndUnderEverySpellingOfItsName() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + string target = NewPath("busy-too"); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var letGo = new ManualResetEventSlim(); + _client.AfterFetchForTesting = () => + { + entered.SetResult(); + letGo.Wait(TimeSpan.FromSeconds(30)); + }; + Task clone = _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(30)); + var other = new LibGit2GitClient(); + var nothing = new PathRecovery(new Dictionary(StringComparer.Ordinal), new HashSet(StringComparer.Ordinal)); + + InvalidOperationException recovery = await Assert.ThrowsAsync( + () => other.ApplyRecoveryAsync(target, nothing, CancellationToken.None)); + Assert.Contains("still working in this folder", recovery.Message, StringComparison.Ordinal); + if(OperatingSystem.IsWindows()) + { + // One folder to Windows: another case, and the long-path spelling. + await Assert.ThrowsAsync( + () => other.FetchAndPullAsync(origin, target.ToUpperInvariant(), Token, CancellationToken.None)); + await Assert.ThrowsAsync( + () => other.FetchAndPullAsync(origin, @"\\?\" + target, Token, CancellationToken.None)); + } + + letGo.Set(); + await clone; + } + + + + // ---------------------------------------------------------------- the missing-index message is for a missing index only + + [Fact] + public async Task AnOrdinaryConflict_WithTheIndexInPlace_IsNotBlamedOnAMissingIndex() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "one")); + string target = NewPath("edited"); + await _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + File.WriteAllText(Path.Combine(target, "readme.txt"), "edited here"); // an edit that is not committed + + string second = NewPath("second"); + await _client.CloneAsync(origin, second, Token, null, CancellationToken.None); + using(var repo = new Repository(second)) + { + File.WriteAllText(Path.Combine(second, "readme.txt"), "two"); + Commands.Stage(repo, "readme.txt"); + Signature who = MakeSignature(); + repo.Commit("second commit", who, who); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Head.CanonicalName, new PushOptions()); + } + + Exception failure = await Assert.ThrowsAnyAsync( + () => _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + + Assert.DoesNotContain("has no index", failure.Message, StringComparison.Ordinal); + Assert.Equal("edited here", File.ReadAllText(Path.Combine(target, "readme.txt"))); + } } diff --git a/gclo.Engine.Tests/WslCloneScriptRunTests.cs b/gclo.Engine.Tests/WslCloneScriptRunTests.cs new file mode 100644 index 0000000..8579009 --- /dev/null +++ b/gclo.Engine.Tests/WslCloneScriptRunTests.cs @@ -0,0 +1,249 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +using System.Diagnostics; +using LibGit2Sharp; +using static gclo.Engine.Tests.GitTestHelpers; + + +namespace gclo.Engine.Tests; + + +/// +/// Runs for real, with a POSIX shell and local +/// repositories, where the machine has one (Git for Windows brings sh): what +/// the script leaves on disk is what matters, and matching its text cannot show +/// that. On a machine without sh and git on the path the tests have +/// nothing to run and pass. +/// +public sealed class WslCloneScriptRunTests : IDisposable +{ + private static readonly (string Sh, string Git)? s_tools = FindTools(); + + private readonly string _root = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + + private readonly string _home; + + private readonly string _origin; + + + + public WslCloneScriptRunTests() + { + _home = Path.Combine(_root, "home"); + Directory.CreateDirectory(_home); + + // An origin with one commit, made through a working copy. + _origin = Path.Combine(_root, "origin.git"); + Repository.Init(_origin, isBare: true); + string seeder = Repository.Clone(_origin, Path.Combine(_root, "seeder")); + using var repo = new Repository(seeder); + File.WriteAllText(Path.Combine(repo.Info.WorkingDirectory, "readme.txt"), "theirs"); + Commands.Stage(repo, "readme.txt"); + Signature who = MakeSignature(); + repo.Commit("first commit", who, who); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Head.CanonicalName, new PushOptions()); + } + + + + public void Dispose() => TryDeleteDirectory(_root); + + + + /// A POSIX shell that can find git, or null when this machine has none. + private static (string Sh, string Git)? FindTools() + { + IEnumerable candidates = (Environment.GetEnvironmentVariable("PATH") ?? "") + .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries) + .Select(folder => Path.Combine(folder, OperatingSystem.IsWindows() ? "sh.exe" : "sh")); + foreach(string sh in candidates.Where(File.Exists)) + { + try + { + var ask = new ProcessStartInfo(sh) { RedirectStandardOutput = true, RedirectStandardError = true, UseShellExecute = false }; + ask.ArgumentList.Add("-c"); + ask.ArgumentList.Add("command -v git"); + using Process process = Process.Start(ask)!; + string git = process.StandardOutput.ReadToEnd().Trim(); + process.WaitForExit(30000); + if(process.ExitCode == 0 && git.Length > 0) + { + return (sh, git); + } + } + catch(Exception) + { + // Not a shell that runs here: try the next one. + } + } + return null; + } + + + + private string Destination(string org, string repo) => Path.Combine(_home, "gclo", org, repo); + + + + private string[] Leftovers() + { + string tmp = Path.Combine(_home, "gclo", ".gclo-tmp"); + return Directory.Exists(tmp) ? [.. Directory.GetFileSystemEntries(tmp).Select(Path.GetFileName).Order(StringComparer.Ordinal)!] : []; + } + + + + /// Runs the script as wsl.exe would hand it to the shell: on standard input, with its four arguments. + private (int ExitCode, string Output, string Error) Run(string org, string repo, string? url = null, string? pathPrefix = null, string? appear = null) + { + var start = new ProcessStartInfo(s_tools!.Value.Sh) + { + RedirectStandardInput = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }; + foreach(string argument in (string[])["-s", "--", org, repo, (url ?? _origin).Replace('\\', '/'), "example.test"]) + { + start.ArgumentList.Add(argument); + } + start.Environment["HOME"] = _home.Replace('\\', '/'); + start.Environment[WslCloner.TokenVariable] = "not-a-token"; + start.Environment["WSL_DISTRO_NAME"] = "test"; + start.Environment["GCLO_TEST_REAL_GIT"] = s_tools.Value.Git; + start.Environment["GCLO_TEST_APPEAR"] = (appear ?? "").Replace('\\', '/'); + if(pathPrefix is not null) + { + start.Environment["PATH"] = pathPrefix + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + } + + using Process process = Process.Start(start)!; + process.StandardInput.Write(WslCloner.CloneScript); + process.StandardInput.Close(); + Task error = process.StandardError.ReadToEndAsync(); + string output = process.StandardOutput.ReadToEnd(); + Assert.True(process.WaitForExit(120000), "the script did not end"); + return (process.ExitCode, output, error.GetAwaiter().GetResult()); + } + + + + [Fact] + public void ANewClone_EndsUpAtTheDestination_AndLeavesNothingBehind_AndASecondRunUpdatesIt() + { + if(s_tools is null) + { + return; + } + + (int exitCode, string output, string error) = Run("acme", "repo"); + + Assert.True(exitCode == 0, error); + Assert.Equal("theirs", File.ReadAllText(Path.Combine(Destination("acme", "repo"), "readme.txt"))); + Assert.Empty(Leftovers()); + Assert.EndsWith("/gclo/acme/repo", output.TrimEnd().Split('\n')[^1].TrimEnd(), StringComparison.Ordinal); + + (int again, _, string secondError) = Run("acme", "repo"); + Assert.True(again == 0, secondError); + } + + + + [Fact] + public void ACloneThatFails_LeavesNoFolderThatLooksLikeAClone() + { + if(s_tools is null) + { + return; + } + + (int exitCode, _, _) = Run("acme", "repo", url: Path.Combine(_root, "no-such-origin.git")); + + Assert.NotEqual(0, exitCode); + Assert.False(Directory.Exists(Destination("acme", "repo"))); + Assert.Empty(Leftovers()); + } + + + + [Fact] + public void AFolderWithSomethingInIt_AtTheDestination_IsRefused_AndAnEmptyOneIsUsed() + { + if(s_tools is null) + { + return; + } + Directory.CreateDirectory(Destination("acme", "full")); + File.WriteAllText(Path.Combine(Destination("acme", "full"), "mine.txt"), "mine"); + Directory.CreateDirectory(Destination("acme", "empty")); + + (int refused, _, string error) = Run("acme", "full"); + (int used, _, string usedError) = Run("acme", "empty"); + + Assert.Equal(1, refused); + Assert.Contains("exists but is not a repository", error, StringComparison.Ordinal); + Assert.Equal("mine", File.ReadAllText(Path.Combine(Destination("acme", "full"), "mine.txt"))); + Assert.False(Directory.Exists(Path.Combine(Destination("acme", "full"), ".git"))); + Assert.True(used == 0, usedError); + Assert.Equal("theirs", File.ReadAllText(Path.Combine(Destination("acme", "empty"), "readme.txt"))); + } + + + + [Fact] + public void ADestinationThatAppearsWhileTheCloneRuns_IsNotMovedInto() + { + if(s_tools is null) + { + return; + } + + // A stand-in for git that runs the real one and then, after a clone, makes + // the destination appear: what another gclo cloning the same repository does. + string wrappers = Path.Combine(_root, "wrappers"); + Directory.CreateDirectory(wrappers); + File.WriteAllText( + Path.Combine(wrappers, "git"), + "#!/bin/sh\n\"$GCLO_TEST_REAL_GIT\" \"$@\"\nrc=$?\n" + + "case \" $* \" in *\" clone \"*) mkdir -p \"$GCLO_TEST_APPEAR\"; echo theirs-too > \"$GCLO_TEST_APPEAR/other.txt\";; esac\n" + + "exit $rc\n"); + string destination = Destination("acme", "repo"); + + (int exitCode, _, string error) = Run("acme", "repo", pathPrefix: wrappers, appear: destination); + + Assert.Equal(1, exitCode); + Assert.Contains("appeared while the repository was being cloned", error, StringComparison.Ordinal); + Assert.Equal(["other.txt"], Directory.GetFileSystemEntries(destination).Select(Path.GetFileName), StringComparer.Ordinal); // no clone nested inside it + Assert.Empty(Leftovers()); + } + + + + [Fact] + public void OnlyOldLeftovers_InGclosOwnTemporaryFolder_AreRemoved() + { + if(s_tools is null) + { + return; + } + string tmp = Path.Combine(_home, "gclo", ".gclo-tmp"); + string old = Directory.CreateDirectory(Path.Combine(tmp, "acme.other.111")).FullName; + File.WriteAllText(Path.Combine(old, "half.txt"), "x"); + Directory.SetLastWriteTimeUtc(old, DateTime.UtcNow.AddDays(-2)); + Directory.CreateDirectory(Path.Combine(tmp, "acme.other.222")); // young: perhaps a clone in progress + + // A real clone whose name looks like a leftover's, and is old: not gclo's to remove. + string sibling = Directory.CreateDirectory(Destination("acme", "repo.gclo-clone.keep")).FullName; + File.WriteAllText(Path.Combine(sibling, "kept.txt"), "mine"); + Directory.SetLastWriteTimeUtc(sibling, DateTime.UtcNow.AddDays(-2)); + + (int exitCode, _, string error) = Run("acme", "repo"); + + Assert.True(exitCode == 0, error); + Assert.Equal(["acme.other.222"], Leftovers(), StringComparer.Ordinal); + Assert.Equal("mine", File.ReadAllText(Path.Combine(sibling, "kept.txt"))); + } +} diff --git a/gclo.Engine.Tests/WslClonerTests.cs b/gclo.Engine.Tests/WslClonerTests.cs index 80a8e17..16747ba 100644 --- a/gclo.Engine.Tests/WslClonerTests.cs +++ b/gclo.Engine.Tests/WslClonerTests.cs @@ -282,7 +282,7 @@ public sealed class WslClonerTests // clone is made beside the destination and moved into place when it is // complete: a clone that is cut off leaves no folder that looks like one. Assert.Contains("clone --quiet -- \"$url\" \"$tmp\"", script); - Assert.Contains("tmp=\"$dest.gclo-clone.$$\"", script); + Assert.Contains("tmp=\"$HOME/gclo/.gclo-tmp/$org.$repo.$$\"", script); Assert.Contains("trap 'rm -rf \"$tmp\"' EXIT", script); Assert.True( script.IndexOf("clone --quiet", StringComparison.Ordinal) < script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal), @@ -290,9 +290,9 @@ public sealed class WslClonerTests Assert.True( script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal) < script.IndexOf("trap - EXIT", StringComparison.Ordinal), "the cleanup stays armed until the folder is in place"); - // What a killed run left is removed only when it is old: a folder of that name - // that is young may be another gclo's clone in progress. - Assert.Contains("-name \"$repo.gclo-clone.*\" -mmin +1440 -exec rm -rf {} +", script); + // What a killed run left is removed only from gclo's own temporary folder, and + // only when it is old: a young one may be another gclo's clone in progress. + Assert.Contains("find \"$HOME/gclo/.gclo-tmp\" -mindepth 1 -maxdepth 1 -mmin +1440 -exec rm -rf {} +", script); Assert.DoesNotContain("rm -rf \"$dest\"", script); // Something that is in the way and is not a repository is not cloned into. Assert.Contains("exists but is not a repository", script); diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index b3f08b9..1cc3506 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -155,11 +155,20 @@ public sealed class LibGit2GitClient : IGitClient private static void OneAtATime(string path, Action operation) { string folder = Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)); + if(folder.StartsWith(@"\\?\UNC\", StringComparison.OrdinalIgnoreCase)) + { + folder = @"\\" + folder[8..]; // \\?\UNC\server\share is \\server\share + } + else if(folder.StartsWith(@"\\?\", StringComparison.Ordinal)) + { + folder = folder[4..]; // \\?\C:\x is C:\x + } if(!s_inFlight.TryAdd(folder, 0)) { throw new InvalidOperationException( - "An earlier operation on this folder is still stopping (a canceled clone or update ends on its own, a moment " - + "after it was canceled). Sync again in a moment."); + "Another git operation of gclo is still working in this folder: one that was canceled and has not ended yet " + + "(it ends on its own a moment after it was canceled), or another workspace syncing the same folder. " + + "Sync again when it has finished."); } try @@ -338,8 +347,10 @@ public sealed class LibGit2GitClient : IGitClient return; } - if(IsCheckoutPending(repo)) + if(IsCheckoutPending(repo) && !repo.Info.IsHeadUnborn) { + // (With an unborn HEAD there is no commit to complete: the first checkout + // was cut off before its branch existed, and it starts over below.) CompletePendingCheckout(repo, ct); return; } @@ -680,12 +691,21 @@ public sealed class LibGit2GitClient : IGitClient /// to populate the working tree. /// /// - /// Which branch: the one HEAD names when origin has it. When it does not, HEAD + /// + /// Which branch: the one HEAD names when origin has it. When origin does not, + /// and this repository has no branch of its own at all (so it is a clone that + /// never had a commit, not a clone in which somebody started a new branch), HEAD /// still names the branch a new repository starts with here, which need not be - /// origin's (a repository that was empty when it was cloned and got its first - /// commit on "main"; a clone that was killed before it had learned origin's - /// default). Origin is then asked, and HEAD is pointed at its default branch. - /// Nothing is lost by that: a branch without commits holds nothing. + /// origin's: origin is asked for its default branch, and that one is checked + /// out. A repository that has local branches is left as it is: an unborn HEAD + /// there is a branch a person started ("git switch --orphan"), and is theirs. + /// + /// + /// Nothing changes before everything was checked: the branch to check out, the + /// paths Windows cannot create, and what is already in the folder + /// (). Only then is HEAD pointed at + /// the branch, the folder marked unfinished, and the branch created. + /// /// private void MaterializeUnbornHead(Repository repo, string url, string token, CancellationToken ct) { @@ -698,12 +718,14 @@ public sealed class LibGit2GitClient : IGitClient string branchName = targetRef[Prefix.Length..]; Branch remoteBranch = repo.Branches["origin/" + branchName]; + bool followOriginsDefault = false; if(remoteBranch?.Tip is null) { - if(repo.Index.Count > 0 || !repo.Branches.Any(b => b.IsRemote)) + if(repo.Index.Count > 0 || !repo.Branches.Any(b => b.IsRemote) || repo.Branches.Any(b => !b.IsRemote)) { - // A branch somebody started here by hand (files are staged), or origin - // is still empty: nothing to pull, and nothing to ask origin about. + // Files are staged or a local branch exists (a branch somebody started + // here by hand), or origin is still empty: nothing to pull, and + // nothing to ask origin about. return; } @@ -716,7 +738,7 @@ public sealed class LibGit2GitClient : IGitClient return; // origin names no default branch, or it was not fetched } - repo.Refs.UpdateTarget("HEAD", defaultRef!); + followOriginsDefault = true; targetRef = defaultRef!; branchName = targetRef[Prefix.Length..]; remoteBranch = defaultBranch; @@ -745,29 +767,32 @@ public sealed class LibGit2GitClient : IGitClient } } - RefuseToCheckOutOverWhatIsThere(repo, remoteBranch.Tip.Tree); + RefuseToWriteOverWhatIsNotOurs(repo, remoteBranch.Tip.Tree, aCutOffWriteIsOurs: false); /////////////////////////////////////////////////////////////////////////////// - // From the moment the branch exists the repository looks like any clone, so // - // the marker goes first: if the checkout below fails or the process dies, // - // the next pull completes it instead of reporting an empty or half-written // - // tree as up to date. // + // From here on the repository changes, in an order in which every point it // + // can be cut off at is one the next pull recovers from: HEAD names the // + // branch (still unborn: this method runs again); the branch's upstream is // + // configured before the branch exists; the folder is marked unfinished // + // before the branch exists, because from then on it looks like any clone // + // and only the marker says that its tree is not written yet. // /////////////////////////////////////////////////////////////////////////////// + if(followOriginsDefault) + { + repo.Refs.UpdateTarget("HEAD", targetRef); + } + repo.Config.Set($"branch.{branchName}.remote", "origin", ConfigurationLevel.Local); + repo.Config.Set($"branch.{branchName}.merge", targetRef, ConfigurationLevel.Local); repo.Config.Set(CheckoutPendingKey, true, ConfigurationLevel.Local); repo.Refs.Add(targetRef, remoteBranch.Tip.Id); Branch local = repo.Branches[branchName]; - repo.Branches.Update(local, b => - { - b.Remote = "origin"; - b.UpstreamBranch = targetRef; - }); BeforeFirstCheckoutForTesting?.Invoke(); ct.ThrowIfCancellationRequested(); try { - // Force: nothing in the folder is in the way (checked above), and a - // checkout that is being completed after a failure must write every file. + // Force: nothing that is not gclo's is in the way (checked above), and + // nothing was ever checked out here, so there is no state to merge with. Commands.Checkout(repo, local, new CheckoutOptions { CheckoutModifiers = CheckoutModifiers.Force, @@ -786,42 +811,67 @@ public sealed class LibGit2GitClient : IGitClient /// - /// The first checkout into a folder gclo has written nothing to yet: whatever is - /// already where the repository has a file (or a folder where it has a file, or a - /// file where it has a folder) was put there by someone, and a forced checkout - /// would replace it. Refused before the branch is created, so nothing has changed. + /// Before a forced checkout into a folder whose tree gclo has not finished writing: + /// refuses, with nothing changed, when something that is not gclo's is where the + /// incoming tree has a file, needs a folder, or has a submodule. A forced checkout + /// replaces all of those without asking (#174). /// - private static void RefuseToCheckOutOverWhatIsThere(Repository repo, Tree tree) + /// + /// The rules are the ones the path recovery follows + /// (): a link on the way, a file where a + /// folder is needed, a folder with files where a file is needed, and a file at a + /// destination that is not what the current commit has there. With + /// a file that is the beginning of what the + /// current commit has also counts as gclo's: a checkout that was cut off leaves + /// the file it was writing short, and completing it must not stop at that file. + /// A submodule's place must be empty: the checkout makes an empty folder of it. + /// + private static void RefuseToWriteOverWhatIsNotOurs(Repository repo, Tree incoming, bool aCutOffWriteIsOurs) { string workdir = WorkingDirectoryOf(repo); - var checkedFolders = new HashSet(StringComparer.Ordinal); - foreach((string? path, _, _) in CollectEffectiveEntries(tree, s_noRecovery)) - { - string? inTheWay = File.Exists(InWorkingDirectory(workdir, path)) || Directory.Exists(InWorkingDirectory(workdir, path)) - ? path - : null; - for(int slash = path.IndexOf('/', StringComparison.Ordinal); inTheWay is null && slash > 0; slash = path.IndexOf('/', slash + 1)) - { - string folder = path[..slash]; - if(checkedFolders.Add(folder) && File.Exists(InWorkingDirectory(workdir, folder))) - { - inTheWay = folder; - } - } + List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(incoming, s_noRecovery); + var effectiveNow = new HashSet(entries.Select(entry => entry.EffectivePath), StringComparer.Ordinal); + RefuseWhatCannotBeWritten(repo, workdir, entries, ours: null, effectiveNow, replaceOnlyWhatTheCommitHas: true, aCutOffWriteIsOurs); - if(inTheWay is not null) + foreach(string submodule in SubmodulePaths(incoming)) + { + WorkingTree.Verify(workdir, submodule); + string full = Path.Combine(workdir, submodule.Replace('/', Path.DirectorySeparatorChar)); + if(File.Exists(full) || (Directory.Exists(full) && Directory.EnumerateFileSystemEntries(full).Any())) { throw new InvalidOperationException( - $"'{inTheWay}' is already in this folder, which gclo has not checked out yet, so it is not gclo's to replace. " - + "Move or delete it (or the whole folder), then sync again; fetched, but nothing was checked out."); + $"'{submodule}' is already in this folder, where the repository has a submodule, and it is not empty. " + + "Move or delete it, then try again."); } } } - private static string InWorkingDirectory(string workdir, string repoPath) - => Path.Combine(workdir, repoPath.Replace('/', Path.DirectorySeparatorChar)); + /// The paths of the submodule entries (gitlinks) of a tree, at any depth. + private static List SubmodulePaths(Tree tree) + { + var found = new List(); + var stack = new Stack<(Tree Tree, string Prefix)>(); + stack.Push((tree, "")); + while(stack.Count > 0) + { + (Tree current, string prefix) = stack.Pop(); + foreach(TreeEntry entry in current) + { + string path = prefix.Length == 0 ? entry.Name : $"{prefix}/{entry.Name}"; + if(entry.TargetType == TreeEntryTargetType.Tree) + { + stack.Push(((Tree)entry.Target, path)); + } + else if(entry.TargetType == TreeEntryTargetType.GitLink) + { + found.Add(path); + } + } + } + return found; + } @@ -1203,7 +1253,8 @@ public sealed class LibGit2GitClient : IGitClient List<(string OriginalPath, string EffectivePath, Blob Blob)> entries, HashSet? ours, HashSet effectiveNow, - bool replaceOnlyWhatTheCommitHas) + bool replaceOnlyWhatTheCommitHas, + bool aCutOffWriteIsOurs = false) { if(File.Exists(Path.Combine(repo.Info.Path, "index.lock"))) { @@ -1225,7 +1276,7 @@ public sealed class LibGit2GitClient : IGitClient WorkingTree.Verify(workdir, effectivePath, verifiedFolders); string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); - if(replaceOnlyWhatTheCommitHas && File.Exists(full) && !IsWhatTheCommitHas(full, originalPath, current)) + if(replaceOnlyWhatTheCommitHas && File.Exists(full) && !IsWhatTheCommitHas(full, originalPath, current, aCutOffWriteIsOurs)) { throw new InvalidOperationException( $"'{effectivePath}' is already in this folder, and it is not a file gclo has a record of writing " @@ -1271,8 +1322,10 @@ 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. + /// With , also true when the file holds the + /// beginning of that and nothing else: what a write that was cut off leaves. /// - private static bool IsWhatTheCommitHas(string full, string originalPath, Tree? commitTree) + private static bool IsWhatTheCommitHas(string full, string originalPath, Tree? commitTree, bool orItsBeginning = false) { if(commitTree?[originalPath]?.Target is not Blob blob) { @@ -1287,10 +1340,15 @@ public sealed class LibGit2GitClient : IGitClient { 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))) + int common = Math.Min(expectedRead, actualRead); + if(actualRead > expectedRead || !expectedBuffer.AsSpan(0, common).SequenceEqual(actualBuffer.AsSpan(0, common))) { return false; } + if(actualRead < expectedRead) + { + return orItsBeginning; // the file ends where the commit's goes on + } if(expectedRead == 0) { return true; @@ -1392,12 +1450,28 @@ public sealed class LibGit2GitClient : IGitClient /// /// Finishes a clone whose checkout never completed and that has no stored /// recovery: moves to the upstream tip, re-validates its paths on Windows, and - /// force-checks-out the whole tree (whatever partial files exist are overwritten - /// to match). If the paths are still invalid, rethrows the same typed failure — - /// the repo must not silently report success with an empty or partial tree. + /// force-checks-out the whole tree. If the paths are still invalid, rethrows the + /// same typed failure — the repo must not silently report success with an empty + /// or partial tree. /// + /// + /// What the unfinished checkout left is overwritten: its files, and the one it + /// was writing when it was cut off. Anything else at a destination was put there + /// since, by somebody, and stops the completion before the branch has moved + /// (, #174). "What the unfinished + /// checkout left" is judged against the commit the branch is on, which is the + /// one that checkout was writing; that is why the check comes before the move. + /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { + // Paths Windows cannot create are reported as such further down, once the + // branch has moved (the recovery that follows works on the newest tree); a + // tree with such paths is not written here at all, so nothing is in its way. + Tree incoming = TargetOf(repo).Tree; + if(!OperatingSystem.IsWindows() || WindowsPathValidator.Validate(incoming).Count == 0) + { + RefuseToWriteOverWhatIsNotOurs(repo, incoming, aCutOffWriteIsOurs: true); + } AdvanceHeadToTrackedTip(repo); Commit? tip = repo.Head.Tip; diff --git a/gclo.Engine/WslCloner.cs b/gclo.Engine/WslCloner.cs index c992a99..45afe43 100644 --- a/gclo.Engine/WslCloner.cs +++ b/gclo.Engine/WslCloner.cs @@ -24,13 +24,15 @@ public sealed class WslCloner : IWslCloner /// host (with a non-default port). Prints the distribution name and the destination /// as its last two lines on success. A folder that already is a clone of the same /// URL is fast-forwarded instead of failing, so the verb is safe to use twice; a - /// folder that is a clone of anything else is refused. A new clone is made under a - /// temporary name beside the destination and moved into place when it is complete, - /// so one that is cut off never leaves a folder that the next attempt would take - /// for a clone and try to update. The temporary folder is removed when the script - /// ends without having moved it; one that a killed script left behind is removed - /// by a later run once it is a day old (not sooner: another gclo may be cloning the - /// same repository right now under such a name). + /// folder that is a clone of anything else is refused, and so is anything else at + /// the destination but an empty folder. A new clone is made in a folder of gclo's + /// own (~/gclo/.gclo-tmp) and moved into place when it is complete, so one + /// that is cut off never leaves a folder that the next attempt would take for a + /// clone and try to update. The temporary folder is removed when the script ends + /// without having moved it, also when it is ended by a signal; what a killed + /// script left there is removed by a later run once it is a day old (not sooner: + /// another gclo may be cloning right now). A destination that appears while the + /// clone runs is not moved into: the clone would end up inside it. /// The credential helper is /// scoped to $4, so git (and anything it starts, such as an LFS filter pointed at /// another server by the repository's own configuration) gets the token for that @@ -48,17 +50,25 @@ public sealed class WslCloner : IWslCloner + " fi\n" + " git -C \"$dest\" -c credential.helper= -c \"credential.https://$host.helper=$helper\" pull --ff-only --quiet\n" + "else\n" - + " if [ -e \"$dest\" ]; then\n" + + " if [ -e \"$dest\" ] && ! rmdir \"$dest\" 2>/dev/null; then\n" + " echo \"$dest exists but is not a repository; move or delete it, then try again\" >&2\n" + " exit 1\n" + " fi\n" - + " mkdir -p \"$HOME/gclo/$org\"\n" - + " tmp=\"$dest.gclo-clone.$$\"\n" - + " find \"$HOME/gclo/$org\" -maxdepth 1 -name \"$repo.gclo-clone.*\" -mmin +1440 -exec rm -rf {} + 2>/dev/null || true\n" + + " mkdir -p \"$HOME/gclo/$org\" \"$HOME/gclo/.gclo-tmp\"\n" + + " find \"$HOME/gclo/.gclo-tmp\" -mindepth 1 -maxdepth 1 -mmin +1440 -exec rm -rf {} + 2>/dev/null || true\n" + + " tmp=\"$HOME/gclo/.gclo-tmp/$org.$repo.$$\"\n" + + " rm -rf \"$tmp\"\n" + " trap 'rm -rf \"$tmp\"' EXIT\n" + + " trap 'exit 129' HUP\n" + + " trap 'exit 130' INT\n" + + " trap 'exit 143' TERM\n" + " git -c credential.helper= -c \"credential.https://$host.helper=$helper\" clone --quiet -- \"$url\" \"$tmp\"\n" + + " if [ -e \"$dest\" ]; then\n" + + " echo \"$dest appeared while the repository was being cloned (another gclo?); nothing was put there\" >&2\n" + + " exit 1\n" + + " fi\n" + " mv \"$tmp\" \"$dest\"\n" - + " trap - EXIT\n" + + " trap - EXIT HUP INT TERM\n" + "fi\n" + "printf '%s\\n%s\\n' \"$WSL_DISTRO_NAME\" \"$dest\"\n"; -- 2.54.0 From 099c2b5ee8036d54f483e87f1c76ebb72194c4b5 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 21:24:25 -0600 Subject: [PATCH 3/5] fix(clone): second review of #175: an unfinished checkout is recognised by its missing index and completed where it stands The second review showed that telling gclo's files from a user's by exact content does not hold: with line endings being converted, the completion refused files its own cut-off checkout had written, and a retry interrupted after the branch had moved refused them for ever. It also found a user's file deleted where a submodule's parent folder is needed. - A marked folder is an unfinished checkout only while it has no index (git writes the index when a checkout has reached its end). A marked folder with an index is updated like any clone, without force. - An unfinished checkout is completed at the commit its branch is on, before the branch moves; what origin has got since comes with the ordinary pull. When that commit or origin's cannot be written on Windows, the typed failure and the marker are kept as before. - A file is the unfinished checkout's own when it holds the commit's content or the beginning of it, carriage returns aside. - Refused as well: a file where a submodule's parent folder is needed, a link at a destination, a folder there that is not empty. - Only a repository without branches of its own is an empty clone, and it follows origin's default branch only when its folder holds nothing but .git. - An ordinary update that would replace files changed or added here says so. - The refusals are Windows' rules and apply on Windows. - WSL: 'mv -T'; the temporary folder's name is refused as an organization. The run tests find Git for Windows' shell beside git.exe and say when they had nothing to run. Refs #174 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/CLI.md | 8 +- docs/security/threat-model.md | 2 +- gclo.Engine.Tests/InterruptedCloneTests.cs | 305 ++++++++++++++- gclo.Engine.Tests/LibGit2GitClientTests.cs | 15 +- gclo.Engine.Tests/WslCloneScriptRunTests.cs | 101 ++++- gclo.Engine.Tests/WslClonerTests.cs | 20 +- gclo.Engine/LibGit2GitClient.cs | 394 +++++++++++++++----- gclo.Engine/WslCloner.cs | 17 +- 9 files changed, 732 insertions(+), 132 deletions(-) diff --git a/README.md b/README.md index ea38584..cf082c3 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, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. -**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. What it completes is its own: a file that is in the folder and is not what the unfinished checkout wrote is never replaced. The row fails and names the file; move it and sync again. The same holds for a repository that was empty when it was cloned and gets its first files later (it then follows the origin's default branch), and a branch you started yourself in a clone is left alone. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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`). +**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. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder; a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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 bcbe0db..d5961a9 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -49,10 +49,10 @@ Options may be written as `--name value` or `--name=value`. Clones every repository of `--org` into `\`. Repositories that are already valid git repositories locally are fetched and fast-forwarded instead. -A file in a target folder that gclo did not write is never replaced: when one is where -the repository has a file (a repository that was empty when it was cloned, or a clone -whose first checkout was interrupted), that repository fails and the message names the -file. +When a repository gets its first checkout into a folder that already holds something +(it was empty when it was cloned, or its first checkout was interrupted), gclo on +Windows replaces a file there only if it is what its own unfinished checkout wrote; +otherwise that repository fails and the message names what is in the way. Repositories fail independently — one failure never stops the rest. Up to `--parallel` git operations run at once (default 8, maximum 64 — the same ceiling the desktop app applies, because more would trip GitHub's secondary diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index a29b097..047dc4a 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -121,4 +121,4 @@ flowchart LR | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated: path recovery writes and deletes only through `WorkingTree` (inside the working directory, never in `.git`, never through a link); `.git` and its Windows aliases are invalid path segments; the stored recovery file's paths are validated on load; cleanup of a failed clone does not follow links (#141) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T15 added: text from repositories and servers is shown with control, separator and format characters made visible, in the activity log and in everything the CLI prints as text, including characters a legacy console code page would turn into control bytes; the desktop app's surfaces are named as not covered (#143) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | The project's home moved to `git.kof22.com`: the Help menu, the About dialog, the CLI's error text and the documentation link there, and the Scoop bucket is added from there (the manifest that pins the download URL and its hash now comes from that server over HTTPS; T2 is unchanged in kind). Help > Share hands a fixed text (what gclo is, where to download it, the sender's gclo version) to the system's mail handler as a `mailto:` link with no recipient, or shows it and copies it to the clipboard; nothing about the user, their accounts, or their repositories is in it, and the address it is built from must be a plain http(s) host and path (#163, #165). The update feed and the release downloads named in package manifests are still GitHub Releases at this revision. | -| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on): a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) goes through the same refusals as the path recovery before anything changes: a link on the way, a file where a folder is needed, a folder with files where a file is needed, a file at a destination that is not what the unfinished checkout wrote, and anything but an empty folder where the repository has a submodule. A repository with local branches whose HEAD names a branch without commits is left alone. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes). The WSL clone is made in gclo's own temporary folder and moved into place when complete; it is not moved into a destination that appeared meanwhile. Not covered: a file created in the folder between the check and the checkout that follows it. | +| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on). On Windows, a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) is preceded by refusals that change nothing: a link on the way or at a destination, a file where a folder is needed (also on the way to a submodule), a folder that is not empty where a file is needed, a file at a destination that is not what the unfinished checkout wrote there (the commit's content or the beginning of it, carriage returns aside), and anything but an empty folder where the repository has a submodule. An unfinished checkout is recognised by its marker and the absence of an index; a marked folder that has an index is updated like any clone, without force. It is completed at the commit its branch is on, before the branch moves. A repository that has local branches is not treated as an empty clone; one without follows origin's default branch only when its folder holds nothing but `.git`. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes, and not across spellings of a path that only the file system can resolve). The WSL clone is made in `~/gclo/.gclo-tmp` and renamed into place when complete (`mv -T`, after a check that nothing is there). Not covered: a file created in the folder between the check and the checkout that follows it; an empty or identical file of the user's at a destination, which is replaced; platforms other than Windows, where the checkout is forced as before. | diff --git a/gclo.Engine.Tests/InterruptedCloneTests.cs b/gclo.Engine.Tests/InterruptedCloneTests.cs index 7b8a094..6448883 100644 --- a/gclo.Engine.Tests/InterruptedCloneTests.cs +++ b/gclo.Engine.Tests/InterruptedCloneTests.cs @@ -20,6 +20,9 @@ public sealed class InterruptedCloneTests : IDisposable { private const string Token = "test-token"; + /// A branch name that is nobody's init.defaultBranch: a new clone's HEAD never names it. + private const string OtherBranch = "not-the-branch-a-clone-starts-with"; + private readonly string _root = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); private readonly LibGit2GitClient _client = new(); @@ -86,6 +89,15 @@ public sealed class InterruptedCloneTests : IDisposable + /// The repository's own configuration, as "key=value" lines in a stable order. + private static string[] LocalConfiguration(string workdir) + { + using var repo = new Repository(workdir); + return [.. repo.Config.Where(entry => entry.Level == ConfigurationLevel.Local).Select(entry => $"{entry.Key}={entry.Value}").Order(StringComparer.Ordinal)]; + } + + + private static bool IsCheckoutPending(string workdir) { using var repo = new Repository(workdir); @@ -100,16 +112,16 @@ public sealed class InterruptedCloneTests : IDisposable public async Task AnEmptyClone_WhoseOriginStartedOnAnotherBranch_GetsOriginsDefaultBranch() { (string origin, string clone) = await CloneAnEmptyOriginAsync(); - string sha = await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + string sha = await SeedAsync(origin, OtherBranch, ("readme.txt", "from the other branch")); await _client.PullAsync(clone, Token, CancellationToken.None); using var repo = new Repository(clone); Assert.False(repo.Info.IsHeadUnborn); - Assert.Equal("trunk", repo.Head.FriendlyName); + Assert.Equal(OtherBranch, repo.Head.FriendlyName); Assert.Equal(sha, repo.Head.Tip.Sha); - Assert.Equal("origin/trunk", repo.Head.TrackedBranch?.FriendlyName); - Assert.Equal("from trunk", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.Equal("origin/" + OtherBranch, repo.Head.TrackedBranch?.FriendlyName); + Assert.Equal("from the other branch", File.ReadAllText(Path.Combine(clone, "readme.txt"))); Assert.False(IsCheckoutPending(clone)); } @@ -134,14 +146,16 @@ public sealed class InterruptedCloneTests : IDisposable File.WriteAllText(mine, "mine"); } await SeedAsync(origin, null, ("readme.txt", "theirs"), ("docs/guide.txt", "theirs")); + string[] configurationBefore = LocalConfiguration(clone); InvalidOperationException refused = await Assert.ThrowsAsync( () => _client.PullAsync(clone, Token, CancellationToken.None)); Assert.Contains(said, refused.Message, StringComparison.Ordinal); + Assert.Equal(configurationBefore, LocalConfiguration(clone)); // nothing was written: no marker, no upstream Assert.Equal("mine", File.ReadAllText(asFolder ? Path.Combine(mine, "kept.txt") : mine)); using var repo = new Repository(clone); - Assert.True(repo.Info.IsHeadUnborn); // nothing was started: no branch, no marker + Assert.True(repo.Info.IsHeadUnborn); // nothing was started: no branch, no marker, no upstream written Assert.False(IsCheckoutPending(clone)); Assert.False(File.Exists(Path.Combine(clone, "docs", "guide.txt"))); } @@ -206,13 +220,13 @@ public sealed class InterruptedCloneTests : IDisposable { Commands.Stage(repo, "draft.txt"); } - await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + await SeedAsync(origin, OtherBranch, ("readme.txt", "from the other branch")); await _client.PullAsync(clone, Token, CancellationToken.None); // fetches; checks nothing out using var after = new Repository(clone); Assert.True(after.Info.IsHeadUnborn); - Assert.False(string.Equals("trunk", after.Head.FriendlyName, StringComparison.Ordinal)); + Assert.False(string.Equals(OtherBranch, after.Head.FriendlyName, StringComparison.Ordinal)); Assert.Equal(1, after.Index.Count); Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "draft.txt"))); @@ -224,7 +238,7 @@ public sealed class InterruptedCloneTests : IDisposable public async Task AnOriginThatNamesNoDefaultBranch_LeavesTheEmptyCloneAsItIs() { (string origin, string clone) = await CloneAnEmptyOriginAsync(); - await SeedAsync(origin, "trunk", ("readme.txt", "from trunk")); + await SeedAsync(origin, OtherBranch, ("readme.txt", "from the other branch")); using(var bare = new Repository(origin)) { bare.Refs.UpdateTarget("HEAD", "refs/heads/does-not-exist"); // HEAD names a branch that is not there @@ -377,9 +391,13 @@ public sealed class InterruptedCloneTests : IDisposable InvalidOperationException refused = await Assert.ThrowsAsync( () => _client.PullAsync(clone, Token, CancellationToken.None)); - Assert.Contains("'notes.txt' is already in this folder", refused.Message, StringComparison.Ordinal); + // The checkout that was cut off is finished at its own commit, which has no + // such file; what origin got since is an ordinary update, and that one does + // not replace a file that was added here. + Assert.Contains("The update would replace files in this folder that were changed or added here", refused.Message, StringComparison.Ordinal); Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "notes.txt"))); - Assert.True(IsCheckoutPending(clone)); + Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); } @@ -429,18 +447,25 @@ public sealed class InterruptedCloneTests : IDisposable branch = repo.Refs.Head.TargetIdentifier["refs/heads/".Length..]; } - // Something that makes creating the branch fail: a reference below its name. - string below = Path.Combine(clone, ".git", "refs", "heads", branch); - Directory.CreateDirectory(below); - File.WriteAllText(Path.Combine(below, "x"), "0000000000000000000000000000000000000000" + (char)10); + // Something that makes creating the branch fail: another git holds its lock. + string heads = Directory.CreateDirectory(Path.Combine(clone, ".git", "refs", "heads")).FullName; + string held = Path.Combine(heads, branch + ".lock"); + File.WriteAllText(held, ""); await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); - // Whatever stopped it, the folder already says that it is not finished. + // Whatever stopped it, the folder already says that it is not finished, and + // the branch already has its upstream: no cut-off leaves one without. Assert.True(IsCheckoutPending(clone)); Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + using(var stopped = new Repository(clone)) + { + Assert.True(stopped.Info.IsHeadUnborn); + Assert.Equal("origin", stopped.Config.Get($"branch.{branch}.remote", ConfigurationLevel.Local)?.Value); + Assert.Equal("refs/heads/" + branch, stopped.Config.Get($"branch.{branch}.merge", ConfigurationLevel.Local)?.Value); + } - Directory.Delete(below, recursive: true); + File.Delete(held); await _client.PullAsync(clone, Token, CancellationToken.None); Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); @@ -597,8 +622,11 @@ public sealed class InterruptedCloneTests : IDisposable // One folder to Windows: another case, and the long-path spelling. await Assert.ThrowsAsync( () => other.FetchAndPullAsync(origin, target.ToUpperInvariant(), Token, CancellationToken.None)); - await Assert.ThrowsAsync( - () => other.FetchAndPullAsync(origin, @"\\?\" + target, Token, CancellationToken.None)); + foreach(string prefix in (string[])[@"\\?\", @"\\.\", @"\??\"]) + { + await Assert.ThrowsAsync( + () => other.FetchAndPullAsync(origin, prefix + target, Token, CancellationToken.None)); + } } letGo.Set(); @@ -635,4 +663,245 @@ public sealed class InterruptedCloneTests : IDisposable Assert.DoesNotContain("has no index", failure.Message, StringComparison.Ordinal); Assert.Equal("edited here", File.ReadAllText(Path.Combine(target, "readme.txt"))); } + + + + // ---------------------------------------------------------------- review of #175, second round + + [Fact] + public async Task ACheckoutCutOffHalfWay_WithLineEndingsBeingConverted_IsStillCompleted() + { + // The tree brings its own line-ending rules, and the user's configuration has + // others: the files written before '.gitattributes' was on disk follow the + // configuration alone. They are gclo's all the same. + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync( + origin, null, + (".gitattributes", "* text=auto eol=lf" + (char)10), + ("a.txt", "one" + (char)10 + "two" + (char)10), + ("zzz.txt", "last" + (char)10)); + string target = Directory.CreateDirectory(NewPath("cut-off-half-way")).FullName; // it exists: a failed clone is then kept + FileStream? held = null; + _client.AfterFetchForTesting = () => + { + using(var fetched = new Repository(target)) + { + fetched.Config.Set("core.autocrlf", true, ConfigurationLevel.Local); + } + // Another program has the last file open: the checkout fails when it gets there. + held = new FileStream(Path.Combine(target, "zzz.txt"), FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None); + }; + try + { + await Assert.ThrowsAnyAsync(() => _client.CloneAsync(origin, target, Token, null, CancellationToken.None)); + } + finally + { + held?.Dispose(); + _client.AfterFetchForTesting = null; + } + + Assert.True(IsCheckoutPending(target)); + Assert.True(File.Exists(Path.Combine(target, ".gitattributes"))); // written before the failure + using(var cutOff = new Repository(target)) + { + Assert.Equal(0, cutOff.Index.Count); // git writes the index last: a checkout that was cut off leaves none + } + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.False(IsCheckoutPending(target)); + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zzz.txt")).TrimEnd()); + Assert.Equal(["one", "two"], File.ReadAllLines(Path.Combine(target, "a.txt"))); + } + + + + [Theory] + [InlineData(150_001, null, true)] // cut off in the middle of a large file, with other line endings + [InlineData(0, null, true)] // cut off before its first byte + [InlineData(150_001, 120_000, false)] // the same, with one byte that is not the commit's + public async Task WhatACutOffCheckoutLeft_IsRecognisedByItsContent_LineEndingsAside(int length, int? changedAt, bool isOurs) + { + string big = string.Concat(Enumerable.Range(0, 20_000).Select(i => $"line {i:D6}" + (char)10)); // more than two buffers + (_, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("big.txt", big)); + char[] onDisk = big.Replace(((char)10).ToString(), "" + (char)13 + (char)10, StringComparison.Ordinal)[..length].ToCharArray(); + if(changedAt is int at) + { + onDisk[at] = onDisk[at] == 'x' ? 'y' : 'x'; + } + File.WriteAllText(Path.Combine(clone, "big.txt"), new string(onDisk)); + + if(isOurs) + { + await _client.PullAsync(clone, Token, CancellationToken.None); + Assert.Equal( + big, + File.ReadAllText(Path.Combine(clone, "big.txt")).Replace("" + (char)13, "", StringComparison.Ordinal)); + Assert.False(IsCheckoutPending(clone)); + } + else + { + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + Assert.Contains("'big.txt' is already in this folder", refused.Message, StringComparison.Ordinal); + Assert.Equal(new string(onDisk), File.ReadAllText(Path.Combine(clone, "big.txt"))); + } + } + + + + [Fact] + public async Task AFileLongerThanTheCommits_IsNotACutOffWrite() + { + (_, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("readme.txt", "theirs")); + File.WriteAllText(Path.Combine(clone, "readme.txt"), "theirs, and what I added"); + + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Equal("theirs, and what I added", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + } + + + + [Fact] + public async Task AMarkedFolderThatHasAnIndex_IsAFinishedCheckout_AndIsUpdatedWithoutForce() + { + // The checkout reached its end and the marker could not be cleared any more; + // or somebody finished it with git. Either way the tree is git's now. + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "one"), ("other.txt", "other")); + string target = NewPath("finished-but-marked"); + await _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + using(var repo = new Repository(target)) + { + repo.Config.Set("gclo.checkoutpending", true, ConfigurationLevel.Local); + } + File.WriteAllText(Path.Combine(target, "other.txt"), "edited here"); // an edit; a forced completion would undo it + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.Equal("edited here", File.ReadAllText(Path.Combine(target, "other.txt"))); + Assert.False(IsCheckoutPending(target)); + } + + + + [Fact] + public async Task AFileWhereASubmodulesParentFolderIsNeeded_IsNotReplaced() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string seeder = NewPath("seeder-with-nested-submodule"); + await _client.CloneAsync(origin, seeder, Token, null, CancellationToken.None); + using(var repo = new Repository(seeder)) + { + Blob readme = repo.ObjectDatabase.CreateBlob(new MemoryStream("theirs"u8.ToArray())); + var definition = new TreeDefinition(); + definition.Add("readme.txt", readme, Mode.NonExecutableFile); + definition.AddGitLink("libs/sub", new ObjectId("1234567890123456789012345678901234567890")); + Tree tree = repo.ObjectDatabase.CreateTree(definition); + Signature who = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit(who, who, "first commit", tree, [], prettifyMessage: false); + repo.Refs.Add(repo.Refs.Head.TargetIdentifier, commit.Id); + repo.Network.Push(repo.Network.Remotes["origin"], repo.Refs.Head.TargetIdentifier, new PushOptions()); + } + File.WriteAllText(Path.Combine(clone, "libs"), "mine"); + + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("'libs' is a file in this folder, and the repository has a folder of that name", refused.Message, StringComparison.Ordinal); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "libs"))); + } + + + + [Theory] + [InlineData(false, "'readme.txt' is a link in this folder")] // a link where the repository has a file + [InlineData(true, "'readme.txt' is a folder that is not empty")] // a folder that holds nothing but a link + public async Task ALinkWhereTheRepositoryHasAFile_IsNotRemoved(bool insideAFolder, string said) + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; + File.WriteAllText(Path.Combine(elsewhere, "kept.txt"), "mine"); + string link = insideAFolder + ? Path.Combine(Directory.CreateDirectory(Path.Combine(clone, "readme.txt")).FullName, "j") + : Path.Combine(clone, "readme.txt"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains(said, refused.Message, StringComparison.Ordinal); + Assert.NotNull(new DirectoryInfo(link).LinkTarget); + Assert.Equal("mine", File.ReadAllText(Path.Combine(elsewhere, "kept.txt"))); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + [Fact] + public async Task AFolderSomebodyPutSomethingInto_KeepsTheBranchItsHeadNames_WhenOriginHasAnother() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string before; + using(var repo = new Repository(clone)) + { + before = repo.Refs.Head.TargetIdentifier; + } + File.WriteAllText(Path.Combine(clone, "page.html"), "mine"); // not the untouched folder of an empty clone any more + await SeedAsync(origin, OtherBranch, ("readme.txt", "from the other branch")); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + using var after = new Repository(clone); + Assert.True(after.Info.IsHeadUnborn); + Assert.Equal(before, after.Refs.Head.TargetIdentifier); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task ABranchSomebodyStarted_IsTheirs_AlsoWhenOriginHasABranchOfThatName() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, null, ("readme.txt", "theirs")); + string clone = NewPath("ordinary-with-a-new-branch"); + await _client.CloneAsync(origin, clone, Token, null, CancellationToken.None); + + // Origin gets a branch 'pages' of its own... + string publisher = NewPath("publisher"); + await _client.CloneAsync(origin, publisher, Token, null, CancellationToken.None); + using(var repo = new Repository(publisher)) + { + Branch pages = repo.CreateBranch("pages"); + repo.Network.Push(repo.Network.Remotes["origin"], pages.CanonicalName, new PushOptions()); + } + + // ...and here somebody starts one of that name from nothing ('git switch --orphan pages'). + using(var repo = new Repository(clone)) + { + repo.Refs.UpdateTarget("HEAD", "refs/heads/pages"); + repo.Index.Clear(); + repo.Index.Write(); + } + File.Delete(Path.Combine(clone, "readme.txt")); + File.WriteAllText(Path.Combine(clone, "page.html"), "mine"); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + using var after = new Repository(clone); + Assert.True(after.Info.IsHeadUnborn); // still theirs: origin's 'pages' was fetched, not checked out + Assert.Equal("pages", after.Head.FriendlyName); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "page.html"))); + } } diff --git a/gclo.Engine.Tests/LibGit2GitClientTests.cs b/gclo.Engine.Tests/LibGit2GitClientTests.cs index 81e2013..77369d8 100644 --- a/gclo.Engine.Tests/LibGit2GitClientTests.cs +++ b/gclo.Engine.Tests/LibGit2GitClientTests.cs @@ -544,10 +544,21 @@ public sealed class LibGit2GitClientTests : IDisposable + /// + /// Sets or clears the marker. Set, the folder is also left without an index, as a + /// checkout that was cut off leaves it: git writes the index when a checkout has + /// reached its end, and a marked folder that has one is taken for finished. + /// private static void SetCheckoutPending(string workdir, bool value) { - using var repo = new Repository(workdir); - repo.Config.Set("gclo.checkoutpending", value, ConfigurationLevel.Local); + using(var repo = new Repository(workdir)) + { + repo.Config.Set("gclo.checkoutpending", value, ConfigurationLevel.Local); + } + if(value) + { + File.Delete(Path.Combine(workdir, ".git", "index")); + } } diff --git a/gclo.Engine.Tests/WslCloneScriptRunTests.cs b/gclo.Engine.Tests/WslCloneScriptRunTests.cs index 8579009..219b77d 100644 --- a/gclo.Engine.Tests/WslCloneScriptRunTests.cs +++ b/gclo.Engine.Tests/WslCloneScriptRunTests.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using LibGit2Sharp; +using Xunit.Abstractions; using static gclo.Engine.Tests.GitTestHelpers; @@ -15,8 +16,8 @@ namespace gclo.Engine.Tests; /// Runs for real, with a POSIX shell and local /// repositories, where the machine has one (Git for Windows brings sh): what /// the script leaves on disk is what matters, and matching its text cannot show -/// that. On a machine without sh and git on the path the tests have -/// nothing to run and pass. +/// that. On a machine with no sh that can find git the tests have +/// nothing to run: they pass, and say so in their output. /// public sealed class WslCloneScriptRunTests : IDisposable { @@ -30,8 +31,13 @@ public sealed class WslCloneScriptRunTests : IDisposable - public WslCloneScriptRunTests() + private readonly ITestOutputHelper _output; + + + + public WslCloneScriptRunTests(ITestOutputHelper output) { + _output = output; _home = Path.Combine(_root, "home"); Directory.CreateDirectory(_home); @@ -53,12 +59,26 @@ public sealed class WslCloneScriptRunTests : IDisposable - /// A POSIX shell that can find git, or null when this machine has none. + /// + /// A POSIX shell that can find git, or null when this machine has none. Looked for + /// on the path and, because Git for Windows puts only its cmd folder there, + /// beside every git.exe on the path and where Git for Windows installs. + /// private static (string Sh, string Git)? FindTools() { - IEnumerable candidates = (Environment.GetEnvironmentVariable("PATH") ?? "") - .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries) - .Select(folder => Path.Combine(folder, OperatingSystem.IsWindows() ? "sh.exe" : "sh")); + string shell = OperatingSystem.IsWindows() ? "sh.exe" : "sh"; + string[] path = (Environment.GetEnvironmentVariable("PATH") ?? "").Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries); + var candidates = new List(path.Select(folder => Path.Combine(folder, shell))); + foreach(string folder in path.Where(folder => File.Exists(Path.Combine(folder, "git.exe")))) + { + candidates.Add(Path.GetFullPath(Path.Combine(folder, "..", "bin", shell))); + candidates.Add(Path.GetFullPath(Path.Combine(folder, "..", "usr", "bin", shell))); + } + foreach(Environment.SpecialFolder programs in (Environment.SpecialFolder[])[Environment.SpecialFolder.ProgramFiles, Environment.SpecialFolder.ProgramFilesX86]) + { + candidates.Add(Path.Combine(Environment.GetFolderPath(programs), "Git", "bin", shell)); + } + foreach(string sh in candidates.Where(File.Exists)) { try @@ -84,6 +104,18 @@ public sealed class WslCloneScriptRunTests : IDisposable + /// False, with a line in the test's output, when this machine has no shell to run the script with. + private bool CanRun() + { + if(s_tools is null) + { + _output.WriteLine("Not run: no 'sh' that can find 'git' on this machine (Git for Windows brings one)."); + } + return s_tools is not null; + } + + + private string Destination(string org, string repo) => Path.Combine(_home, "gclo", org, repo); @@ -134,7 +166,7 @@ public sealed class WslCloneScriptRunTests : IDisposable [Fact] public void ANewClone_EndsUpAtTheDestination_AndLeavesNothingBehind_AndASecondRunUpdatesIt() { - if(s_tools is null) + if(!CanRun()) { return; } @@ -155,7 +187,7 @@ public sealed class WslCloneScriptRunTests : IDisposable [Fact] public void ACloneThatFails_LeavesNoFolderThatLooksLikeAClone() { - if(s_tools is null) + if(!CanRun()) { return; } @@ -172,7 +204,7 @@ public sealed class WslCloneScriptRunTests : IDisposable [Fact] public void AFolderWithSomethingInIt_AtTheDestination_IsRefused_AndAnEmptyOneIsUsed() { - if(s_tools is null) + if(!CanRun()) { return; } @@ -196,7 +228,7 @@ public sealed class WslCloneScriptRunTests : IDisposable [Fact] public void ADestinationThatAppearsWhileTheCloneRuns_IsNotMovedInto() { - if(s_tools is null) + if(!CanRun()) { return; } @@ -225,7 +257,7 @@ public sealed class WslCloneScriptRunTests : IDisposable [Fact] public void OnlyOldLeftovers_InGclosOwnTemporaryFolder_AreRemoved() { - if(s_tools is null) + if(!CanRun()) { return; } @@ -246,4 +278,49 @@ public sealed class WslCloneScriptRunTests : IDisposable Assert.Equal(["acme.other.222"], Leftovers(), StringComparer.Ordinal); Assert.Equal("mine", File.ReadAllText(Path.Combine(sibling, "kept.txt"))); } + + + + [Fact] + public async Task AScriptThatIsEndedByASignal_LeavesNoTemporaryFolder_AndNoClone() + { + if(!CanRun()) + { + return; + } + + // A stand-in for git that clones and then stays busy for a moment, so that the + // signal arrives while the script is waiting for it. A shell runs its handler + // when the command it waits for has ended; the handler ends the script, and + // ending it removes the temporary folder. + string wrappers = Path.Combine(_root, "wrappers"); + Directory.CreateDirectory(wrappers); + File.WriteAllText( + Path.Combine(wrappers, "git"), + "#!/bin/sh\n\"$GCLO_TEST_REAL_GIT\" \"$@\"\nrc=$?\ncase \" $* \" in *\" clone \"*) sleep 3;; esac\nexit $rc\n"); + string script = Path.Combine(_root, "script.sh"); + File.WriteAllText(script, WslCloner.CloneScript); + + var start = new ProcessStartInfo(s_tools!.Value.Sh) { RedirectStandardOutput = true, RedirectStandardError = true, UseShellExecute = false }; + start.ArgumentList.Add("-c"); + start.ArgumentList.Add( + "sh \"$GCLO_TEST_SCRIPT\" acme repo \"$GCLO_TEST_URL\" example.test & pid=$!; sleep 1; kill -TERM $pid; wait $pid; echo \"exit=$?\""); + start.Environment["HOME"] = _home.Replace('\\', '/'); + start.Environment[WslCloner.TokenVariable] = "not-a-token"; + start.Environment["WSL_DISTRO_NAME"] = "test"; + start.Environment["GCLO_TEST_REAL_GIT"] = s_tools.Value.Git; + start.Environment["GCLO_TEST_SCRIPT"] = script.Replace('\\', '/'); + start.Environment["GCLO_TEST_URL"] = _origin.Replace('\\', '/'); + start.Environment["PATH"] = wrappers + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + + using Process process = Process.Start(start)!; + Task errorText = process.StandardError.ReadToEndAsync(); + string output = await process.StandardOutput.ReadToEndAsync(); + string error = await errorText; + await process.WaitForExitAsync().WaitAsync(TimeSpan.FromMinutes(2)); + + Assert.Contains("exit=143", output, StringComparison.Ordinal); // ended by the signal, not by finishing + Assert.False(Directory.Exists(Destination("acme", "repo")), error); + Assert.Empty(Leftovers()); + } } diff --git a/gclo.Engine.Tests/WslClonerTests.cs b/gclo.Engine.Tests/WslClonerTests.cs index 16747ba..d0a117c 100644 --- a/gclo.Engine.Tests/WslClonerTests.cs +++ b/gclo.Engine.Tests/WslClonerTests.cs @@ -285,11 +285,12 @@ public sealed class WslClonerTests Assert.Contains("tmp=\"$HOME/gclo/.gclo-tmp/$org.$repo.$$\"", script); Assert.Contains("trap 'rm -rf \"$tmp\"' EXIT", script); Assert.True( - script.IndexOf("clone --quiet", StringComparison.Ordinal) < script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal), + script.IndexOf("clone --quiet", StringComparison.Ordinal) < script.IndexOf("mv -T \"$tmp\" \"$dest\"", StringComparison.Ordinal), "the folder is moved into place after the clone"); Assert.True( - script.IndexOf("mv \"$tmp\" \"$dest\"", StringComparison.Ordinal) < script.IndexOf("trap - EXIT", StringComparison.Ordinal), + script.IndexOf("mv -T \"$tmp\" \"$dest\"", StringComparison.Ordinal) < script.IndexOf("trap - EXIT", StringComparison.Ordinal), "the cleanup stays armed until the folder is in place"); + Assert.Contains("/gclo/" + WslCloner.TemporaryFolderName + "/", script); // What a killed run left is removed only from gclo's own temporary folder, and // only when it is old: a young one may be another gclo's clone in progress. Assert.Contains("find \"$HOME/gclo/.gclo-tmp\" -mindepth 1 -maxdepth 1 -mmin +1440 -exec rm -rf {} +", script); @@ -340,6 +341,21 @@ public sealed class WslClonerTests + [Theory] + [InlineData(".gclo-tmp")] + [InlineData(".GCLO-TMP")] + public async Task Clone_IntoGclosOwnTemporaryFolder_IsRefused_WithoutStartingWsl(string organization) + { + // Unfinished clones are made in ~/gclo/.gclo-tmp, and old ones there are removed. + WslCloneException ex = await Assert.ThrowsAsync( + () => Create().CloneAsync("https://github.com/acme/repo.git", organization, "repo", Token, CancellationToken.None)); + + Assert.Contains("gclo's own temporary folder", ex.Message); + Assert.Empty(_runner.Calls); + } + + + [Theory] [InlineData("http://github.com/acme/repo.git")] [InlineData("ssh://git@github.com/acme/repo.git")] diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 1cc3506..7703e92 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -154,15 +154,28 @@ public sealed class LibGit2GitClient : IGitClient /// private static void OneAtATime(string path, Action operation) { - string folder = Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)); - if(folder.StartsWith(@"\\?\UNC\", StringComparison.OrdinalIgnoreCase)) + /////////////////////////////////////////////////////////////////////////////// + // The device and long-path spellings of a path name the same folder, and // + // GetFullPath leaves them as they are: they are taken off first. Spellings // + // only the file system can resolve (a share on this machine, a substituted // + // drive, a link in a parent) are not recognised as the same folder. // + /////////////////////////////////////////////////////////////////////////////// + string folder = path; + foreach(string prefix in (string[])[@"\\?\UNC\", @"\\.\UNC\", @"\??\UNC\"]) { - folder = @"\\" + folder[8..]; // \\?\UNC\server\share is \\server\share + if(folder.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + folder = @"\\" + folder[prefix.Length..]; + } } - else if(folder.StartsWith(@"\\?\", StringComparison.Ordinal)) + foreach(string prefix in (string[])[@"\\?\", @"\\.\", @"\??\"]) { - folder = folder[4..]; // \\?\C:\x is C:\x + if(folder.StartsWith(prefix, StringComparison.Ordinal)) + { + folder = folder[prefix.Length..]; + } } + folder = Path.TrimEndingDirectorySeparator(Path.GetFullPath(folder)); if(!s_inFlight.TryAdd(folder, 0)) { throw new InvalidOperationException( @@ -257,11 +270,13 @@ public sealed class LibGit2GitClient : IGitClient ct.ThrowIfCancellationRequested(); Commands.Checkout(repo, repo.Head, new CheckoutOptions { - // Force: materialize the fresh working tree. The hook lets a - // cancellation stop the checkout before it has written much; once - // libgit2 is writing, it may run to the end, and the cancellation is - // then the caller's to see (the engine stops waiting; the folder stays - // marked as busy until this returns). + //////////////////////////////////////////////////////////////////////// + // Force: materialize the fresh working tree. The hook lets a // + // cancellation stop the checkout before it has written much; once // + // libgit2 is writing, it may run to the end, and the cancellation is // + // then the caller's to see (the engine stops waiting; the folder // + // stays marked as busy until this returns). // + //////////////////////////////////////////////////////////////////////// CheckoutModifiers = CheckoutModifiers.Force, CheckoutNotifyFlags = CheckoutNotifyFlags.Updated, OnCheckoutNotify = (_, _) => !ct.IsCancellationRequested, @@ -349,10 +364,12 @@ public sealed class LibGit2GitClient : IGitClient if(IsCheckoutPending(repo) && !repo.Info.IsHeadUnborn) { - // (With an unborn HEAD there is no commit to complete: the first checkout - // was cut off before its branch existed, and it starts over below.) - CompletePendingCheckout(repo, ct); - return; + ///////////////////////////////////////////////////////////////////// + // With an unborn HEAD there is no commit to complete: the first // + // checkout was cut off before its branch existed, and starts over // + // below. // + ///////////////////////////////////////////////////////////////////// + FinishOrReleaseAPendingCheckout(repo, ct); } if(repo.Info.IsHeadDetached) @@ -430,7 +447,13 @@ public sealed class LibGit2GitClient : IGitClient $"'{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.", ex); } - catch(CheckoutConflictException ex) when(!File.Exists(Path.Combine(repo.Info.Path, "index"))) + catch(CheckoutConflictException ex) when(File.Exists(Path.Combine(repo.Info.Path, "index"))) + { + throw new InvalidOperationException( + "The update would replace files in this folder that were changed or added here, so nothing was updated " + + $"({ex.Message}). Commit, move or delete them ('git status' lists them), then sync again.", ex); + } + catch(CheckoutConflictException ex) { /////////////////////////////////////////////////////////////////////////// // Without an index git cannot tell which files are unchanged, takes // @@ -692,13 +715,13 @@ public sealed class LibGit2GitClient : IGitClient /// /// /// - /// Which branch: the one HEAD names when origin has it. When origin does not, - /// and this repository has no branch of its own at all (so it is a clone that - /// never had a commit, not a clone in which somebody started a new branch), HEAD - /// still names the branch a new repository starts with here, which need not be - /// origin's: origin is asked for its default branch, and that one is checked - /// out. A repository that has local branches is left as it is: an unborn HEAD - /// there is a branch a person started ("git switch --orphan"), and is theirs. + /// Only a repository without any branch of its own is treated as an empty + /// clone; one that has local branches is left as it is. Which branch is then + /// checked out: the one HEAD names when origin has it. When origin does not, and + /// the folder holds nothing but its .git (nobody has put anything into it), + /// HEAD still names the branch a new repository starts with here, which need not + /// be origin's: origin is asked for its default branch, and that one is checked + /// out. A folder that somebody has put files into keeps the branch HEAD names. /// /// /// Nothing changes before everything was checked: the branch to check out, the @@ -717,20 +740,33 @@ public sealed class LibGit2GitClient : IGitClient } string branchName = targetRef[Prefix.Length..]; + + ///////////////////////////////////////////////////////////////////////////////// + // A repository with branches of its own is not an empty clone. Its unborn // + // HEAD is a branch a person started ("git switch --orphan"), and it is // + // theirs, whatever origin has under that name. (With files staged it is the // + // refusal further down that says so.) // + ///////////////////////////////////////////////////////////////////////////////// + bool hasBranchesOfItsOwn = repo.Branches.Any(b => !b.IsRemote); Branch remoteBranch = repo.Branches["origin/" + branchName]; bool followOriginsDefault = false; if(remoteBranch?.Tip is null) { - if(repo.Index.Count > 0 || !repo.Branches.Any(b => b.IsRemote) || repo.Branches.Any(b => !b.IsRemote)) + if(hasBranchesOfItsOwn || repo.Index.Count > 0 || !repo.Branches.Any(b => b.IsRemote) || !HoldsNothingButItsGitFolder(repo)) { - // Files are staged or a local branch exists (a branch somebody started - // here by hand), or origin is still empty: nothing to pull, and - // nothing to ask origin about. + ///////////////////////////////////////////////////////////////////////// + // Origin is still empty; or this is not the untouched folder of an // + // empty clone: it has branches, files are staged, or something was // + // put into it. Then the branch HEAD names may be one somebody chose, // + // and it is left. // + ///////////////////////////////////////////////////////////////////////// return; } - // A provider of its own: the fetch already spent one of the two requests - // the replay guard allows. + ///////////////////////////////////////////////////////////////////////////// + // A provider of its own: the fetch already spent one of the two requests // + // the replay guard allows. // + ///////////////////////////////////////////////////////////////////////////// string? defaultRef = RemoteDefaultBranch(repo, url, MakeCredentialsProvider(token, url)); Branch? defaultBranch = defaultRef is null ? null : repo.Branches["origin/" + defaultRef[Prefix.Length..]]; if(defaultBranch?.Tip is null) @@ -750,6 +786,11 @@ public sealed class LibGit2GitClient : IGitClient // the branch it was started from. Checking out over that, with force, would // // replace someone's files. // /////////////////////////////////////////////////////////////////////////////// + if(hasBranchesOfItsOwn && repo.Index.Count == 0) + { + return; + } + if(repo.Index.Count > 0) { throw new InvalidOperationException( @@ -791,8 +832,10 @@ public sealed class LibGit2GitClient : IGitClient ct.ThrowIfCancellationRequested(); try { - // Force: nothing that is not gclo's is in the way (checked above), and - // nothing was ever checked out here, so there is no state to merge with. + //////////////////////////////////////////////////////////////////////////// + // Force: nothing that is not gclo's is in the way (checked above), and // + // nothing was ever checked out here, so there is no state to merge. // + //////////////////////////////////////////////////////////////////////////// Commands.Checkout(repo, local, new CheckoutOptions { CheckoutModifiers = CheckoutModifiers.Force, @@ -810,6 +853,13 @@ public sealed class LibGit2GitClient : IGitClient + /// True when the working directory holds nothing but the repository's own .git. + private static bool HoldsNothingButItsGitFolder(Repository repo) + => Directory.EnumerateFileSystemEntries(WorkingDirectoryOf(repo)) + .All(entry => string.Equals(Path.GetFileName(entry), ".git", StringComparison.OrdinalIgnoreCase)); + + + /// /// Before a forced checkout into a folder whose tree gclo has not finished writing: /// refuses, with nothing changed, when something that is not gclo's is where the @@ -817,30 +867,81 @@ public sealed class LibGit2GitClient : IGitClient /// replaces all of those without asking (#174). /// /// + /// /// The rules are the ones the path recovery follows - /// (): a link on the way, a file where a - /// folder is needed, a folder with files where a file is needed, and a file at a - /// destination that is not what the current commit has there. With - /// a file that is the beginning of what the - /// current commit has also counts as gclo's: a checkout that was cut off leaves - /// the file it was writing short, and completing it must not stop at that file. - /// A submodule's place must be empty: the checkout makes an empty folder of it. + /// (), and a few it has no need for: a link + /// on the way or at a destination, a file where a folder is needed (also on the way + /// to a submodule), a folder with anything in it where a file is needed, a file at + /// a destination that is not what the current commit has there, and anything but an + /// empty folder where the repository has a submodule (the checkout makes an empty + /// folder of it). + /// + /// + /// With the checkout being completed has + /// written files already. One of them at a destination is recognised by holding + /// what the current commit has for that path, or the beginning of it (the file it + /// was writing when it was cut off), line endings aside: the checkout converts + /// them by rules that the tree itself brings, and those were not all on disk yet + /// when the first files were written. + /// + /// + /// Windows only. The rules about names and links are Windows' rules; elsewhere the + /// checkout is done as it always was. + /// /// private static void RefuseToWriteOverWhatIsNotOurs(Repository repo, Tree incoming, bool aCutOffWriteIsOurs) { + if(!OperatingSystem.IsWindows()) + { + return; + } + string workdir = WorkingDirectoryOf(repo); List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(incoming, s_noRecovery); var effectiveNow = new HashSet(entries.Select(entry => entry.EffectivePath), StringComparer.Ordinal); RefuseWhatCannotBeWritten(repo, workdir, entries, ours: null, effectiveNow, replaceOnlyWhatTheCommitHas: true, aCutOffWriteIsOurs); + /////////////////////////////////////////////////////////////////////////////// + // What that check lets through because the recovery writes file by file and // + // a forced checkout does not: a link at a destination (replaced by the // + // file), and a folder there that holds no file but something else (links, // + // empty folders), which the checkout removes with what is in it. // + /////////////////////////////////////////////////////////////////////////////// + foreach((_, string effectivePath, _) in entries) + { + string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); + if(IsLink(full)) + { + throw new InvalidOperationException( + $"'{effectivePath}' is a link in this folder, where the repository has a file. Move or delete the link, then try again."); + } + if(Directory.Exists(full) && Directory.EnumerateFileSystemEntries(full).Any()) + { + throw new InvalidOperationException( + $"'{effectivePath}' is a folder that is not empty, where the repository has a file. Move or delete the folder, then try again."); + } + } + + var checkedFolders = new HashSet(StringComparer.Ordinal); foreach(string submodule in SubmodulePaths(incoming)) { WorkingTree.Verify(workdir, submodule); + for(int slash = submodule.IndexOf('/', StringComparison.Ordinal); slash > 0; slash = submodule.IndexOf('/', slash + 1)) + { + string folder = submodule[..slash]; + if(checkedFolders.Add(folder) && 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."); + } + } + string full = Path.Combine(workdir, submodule.Replace('/', Path.DirectorySeparatorChar)); - if(File.Exists(full) || (Directory.Exists(full) && Directory.EnumerateFileSystemEntries(full).Any())) + if(IsLink(full) || File.Exists(full) || (Directory.Exists(full) && Directory.EnumerateFileSystemEntries(full).Any())) { throw new InvalidOperationException( - $"'{submodule}' is already in this folder, where the repository has a submodule, and it is not empty. " + $"'{submodule}' is already in this folder, where the repository has a submodule, and it is not an empty folder. " + "Move or delete it, then try again."); } } @@ -848,6 +949,21 @@ public sealed class LibGit2GitClient : IGitClient + /// True when something is at and it is a link (a junction, a symbolic link). + private static bool IsLink(string path) + { + try + { + return File.GetAttributes(path).HasFlag(FileAttributes.ReparsePoint); + } + catch(IOException) + { + return false; // nothing is there + } + } + + + /// The paths of the submodule entries (gitlinks) of a tree, at any depth. private static List SubmodulePaths(Tree tree) { @@ -1322,8 +1438,10 @@ 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. - /// With , also true when the file holds the - /// beginning of that and nothing else: what a write that was cut off leaves. + /// With the question is whether an unfinished + /// checkout wrote the file: then it holds the commit's content for the path, or + /// the beginning of it, line endings aside + /// (). /// private static bool IsWhatTheCommitHas(string full, string originalPath, Tree? commitTree, bool orItsBeginning = false) { @@ -1332,23 +1450,24 @@ public sealed class LibGit2GitClient : IGitClient return false; } - using Stream expected = blob.GetContentStream(new FilteringOptions(originalPath)); using var actual = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + if(orItsBeginning) + { + using Stream stored = blob.GetContentStream(); + return HoldsTheBeginningOf(stored, actual); + } + + using Stream expected = blob.GetContentStream(new FilteringOptions(originalPath)); 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); - int common = Math.Min(expectedRead, actualRead); - if(actualRead > expectedRead || !expectedBuffer.AsSpan(0, common).SequenceEqual(actualBuffer.AsSpan(0, common))) + if(expectedRead != actualRead || !expectedBuffer.AsSpan(0, expectedRead).SequenceEqual(actualBuffer.AsSpan(0, actualRead))) { return false; } - if(actualRead < expectedRead) - { - return orItsBeginning; // the file ends where the commit's goes on - } if(expectedRead == 0) { return true; @@ -1358,6 +1477,62 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// True when holds what holds, + /// or the beginning of it and nothing else, carriage returns aside: a file a + /// checkout wrote, or was writing when it was cut off. Carriage returns are left + /// out on both sides because a checkout converts line endings by the repository's + /// attributes and the user's configuration, and a text that differs from the + /// commit's only in its line endings is the commit's text. + /// + private static bool HoldsTheBeginningOf(Stream expected, Stream actual) + { + const byte CarriageReturn = 13; + byte[] expectedBuffer = new byte[81920]; + byte[] actualBuffer = new byte[81920]; + int expectedCount = 0; + int expectedAt = 0; + while(true) + { + int read = actual.Read(actualBuffer, 0, actualBuffer.Length); + if(read == 0) + { + return true; // the file has ended: all of it was the commit's, from its start + } + + for(int i = 0; i < read; i++) + { + if(actualBuffer[i] == CarriageReturn) + { + continue; + } + + byte next; + do + { + if(expectedAt == expectedCount) + { + expectedCount = expected.Read(expectedBuffer, 0, expectedBuffer.Length); + expectedAt = 0; + if(expectedCount == 0) + { + return false; // the file goes on where the commit's has ended + } + } + next = expectedBuffer[expectedAt++]; + } + while(next == CarriageReturn); + + if(next != actualBuffer[i]) + { + return false; + } + } + } + } + + + /// /// Overlays onto . The newer /// choice for a path wins whatever kind it is: an incoming rename replaces a stored @@ -1448,58 +1623,97 @@ public sealed class LibGit2GitClient : IGitClient /// - /// Finishes a clone whose checkout never completed and that has no stored - /// recovery: moves to the upstream tip, re-validates its paths on Windows, and - /// force-checks-out the whole tree. If the paths are still invalid, rethrows the - /// same typed failure — the repo must not silently report success with an empty - /// or partial tree. + /// True when the commit a pull would move this branch to has paths Windows cannot + /// create. False when there is no such commit to move to (histories that diverged + /// are the ordinary pull's to report). + /// + private static bool OriginHasPathsWindowsCannotCreate(Repository repo) + { + Commit? upstream = repo.Head.TrackedBranch?.Tip; + return upstream is not null + && !string.Equals(upstream.Sha, repo.Head.Tip.Sha, StringComparison.Ordinal) + && WindowsPathValidator.Validate(upstream.Tree).Count > 0; + } + + + + /// + /// What a marked folder needs before the ordinary pull: its checkout finished, or + /// only its marker taken off. /// /// - /// What the unfinished checkout left is overwritten: its files, and the one it - /// was writing when it was cut off. Anything else at a destination was put there - /// since, by somebody, and stops the completion before the branch has moved - /// (, #174). "What the unfinished - /// checkout left" is judged against the commit the branch is on, which is the - /// one that checkout was writing; that is why the check comes before the move. + /// git writes the index when a checkout has reached its end. A marked folder + /// without an index is therefore a tree that was never finished: it is completed, + /// with force, at the commit its branch is on + /// (). A marked folder WITH an index is one + /// whose checkout did finish (gclo's own, whose marker could not be cleared any + /// more, or one a person made with git): its tree is git's to update, and the + /// ordinary pull does that without replacing anything that was changed. Either + /// way the pull that follows brings what origin has got since. + /// + private static void FinishOrReleaseAPendingCheckout(Repository repo, CancellationToken ct) + { + if(repo.Index.Count == 0) + { + CompletePendingCheckout(repo, ct); + } + else + { + repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); + } + } + + + + /// + /// Finishes a clone whose checkout never completed and that has no stored + /// recovery: force-checks-out the commit the branch is on, re-validating its paths + /// on Windows. If the paths are invalid, rethrows the typed failure — the repo must + /// not silently report success with an empty or partial tree. + /// + /// + /// + /// The branch is not moved first: every attempt to finish writes the same commit, + /// so what an earlier attempt left on disk can be told from what somebody put + /// there since (, #174). What origin + /// has got meanwhile is brought by the ordinary pull that follows, over a tree + /// that then has an index. + /// + /// + /// The exception is a tree Windows cannot write: the commit the branch is on, or + /// the one origin has now. Then the branch moves to origin's first, as it always + /// did, and nothing is written here: if origin renamed what was in the way the + /// checkout goes ahead at its commit, and if not, the failure names the paths of + /// the newest tree, which is the one the recovery that follows will write. + /// /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { - // Paths Windows cannot create are reported as such further down, once the - // branch has moved (the recovery that follows works on the newest tree); a - // tree with such paths is not written here at all, so nothing is in its way. - Tree incoming = TargetOf(repo).Tree; - if(!OperatingSystem.IsWindows() || WindowsPathValidator.Validate(incoming).Count == 0) + if(OperatingSystem.IsWindows() && (HasPathsWindowsCannotCreate(repo) || OriginHasPathsWindowsCannotCreate(repo))) { - RefuseToWriteOverWhatIsNotOurs(repo, incoming, aCutOffWriteIsOurs: true); + AdvanceHeadToTrackedTip(repo); + IReadOnlyList invalidPaths = WindowsPathValidator.Validate(repo.Head.Tip.Tree); + if(invalidPaths.Count > 0) + { + throw new InvalidRepositoryPathsException(invalidPaths); + } } - AdvanceHeadToTrackedTip(repo); - Commit? tip = repo.Head.Tip; - if(tip is not null) + RefuseToWriteOverWhatIsNotOurs(repo, repo.Head.Tip.Tree, aCutOffWriteIsOurs: true); + + ct.ThrowIfCancellationRequested(); + try { - if(OperatingSystem.IsWindows()) + Commands.Checkout(repo, repo.Head, new CheckoutOptions { - IReadOnlyList invalidPaths = WindowsPathValidator.Validate(tip.Tree); - if(invalidPaths.Count > 0) - { - throw new InvalidRepositoryPathsException(invalidPaths); - } - } - - ct.ThrowIfCancellationRequested(); - try - { - Commands.Checkout(repo, repo.Head, new CheckoutOptions - { - CheckoutModifiers = CheckoutModifiers.Force, // nothing was ever checked out - CheckoutNotifyFlags = CheckoutNotifyFlags.Updated, - OnCheckoutNotify = (_, _) => !ct.IsCancellationRequested, - }); - } - catch(UserCancelledException) when(ct.IsCancellationRequested) - { - throw new OperationCanceledException(ct); - } + CheckoutModifiers = CheckoutModifiers.Force, // what is in the way is the unfinished checkout's own (checked above) + CheckoutNotifyFlags = CheckoutNotifyFlags.Updated, + OnCheckoutNotify = (_, _) => !ct.IsCancellationRequested, + }); + } + catch(UserCancelledException) when(ct.IsCancellationRequested) + { + throw new OperationCanceledException(ct); } repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); diff --git a/gclo.Engine/WslCloner.cs b/gclo.Engine/WslCloner.cs index 45afe43..ef810ac 100644 --- a/gclo.Engine/WslCloner.cs +++ b/gclo.Engine/WslCloner.cs @@ -32,12 +32,17 @@ public sealed class WslCloner : IWslCloner /// without having moved it, also when it is ended by a signal; what a killed /// script left there is removed by a later run once it is a day old (not sooner: /// another gclo may be cloning right now). A destination that appears while the - /// clone runs is not moved into: the clone would end up inside it. + /// clone runs is not moved into: the clone would end up inside it (checked before + /// the move, and the move itself is mv -T, which renames onto the name and + /// never into a folder of that name). /// The credential helper is /// scoped to $4, so git (and anything it starts, such as an LFS filter pointed at /// another server by the repository's own configuration) gets the token for that /// host only (#139). /// + /// The folder under ~/gclo in which clones are made before they are moved into place. + internal const string TemporaryFolderName = ".gclo-tmp"; + internal const string CloneScript = "set -e\n" + "org=\"$1\"; repo=\"$2\"; url=\"$3\"; host=\"$4\"\n" @@ -67,7 +72,7 @@ public sealed class WslCloner : IWslCloner + " echo \"$dest appeared while the repository was being cloned (another gclo?); nothing was put there\" >&2\n" + " exit 1\n" + " fi\n" - + " mv \"$tmp\" \"$dest\"\n" + + " mv -T \"$tmp\" \"$dest\"\n" + " trap - EXIT HUP INT TERM\n" + "fi\n" + "printf '%s\\n%s\\n' \"$WSL_DISTRO_NAME\" \"$dest\"\n"; @@ -149,6 +154,14 @@ public sealed class WslCloner : IWslCloner throw new WslCloneException($"Clone in WSL refused: '{name}' cannot be used as a folder name ({reason})."); } } + if(string.Equals(organization, TemporaryFolderName, StringComparison.OrdinalIgnoreCase)) + { + //////////////////////////////////////////////////////////////////////// + // ~/gclo/.gclo-tmp is where unfinished clones are made, and old ones // + // there are removed. // + //////////////////////////////////////////////////////////////////////// + throw new WslCloneException($"Clone in WSL refused: '{organization}' is the name of gclo's own temporary folder there."); + } string host = RemoteUrl.HttpsAuthority(url) ?? throw new WslCloneException("Clone in WSL refused: the clone URL is not an HTTPS URL."); -- 2.54.0 From 41f3712a8a6d6ba3117135b94f7b48c13070905e Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 22:30:40 -0600 Subject: [PATCH 4/5] fix(clone): third review of #175: the cases in which an unfinished clone was refused where it used to be completed The third review confirmed, under real process kills, that a checkout that was cut off leaves no index, and found no way left to replace or delete a user's file. It found five narrow cases in which gclo refused, failed or said done where it used to complete the tree. - When the branch of an unfinished checkout must move before it is written (a tree Windows cannot write, or a history origin rewrote), the commit it leaves is recorded, and its files on disk are recognised by it as well. A history origin rewrote is followed again, as on main. - A file is the unfinished checkout's own when it holds what the commit stores or what the checkout filters make of it (the 'ident' attribute). - Where the repository has a symbolic link, a link in the folder is the checkout's own. - A marked folder is taken for finished only when its index and its files hold the whole tree of its commit: an index somebody's git wrote over an unfinished tree does not make it whole. - A link where the repository has a submodule is refused; a link is something with a target, not anything with a reparse point. - Tests for each, for a file stored with carriage returns, for a completion that fails half-way, and for the order of what following origin's default branch writes. The message of an ordinary update that would replace files no longer says who changed them. Refs #174 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/security/threat-model.md | 2 +- gclo.Engine.Tests/InterruptedCloneTests.cs | 342 ++++++++++++++++++++- gclo.Engine/LibGit2GitClient.cs | 254 +++++++++++---- gclo.Engine/WslCloner.cs | 6 +- 5 files changed, 537 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index cf082c3..0c220b8 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, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. -**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. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder; a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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`). +**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. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder (otherwise it keeps the branch its HEAD names, and stays empty until origin has that branch); a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). If you have worked in an unfinished clone with git in the meantime, gclo finishes it only when every file there is still the repository's own. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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/security/threat-model.md b/docs/security/threat-model.md index 047dc4a..4288170 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -121,4 +121,4 @@ flowchart LR | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated: path recovery writes and deletes only through `WorkingTree` (inside the working directory, never in `.git`, never through a link); `.git` and its Windows aliases are invalid path segments; the stored recovery file's paths are validated on load; cleanup of a failed clone does not follow links (#141) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T15 added: text from repositories and servers is shown with control, separator and format characters made visible, in the activity log and in everything the CLI prints as text, including characters a legacy console code page would turn into control bytes; the desktop app's surfaces are named as not covered (#143) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | The project's home moved to `git.kof22.com`: the Help menu, the About dialog, the CLI's error text and the documentation link there, and the Scoop bucket is added from there (the manifest that pins the download URL and its hash now comes from that server over HTTPS; T2 is unchanged in kind). Help > Share hands a fixed text (what gclo is, where to download it, the sender's gclo version) to the system's mail handler as a `mailto:` link with no recipient, or shows it and copies it to the clipboard; nothing about the user, their accounts, or their repositories is in it, and the address it is built from must be a plain http(s) host and path (#163, #165). The update feed and the release downloads named in package manifests are still GitHub Releases at this revision. | -| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on). On Windows, a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) is preceded by refusals that change nothing: a link on the way or at a destination, a file where a folder is needed (also on the way to a submodule), a folder that is not empty where a file is needed, a file at a destination that is not what the unfinished checkout wrote there (the commit's content or the beginning of it, carriage returns aside), and anything but an empty folder where the repository has a submodule. An unfinished checkout is recognised by its marker and the absence of an index; a marked folder that has an index is updated like any clone, without force. It is completed at the commit its branch is on, before the branch moves. A repository that has local branches is not treated as an empty clone; one without follows origin's default branch only when its folder holds nothing but `.git`. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes, and not across spellings of a path that only the file system can resolve). The WSL clone is made in `~/gclo/.gclo-tmp` and renamed into place when complete (`mv -T`, after a check that nothing is there). Not covered: a file created in the folder between the check and the checkout that follows it; an empty or identical file of the user's at a destination, which is replaced; platforms other than Windows, where the checkout is forced as before. | +| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on). On Windows, a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) is preceded by refusals that change nothing: a link on the way or at a destination (except where the repository itself has a symbolic link), a file where a folder is needed (also on the way to a submodule), a folder that is not empty where a file is needed, a file at a destination that is not what the unfinished checkout wrote there, and anything but an empty folder where the repository has a submodule. "What the unfinished checkout wrote" is the content, or the beginning of it, of the commit the branch is on or of a commit recorded in `gclo.checkoutwrittenbefore`, as stored or as the checkout filters make it, carriage returns aside. An unfinished checkout is a marked folder that has no index, or whose index and files do not hold the whole tree of its commit; a marked folder that holds it is updated like any clone, without force. An unfinished checkout is completed at the commit its branch is on, before the branch moves, except where the branch must move first (a tree Windows cannot write, or a history origin rewrote); the commit it leaves is then recorded. A repository that has local branches is not treated as an empty clone; one without follows origin's default branch only when its folder holds nothing but `.git`. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes, and not across spellings of a path that only the file system can resolve). The WSL clone is made in `~/gclo/.gclo-tmp` and renamed into place when complete (`mv -T`, after a check that nothing is there). Not covered: a file created in the folder between the check and the checkout that follows it; a file of the user's at a destination that is empty, or holds the repository's content or the beginning of it, which is replaced; a link of the user's where the repository has a symbolic link, which is replaced (what it leads to is not touched); an unfinished clone in which a folder of an earlier commit became a file, or that was cut off by a power loss leaving files of zeros, which is refused until the folder is cleared; platforms other than Windows, where the checkout is forced as before. | diff --git a/gclo.Engine.Tests/InterruptedCloneTests.cs b/gclo.Engine.Tests/InterruptedCloneTests.cs index 6448883..43130ce 100644 --- a/gclo.Engine.Tests/InterruptedCloneTests.cs +++ b/gclo.Engine.Tests/InterruptedCloneTests.cs @@ -394,7 +394,7 @@ public sealed class InterruptedCloneTests : IDisposable // The checkout that was cut off is finished at its own commit, which has no // such file; what origin got since is an ordinary update, and that one does // not replace a file that was added here. - Assert.Contains("The update would replace files in this folder that were changed or added here", refused.Message, StringComparison.Ordinal); + Assert.Contains("The update would replace files in this folder that are not as git recorded them", refused.Message, StringComparison.Ordinal); Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "notes.txt"))); Assert.Equal("theirs", File.ReadAllText(Path.Combine(clone, "readme.txt"))); Assert.False(IsCheckoutPending(clone)); @@ -904,4 +904,344 @@ public sealed class InterruptedCloneTests : IDisposable Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); Assert.Equal("mine", File.ReadAllText(Path.Combine(clone, "page.html"))); } + + + + // ---------------------------------------------------------------- review of #175, third round + + private const string LF = "\n"; + + private const string CRLF = "\r\n"; + + + + /// + /// Adds a commit to the origin itself (no working copy in between), on the branch + /// its HEAD names: the tree of the commit before it, changed by + /// ; with a commit without + /// a parent, which is what a rewritten history looks like from a clone. + /// + private static string CommitToOrigin(string origin, Func change, bool asANewRoot = false) + { + using var repo = new Repository(origin); + Commit? before = repo.Head.Tip; + TreeDefinition definition = change(repo, before is null || asANewRoot ? new TreeDefinition() : TreeDefinition.From(before.Tree)); + Signature who = MakeSignature(); + Commit commit = repo.ObjectDatabase.CreateCommit( + who, who, "written into origin", repo.ObjectDatabase.CreateTree(definition), + before is null || asANewRoot ? [] : [before], prettifyMessage: false); + if(before is null) + { + repo.Refs.Add(repo.Refs.Head.TargetIdentifier, commit.Id); + } + else + { + repo.Refs.UpdateTarget(repo.Refs.Head.ResolveToDirectReference(), commit.Id); + } + return commit.Sha; + } + + + + private static Blob BlobOf(Repository repo, string content) + => repo.ObjectDatabase.CreateBlob(new MemoryStream(System.Text.Encoding.UTF8.GetBytes(content))); + + + + private static string? WrittenBefore(string workdir) + { + using var repo = new Repository(workdir); + return repo.Config.Get("gclo.checkoutwrittenbefore", ConfigurationLevel.Local)?.Value; + } + + + + [Fact] + public async Task OriginGetsAPathWindowsCannotCreate_AndLosesItAgain_TheUnfinishedCloneIsStillCompleted() + { + if(!OperatingSystem.IsWindows()) + { + return; // the paths are Windows' to refuse + } + (string origin, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one"), ("b.txt", "b")); + File.WriteAllText(Path.Combine(clone, "a.txt"), "one"); // what the cut-off checkout had written + File.WriteAllText(Path.Combine(clone, "b.txt"), "b"); + string first = HeadSha(clone); + + CommitToOrigin(origin, (repo, tree) => tree.Add("bad:name.txt", BlobOf(repo, "bad"), Mode.NonExecutableFile)); + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + Assert.True(IsCheckoutPending(clone)); + Assert.Equal(first, WrittenBefore(clone)); // the branch moved on; the commit whose files are on disk is remembered + + string third = CommitToOrigin( + origin, (repo, tree) => tree.Remove("bad:name.txt").Add("a.txt", BlobOf(repo, "two"), Mode.NonExecutableFile)); + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal(third, HeadSha(clone)); + Assert.Equal("two", File.ReadAllText(Path.Combine(clone, "a.txt"))); + Assert.Equal("b", File.ReadAllText(Path.Combine(clone, "b.txt"))); + Assert.False(IsCheckoutPending(clone)); + Assert.Null(WrittenBefore(clone)); + } + + + + [Fact] + public async Task OriginRewritesItsHistory_BeforeAnUnfinishedCloneIsSyncedAgain_AndTheCloneFollowsIt() + { + (string origin, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one"), ("b.txt", "b")); + File.WriteAllText(Path.Combine(clone, "a.txt"), "one"); + string rewritten = CommitToOrigin( + origin, (repo, tree) => tree.Add("a.txt", BlobOf(repo, "rewritten"), Mode.NonExecutableFile), asANewRoot: true); + + await _client.PullAsync(clone, Token, CancellationToken.None); // nobody committed here: nothing to push or reset + + Assert.Equal(rewritten, HeadSha(clone)); + Assert.Equal("rewritten", File.ReadAllText(Path.Combine(clone, "a.txt"))); + Assert.False(File.Exists(Path.Combine(clone, "b.txt"))); + Assert.False(IsCheckoutPending(clone)); + Assert.Null(WrittenBefore(clone)); + } + + + + /// + /// A clone into a folder that exists, whose checkout fails when it gets to + /// zzz.txt because another program has that file open: the files before it + /// are written, the marker is set, and there is no index. + /// + private async Task ACloneWhoseCheckoutFailedHalfWayAsync(string origin, string name) + { + string target = Directory.CreateDirectory(NewPath(name)).FullName; + FileStream? held = null; + _client.AfterFetchForTesting = () => + held = new FileStream(Path.Combine(target, "zzz.txt"), FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None); + try + { + await Assert.ThrowsAnyAsync(() => _client.CloneAsync(origin, target, Token, null, CancellationToken.None)); + } + finally + { + held?.Dispose(); + _client.AfterFetchForTesting = null; + } + Assert.True(IsCheckoutPending(target)); + return target; + } + + + + [Fact] + public async Task AFileTheCheckoutWroteAnObjectIdInto_IsStillItsOwn() + { + // The 'ident' attribute: what is written is not what is stored. + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add(".gitattributes", BlobOf(repo, "*.txt ident" + LF), Mode.NonExecutableFile) + .Add("a.txt", BlobOf(repo, "$Id$" + LF + "hello" + LF), Mode.NonExecutableFile) + .Add("zzz.txt", BlobOf(repo, "last" + LF), Mode.NonExecutableFile)); + string target = await ACloneWhoseCheckoutFailedHalfWayAsync(origin, "ident"); + Assert.Contains("$Id: ", File.ReadAllText(Path.Combine(target, "a.txt")), StringComparison.Ordinal); + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.False(IsCheckoutPending(target)); + Assert.StartsWith("last", File.ReadAllText(Path.Combine(target, "zzz.txt")), StringComparison.Ordinal); + } + + + + [Theory] + [InlineData(false)] // 'git reset': the index names every file, the folder does not hold them all + [InlineData(true)] // 'git add -A': the index names what is in the folder + public async Task AnIndexSomebodysGitWrote_OverAnUnfinishedTree_DoesNotMakeTheTreeWhole(bool onlyWhatIsThere) + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("a.txt", BlobOf(repo, "a"), Mode.NonExecutableFile) + .Add("b.txt", BlobOf(repo, "b"), Mode.NonExecutableFile) + .Add("zzz.txt", BlobOf(repo, "last"), Mode.NonExecutableFile)); + string target = await ACloneWhoseCheckoutFailedHalfWayAsync(origin, "then-git"); + File.Delete(Path.Combine(target, "zzz.txt")); + using(var repo = new Repository(target)) + { + if(onlyWhatIsThere) + { + Commands.Stage(repo, "*"); + } + else + { + repo.Index.Replace(repo.Head.Tip); + repo.Index.Write(); + } + Assert.True(repo.Index.Count > 0); + } + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zzz.txt"))); // completed, not released with a file missing + Assert.False(IsCheckoutPending(target)); + } + + + + [Theory] + [InlineData("one" + LF + "tw")] // written with other line endings, and cut off + [InlineData("one" + CRLF + "two" + CRLF)] // written as it is stored + public async Task AFileStoredWithCarriageReturns_IsRecognisedTheSameWay(string onDisk) + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree.Add("crlf.txt", BlobOf(repo, "one" + CRLF + "two" + CRLF), Mode.NonExecutableFile)); + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + File.WriteAllText(Path.Combine(clone, "crlf.txt"), onDisk); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal(["one", "two"], File.ReadAllLines(Path.Combine(clone, "crlf.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task ACompletionThatFailsHalfWay_LeavesTheFolderMarked() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("a.txt", BlobOf(repo, "a"), Mode.NonExecutableFile) + .Add("zzz.txt", BlobOf(repo, "last"), Mode.NonExecutableFile)); + string target = await ACloneWhoseCheckoutFailedHalfWayAsync(origin, "twice"); + + // The next sync is the completion, and it fails at the same file: it can be + // read (so it is seen to be the checkout's own), not written. + using(var held = new FileStream(Path.Combine(target, "zzz.txt"), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + await Assert.ThrowsAnyAsync(() => _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + } + Assert.True(IsCheckoutPending(target)); // not taken off before the tree is written + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zzz.txt"))); + Assert.False(IsCheckoutPending(target)); + } + + + + [Fact] + public async Task ALinkWhereTheRepositoryHasASymbolicLink_IsTakenForTheCheckoutsOwn_AndWhatItLeadsToIsNotTouched() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("readme.txt", BlobOf(repo, "theirs"), Mode.NonExecutableFile) + .Add("alink", BlobOf(repo, "readme.txt"), Mode.SymbolicLink)); + using(var repo = new Repository(clone)) + { + repo.Config.Set("core.symlinks", false, ConfigurationLevel.Local); // the link is then written as a file that names its target + } + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; + File.WriteAllText(Path.Combine(elsewhere, "kept.txt"), "mine"); + string link = Path.Combine(clone, "alink"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("readme.txt", File.ReadAllText(link)); + Assert.Equal("mine", File.ReadAllText(Path.Combine(elsewhere, "kept.txt"))); + Assert.False(IsCheckoutPending(clone)); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + [Fact] + public async Task ALinkWhereTheRepositoryHasASubmodule_IsNotReplaced() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("readme.txt", BlobOf(repo, "theirs"), Mode.NonExecutableFile) + .AddGitLink("sub", new ObjectId("1234567890123456789012345678901234567890"))); + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; // empty: only the link itself is in the way + string link = Path.Combine(clone, "sub"); + WorkingTreeTests.CreateDirectoryLink(link, elsewhere); + try + { + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.NotNull(new DirectoryInfo(link).LinkTarget); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + [Fact] + public async Task FollowingOriginsDefaultBranch_ChangesNothing_BeforeItsTreeIsKnownToBeWritable() + { + if(!OperatingSystem.IsWindows()) + { + return; // the paths are Windows' to refuse + } + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + using(var bare = new Repository(origin)) + { + bare.Refs.UpdateTarget("HEAD", "refs/heads/" + OtherBranch); + } + CommitToOrigin(origin, (repo, tree) => tree.Add("bad:name.txt", BlobOf(repo, "bad"), Mode.NonExecutableFile)); + string[] before = LocalConfiguration(clone); + string headBefore; + using(var repo = new Repository(clone)) + { + headBefore = repo.Refs.Head.TargetIdentifier; + } + + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Equal(before, LocalConfiguration(clone)); // no marker, no upstream for a branch that was not created + using var after = new Repository(clone); + Assert.Equal(headBefore, after.Refs.Head.TargetIdentifier); // and HEAD names what it named + } + + + + [Fact] + public async Task FollowingOriginsDefaultBranch_TheUpstreamIsConfigured_BeforeTheBranchExists() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + await SeedAsync(origin, OtherBranch, ("readme.txt", "from the other branch")); + string heads = Directory.CreateDirectory(Path.Combine(clone, ".git", "refs", "heads")).FullName; + string held = Path.Combine(heads, OtherBranch + ".lock"); // another git holds the lock: the branch cannot be created + File.WriteAllText(held, ""); + + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + + using(var stopped = new Repository(clone)) + { + Assert.True(stopped.Info.IsHeadUnborn); + Assert.Equal("refs/heads/" + OtherBranch, stopped.Refs.Head.TargetIdentifier); + Assert.Equal("refs/heads/" + OtherBranch, stopped.Config.Get($"branch.{OtherBranch}.merge", ConfigurationLevel.Local)?.Value); + } + Assert.True(IsCheckoutPending(clone)); + + File.Delete(held); + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("from the other branch", File.ReadAllText(Path.Combine(clone, "readme.txt"))); + Assert.False(IsCheckoutPending(clone)); + } } diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 7703e92..06c9070 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -48,6 +48,13 @@ public sealed class LibGit2GitClient : IGitClient /// private const string CheckoutPendingKey = "gclo.checkoutpending"; + /// + /// Beside the marker: the commits an unfinished checkout here was writing before + /// the branch was moved on (ids, separated by commas). Their files may still be + /// on disk, and are told from a person's by what those commits hold (#174). + /// + private const string WrittenBeforeKey = "gclo.checkoutwrittenbefore"; + /// /// Local git config flag meaning "gclo writes this repository's working tree /// itself, through a path recovery". It outlives the recovery file, so a @@ -450,8 +457,9 @@ public sealed class LibGit2GitClient : IGitClient catch(CheckoutConflictException ex) when(File.Exists(Path.Combine(repo.Info.Path, "index"))) { throw new InvalidOperationException( - "The update would replace files in this folder that were changed or added here, so nothing was updated " - + $"({ex.Message}). Commit, move or delete them ('git status' lists them), then sync again.", ex); + "The update would replace files in this folder that are not as git recorded them (changed or added here, or " + + $"left by an update that was interrupted), so nothing was updated ({ex.Message}). 'git status' lists them: " + + "commit, move or delete them, then sync again.", ex); } catch(CheckoutConflictException ex) { @@ -879,17 +887,20 @@ public sealed class LibGit2GitClient : IGitClient /// /// With the checkout being completed has /// written files already. One of them at a destination is recognised by holding - /// what the current commit has for that path, or the beginning of it (the file it - /// was writing when it was cut off), line endings aside: the checkout converts - /// them by rules that the tree itself brings, and those were not all on disk yet - /// when the first files were written. + /// what the current commit has for that path (or a commit of + /// : one the branch was on while the checkout was + /// unfinished), or the beginning of it (the file it was writing when it was cut + /// off), line endings aside: the checkout converts them by rules that the tree + /// itself brings, and those were not all on disk yet when the first files were + /// written. /// /// /// Windows only. The rules about names and links are Windows' rules; elsewhere the /// checkout is done as it always was. /// /// - private static void RefuseToWriteOverWhatIsNotOurs(Repository repo, Tree incoming, bool aCutOffWriteIsOurs) + private static void RefuseToWriteOverWhatIsNotOurs( + Repository repo, Tree incoming, bool aCutOffWriteIsOurs, IReadOnlyList? writtenBefore = null) { if(!OperatingSystem.IsWindows()) { @@ -898,8 +909,19 @@ public sealed class LibGit2GitClient : IGitClient string workdir = WorkingDirectoryOf(repo); List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(incoming, s_noRecovery); + + /////////////////////////////////////////////////////////////////////////////// + // Where the repository has a symbolic link, a link in the folder is what a // + // checkout writes there (when the user's configuration lets it): it is not // + // looked at further. Replacing a link never touches what it leads to. // + /////////////////////////////////////////////////////////////////////////////// + entries.RemoveAll(entry => + incoming[entry.OriginalPath]?.Mode == Mode.SymbolicLink + && IsLink(Path.Combine(workdir, entry.EffectivePath.Replace('/', Path.DirectorySeparatorChar)))); + var effectiveNow = new HashSet(entries.Select(entry => entry.EffectivePath), StringComparer.Ordinal); - RefuseWhatCannotBeWritten(repo, workdir, entries, ours: null, effectiveNow, replaceOnlyWhatTheCommitHas: true, aCutOffWriteIsOurs); + RefuseWhatCannotBeWritten( + repo, workdir, entries, ours: null, effectiveNow, replaceOnlyWhatTheCommitHas: true, aCutOffWriteIsOurs, writtenBefore); /////////////////////////////////////////////////////////////////////////////// // What that check lets through because the recovery writes file by file and // @@ -949,18 +971,13 @@ public sealed class LibGit2GitClient : IGitClient - /// True when something is at and it is a link (a junction, a symbolic link). + /// + /// True when something is at and it is a link (a junction, a + /// symbolic link): recognised by having a target, as does, + /// not by the reparse-point attribute, which other things carry too. + /// private static bool IsLink(string path) - { - try - { - return File.GetAttributes(path).HasFlag(FileAttributes.ReparsePoint); - } - catch(IOException) - { - return false; // nothing is there - } - } + => new FileInfo(path).LinkTarget is not null || new DirectoryInfo(path).LinkTarget is not null; @@ -1370,7 +1387,8 @@ public sealed class LibGit2GitClient : IGitClient HashSet? ours, HashSet effectiveNow, bool replaceOnlyWhatTheCommitHas, - bool aCutOffWriteIsOurs = false) + bool aCutOffWriteIsOurs = false, + IReadOnlyList? writtenBefore = null) { if(File.Exists(Path.Combine(repo.Info.Path, "index.lock"))) { @@ -1392,7 +1410,10 @@ public sealed class LibGit2GitClient : IGitClient WorkingTree.Verify(workdir, effectivePath, verifiedFolders); string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); - if(replaceOnlyWhatTheCommitHas && File.Exists(full) && !IsWhatTheCommitHas(full, originalPath, current, aCutOffWriteIsOurs)) + if(replaceOnlyWhatTheCommitHas + && File.Exists(full) + && !IsWhatTheCommitHas(full, originalPath, current, aCutOffWriteIsOurs) + && !(writtenBefore ?? []).Any(earlier => IsWhatTheCommitHas(full, originalPath, earlier, aCutOffWriteIsOurs))) { throw new InvalidOperationException( $"'{effectivePath}' is already in this folder, and it is not a file gclo has a record of writing " @@ -1450,13 +1471,28 @@ public sealed class LibGit2GitClient : IGitClient return false; } - using var actual = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); if(orItsBeginning) { - using Stream stored = blob.GetContentStream(); - return HoldsTheBeginningOf(stored, actual); + ///////////////////////////////////////////////////////////////////////////// + // What is stored, or what the filters make of it on the way out (the // + // "ident" attribute writes the object's id into the file): either is // + // what a checkout writes, depending on which rules were on disk then. // + ///////////////////////////////////////////////////////////////////////////// + using(Stream stored = blob.GetContentStream()) + using(var written = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) + { + if(HoldsTheBeginningOf(stored, written)) + { + return true; + } + } + + using Stream filtered = blob.GetContentStream(new FilteringOptions(originalPath)); + using var again = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + return HoldsTheBeginningOf(filtered, again); } + using var actual = new FileStream(full, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); using Stream expected = blob.GetContentStream(new FilteringOptions(originalPath)); byte[] expectedBuffer = new byte[81920]; byte[] actualBuffer = new byte[81920]; @@ -1622,21 +1658,6 @@ public sealed class LibGit2GitClient : IGitClient - /// - /// True when the commit a pull would move this branch to has paths Windows cannot - /// create. False when there is no such commit to move to (histories that diverged - /// are the ordinary pull's to report). - /// - private static bool OriginHasPathsWindowsCannotCreate(Repository repo) - { - Commit? upstream = repo.Head.TrackedBranch?.Tip; - return upstream is not null - && !string.Equals(upstream.Sha, repo.Head.Tip.Sha, StringComparison.Ordinal) - && WindowsPathValidator.Validate(upstream.Tree).Count > 0; - } - - - /// /// What a marked folder needs before the ordinary pull: its checkout finished, or /// only its marker taken off. @@ -1644,22 +1665,89 @@ public sealed class LibGit2GitClient : IGitClient /// /// git writes the index when a checkout has reached its end. A marked folder /// without an index is therefore a tree that was never finished: it is completed, - /// with force, at the commit its branch is on - /// (). A marked folder WITH an index is one - /// whose checkout did finish (gclo's own, whose marker could not be cleared any - /// more, or one a person made with git): its tree is git's to update, and the - /// ordinary pull does that without replacing anything that was changed. Either - /// way the pull that follows brings what origin has got since. + /// with force (). A marked folder whose index + /// and files hold the whole tree of its commit is one whose checkout did finish + /// (gclo's own, whose marker could not be cleared any more, or one a person + /// completed with git): its tree is git's to update, and the ordinary pull does + /// that without replacing anything that was changed. An index that somebody's git + /// wrote over an unfinished tree (a "git reset", a "git add") does not make the + /// tree whole, and that folder is completed like one without an index. Either way + /// the pull that follows brings what origin has got since. /// private static void FinishOrReleaseAPendingCheckout(Repository repo, CancellationToken ct) { - if(repo.Index.Count == 0) + if(repo.Index.Count == 0 || !HoldsItsWholeTree(repo)) { CompletePendingCheckout(repo, ct); } else { - repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); + MarkFinished(repo); + } + } + + + + /// True when every file of the current commit is in the index and in the folder. + private static bool HoldsItsWholeTree(Repository repo) + { + string workdir = WorkingDirectoryOf(repo); + foreach((string originalPath, _, _) in CollectEffectiveEntries(repo.Head.Tip.Tree, s_noRecovery)) + { + string full = Path.Combine(workdir, originalPath.Replace('/', Path.DirectorySeparatorChar)); + if(repo.Index[originalPath] is null || !(File.Exists(full) || IsLink(full))) + { + return false; + } + } + return true; + } + + + + /// The folder's tree is written: the marker and what goes with it are taken off. + private static void MarkFinished(Repository repo) + { + repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); + repo.Config.Unset(WrittenBeforeKey, ConfigurationLevel.Local); + } + + + + /// The trees of the commits recorded under that are still in the repository. + private static List WrittenBefore(Repository repo) + { + string ids = repo.Config.Get(WrittenBeforeKey, ConfigurationLevel.Local)?.Value ?? ""; + var trees = new List(); + foreach(string id in ids.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) + { + if(ObjectId.TryParse(id, out ObjectId parsed) && repo.Lookup(parsed) is { } commit) + { + trees.Add(commit.Tree); + } + } + return trees; + } + + + + /// + /// Records, before the branch of an unfinished checkout is moved, the commit it was + /// on. The latest sixteen are kept: a folder that was cut off more often than that + /// while origin kept moving has to be moved out of the way by hand. + /// + private static void RememberAsWrittenBefore(Repository repo, Commit commit) + { + const int Kept = 16; + List ids = + [ + .. (repo.Config.Get(WrittenBeforeKey, ConfigurationLevel.Local)?.Value ?? "") + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries), + ]; + if(!ids.Contains(commit.Sha, StringComparer.Ordinal)) + { + ids.Add(commit.Sha); + repo.Config.Set(WrittenBeforeKey, string.Join(',', ids.TakeLast(Kept)), ConfigurationLevel.Local); } } @@ -1673,33 +1761,41 @@ public sealed class LibGit2GitClient : IGitClient /// /// /// - /// The branch is not moved first: every attempt to finish writes the same commit, - /// so what an earlier attempt left on disk can be told from what somebody put - /// there since (, #174). What origin - /// has got meanwhile is brought by the ordinary pull that follows, over a tree - /// that then has an index. + /// Where origin's branch has only got ahead, the branch is not moved first: every + /// attempt to finish writes the same commit, so what an earlier attempt left on + /// disk can be told from what somebody put there since + /// (, #174). What origin has got + /// meanwhile is brought by the ordinary pull that follows, over a tree that then + /// has an index. /// /// - /// The exception is a tree Windows cannot write: the commit the branch is on, or - /// the one origin has now. Then the branch moves to origin's first, as it always - /// did, and nothing is written here: if origin renamed what was in the way the - /// checkout goes ahead at its commit, and if not, the failure names the paths of - /// the newest tree, which is the one the recovery that follows will write. + /// The branch has to move first in two cases, as it always did here: a tree + /// Windows cannot write (the commit the branch is on, or origin's; the failure + /// then names the paths of the newest tree, which is the one the recovery that + /// follows will write), and a history origin has rewritten (there is nothing to + /// fast-forward from). The commit the branch leaves is recorded then + /// (), because its files may be on disk and are + /// judged against it as well. /// /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { - if(OperatingSystem.IsWindows() && (HasPathsWindowsCannotCreate(repo) || OriginHasPathsWindowsCannotCreate(repo))) + Commit head = repo.Head.Tip; + if(MustMoveBeforeItIsWritten(repo, head)) { + RememberAsWrittenBefore(repo, head); AdvanceHeadToTrackedTip(repo); - IReadOnlyList invalidPaths = WindowsPathValidator.Validate(repo.Head.Tip.Tree); - if(invalidPaths.Count > 0) + if(OperatingSystem.IsWindows()) { - throw new InvalidRepositoryPathsException(invalidPaths); + IReadOnlyList invalidPaths = WindowsPathValidator.Validate(repo.Head.Tip.Tree); + if(invalidPaths.Count > 0) + { + throw new InvalidRepositoryPathsException(invalidPaths); + } } } - RefuseToWriteOverWhatIsNotOurs(repo, repo.Head.Tip.Tree, aCutOffWriteIsOurs: true); + RefuseToWriteOverWhatIsNotOurs(repo, repo.Head.Tip.Tree, aCutOffWriteIsOurs: true, WrittenBefore(repo)); ct.ThrowIfCancellationRequested(); try @@ -1716,7 +1812,39 @@ public sealed class LibGit2GitClient : IGitClient throw new OperationCanceledException(ct); } - repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); + MarkFinished(repo); + } + + + + /// + /// True when an unfinished checkout cannot be completed at the commit its branch + /// is on: Windows cannot write that commit's tree or the one origin has now, or + /// origin's branch is not a continuation of it (origin rewrote its history, and + /// by nobody committed here). Histories that diverged + /// because of commits made here are not such a case: the commit here is completed, + /// and the ordinary pull says what is wrong. + /// + private static bool MustMoveBeforeItIsWritten(Repository repo, Commit head) + { + Commit target; + try + { + target = TargetOf(repo); + } + catch(InvalidOperationException) + { + target = head; + } + + if(OperatingSystem.IsWindows() + && (WindowsPathValidator.Validate(head.Tree).Count > 0 || WindowsPathValidator.Validate(target.Tree).Count > 0)) + { + return true; + } + + return !string.Equals(target.Sha, head.Sha, StringComparison.Ordinal) + && !string.Equals(repo.ObjectDatabase.FindMergeBase(head, target)?.Sha, head.Sha, StringComparison.Ordinal); } diff --git a/gclo.Engine/WslCloner.cs b/gclo.Engine/WslCloner.cs index ef810ac..064cb50 100644 --- a/gclo.Engine/WslCloner.cs +++ b/gclo.Engine/WslCloner.cs @@ -19,6 +19,9 @@ public sealed class WslCloner : IWslCloner /// Environment variable the inline credential helper reads the token from. public const string TokenVariable = "GCLO_GIT_TOKEN"; + /// The folder under ~/gclo in which clones are made before they are moved into place. + internal const string TemporaryFolderName = ".gclo-tmp"; + /// /// The clone script. $1 organization, $2 repository, $3 clone URL, $4 the URL's /// host (with a non-default port). Prints the distribution name and the destination @@ -40,9 +43,6 @@ public sealed class WslCloner : IWslCloner /// another server by the repository's own configuration) gets the token for that /// host only (#139). /// - /// The folder under ~/gclo in which clones are made before they are moved into place. - internal const string TemporaryFolderName = ".gclo-tmp"; - internal const string CloneScript = "set -e\n" + "org=\"$1\"; repo=\"$2\"; url=\"$3\"; host=\"$4\"\n" -- 2.54.0 From 5124ed3a5912b9f4cb2c828778535c98a0c4d07d Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 23:22:43 -0600 Subject: [PATCH 5/5] fix(clone): fourth review of #175: finishing a checkout removes nothing that was staged and writes through no link The fourth review looked only at the changes of the third and found two ways, both opened by those changes, in which finishing an unfinished checkout cost the user something, and a few smaller things. - A file somebody added to the index with git, and that the commit does not have, was removed by the forced checkout. The completion is now refused, names the file, and says how to go on ('git reset', or move the folder). - Where the repository has a symbolic link, the folders on the way to it were no longer checked for links, so a link behind a linked folder could be replaced outside the clone. The way is checked again, and a link in the folder is the checkout's own only when it leads where the repository's does (also on a first checkout, where any link was taken before). - Whether a marked folder holds its whole tree is asked of the files, not of the index: what the index says is whoever's git wrote it. - A path recovery of an unfinished checkout whose branch had to move accepts the files of the commit it left (whole content), and takes the list of those commits off when it has written the tree. - Tests for each, for a list that holds what is not a commit, for the list's cap and its entries being recorded once, for a file written before the attributes that change it, and for a link to a folder in a finished tree. The test of a rewritten history now writes the file it makes a statement about, and states what happens to it: it stays, untracked. - README and threat model say what the code does, and name what is not covered: a marked folder whose index somebody's git wrote and whose last file was cut off is taken for finished; files of a commit the branch left stay in the folder. - The tests that run the WSL clone script use the shell Git for Windows' launcher starts, not the launcher: it puts git's folders in front of the path, so the stand-in for git was never found when the tests were started from PowerShell (as CI starts them), and two of them failed there. Refs #174 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- docs/security/threat-model.md | 2 +- gclo.Engine.Tests/InterruptedCloneTests.cs | 340 +++++++++++++++++++- gclo.Engine.Tests/WslCloneScriptRunTests.cs | 31 +- gclo.Engine/LibGit2GitClient.cs | 91 ++++-- 5 files changed, 425 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 0c220b8..fd4d00a 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, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. -**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. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder (otherwise it keeps the branch its HEAD names, and stays empty until origin has that branch); a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). If you have worked in an unfinished clone with git in the meantime, gclo finishes it only when every file there is still the repository's own. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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`). +**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. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder (otherwise it keeps the branch its HEAD names, and stays empty until origin has that branch); a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). If you have worked in an unfinished clone with git in the meantime, gclo finishes it only when every file there is still the repository's own and nothing you added with `git add` would be removed; otherwise the row fails and names the file. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. 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/security/threat-model.md b/docs/security/threat-model.md index 4288170..fa0ea39 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -121,4 +121,4 @@ flowchart LR | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T3/T4 updated: path recovery writes and deletes only through `WorkingTree` (inside the working directory, never in `.git`, never through a link); `.git` and its Windows aliases are invalid path segments; the stored recovery file's paths are validated on load; cleanup of a failed clone does not follow links (#141) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T15 added: text from repositories and servers is shown with control, separator and format characters made visible, in the activity log and in everything the CLI prints as text, including characters a legacy console code page would turn into control bytes; the desktop app's surfaces are named as not covered (#143) | | 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | The project's home moved to `git.kof22.com`: the Help menu, the About dialog, the CLI's error text and the documentation link there, and the Scoop bucket is added from there (the manifest that pins the download URL and its hash now comes from that server over HTTPS; T2 is unchanged in kind). Help > Share hands a fixed text (what gclo is, where to download it, the sender's gclo version) to the system's mail handler as a `mailto:` link with no recipient, or shows it and copies it to the clipboard; nothing about the user, their accounts, or their repositories is in it, and the address it is built from must be a plain http(s) host and path (#163, #165). The update feed and the release downloads named in package manifests are still GitHub Releases at this revision. | -| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on). On Windows, a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) is preceded by refusals that change nothing: a link on the way or at a destination (except where the repository itself has a symbolic link), a file where a folder is needed (also on the way to a submodule), a folder that is not empty where a file is needed, a file at a destination that is not what the unfinished checkout wrote there, and anything but an empty folder where the repository has a submodule. "What the unfinished checkout wrote" is the content, or the beginning of it, of the commit the branch is on or of a commit recorded in `gclo.checkoutwrittenbefore`, as stored or as the checkout filters make it, carriage returns aside. An unfinished checkout is a marked folder that has no index, or whose index and files do not hold the whole tree of its commit; a marked folder that holds it is updated like any clone, without force. An unfinished checkout is completed at the commit its branch is on, before the branch moves, except where the branch must move first (a tree Windows cannot write, or a history origin rewrote); the commit it leaves is then recorded. A repository that has local branches is not treated as an empty clone; one without follows origin's default branch only when its folder holds nothing but `.git`. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes, and not across spellings of a path that only the file system can resolve). The WSL clone is made in `~/gclo/.gclo-tmp` and renamed into place when complete (`mv -T`, after a check that nothing is there). Not covered: a file created in the folder between the check and the checkout that follows it; a file of the user's at a destination that is empty, or holds the repository's content or the beginning of it, which is replaced; a link of the user's where the repository has a symbolic link, which is replaced (what it leads to is not touched); an unfinished clone in which a folder of an earlier commit became a file, or that was cut off by a power loss leaving files of zeros, which is refused until the folder is cleared; platforms other than Windows, where the checkout is forced as before. | +| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | Working tree (#174; the T3 and T4 rows are unchanged, this adds to what they rely on). On Windows, a forced checkout into a folder whose tree is not finished (the first checkout of a clone that was empty, and the completion of a checkout that was cut off) is preceded by refusals that change nothing: a link on the way or at a destination (except, at a destination where the repository itself has a symbolic link, a link that leads where the repository's does), a file where a folder is needed (also on the way to a submodule), a folder that is not empty where a file is needed, a file at a destination that is not what the unfinished checkout wrote there, and anything but an empty folder where the repository has a submodule. "What the unfinished checkout wrote" is the content, or the beginning of it, of the commit the branch is on or of a commit recorded in `gclo.checkoutwrittenbefore`, as stored or as the checkout filters make it, carriage returns aside. An unfinished checkout is a marked folder that has no index, or that does not hold every file of its commit; a marked folder that has an index and every file is updated like any clone, without force. A completion is refused when the index names a file the commit does not have (somebody added it with git, and the forced checkout would remove it). An unfinished checkout is completed at the commit its branch is on, before the branch moves, except where the branch must move first (a tree Windows cannot write, or a history origin rewrote); the commit it leaves is then recorded, and a path recovery of that folder accepts a file that is, whole, what a recorded commit has. A repository that has local branches is not treated as an empty clone; one without follows origin's default branch only when its folder holds nothing but `.git`. The folder is marked unfinished before its branch is created. One git operation per folder at a time within a process (not across processes, and not across spellings of a path that only the file system can resolve). The WSL clone is made in `~/gclo/.gclo-tmp` and renamed into place when complete (`mv -T`, after a check that nothing is there). Not covered: a file created in the folder between the check and the checkout that follows it; a file of the user's at a destination that is empty, or holds the repository's content or the beginning of it, which is replaced; a link of the user's that leads exactly where the repository's symbolic link at that path does, which is replaced by the repository's; a marked folder whose index somebody's git wrote and whose last file was cut off, which is taken for finished (git shows that file as changed); files of a commit the branch left that the new commit does not have, which stay in the folder untracked; an unfinished clone in which a folder of an earlier commit became a file, or that was cut off by a power loss leaving files of zeros, which is refused until the folder is cleared; platforms other than Windows, where the checkout is forced as before. | diff --git a/gclo.Engine.Tests/InterruptedCloneTests.cs b/gclo.Engine.Tests/InterruptedCloneTests.cs index 43130ce..90d32ce 100644 --- a/gclo.Engine.Tests/InterruptedCloneTests.cs +++ b/gclo.Engine.Tests/InterruptedCloneTests.cs @@ -990,7 +990,8 @@ public sealed class InterruptedCloneTests : IDisposable public async Task OriginRewritesItsHistory_BeforeAnUnfinishedCloneIsSyncedAgain_AndTheCloneFollowsIt() { (string origin, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one"), ("b.txt", "b")); - File.WriteAllText(Path.Combine(clone, "a.txt"), "one"); + File.WriteAllText(Path.Combine(clone, "a.txt"), "one"); // what the cut-off checkout had written + File.WriteAllText(Path.Combine(clone, "b.txt"), "b"); string rewritten = CommitToOrigin( origin, (repo, tree) => tree.Add("a.txt", BlobOf(repo, "rewritten"), Mode.NonExecutableFile), asANewRoot: true); @@ -998,7 +999,7 @@ public sealed class InterruptedCloneTests : IDisposable Assert.Equal(rewritten, HeadSha(clone)); Assert.Equal("rewritten", File.ReadAllText(Path.Combine(clone, "a.txt"))); - Assert.False(File.Exists(Path.Combine(clone, "b.txt"))); + Assert.Equal("b", File.ReadAllText(Path.Combine(clone, "b.txt"))); // not in the new history: it stays, as a file git does not track Assert.False(IsCheckoutPending(clone)); Assert.Null(WrittenBefore(clone)); } @@ -1130,32 +1131,51 @@ public sealed class InterruptedCloneTests : IDisposable - [Fact] - public async Task ALinkWhereTheRepositoryHasASymbolicLink_IsTakenForTheCheckoutsOwn_AndWhatItLeadsToIsNotTouched() + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] // a link of the user's, in a folder nothing was ever checked out into + public async Task ALinkWhereTheRepositoryHasASymbolicLink_IsTheCheckoutsOwn_OnlyWhenItLeadsWhereTheRepositorysDoes( + bool afterACutOff, bool leadsThere) { (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; + File.WriteAllText(Path.Combine(elsewhere, "kept.txt"), "mine"); + // A repository names the target of a link with '/'; a link on Windows has '\\'. + string named = leadsThere ? elsewhere.Replace('\\', '/') : "readme.txt"; CommitToOrigin(origin, (repo, tree) => tree .Add("readme.txt", BlobOf(repo, "theirs"), Mode.NonExecutableFile) - .Add("alink", BlobOf(repo, "readme.txt"), Mode.SymbolicLink)); + .Add("alink", BlobOf(repo, named), Mode.SymbolicLink)); using(var repo = new Repository(clone)) { repo.Config.Set("core.symlinks", false, ConfigurationLevel.Local); // the link is then written as a file that names its target } - using var cts = new CancellationTokenSource(); - _client.BeforeFirstCheckoutForTesting = cts.Cancel; - await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); - _client.BeforeFirstCheckoutForTesting = null; - string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; - File.WriteAllText(Path.Combine(elsewhere, "kept.txt"), "mine"); + if(afterACutOff) + { + await CutOffBeforeTheFirstFileAsync(clone); + } string link = Path.Combine(clone, "alink"); WorkingTreeTests.CreateDirectoryLink(link, elsewhere); try { - await _client.PullAsync(clone, Token, CancellationToken.None); + if(leadsThere) + { + await _client.PullAsync(clone, Token, CancellationToken.None); - Assert.Equal("readme.txt", File.ReadAllText(link)); + Assert.Equal(named, File.ReadAllText(link)); + Assert.False(IsCheckoutPending(clone)); + } + else + { + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("'alink' is a link", refused.Message, StringComparison.Ordinal); + Assert.NotNull(new DirectoryInfo(link).LinkTarget); + Assert.False(File.Exists(Path.Combine(clone, "readme.txt"))); + } Assert.Equal("mine", File.ReadAllText(Path.Combine(elsewhere, "kept.txt"))); - Assert.False(IsCheckoutPending(clone)); } finally { @@ -1244,4 +1264,296 @@ public sealed class InterruptedCloneTests : IDisposable Assert.Equal("from the other branch", File.ReadAllText(Path.Combine(clone, "readme.txt"))); Assert.False(IsCheckoutPending(clone)); } + + + + // ---------------------------------------------------------------- review of #175, fourth round + + /// Cuts the first checkout of an empty clone off once its branch exists and before a file is written. + private async Task CutOffBeforeTheFirstFileAsync(string clone) + { + using var cts = new CancellationTokenSource(); + _client.BeforeFirstCheckoutForTesting = cts.Cancel; + await Assert.ThrowsAnyAsync(() => _client.PullAsync(clone, Token, cts.Token)); + _client.BeforeFirstCheckoutForTesting = null; + Assert.True(IsCheckoutPending(clone)); + } + + + + /// An origin with a.txt, b.txt and zzz.txt (the file a checkout that fails half-way stops at). + private async Task AnOriginOfThreeFilesAsync() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("a.txt", BlobOf(repo, "a"), Mode.NonExecutableFile) + .Add("b.txt", BlobOf(repo, "b"), Mode.NonExecutableFile) + .Add("zzz.txt", BlobOf(repo, "last"), Mode.NonExecutableFile)); + return origin; + } + + + + [Theory] + [InlineData("mine.txt", false)] // 'git add mine.txt' + [InlineData("mywork/staged.txt", false)] + [InlineData("mine.txt", true)] // 'git add -A' + public async Task AFileSomebodyStaged_InAnUnfinishedTree_IsNotRemovedByTheCompletion(string path, bool everything) + { + string origin = await AnOriginOfThreeFilesAsync(); + string target = await ACloneWhoseCheckoutFailedHalfWayAsync(origin, "staged"); + File.Delete(Path.Combine(target, "zzz.txt")); // the tree is not whole + string mine = Path.Combine(target, path.Replace('/', Path.DirectorySeparatorChar)); + Directory.CreateDirectory(Path.GetDirectoryName(mine)!); + File.WriteAllText(mine, "my notes"); + using(var repo = new Repository(target)) + { + Commands.Stage(repo, everything ? "*" : path); + } + + // A forced checkout removes what the index names and the commit does not have. + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + + Assert.Contains($"'{path}'", refused.Message, StringComparison.Ordinal); + Assert.Contains("git reset", refused.Message, StringComparison.Ordinal); + Assert.Equal("my notes", File.ReadAllText(mine)); + Assert.True(IsCheckoutPending(target)); + + // Out of the index it is a file beside the checkout, and the checkout is finished. + using(var repo = new Repository(target)) + { + repo.Index.Replace(repo.Head.Tip); // 'git reset' + repo.Index.Write(); + } + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.Equal("my notes", File.ReadAllText(mine)); + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zzz.txt"))); + Assert.False(IsCheckoutPending(target)); + } + + + + [Fact] + public async Task ARenameStagedWithGit_InAFinishedCheckoutWhoseMarkerStayed_IsNotUndone() + { + string origin = await AnOriginOfThreeFilesAsync(); + string target = NewPath("renamed-here"); + await _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + string renamed = Path.Combine(target, "renamed.txt"); + using(var repo = new Repository(target)) + { + repo.Config.Set("gclo.checkoutpending", true, ConfigurationLevel.Local); + File.Move(Path.Combine(target, "b.txt"), renamed); + Commands.Stage(repo, "b.txt"); + Commands.Stage(repo, "renamed.txt"); // 'git mv b.txt renamed.txt' + } + File.WriteAllText(renamed, "b, and an afternoon of work on it"); + + await Assert.ThrowsAsync(() => _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None)); + + Assert.Equal("b, and an afternoon of work on it", File.ReadAllText(renamed)); + Assert.False(File.Exists(Path.Combine(target, "b.txt"))); + } + + + + [Fact] + public async Task ALinkOnTheWayToASymbolicLinkOfTheRepository_IsRefused_AndNothingBehindItIsTouched() + { + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + string elsewhere = Directory.CreateDirectory(NewPath("elsewhere")).FullName; + string further = Directory.CreateDirectory(NewPath("further")).FullName; + CommitToOrigin(origin, (repo, tree) => tree + .Add("readme.txt", BlobOf(repo, "theirs"), Mode.NonExecutableFile) + .Add("dir/alink", BlobOf(repo, further), Mode.SymbolicLink)); + await CutOffBeforeTheFirstFileAsync(clone); + + // The user's own link, outside the clone, which leads where the repository's + // does; and the clone's "dir" leads to the folder that holds it. + string outside = Path.Combine(elsewhere, "alink"); + WorkingTreeTests.CreateDirectoryLink(outside, further); + string onTheWay = Path.Combine(clone, "dir"); + WorkingTreeTests.CreateDirectoryLink(onTheWay, elsewhere); + try + { + InvalidOperationException refused = await Assert.ThrowsAsync( + () => _client.PullAsync(clone, Token, CancellationToken.None)); + + Assert.Contains("is a link to another location", refused.Message, StringComparison.Ordinal); + Assert.NotNull(new DirectoryInfo(outside).LinkTarget); + Assert.NotNull(new DirectoryInfo(onTheWay).LinkTarget); + Assert.True(IsCheckoutPending(clone)); + } + finally + { + WorkingTreeTests.RemoveLink(onTheWay); + WorkingTreeTests.RemoveLink(outside); + } + } + + + + [Fact] + public async Task TheLinksACheckoutItselfWrote_AreItsOwn_WhateverThisMachineLetsItWrite() + { + // With the right to create symbolic links a checkout writes links (the target + // with this system's separators); without it, files that name their targets. + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("alink", BlobOf(repo, "readme.txt"), Mode.SymbolicLink) + .Add("docs/up", BlobOf(repo, "../readme.txt"), Mode.SymbolicLink) + .Add("readme.txt", BlobOf(repo, "theirs"), Mode.NonExecutableFile) + .Add("zzz.txt", BlobOf(repo, "last"), Mode.NonExecutableFile)); + string target = await ACloneWhoseCheckoutFailedHalfWayAsync(origin, "links"); + + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.Equal("last", File.ReadAllText(Path.Combine(target, "zzz.txt"))); + Assert.False(IsCheckoutPending(target)); + } + + + + [Fact] + public async Task AFinishedCheckoutWhoseMarkerStayed_IsWhole_AlsoWhereItHoldsALinkToAFolder() + { + (string origin, _) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("alink", BlobOf(repo, "docs"), Mode.SymbolicLink) + .Add("docs/guide.txt", BlobOf(repo, "guide"), Mode.NonExecutableFile) + .Add("other.txt", BlobOf(repo, "other"), Mode.NonExecutableFile)); + string target = NewPath("finished-with-a-link"); + await _client.CloneAsync(origin, target, Token, null, CancellationToken.None); + string link = Path.Combine(target, "alink"); + if(File.GetAttributes(link).HasFlag(FileAttributes.Directory)) + { + Directory.Delete(link); + } + else + { + File.Delete(link); + } + WorkingTreeTests.CreateDirectoryLink(link, Path.Combine(target, "docs")); // a link to a folder: no file is there + using(var repo = new Repository(target)) + { + repo.Config.Set("gclo.checkoutpending", true, ConfigurationLevel.Local); + } + File.WriteAllText(Path.Combine(target, "other.txt"), "edited here"); // a completion would refuse this, or undo it + try + { + await _client.FetchAndPullAsync(origin, target, Token, CancellationToken.None); + + Assert.Equal("edited here", File.ReadAllText(Path.Combine(target, "other.txt"))); + Assert.False(IsCheckoutPending(target)); + } + finally + { + WorkingTreeTests.RemoveLink(link); + } + } + + + + [Fact] + public async Task AFileWrittenBeforeTheRulesThatChangeIt_IsStillTheCheckoutsOwn() + { + // "-early.txt" is written before ".gitattributes": as it is stored, not as the + // 'ident' rule in that file makes it. + (string origin, string clone) = await CloneAnEmptyOriginAsync(); + CommitToOrigin(origin, (repo, tree) => tree + .Add("-early.txt", BlobOf(repo, "$Id$" + LF + "hello" + LF), Mode.NonExecutableFile) + .Add(".gitattributes", BlobOf(repo, "*.txt ident" + LF), Mode.NonExecutableFile)); + await CutOffBeforeTheFirstFileAsync(clone); + File.WriteAllText(Path.Combine(clone, "-early.txt"), "$Id$" + LF + "hello" + LF); + File.WriteAllText(Path.Combine(clone, ".gitattributes"), "*.txt ident" + LF); + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.False(IsCheckoutPending(clone)); + } + + + + [Fact] + public async Task ARecoveryAfterTheBranchMoved_KnowsTheFilesOfTheCommitItLeft_AndTakesTheListOff() + { + if(!OperatingSystem.IsWindows()) + { + return; // the paths are Windows' to refuse + } + (string origin, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one")); + File.WriteAllText(Path.Combine(clone, "a.txt"), "one"); // what the cut-off checkout had written + CommitToOrigin(origin, (repo, tree) => tree + .Add("a.txt", BlobOf(repo, "two"), Mode.NonExecutableFile) + .Add("bad:name.txt", BlobOf(repo, "bad"), Mode.NonExecutableFile)); + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + Assert.NotNull(WrittenBefore(clone)); + + await _client.ApplyRecoveryAsync( + clone, + new PathRecovery( + new Dictionary(StringComparer.Ordinal) { ["bad:name.txt"] = "bad_name.txt" }, + new HashSet(StringComparer.Ordinal)), + CancellationToken.None); + + Assert.Equal("two", File.ReadAllText(Path.Combine(clone, "a.txt"))); + Assert.Equal("bad", File.ReadAllText(Path.Combine(clone, "bad_name.txt"))); + Assert.False(IsCheckoutPending(clone)); + Assert.Null(WrittenBefore(clone)); + } + + + + [Fact] + public async Task AListOfEarlierCommits_ThatHoldsWhatIsNotACommitHere_IsReadWithoutFailing() + { + (_, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one")); + File.WriteAllText(Path.Combine(clone, "a.txt"), "on"); // the file the checkout was writing + using(var repo = new Repository(clone)) + { + string blob = repo.Head.Tip["a.txt"].Target.Sha; + string tree = repo.Head.Tip.Tree.Sha; + repo.Config.Set( + "gclo.checkoutwrittenbefore", + $"garbage,0123456789012345678901234567890123456789,{blob},{tree}, ,abc,{new string('f', 40)},", + ConfigurationLevel.Local); + } + + await _client.PullAsync(clone, Token, CancellationToken.None); + + Assert.Equal("one", File.ReadAllText(Path.Combine(clone, "a.txt"))); + Assert.False(IsCheckoutPending(clone)); + Assert.Null(WrittenBefore(clone)); + } + + + + [Fact] + public async Task TheCommitsAnUnfinishedCheckoutLeft_AreRecordedOnce_AndTheLatestSixteenAreKept() + { + if(!OperatingSystem.IsWindows()) + { + return; // the paths are Windows' to refuse + } + (string origin, string clone) = await ACloneWhoseFirstCheckoutWasCutOffAsync(("a.txt", "one")); + var left = new List { HeadSha(clone) }; + for(int round = 0; round < 17; round++) + { + string content = new('x', round + 1); + string added = CommitToOrigin(origin, (repo, tree) => tree.Add("bad:name.txt", BlobOf(repo, content), Mode.NonExecutableFile)); + + // The first sync moves the branch off the commit before; the second, with + // nothing new at origin, finds the branch on a commit Windows cannot write + // and records that one; a third would record nothing new. + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + await Assert.ThrowsAsync(() => _client.PullAsync(clone, Token, CancellationToken.None)); + left.Add(added); + } + + Assert.Equal(18, left.Count); + Assert.Equal(string.Join(',', left.TakeLast(16)), WrittenBefore(clone)); + } } diff --git a/gclo.Engine.Tests/WslCloneScriptRunTests.cs b/gclo.Engine.Tests/WslCloneScriptRunTests.cs index 219b77d..a3e01ab 100644 --- a/gclo.Engine.Tests/WslCloneScriptRunTests.cs +++ b/gclo.Engine.Tests/WslCloneScriptRunTests.cs @@ -64,6 +64,14 @@ public sealed class WslCloneScriptRunTests : IDisposable /// on the path and, because Git for Windows puts only its cmd folder there, /// beside every git.exe on the path and where Git for Windows installs. /// + /// + /// Git for Windows' bin\sh.exe is a launcher: it puts git's own folders in + /// front of the path before it starts the shell, and a stand-in for git that a + /// test puts on the path is then never found. The shell it starts + /// (usr\bin\sh.exe) is used instead, with its own folder on the path for + /// the tools a script needs (). Which of the two a test run + /// finds first depends on where it was started from. + /// private static (string Sh, string Git)? FindTools() { string shell = OperatingSystem.IsWindows() ? "sh.exe" : "sh"; @@ -79,11 +87,14 @@ public sealed class WslCloneScriptRunTests : IDisposable candidates.Add(Path.Combine(Environment.GetFolderPath(programs), "Git", "bin", shell)); } - foreach(string sh in candidates.Where(File.Exists)) + foreach(string found in candidates.Where(File.Exists)) { + string started = Path.GetFullPath(Path.Combine(Path.GetDirectoryName(found)!, "..", "usr", "bin", shell)); + string sh = OperatingSystem.IsWindows() && File.Exists(started) ? started : found; try { var ask = new ProcessStartInfo(sh) { RedirectStandardOutput = true, RedirectStandardError = true, UseShellExecute = false }; + ask.Environment["PATH"] = PathWith(sh, null); ask.ArgumentList.Add("-c"); ask.ArgumentList.Add("command -v git"); using Process process = Process.Start(ask)!; @@ -104,6 +115,17 @@ public sealed class WslCloneScriptRunTests : IDisposable + /// + /// The path a shell is run with: (a test's stand-ins, + /// when it has any), the shell's own folder, then the path of this process. + /// + private static string PathWith(string sh, string? first) + => string.Join( + Path.PathSeparator, + new[] { first, Path.GetDirectoryName(sh), Environment.GetEnvironmentVariable("PATH") }.Where(part => !string.IsNullOrEmpty(part))); + + + /// False, with a line in the test's output, when this machine has no shell to run the script with. private bool CanRun() { @@ -147,10 +169,7 @@ public sealed class WslCloneScriptRunTests : IDisposable start.Environment["WSL_DISTRO_NAME"] = "test"; start.Environment["GCLO_TEST_REAL_GIT"] = s_tools.Value.Git; start.Environment["GCLO_TEST_APPEAR"] = (appear ?? "").Replace('\\', '/'); - if(pathPrefix is not null) - { - start.Environment["PATH"] = pathPrefix + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); - } + start.Environment["PATH"] = PathWith(s_tools.Value.Sh, pathPrefix); using Process process = Process.Start(start)!; process.StandardInput.Write(WslCloner.CloneScript); @@ -311,7 +330,7 @@ public sealed class WslCloneScriptRunTests : IDisposable start.Environment["GCLO_TEST_REAL_GIT"] = s_tools.Value.Git; start.Environment["GCLO_TEST_SCRIPT"] = script.Replace('\\', '/'); start.Environment["GCLO_TEST_URL"] = _origin.Replace('\\', '/'); - start.Environment["PATH"] = wrappers + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + start.Environment["PATH"] = PathWith(s_tools.Value.Sh, wrappers); using Process process = Process.Start(start)!; Task errorText = process.StandardError.ReadToEndAsync(); diff --git a/gclo.Engine/LibGit2GitClient.cs b/gclo.Engine/LibGit2GitClient.cs index 06c9070..40e8c7c 100644 --- a/gclo.Engine/LibGit2GitClient.cs +++ b/gclo.Engine/LibGit2GitClient.cs @@ -878,11 +878,12 @@ public sealed class LibGit2GitClient : IGitClient /// /// The rules are the ones the path recovery follows /// (), and a few it has no need for: a link - /// on the way or at a destination, a file where a folder is needed (also on the way - /// to a submodule), a folder with anything in it where a file is needed, a file at - /// a destination that is not what the current commit has there, and anything but an - /// empty folder where the repository has a submodule (the checkout makes an empty - /// folder of it). + /// on the way or at a destination (at a destination where the repository has a + /// symbolic link, only a link that leads elsewhere), a file where a folder is + /// needed (also on the way to a submodule), a folder with anything in it where a + /// file is needed, a file at a destination that is not what the current commit has + /// there, and anything but an empty folder where the repository has a submodule + /// (the checkout makes an empty folder of it). /// /// /// With the checkout being completed has @@ -911,13 +912,13 @@ public sealed class LibGit2GitClient : IGitClient List<(string OriginalPath, string EffectivePath, Blob Blob)> entries = CollectEffectiveEntries(incoming, s_noRecovery); /////////////////////////////////////////////////////////////////////////////// - // Where the repository has a symbolic link, a link in the folder is what a // - // checkout writes there (when the user's configuration lets it): it is not // - // looked at further. Replacing a link never touches what it leads to. // + // Where the repository has a symbolic link, a link in the folder that leads // + // where the repository's does is what a checkout writes there (when the // + // user's configuration lets it), or the same thing made by hand: it is not // + // looked at further. The folders on the way to it are, as for every entry. // /////////////////////////////////////////////////////////////////////////////// entries.RemoveAll(entry => - incoming[entry.OriginalPath]?.Mode == Mode.SymbolicLink - && IsLink(Path.Combine(workdir, entry.EffectivePath.Replace('/', Path.DirectorySeparatorChar)))); + incoming[entry.OriginalPath]?.Mode == Mode.SymbolicLink && IsTheLinkTheRepositoryHas(workdir, entry.EffectivePath, entry.Blob)); var effectiveNow = new HashSet(entries.Select(entry => entry.EffectivePath), StringComparer.Ordinal); RefuseWhatCannotBeWritten( @@ -981,6 +982,24 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// True when a link is at and it leads where the + /// repository's symbolic link there does (, whose content + /// is the target). A link on the way to it is refused, as on the way to anything + /// that is written. + /// + private static bool IsTheLinkTheRepositoryHas(string workdir, string effectivePath, Blob link) + { + WorkingTree.Verify(workdir, effectivePath); + + string full = Path.Combine(workdir, effectivePath.Replace('/', Path.DirectorySeparatorChar)); + string? target = new FileInfo(full).LinkTarget ?? new DirectoryInfo(full).LinkTarget; + return target is not null + && string.Equals(target.Replace('/', '\\'), link.GetContentText().Replace('/', '\\'), StringComparison.Ordinal); + } + + + /// The paths of the submodule entries (gitlinks) of a tree, at any depth. private static List SubmodulePaths(Tree tree) { @@ -1104,7 +1123,8 @@ 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, replaceOnlyWhatTheCommitHas: !managedByRecord); + RefuseWhatCannotBeWritten( + repo, workdir, entries, ours, effectiveNow, replaceOnlyWhatTheCommitHas: !managedByRecord, writtenBefore: WrittenBefore(repo)); ///////////////////////////////////////////////////////////////////////////// // The point of no return. Record first that this tree is being rewritten, // @@ -1147,7 +1167,7 @@ public sealed class LibGit2GitClient : IGitClient } SaveRecovery(repo, recovery, effectiveNow, tip.Sha); - repo.Config.Set(CheckoutPendingKey, false, ConfigurationLevel.Local); + MarkFinished(repo); } @@ -1665,14 +1685,14 @@ public sealed class LibGit2GitClient : IGitClient /// /// git writes the index when a checkout has reached its end. A marked folder /// without an index is therefore a tree that was never finished: it is completed, - /// with force (). A marked folder whose index - /// and files hold the whole tree of its commit is one whose checkout did finish + /// with force (). A marked folder that has an + /// index and holds every file of its commit is one whose checkout did finish /// (gclo's own, whose marker could not be cleared any more, or one a person /// completed with git): its tree is git's to update, and the ordinary pull does /// that without replacing anything that was changed. An index that somebody's git - /// wrote over an unfinished tree (a "git reset", a "git add") does not make the - /// tree whole, and that folder is completed like one without an index. Either way - /// the pull that follows brings what origin has got since. + /// wrote over an unfinished tree (a "git reset", a "git add") does not put the + /// missing files there, and that folder is completed like one without an index. + /// Either way the pull that follows brings what origin has got since. /// private static void FinishOrReleaseAPendingCheckout(Repository repo, CancellationToken ct) { @@ -1688,14 +1708,18 @@ public sealed class LibGit2GitClient : IGitClient - /// True when every file of the current commit is in the index and in the folder. + /// + /// True when every file of the current commit is in the folder (as a file, or as a + /// link: the repository may have a symbolic link there). What the index says is + /// not asked: it is whoever's git wrote it. + /// private static bool HoldsItsWholeTree(Repository repo) { string workdir = WorkingDirectoryOf(repo); foreach((string originalPath, _, _) in CollectEffectiveEntries(repo.Head.Tip.Tree, s_noRecovery)) { string full = Path.Combine(workdir, originalPath.Replace('/', Path.DirectorySeparatorChar)); - if(repo.Index[originalPath] is null || !(File.Exists(full) || IsLink(full))) + if(!(File.Exists(full) || IsLink(full))) { return false; } @@ -1777,6 +1801,13 @@ public sealed class LibGit2GitClient : IGitClient /// (), because its files may be on disk and are /// judged against it as well. /// + /// + /// A file somebody added to the index with git, and that the commit does not have, + /// would be removed from the folder by the forced checkout: the completion is + /// refused instead (). A file of a commit + /// the branch left that the new commit does not have is not looked for: it stays + /// in the folder, as a file git does not track. + /// /// private static void CompletePendingCheckout(Repository repo, CancellationToken ct) { @@ -1795,6 +1826,7 @@ public sealed class LibGit2GitClient : IGitClient } } + RefuseToRemoveWhatIsStaged(repo); RefuseToWriteOverWhatIsNotOurs(repo, repo.Head.Tip.Tree, aCutOffWriteIsOurs: true, WrittenBefore(repo)); ct.ThrowIfCancellationRequested(); @@ -1817,6 +1849,27 @@ public sealed class LibGit2GitClient : IGitClient + /// + /// Before a forced checkout into a folder that has an index: refuses, with nothing + /// changed, when the index names a file the commit does not have. A forced checkout + /// removes such a file from the folder, and it is there because somebody added it + /// with git (#174). + /// + private static void RefuseToRemoveWhatIsStaged(Repository repo) + { + Tree tree = repo.Head.Tip.Tree; + IndexEntry? staged = repo.Index.FirstOrDefault(entry => tree[entry.Path] is null); + if(staged is not null) + { + throw new InvalidOperationException( + $"'{staged.Path}' has been added to the index of this folder with git, and the commit whose checkout gclo has to finish " + + "here does not have it: finishing would remove the file. Take it out of the index ('git reset'), or move the folder " + + "away, then sync again."); + } + } + + + /// /// True when an unfinished checkout cannot be completed at the commit its branch /// is on: Windows cannot write that commit's tree or the one origin has now, or -- 2.54.0