Improve missing disk layout metadata error

[Tyrie Vella (tyrielv)]

Problem and Context

When an enlistment is missing its persisted disk layout version, gvfs mount reports that a breaking disk-layout change can have occurred since clone. That guidance is misleading for current users because the more likely cause is incomplete .gvfs metadata.

Automation also cannot reliably distinguish this condition from other generic mount failures because the command exits with the generic error code.

Changes

• Add a distinct MissingDiskLayoutVersion return code for incomplete disk-layout metadata.
• Preserve existing generic handling for other disk-layout read and upgrade failures.
• Report clearer reclone guidance when the disk layout version is absent from repo metadata.
• Propagate structured disk-layout failure details from the upgrade path to gvfs mount instead of re-reading metadata after failure.
• Add functional coverage for both a missing disk-layout key and a missing repo metadata file.

[Nathan Baird (ShiningMassXAcc)]

The user-facing goal is right — the old "check if a breaking change has been made to GVFS since cloning" text is genuinely misleading, and a distinct exit code is the correct fix for the automation problem. CI is green across the full matrix. But the central mechanism has a hole.

Seven findings, ranked.

---

1. High — GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs:139-148

TryCheckDiskLayoutVersion calls the 3-arg TryGetDiskLayoutVersion, throwing away the returnCode this PR just added, and then re-derives the decision with string equality:

if (error == RepoMetadata.MissingDiskLayoutVersionMessage)
{
    returnCode = ReturnCode.MissingDiskLayoutVersion;
}

That string comparison is load-bearing for the new exit code, and nothing tests it. Append the log path to the message, localize it, add a trailing period — and exit code 12 silently degrades to 3, which is exactly the automation problem this PR exists to fix.

It is also internally inconsistent: TryRunAllUpgrades -> TryFindUpgrade -> TryGetDiskLayoutVersion all plumb returnCode properly. Only the one method MountVerb depends on for the new code does not.

Fix should be small: call the 5-arg TryGetDiskLayoutVersion overload and use what it returns. It already sets GenericError at the top and MissingDiskLayoutVersion in exactly the right branch, so the whole string check can be deleted. returnCode is assigned at the top of the method, so definite assignment across the try/finally is fine.

2. Medium — UTF-8 BOM added to four files

GVFS.Common/RepoMetadata.cs, GVFS/CommandLine/MountVerb.cs, GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs and GVFS.FunctionalTests/Tools/GVFSHelpers.cs each gained a UTF-8 BOM (EF BB BF) — verified by byte-comparing the blobs against the merge base. That is the only change to line 1 of each file, it is unrelated to the fix, and it permanently dirties git blame on the first line. Please strip it.

3. Medium — DiskLayoutUpgrade.cs:19, dead overload

The new 1-arg TryRunAllUpgrades(string enlistmentRoot) has zero callers — MountVerb was the only one and it moved to the 3-arg form. Dead code on day one; delete it.

(The 3-arg TryCheckDiskLayoutVersion overload is a different story — DehydrateVerb.cs:226 and RepairVerb.cs:95 still use it, so that one earns its keep.)

4. Medium — RepairVerb.cs:95 and DehydrateVerb.cs:226

Both still call the overload that discards the return code. gvfs repair is the command a user runs after hitting this failure, and gvfs dehydrate hits the same condition — so automation still cannot distinguish it from either. Is it intentional that only mount reports code 12?

5. Low — MountTests.cs:103

private const int GVFSMissingDiskLayoutVersion = 12; duplicates the enum value. The functional test project already references GVFS.Common (see the using GVFS.Common; in GVFSHelpers.cs), so (int)ReturnCode.MissingDiskLayoutVersion keeps them in lockstep. Worth doing for the neighbouring GVFSGenericError = 3 while you are there.

6. Low — GVFSHelpers.DeletePersistedValue

Near-verbatim copy of SavePersistedValue: identical FileStream/StreamReader loop, identical json.Substring(0, 2).ShouldEqual("A ") assertion, identical rewrite loop. The only difference is repoMetadata.Add(kvp.Key, kvp.Value) versus the key filter. Factor the read into a shared helper and have both write from the resulting dictionary.

7. Low — assert against the const, not the prose

Both functional tests assert on a substring of the message text. The message is now a public const, so RepoMetadata.MissingDiskLayoutVersionMessage would stop it drifting — and would also directly guard finding 1 if you would rather not restructure that method.

---

No AI-cruft or stray .github artifacts in this diff.

[Nathan Baird (ShiningMassXAcc)]

Cross-cutting note on base branches (posting here because this PR is one of the master-targeted ones; it applies equally to #2117 and #2018).

The eight PRs tagged in pr-review-chat today split across two bases:

• vnext: Do not fall back to anonymous when the /gvfs/config probe is indeterminate #2104, Pin git init to sha1 during clone #2115, Fail fast on SHA256 repositories in FastFetch and gvfs clone #2116, Fail fast on reftable repositories, and pin clone to files ref storage #2121, Stream large -z git output instead of buffering it against the cap #2045
• master: Add functional regression tests for reset --mixed skip-worktree on hydrated placeholders #2018, Retry native C++ project builds on transient MSBuild PCH failure #2117, Improve missing disk layout metadata error #2118

The branches have genuinely diverged — 22 commits in vnext that are not in master, and 7 in master that are not in vnext — so both are live and this is not automatically wrong.

But master is the repo's GitHub default branch, which means a PR opened without an explicit base lands there by accident. Worth confirming #2018, #2117 and #2118 are deliberately targeting master. The disk-layout error change in particular ships to a different audience than the SHA256 (#2116) and reftable (#2121) work it conceptually sits beside, and #2116's own description cross-references #2119 as the shared root cause — so if these are meant to travel together, three of them are currently on the wrong branch.

[Tyrie Vella (tyrielv)]

Note

🤖 Machine-drafted reply — reviewed & approved by Tyrie before posting

Thanks, confirmed these targets are intentional.

#2018 and #2117 are test infrastructure changes, so they do not affect shipped runtime behavior and are fine for master.

#2118 is a small improvement to an existing error condition: clearer guidance and a distinct exit code when disk layout metadata is missing. I want that on the shippable line, so master is intentional.

#2116, #2121, and the related Git 3.0 preparation work are different. Those preemptively fix issues that are rare today but will become common when Git 3.0 ships. vnext should ship before then, so vnext is the intended target for that work.