Skip to content
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

GitHasStagedChanges only considers modified files, not new/deleted/renamed #77

Open
andreasnilsen opened this issue Sep 10, 2018 · 3 comments

Comments

@andreasnilsen
Copy link

Looking at the source code for LibGit2Sharp's RepositoryStatus, it seems that the "Staged" list only contains existing files that were modified, not files that were added, deleted or renamed. These are stored in separate lists.
https://github.com/libgit2/libgit2sharp/blob/f8e2d42ed9051fa5a5348c1a13d006f0cc069bc7/LibGit2Sharp/RepositoryStatus.cs#L42

return new Dictionary<FileStatus, Action<RepositoryStatus, StatusEntry>>
            {
                { FileStatus.NewInWorkdir, (rs, s) => rs.untracked.Add(s) },
                { FileStatus.ModifiedInWorkdir, (rs, s) => rs.modified.Add(s) },
                { FileStatus.DeletedFromWorkdir, (rs, s) => rs.missing.Add(s) },
                { FileStatus.NewInIndex, (rs, s) => rs.added.Add(s) },
                { FileStatus.ModifiedInIndex, (rs, s) => rs.staged.Add(s) },
                { FileStatus.DeletedFromIndex, (rs, s) => rs.removed.Add(s) },
                { FileStatus.RenamedInIndex, (rs, s) => rs.renamedInIndex.Add(s) },
                { FileStatus.Ignored, (rs, s) => rs.ignored.Add(s) },
                { FileStatus.RenamedInWorkdir, (rs, s) => rs.renamedInWorkDir.Add(s) },
            };
        }

The Cake.Git method GitAliases.Repository.GitHasStagedChanges only checks the RepositoryStatus.Staged list, not the NewInIndex, DeletedFromIndex, RenamedInIndex.

public static bool GitHasStagedChanges(this ICakeContext context, DirectoryPath path)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}
if (path == null)
{
throw new ArgumentNullException(nameof(path));
}
if (!context.FileSystem.Exist(path))
{
throw new RepositoryNotFoundException($"Path '{path}' doesn't exists.");
}
return context.UseRepository(
path,
repository => repository.RetrieveStatus().Staged.Any());
}

So if your changes only contain new files/deleted files/renamed files, GitHasStagedChanges will return false.

I'm not an expert on Git terminology, but I believe "staged" means any changes staged for commit, not just modified files?

If so, I guess it's a matter of discussion where the error lies, LibGit2Sharp's naming of the Staged-list is maybe a bit unfortunate, but I also guess that Cake.Git is consuming the RepositoryStatus "API" wrongly?

@devlead
Copy link
Member

devlead commented Sep 10, 2018

@andreasnilsen to not break anything, perhaps add an GitHasChanges alias that checks all places?

@andreasnilsen
Copy link
Author

@devlead I understand your concern, but imho that name is a bit vague, it's not intuitive to me how it relates to/differs from GitHasUncommitedChanges (which will consider any file that is not ignored or unaltered as a change, including both unstaged and staged files), and some people are already using the existing method in the belief that it will consider all staged files. How about GitHasIndexedChanges, which was the original name of the GitHasStagedChanges? And maybe mark GitHasStagedChanges for deprecation?

Or maybe give it an optional flags enum for the places as an argument, which defaults to only modified files, i.e. current the functionality?

@benfoster
Copy link

I also ran into this today when pushing to my gh-pages branch for the first time. I've replaced it with GitHasUncommitedChanges but this is not ideal either as this doesn't necessarily mean the changes have been staged.

gep13 added a commit to chocolatey/boxstarter.org that referenced this issue Jan 3, 2022
cake-contrib/Cake_Git#77

Since there are only new/deleted files in this commit, need to ignore
this check for now, and add it back in afterwards.
choco-bot pushed a commit to chocolatey/boxstarter.org that referenced this issue Jan 3, 2022
(build) Running into issue with Cake.Git

cake-contrib/Cake_Git#77

Since there are only new/deleted files in this commit, need to ignore
this check for now, and add it back in afterwards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants