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.

Sign up for GitHub

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

Jump to bottom

Misc improvements #110

Merged
merged 6 commits into from
Jun 11, 2023
Merged

Misc improvements #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cf8a04f
WCA: support wildcard statusbranch patterns; include binaries in bPen…
jeffason Jun 24, 2022
2fecb56
WCA fixes
jeffason Nov 11, 2022
42ded55
Merge remote-tracking branch 'origin/release' into feature/wca-dev
jeffason Feb 7, 2023
928a810
Fix for read-only file attributes getting out-of-sync with LFS lock s…
jeffason Jun 8, 2023
75f8196
Merge remote-tracking branch 'origin' into feature/wca-dev
jeffason Jun 8, 2023
a9c643e
Remove WCA tags
jeffason Jun 8, 2023
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
9 changes: 0 additions & 9 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
# PBGet
Binaries/
/Intermediate
.DS_Store

### whitelisted files
!Resources/*
!Screenshots/*
!git-lfs.exe
!git-lfs
!git-lfs-mac-amd64
!git-lfs-mac-arm64
11 changes: 4 additions & 7 deletions Source/GitSourceControl/Private/GitSourceControlOperations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,11 @@ bool FGitCheckOutWorker::Execute(FGitSourceControlCommand& InCommand)
for (const auto& RelativeFile : RelativeFiles)
{
FString AbsoluteFile = FPaths::Combine(InCommand.PathToGitRoot, RelativeFile);
FGitLockedFilesCache::LockedFiles.Add(AbsoluteFile, LockUser);
FGitLockedFilesCache::AddLockedFile(AbsoluteFile, LockUser);
FPaths::NormalizeFilename(AbsoluteFile);
AbsoluteFiles.Add(AbsoluteFile);
}
for (const auto& File : AbsoluteFiles)
{
FPlatformFileManager::Get().GetPlatformFile().SetReadOnly(*File, false);
}

GitSourceControlUtils::CollectNewStates(AbsoluteFiles, States, EFileState::Unset, ETreeState::Unset, ELockState::Locked);
for (auto& State : States)
{
Expand Down Expand Up @@ -345,7 +342,7 @@ bool FGitCheckInWorker::Execute(FGitSourceControlCommand& InCommand)
{
for (const auto& File : LockedFiles)
{
FGitLockedFilesCache::LockedFiles.Remove(File);
FGitLockedFilesCache::RemoveLockedFile(File);
}
}
}
Expand Down Expand Up @@ -584,7 +581,7 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand)
{
for (const auto& File : LockedFiles)
{
FGitLockedFilesCache::LockedFiles.Remove(File);
FGitLockedFilesCache::RemoveLockedFile(File);
}
}
}
Expand Down
25 changes: 24 additions & 1 deletion Source/GitSourceControl/Private/GitSourceControlProvider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ bool FGitSourceControlProvider::QueryStateBranchConfig(const FString& ConfigSrc,

void FGitSourceControlProvider::RegisterStateBranches(const TArray<FString>& BranchNames, const FString& ContentRootIn)
{
StatusBranchNames = BranchNames;
StatusBranchNamePatternsInternal = BranchNames;
}

int32 FGitSourceControlProvider::GetStateBranchIndex(const FString& StateBranchName) const
Expand All @@ -815,6 +815,7 @@ int32 FGitSourceControlProvider::GetStateBranchIndex(const FString& StateBranchN

// Check if we are checking the index of the current branch
// UE uses FEngineVersion for the current branch name because of UEGames setup, but we want to handle otherwise for Git repos.
auto StatusBranchNames = GetStatusBranchNames();
if (StateBranchName == FEngineVersion::Current().GetBranch())
{
const int32 CurrentBranchStatusIndex = StatusBranchNames.IndexOfByKey(BranchName);
Expand All @@ -837,4 +838,26 @@ int32 FGitSourceControlProvider::GetStateBranchIndex(const FString& StateBranchN
return StatusBranchNames.IndexOfByKey(StateBranchName);
}

TArray<FString> FGitSourceControlProvider::GetStatusBranchNames() const
{
TArray<FString> StatusBranches;
if(PathToGitBinary.IsEmpty() || PathToRepositoryRoot.IsEmpty())
return StatusBranches;

for (int i = 0; i < StatusBranchNamePatternsInternal.Num(); i++)
{
TArray<FString> Matches;
bool bResult = GitSourceControlUtils::GetRemoteBranchesWildcard(PathToGitBinary, PathToRepositoryRoot, StatusBranchNamePatternsInternal[i], Matches);
if (bResult && Matches.Num() > 0)
{
for (int j = 0; j < Matches.Num(); j++)
{
StatusBranches.Add(Matches[j].TrimStartAndEnd());
}
}
}

return StatusBranches;
}

#undef LOCTEXT_NAMESPACE
13 changes: 5 additions & 8 deletions Source/GitSourceControl/Private/GitSourceControlProvider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,8 @@ class FGitSourceControlProvider : public ISourceControlProvider

const FString& GetRemoteBranchName() const { return RemoteBranchName; }

const TArray<FString>& GetStatusBranchNames() const
{
return StatusBranchNames;
}

TArray<FString> GetStatusBranchNames() const;

/** Indicates editor binaries are to be updated upon next sync */
bool bPendingRestart;

Expand Down Expand Up @@ -300,8 +297,8 @@ class FGitSourceControlProvider : public ISourceControlProvider
*/
TArray<FString> IgnoreForceCache;

/** Array of branch names for status queries */
TArray<FString> StatusBranchNames;

/** Array of branch name patterns for status queries */
TArray<FString> StatusBranchNamePatternsInternal;
class FGitSourceControlRunner* Runner = nullptr;
};
84 changes: 77 additions & 7 deletions Source/GitSourceControl/Private/GitSourceControlUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,49 @@ const FString& FGitScopedTempFile::GetFilename() const
FDateTime FGitLockedFilesCache::LastUpdated = FDateTime::MinValue();
TMap<FString, FString> FGitLockedFilesCache::LockedFiles = TMap<FString, FString>();

void FGitLockedFilesCache::SetLockedFiles(const TMap<FString, FString>& newLocks)
{
for (auto lock : LockedFiles)
{
if (!newLocks.Contains(lock.Key))
{
OnFileLockChanged(lock.Key, lock.Value, false);
}
}

for (auto lock : newLocks)
{
if (!LockedFiles.Contains(lock.Key))
{
OnFileLockChanged(lock.Key, lock.Value, true);
}
}

LockedFiles = newLocks;
}

void FGitLockedFilesCache::AddLockedFile(const FString& filePath, const FString& lockUser)
{
LockedFiles.Add(filePath, lockUser);
OnFileLockChanged(filePath, lockUser, true);
}

void FGitLockedFilesCache::RemoveLockedFile(const FString& filePath)
{
FString user;
LockedFiles.RemoveAndCopyValue(filePath, user);
OnFileLockChanged(filePath, user, false);
}

void FGitLockedFilesCache::OnFileLockChanged(const FString& filePath, const FString& lockUser, bool locked)
{
const FString& LfsUserName = FGitSourceControlModule::Get().GetProvider().GetLockUser();
if (LfsUserName == lockUser)
{
FPlatformFileManager::Get().GetPlatformFile().SetReadOnly(*filePath, !locked);
}
}

namespace GitSourceControlUtils
{
FString ChangeRepositoryRootIfSubmodule(const TArray<FString>& AbsoluteFilePaths, const FString& PathToRepositoryRoot)
Expand Down Expand Up @@ -639,6 +682,31 @@ bool GetRemoteBranchName(const FString& InPathToGitBinary, const FString& InRepo
return bResults;
}

bool GetRemoteBranchesWildcard(const FString& InPathToGitBinary, const FString& InRepositoryRoot, const FString& PatternMatch, TArray<FString>& OutBranchNames)
{
TArray<FString> InfoMessages;
TArray<FString> ErrorMessages;
TArray<FString> Parameters;
Parameters.Add(TEXT("--remotes"));
Parameters.Add(TEXT("--list"));
bool bResults = RunCommand(TEXT("branch"), InPathToGitBinary, InRepositoryRoot, Parameters, { PatternMatch },
InfoMessages, ErrorMessages);
if (bResults && InfoMessages.Num() > 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when using origin/* this includes origin/HEAD -> origin/main right? Does this break the feature, or does it not matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at a computer but why would ‘git branch —list’ return origin/HEAD which is not a branch?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why, I don't know. but in all my git(lab) projects it does:

C:\Users\...>git branch --remote --list origin/*
  origin/HEAD -> origin/main
  origin/main

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you confirm this is broken by setting a breakpoint in the debugger?

It seems atypical to define origin/* as a status branch pattern but if it breaks then we should handle it elegantly.

These branch patterns were added to support common, standardized branching patterns in git like gitflow - https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow

Copy link

@meruiden meruiden Jun 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screenshot1

See screenshot, confirmed that it adds it to the results

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find. Try git branch --format='%(refname:short)'

{
OutBranchNames = InfoMessages;
}
if (!bResults)
{
static bool bRunOnce = true;
if (bRunOnce)
{
UE_LOG(LogSourceControl, Warning, TEXT("No remote branches matching pattern \"%s\" were found."), *PatternMatch);
bRunOnce = false;
}
}
return bResults;
}

bool GetCommitInfo(const FString& InPathToGitBinary, const FString& InRepositoryRoot, FString& OutCommitId, FString& OutCommitSummary)
{
bool bResults;
Expand Down Expand Up @@ -1332,9 +1400,8 @@ void CheckRemote(const FString& InPathToGitBinary, const FString& InRepositoryRo
//const TArray<FString>& RelativeFiles = RelativeFilenames(Files, InRepositoryRoot);
// Get the full remote status of the Content folder, since it's the only lockable folder we track in editor.
// This shows any new files as well.
// Also update the status of `.checksum` and `Binaries` since that lets us know if editor binaries got updated.
TArray<FString> FilesToDiff{FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()), ".checksum", "Binaries"};

// Also update the status of `.checksum`.
TArray<FString> FilesToDiff{FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()), ".checksum", "Binaries/", "Plugins/"};
TArray<FString> ParametersLog{TEXT("--pretty="), TEXT("--name-only"), TEXT(""), TEXT("--")};
for (auto& Branch : BranchesToDiff)
{
Expand All @@ -1360,7 +1427,8 @@ void CheckRemote(const FString& InPathToGitBinary, const FString& InRepositoryRo
if (!IsFileLFSLockable(NewerFileName))
{
// Check if there's newer binaries pending on this branch
if (bCurrentBranch && (NewerFileName == TEXT(".checksum") || NewerFileName.StartsWith("Binaries")))
if (bCurrentBranch && (NewerFileName == TEXT(".checksum") || NewerFileName.StartsWith("Binaries/", ESearchCase::IgnoreCase) ||
NewerFileName.StartsWith("Plugins/", ESearchCase::IgnoreCase)))
{
Provider.bPendingRestart = true;
}
Expand Down Expand Up @@ -1426,7 +1494,7 @@ bool GetAllLocks(const FString& InRepositoryRoot, const FString& GitBinaryFallba
OutLocks.Add(MoveTemp(LockFile.LocalFilename), MoveTemp(LockFile.LockUser));
}
FGitLockedFilesCache::LastUpdated = CurrentTime;
FGitLockedFilesCache::LockedFiles = OutLocks;
FGitLockedFilesCache::SetLockedFiles(OutLocks);
return bResult;
}
// We tried to invalidate the UE cache, but we failed for some reason. Try updating lock state from LFS cache.
Expand Down Expand Up @@ -1481,7 +1549,7 @@ bool GetAllLocks(const FString& InRepositoryRoot, const FString& GitBinaryFallba
if (!bResult)
{
// We can use our internally tracked local lock cache (an effective combination of --cached and --local)
OutLocks = FGitLockedFilesCache::LockedFiles;
OutLocks = FGitLockedFilesCache::GetLockedFiles();
bResult = true;
}
return bResult;
Expand Down Expand Up @@ -2154,7 +2222,9 @@ bool FetchRemote(const FString& InPathToGitBinary, const FString& InPathToReposi
TArray<FString> Params{"--no-tags"};
// fetch latest repo
// TODO specify branches?
return RunCommand(TEXT("fetch"), InPathToGitBinary, InPathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(),

Params.Add(TEXT("--prune"));
return RunCommand(TEXT("fetch"), InPathToGitBinary, InPathToRepositoryRoot, Params,
FGitSourceControlModule::GetEmptyStringArray(), OutResults, OutErrorMessages);
}

Expand Down
Loading