Repository navigation
Fail fast on SHA256 repositories in FastFetch, gvfs clone, gvfs mount, and gvfs repair #2116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Tyrie Vella (tyrielv)
wants to merge
11
commits into
microsoft:vnext
Choose a base branch
from
tyrielv:tyrielv/sha256-failfast
base: vnext
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
59f2d7e
Fail fast on SHA256 repositories in FastFetch and gvfs clone
tyrielv 0909d96
Address review-swarm feedback on SHA256 detection
tyrielv 0890bbf
Also fail fast on SHA256 repositories in gvfs mount
tyrielv 7da04ed
Address review-swarm feedback on mount-side SHA256 detection
tyrielv 7c7b521
Reconcile SHA256 read-failure policy with the ref-storage (reftable) fix
tyrielv 8c5bf19
Rebase onto vnext and address remaining review comments from Nathan
tyrielv f1a899c
Fix stale GVFSEnlistment.GitVersion comment flagged on #2121
tyrielv 4722f37
Address review-swarm feedback after rebase: fix overclaiming comments
tyrielv 332ffc7
Address second review round from Nathan on PR 2116
tyrielv b261297
Address third review round from Nathan on PR 2116
tyrielv 1be6e2b
Address fourth review round from Nathan on PR 2116
tyrielv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| using System; | ||
|
|
||
| namespace GVFS.Common.Git | ||
| { | ||
| /// <summary> | ||
| /// Detects whether a git repository was initialized with the SHA256 object format | ||
| /// (git's newer, opt-in hash algorithm) rather than the SHA1 format VFS for Git | ||
| /// assumes throughout its object id handling. | ||
| /// | ||
| /// There is no supported way to convert a SHA256 repository to SHA1 in place: an | ||
| /// object's id is a hash of its content plus the algorithm used to compute it, so | ||
| /// "downgrading" would mean rewriting every object with a new identity - effectively | ||
| /// a fresh clone. Callers should use this detection to fail fast with a clear error | ||
| /// rather than let SHA1-only code (e.g. Sha1Id, SHA1Util, index parsing) hit | ||
| /// corruption or an unpredictable crash on 20-byte/40-hex-char assumptions. | ||
| /// </summary> | ||
| public static class ObjectFormat | ||
| { | ||
| public const string ObjectFormatConfigName = "extensions.objectformat"; | ||
| public const string Sha256Value = "sha256"; | ||
|
|
||
| /// <summary> | ||
| /// Length, in hex characters, of a SHA256 object id (32 bytes) - twice | ||
| /// GVFSConstants.ShaStringLength, the equivalent SHA1 length. Object ids are | ||
| /// written at full hex width wherever git records them as text - notably | ||
| /// .git/packed-refs ("<objectid> <refname>") and loose ref files - so this | ||
| /// is a physical, on-disk signal of a repository's object format that survives | ||
| /// even when git config is unreadable. Unlike loose objects, packed-refs is not | ||
| /// touched by git's own packing maintenance, and VFS for Git writes it directly | ||
|
tyrielv marked this conversation as resolved.
Outdated
|
||
| /// on every clone (see GitRefs.ToPackedRefs, CloneVerb.TryInitRepo), so it is | ||
| /// reliably present to probe. | ||
| /// </summary> | ||
| public const int Sha256HexIdLength = 64; | ||
|
|
||
| public const string UnsupportedSha256ErrorMessage = | ||
| "VFS for Git only supports SHA1 repositories, but this repository was initialized " + | ||
| "with the SHA256 object format. There is no supported way to convert a SHA256 " + | ||
| "repository to SHA1 in place. Re-clone or re-initialize the repository as a SHA1 " + | ||
| "repository (for example, 'git init --object-format=sha1')."; | ||
|
|
||
| /// <summary> | ||
| /// Returns true if the given value of extensions.objectformat identifies a SHA256 | ||
| /// repository. A missing/empty value means the repository defaults to SHA1. | ||
| /// </summary> | ||
| public static bool IsSha256(string objectFormatConfigValue) | ||
|
tyrielv marked this conversation as resolved.
|
||
| { | ||
| return string.Equals(objectFormatConfigValue?.Trim(), Sha256Value, StringComparison.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Reads extensions.objectformat from the local config of the repo backing the given | ||
| /// GitProcess, and returns true if it identifies a SHA256 repository. | ||
| /// | ||
| /// A config read failure (e.g. a broken git invocation, as opposed to the key simply | ||
| /// being absent) is treated the same as "not SHA256" here, matching the established | ||
| /// behavior of GitProcess.TryGetFromConfig elsewhere in this codebase for optional | ||
| /// config reads. Use <see cref="TryIsSha256Repo"/> if the caller wants to distinguish | ||
| /// and surface that failure instead of silently treating it as SHA1. | ||
| /// </summary> | ||
| public static bool IsSha256Repo(GitProcess git) | ||
| { | ||
| TryIsSha256Repo(git, out bool isSha256, out string _); | ||
| return isSha256; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Reads extensions.objectformat from the local config of the repo backing the given | ||
| /// GitProcess. Returns false and populates <paramref name="error"/> if the config | ||
| /// could not be read at all (a missing key is not an error - it simply means the | ||
| /// repository defaults to SHA1, and isSha256 is set to false). | ||
| /// | ||
| /// Note the limits of this distinction: 'git config' reports a missing key and a | ||
| /// missing/unreadable .git/config file identically - exit code 1 with no stderr | ||
| /// content - so a repository whose entire config is gone is treated as "not SHA256" | ||
| /// here, not as a read failure. This matches the codebase-wide convention for | ||
| /// optional config reads (GitProcess.TryGetFromConfig has the same ambiguity) rather | ||
| /// than a gap specific to this check. | ||
| /// </summary> | ||
| public static bool TryIsSha256Repo(GitProcess git, out bool isSha256, out string error) | ||
| { | ||
| GitProcess.ConfigResult result = git.GetFromLocalConfig(ObjectFormatConfigName); | ||
| if (!result.TryParseAsString(out string value, out error)) | ||
| { | ||
| isSha256 = false; | ||
| return false; | ||
| } | ||
|
|
||
| isSha256 = IsSha256(value); | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
GVFS/GVFS.UnitTests/CommandLine/GitConfigRepairJobTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| using GVFS.Common; | ||
| using GVFS.RepairJobs; | ||
| using GVFS.Tests.Should; | ||
| using GVFS.UnitTests.Mock.Common; | ||
| using NUnit.Framework; | ||
| using System; | ||
| using System.IO; | ||
|
|
||
| namespace GVFS.UnitTests.CommandLine | ||
|
tyrielv marked this conversation as resolved.
Outdated
|
||
| { | ||
| [TestFixture] | ||
| public class GitConfigRepairJobTests | ||
| { | ||
| private string testEnlistmentRoot; | ||
| private string dotGitRoot; | ||
|
|
||
| [SetUp] | ||
| public void CreateTestEnlistmentRoot() | ||
| { | ||
| this.testEnlistmentRoot = Path.Combine(Path.GetTempPath(), "GVFSUnitTests_GitConfigRepairJob_" + Guid.NewGuid().ToString("N")); | ||
|
|
||
| // Construct a real (non-mock) GVFSEnlistment pointed at a scratch directory purely | ||
| // to get DotGitRoot's platform-specific layout right - PackedRefsLookSha256 only | ||
| // does real file I/O under that path, so there is no need to mock GitProcess or | ||
| // register GVFSPlatform for these tests specifically (though the [SetUpFixture] in | ||
| // GVFS.UnitTests.Setup must still have run, as it does for the whole assembly). | ||
| GVFSEnlistment enlistment = new GVFSEnlistment(this.testEnlistmentRoot, "mock://repoUrl", "mock:\\git.exe", authentication: null); | ||
| this.dotGitRoot = enlistment.DotGitRoot; | ||
| Directory.CreateDirectory(this.dotGitRoot); | ||
| } | ||
|
|
||
| [TearDown] | ||
| public void DeleteTestEnlistmentRoot() | ||
| { | ||
| if (Directory.Exists(this.testEnlistmentRoot)) | ||
| { | ||
| Directory.Delete(this.testEnlistmentRoot, recursive: true); | ||
| } | ||
| } | ||
|
|
||
| [TestCase] | ||
| public void PackedRefsLookSha256IsTrueForSha256WidthObjectId() | ||
| { | ||
| this.WritePackedRefs("bb3fd16685b64a36d8d433fd0684d192277bb3b64f2b8441c5832d8d3ba8da91 refs/heads/master"); | ||
|
|
||
| this.CreateJob().PackedRefsLookSha256().ShouldEqual(true); | ||
| } | ||
|
|
||
| [TestCase] | ||
| public void PackedRefsLookSha256IsFalseForSha1WidthObjectId() | ||
| { | ||
| this.WritePackedRefs("a3541ccec5694b0334d91298e348dc7401c07ff7 refs/heads/master"); | ||
|
|
||
| this.CreateJob().PackedRefsLookSha256().ShouldEqual(false); | ||
| } | ||
|
|
||
| [TestCase] | ||
| public void PackedRefsLookSha256SkipsHeaderCommentAndPeeledLines() | ||
| { | ||
| // Matches the real shape 'git pack-refs'/GitRefs.ToPackedRefs produce: a leading | ||
| // '#' comment, and '^'-prefixed peeled object ids for annotated tags that must not | ||
| // be mistaken for the preceding ref's own object id. | ||
| this.WritePackedRefs( | ||
| "# pack-refs with: peeled fully-peeled", | ||
| "bb3fd16685b64a36d8d433fd0684d192277bb3b64f2b8441c5832d8d3ba8da91 refs/tags/v1", | ||
| "^0000000000000000000000000000000000000000000000000000000000000000"); | ||
|
|
||
| this.CreateJob().PackedRefsLookSha256().ShouldEqual(true); | ||
| } | ||
|
|
||
| [TestCase] | ||
| public void PackedRefsLookSha256IsFalseWhenFileIsMissing() | ||
| { | ||
| this.CreateJob().PackedRefsLookSha256().ShouldEqual(false); | ||
| } | ||
|
|
||
| private GitConfigRepairJob CreateJob() | ||
| { | ||
| GVFSEnlistment enlistment = new GVFSEnlistment(this.testEnlistmentRoot, "mock://repoUrl", "mock:\\git.exe", authentication: null); | ||
| return new GitConfigRepairJob(new MockTracer(), new StringWriter(), enlistment); | ||
| } | ||
|
|
||
| private void WritePackedRefs(params string[] lines) | ||
| { | ||
| File.WriteAllLines(Path.Combine(this.dotGitRoot, GVFSConstants.DotGit.PackedRefsName), lines); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.