SRombauts / UEGitPlugin

Unreal Engine 5 Git LFS 2 Source Control Plugin (beta)
http://srombauts.github.io/UEGitPlugin
MIT License
806 stars 165 forks source link

When HEAD is ahead of remote, "Newer on Server" flag is incorrectly set #134

Closed sinbad closed 4 years ago

sinbad commented 4 years ago

I'm very new to this codebase but I know git fairly well and I had a really weird problem where, because I had an unpushed change to a file in my local repo, in UE4 no changes to that file would ever show up in the Submit Changes dialog.

The problem is that when the plugin deems there's a newer file on the server, it excludes the file from being able to be checked in (silently). I picked through the debug and tracked down the bug to this code (line 1088, GitSourceControlUtils.cpp):

TArray<FString> ParametersDiff;
ParametersDiff.Add(TEXT("--name-only"));
ParametersDiff.Add(bDiffAgainstRemote ? FString::Printf(TEXT("origin/%s "), *BranchName) : BranchName);
ParametersDiff.Add(TEXT("HEAD"));
const bool bResultDiff = RunCommand(TEXT("diff"), InPathToGitBinary, InRepositoryRoot, ParametersDiff, OnePath, Results, ErrorMessages);
OutErrorMessages.Append(ErrorMessages);
if (bResultDiff)
{
    for (const FString& NewerFileName : Results)
    {
        const FString NewerFilePath = FPaths::ConvertRelativePathToFull(InRepositoryRoot, NewerFileName);

        // Find existing corresponding file state to update it (not found would mean new file or not in the current path)
        if (FGitSourceControlState* FileStatePtr = OutStates.FindByPredicate([NewerFilePath](FGitSourceControlState& FileState) { return FileState.LocalFilename == NewerFilePath; }))
        {
            FileStatePtr->bNewerVersionOnServer = true;
        }
    }
}

This is running git diff --name-only origin/master HEAD. The problem is that this will report all changes in HEAD which haven't been pushed to origin/master yet, as well as any changes in the other direction. So it's not detecting that the file is newer on the server, just that they're different. It's impossible to get out of this situation without manually pushing, and you wouldn't know to do that because there's no information in the UI, just "No assets to check in!".

Git Diff can only compare 2 revisions without any reference to their ancestry. To truly detect whether there's a newer change on the server, you need to run something that accesses the history, like git log, or git rev-list. They both work but both have slightly different extra output you have to ignore

There are 2 things that could work:

  1. git rev-list --objects HEAD..origin/master

This gives you the changed files but unfortunately also all the objects leading up to it so there's some kruft to sift through.

  1. git log --oneline --name-only HEAD..origin/master

This output is a little more familiar since it's the same as git status --porcelain but it does also include a line per commit as well, which would need to be ignored.

I'm not productive enough in this codebase to submit a PR for this yet, because of the parsing changes needed, sorry. But I thought I'd let you know.

sinbad commented 4 years ago

Also a nice way of not having to look up the branch is to use HEAD..HEAD@{upstream} as the range.

sinbad commented 4 years ago

Actually I figured out how to do it, using --pretty= to omit the commit line, so the previous parsing code works OK. Submitted PR #135