Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ Non-obvious, load-bearing design points:
`GitCommandBuilder.BuildArguments` already pins — same reasoning, and this was the one verb still
leaving a host-configurable default unspecified. `WithUntrackedFiles(...)` replaces the default
rather than joining it, so the vector never carries two `--untracked-files` values.
`Status()`, `Diff()` and `Patch()` likewise pin `--ignore-submodules=none`: a host's
`diff.ignoreSubmodules=all` would otherwise hide a submodule's new commits and untracked files,
and `Status()` would report clean over unpushed submodule work (#190).

15. **`Log()` passes `--no-show-signature`.** With `log.showSignature=true`, which is common among
people who sign commits, git prints each signed commit's verification to standard output ahead
Expand Down
3 changes: 3 additions & 0 deletions GitIntegration.Test/Builders/GitDiffBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ public void BuildsTheDefaultDiffVector()
"diff",
"--name-status",
"-z",

// Pinned so a host's diff.ignoreSubmodules cannot drop a changed submodule.
"--ignore-submodules=none",
"--no-renames",
];
CollectionAssert.AreEqual(expectedArguments, arguments.ToArray());
Expand Down
1 change: 1 addition & 0 deletions GitIntegration.Test/Builders/GitPatchBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public void BuildsTheDefaultPatchVector()
"--src-prefix=a/",
"--dst-prefix=b/",
"--submodule=short",
"--ignore-submodules=none",
"-U3",
"--no-renames",
];
Expand Down
4 changes: 4 additions & 0 deletions GitIntegration.Test/Builders/GitStatusBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public void BuildsTheDefaultStatusVector()
// status.showUntrackedFiles, and a host that sets it to "no" would report a working copy
// with untracked files in it as clean.
"--untracked-files=normal",

// The same for submodules: a host's diff.ignoreSubmodules=all would hide a submodule's
// unpushed commits and untracked files.
"--ignore-submodules=none",
];
CollectionAssert.AreEqual(expectedArguments, arguments.ToArray());
}
Expand Down
50 changes: 50 additions & 0 deletions GitIntegration.Test/Integration/GitSubmoduleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,56 @@
.ExecuteAsync(cancellationToken).ConfigureAwait(false);
}

[TestMethod]
public async Task StatusDiffAndPatchReportASubmoduleTheHostIgnoresAsync()
{
// diff.ignoreSubmodules is a host setting, and git applies it to status, diff and a patch
// alike when the command carries no --ignore-submodules of its own. Written into the
// superproject's own config, the nearest scope, so the host is left alone.
CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token;

Check warning on line 425 in GitIntegration.Test/Integration/GitSubmoduleTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'TestContext.CancellationToken' instead of 'TestContext.CancellationTokenSource.Token'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitIntegration&issues=AaFZSpCU6fz0Vb7cXPfv&open=AaFZSpCU6fz0Vb7cXPfv&pullRequest=222
await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false);

using TemporaryRepository subDirectory = new();
using TemporaryRepository superDirectory = new();

GitRepository sub = await CreateRepositoryAsync(subDirectory, "s.txt", cancellationToken).ConfigureAwait(false);
GitRepository super = await CreateRepositoryAsync(superDirectory, "m.txt", cancellationToken).ConfigureAwait(false);

await AddSubmoduleAsync(super, sub.LocalPath!, "libs/sub", cancellationToken).ConfigureAwait(false);

_ = await new GitTextBuilder(
super.ProcessRunner!, super.LocalPath, "config", "diff.ignoreSubmodules", "all")
.ExecuteAsync(cancellationToken).ConfigureAwait(false);

// Unpushed work in the submodule: a new commit, and an untracked file beside it.
GitRepository checkout = new()
{
LocalPath = Path.Join(superDirectory.RootPath, "libs", "sub").As<AbsoluteDirectoryPath>(),
ProcessRunner = super.ProcessRunner,
};
await IntegrationGitFixture.ConfigureIdentityAsync(checkout, AuthorName, AuthorEmail, cancellationToken)
.ConfigureAwait(false);
_ = await CommitInSubmoduleAsync(checkout, superDirectory, Main, "two\n", cancellationToken).ConfigureAwait(false);
superDirectory.WriteFile("libs/sub/untracked.txt", "three\n");

// Compared as a typed path rather than as text, since the path type spells separators
// natively and the text differs on Windows.
RelativeFilePath submodulePath = "libs/sub".As<RelativeFilePath>();

GitStatus status = await super.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false);

// IsClean is the property a caller reads before discarding a working copy, and true here
// would mean losing the submodule's commit.
Assert.IsFalse(status.IsClean, "Status() reported clean because the host ignores submodules.");
Assert.Contains(entry => entry.Path == submodulePath, status.Entries);

IReadOnlyList<GitDiffEntry> diff = await super.Diff().ExecuteAsync(cancellationToken).ConfigureAwait(false);
Assert.Contains(entry => entry.Path == submodulePath, diff, "Diff() dropped the submodule.");

GitPatch patch = await super.Patch().ExecuteAsync(cancellationToken).ConfigureAwait(false);
Assert.Contains(file => file.Path == submodulePath, patch.Files, "Patch() dropped the submodule.");
}

[TestMethod]
public async Task ReportsNoSubmodulesForARepositoryWithNoneAsync()
{
Expand Down
5 changes: 5 additions & 0 deletions GitIntegration/Builders/GitDiffBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ protected override void AppendVerbArguments(ICollection<string> arguments)

arguments.Add("-z");

// diff.ignoreSubmodules=all in a host's config would drop a moved or dirty submodule from the
// list entirely, so the same working copy reports different changes on different machines
// (ktsu-dev/GitIntegration#190).
arguments.Add("--ignore-submodules=none");

if (_staged)
{
arguments.Add("--cached");
Expand Down
4 changes: 4 additions & 0 deletions GitIntegration/Builders/GitPatchBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ protected override void AppendVerbArguments(ICollection<string> arguments)
// Only the short form is a gitlink patch that stages (ktsu-dev/GitIntegration#124).
arguments.Add("--submodule=short");

// And diff.ignoreSubmodules=all would leave the gitlink out of the patch altogether, so the
// submodule's change could never be staged from it (ktsu-dev/GitIntegration#190).
arguments.Add("--ignore-submodules=none");

if (_staged)
{
arguments.Add("--cached");
Expand Down
6 changes: 6 additions & 0 deletions GitIntegration/Builders/GitStatusBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ protected override void AppendVerbArguments(ICollection<string> arguments)
// beside --no-pager, core.quotepath and color.ui rather than left to the host.
arguments.Add("--untracked-files=" + ToOptionValue(_untrackedFiles));

// Same reasoning, for submodules. diff.ignoreSubmodules=all in a host's config (status reads
// it too) hides a submodule's new commits and untracked files, so the superproject reports
// IsClean == true over unpushed work. Pinning "none" reports the submodule however the host
// is configured (ktsu-dev/GitIntegration#190).
arguments.Add("--ignore-submodules=none");

if (_includeIgnored)
{
// "matching" rather than "traditional": it lists the ignored paths themselves instead of
Expand Down
Loading