From c55d32ff94b74f601b138ddae7d4debdff33fea8 Mon Sep 17 00:00:00 2001 From: Darcy Abell Date: Wed, 20 Nov 2024 20:56:04 +1100 Subject: [PATCH 1/4] Simplify revert logic - rm added files, checkout the rest to HEAD --- .../Private/GitSourceControlOperations.cpp | 100 +++++------------- 1 file changed, 29 insertions(+), 71 deletions(-) diff --git a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp index ecf3024a..54cc7b3a 100644 --- a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp +++ b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp @@ -470,8 +470,7 @@ bool FGitDeleteWorker::UpdateStates() const } -// Get lists of Missing files (ie "deleted"), Modified files, and "other than Added" Existing files -void GetMissingVsExistingFiles(const TArray& InFiles, TArray& OutMissingFiles, TArray& OutAllExistingFiles, TArray& OutOtherThanAddedExistingFiles) +void GroupFileCommandsForRevert(const TArray& InFiles, TArray& FilesToRemove, TArray& FilesToCheckout) { FGitSourceControlModule& GitSourceControl = FGitSourceControlModule::Get(); FGitSourceControlProvider& Provider = GitSourceControl.GetProvider(); @@ -482,29 +481,13 @@ void GetMissingVsExistingFiles(const TArray& InFiles, TArray& Provider.GetState(Files, LocalStates, EStateCacheUsage::Use); for (const auto& State : LocalStates) { - if (FPaths::FileExists(State->GetFilename())) + if (State->IsAdded()) { - if (State->IsAdded()) - { - OutAllExistingFiles.Add(State->GetFilename()); - } - else if (State->IsModified()) - { - OutOtherThanAddedExistingFiles.Add(State->GetFilename()); - OutAllExistingFiles.Add(State->GetFilename()); - } - else if (State->CanRevert()) // for locked but unmodified files - { - OutOtherThanAddedExistingFiles.Add(State->GetFilename()); - } + FilesToRemove.Add(State->GetFilename()); } - else + else if (State->CanRevert()) { - // If already queued for deletion, don't try to delete again - if (State->IsSourceControlled() && !State->IsDeleted()) - { - OutMissingFiles.Add(State->GetFilename()); - } + FilesToCheckout.Add(State->GetFilename()); } } } @@ -519,54 +502,39 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) InCommand.bCommandSuccessful = true; // Filter files by status - TArray MissingFiles; - TArray AllExistingFiles; - TArray OtherThanAddedExistingFiles; - GetMissingVsExistingFiles(InCommand.Files, MissingFiles, AllExistingFiles, OtherThanAddedExistingFiles); - + TArray AllFilesToRevert = InCommand.Files.Num() == 0 ? FGitSourceControlModule::Get().GetProvider().GetFilesInCache() : InCommand.Files; const bool bRevertAll = InCommand.Files.Num() < 1; if (bRevertAll) { - TArray Parms; - Parms.Add(TEXT("--hard")); - InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("reset"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, Parms, FGitSourceControlModule::GetEmptyStringArray(), InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); + TArray Params; + Params.Add(TEXT("--hard")); + InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("reset"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, Params, FGitSourceControlModule::GetEmptyStringArray(), InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); - Parms.Reset(2); - Parms.Add(TEXT("-f")); // force - Parms.Add(TEXT("-d")); // remove directories - InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("clean"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, Parms, FGitSourceControlModule::GetEmptyStringArray(), InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); + Params.Reset(2); + Params.Add(TEXT("-f")); // force + Params.Add(TEXT("-d")); // remove directories + InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("clean"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, Params, FGitSourceControlModule::GetEmptyStringArray(), InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); } else { - if (MissingFiles.Num() > 0) + TArray FilesToRemove; + TArray FilesToCheckout; + GroupFileCommandsForRevert(InCommand.Files, FilesToRemove, FilesToCheckout); + + // We've missed performing an operation on some files. + ensure(FilesToRemove.Num() + FilesToCheckout.Num() == InCommand.Files.Num()); + if (FilesToRemove.Num() > 0) { // "Added" files that have been deleted needs to be removed from revision control - InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("rm"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), MissingFiles, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); + InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("rm"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), FilesToRemove, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); } - if (AllExistingFiles.Num() > 0) + if (FilesToCheckout.Num() > 0) { - // reset and revert any changes already added to the index - InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("reset"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), AllExistingFiles, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); - InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("checkout"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), AllExistingFiles, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); - } - if (OtherThanAddedExistingFiles.Num() > 0) - { - // revert any changes in working copy (this would fails if the asset was in "Added" state, since after "reset" it is now "untracked") - // may need to try a few times due to file locks from prior operations - bool CheckoutSuccess = false; - int32 Attempts = 10; - while( Attempts-- > 0 ) - { - CheckoutSuccess = GitSourceControlUtils::RunCommand(TEXT("checkout"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), OtherThanAddedExistingFiles, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); - if (CheckoutSuccess) - { - break; - } - - FPlatformProcess::Sleep(0.1f); - } - - InCommand.bCommandSuccessful &= CheckoutSuccess; + // HEAD param allows us to re-pull files which have been deleted. + TArray Params; + Params.Add(TEXT("HEAD")); + // Checkout back to the last commit for any modified files. + InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("checkout"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, Params, FilesToCheckout, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); } } @@ -575,7 +543,7 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) // unlock files: execute the LFS command on relative filenames // (unlock only locked files, that is, not Added files) TArray LockedFiles; - GitSourceControlUtils::GetLockedFiles(OtherThanAddedExistingFiles, LockedFiles); + GitSourceControlUtils::GetLockedFiles(AllFilesToRevert, LockedFiles); if (LockedFiles.Num() > 0) { const TArray& RelativeFiles = GitSourceControlUtils::RelativeFilenames(LockedFiles, InCommand.PathToGitRoot); @@ -591,19 +559,9 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) } } - // If no files were specified (full revert), refresh all relevant files instead of the specified files (which is an empty list in full revert) - // This is required so that files that were "Marked for add" have their status updated after a full revert. - TArray FilesToUpdate = InCommand.Files; - if (InCommand.Files.Num() <= 0) - { - for (const auto& File : MissingFiles) FilesToUpdate.Add(File); - for (const auto& File : AllExistingFiles) FilesToUpdate.Add(File); - for (const auto& File : OtherThanAddedExistingFiles) FilesToUpdate.Add(File); - } - // now update the status of our files TMap UpdatedStates; - bool bSuccess = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, FilesToUpdate, InCommand.ResultInfo.ErrorMessages, UpdatedStates); + bool bSuccess = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, AllFilesToRevert, InCommand.ResultInfo.ErrorMessages, UpdatedStates); if (bSuccess) { GitSourceControlUtils::CollectNewStates(UpdatedStates, States); From 4563e795b25b914dde92fd327ed4e9c396a10f68 Mon Sep 17 00:00:00 2001 From: Darcy Abell Date: Wed, 20 Nov 2024 21:09:08 +1100 Subject: [PATCH 2/4] Remove empty file array handling in GroupFilesForRevert - we never pass empty array in here anymore. --- .../GitSourceControl/Private/GitSourceControlOperations.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp index 54cc7b3a..f1026b53 100644 --- a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp +++ b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp @@ -474,11 +474,9 @@ void GroupFileCommandsForRevert(const TArray& InFiles, TArray& { FGitSourceControlModule& GitSourceControl = FGitSourceControlModule::Get(); FGitSourceControlProvider& Provider = GitSourceControl.GetProvider(); - - const TArray Files = (InFiles.Num() > 0) ? (InFiles) : (Provider.GetFilesInCache()); - + TArray> LocalStates; - Provider.GetState(Files, LocalStates, EStateCacheUsage::Use); + Provider.GetState(InFiles, LocalStates, EStateCacheUsage::Use); for (const auto& State : LocalStates) { if (State->IsAdded()) From 8dba009f7e38fa1eb50e4d542fe17cf386d57b70 Mon Sep 17 00:00:00 2001 From: Darcy Abell Date: Wed, 27 Nov 2024 09:45:01 +1100 Subject: [PATCH 3/4] Add support for "ShouldDeleteNewFilesOnRevert" option from engine config --- .../Private/GitSourceControlOperations.cpp | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp index f1026b53..98079a98 100644 --- a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp +++ b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp @@ -15,6 +15,7 @@ #include "SourceControlHelpers.h" #include "Logging/MessageLog.h" #include "Misc/MessageDialog.h" +#include "HAL/FileManager.h" #include "HAL/PlatformProcess.h" #include "GenericPlatform/GenericPlatformFile.h" #if ENGINE_MAJOR_VERSION >= 5 @@ -470,19 +471,38 @@ bool FGitDeleteWorker::UpdateStates() const } -void GroupFileCommandsForRevert(const TArray& InFiles, TArray& FilesToRemove, TArray& FilesToCheckout) +void GroupFileCommandsForRevert(const TArray& InFiles, TArray& FilesToRemove, TArray& FilesToCheckout, TArray& FilesToReset, TArray& FilesToDelete) { FGitSourceControlModule& GitSourceControl = FGitSourceControlModule::Get(); FGitSourceControlProvider& Provider = GitSourceControl.GetProvider(); - + TArray> LocalStates; Provider.GetState(InFiles, LocalStates, EStateCacheUsage::Use); for (const auto& State : LocalStates) { if (State->IsAdded()) { - FilesToRemove.Add(State->GetFilename()); + if (FPaths::FileExists(State->GetFilename())) + { + // Git rm won't delete the file because the engine still has it in use, and reset won't work on a file which doesn't exist on disk + // so we have to delete it ourselves, and then remove it from the index. + if (USourceControlPreferences::ShouldDeleteNewFilesOnRevert()) + { + FilesToDelete.Add(State->GetFilename()); + FilesToRemove.Add(State->GetFilename()); + } + else + { + FilesToReset.Add(State->GetFilename()); + } + } + else + { + // When you delete a file through content browser, UE will send a revert command to allow us to clean up the stage state. + FilesToRemove.Add(State->GetFilename()); + } } + // Checkout to head will reset the file back to what it is in your current commit, and reset the index. else if (State->CanRevert()) { FilesToCheckout.Add(State->GetFilename()); @@ -500,8 +520,7 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) InCommand.bCommandSuccessful = true; // Filter files by status - TArray AllFilesToRevert = InCommand.Files.Num() == 0 ? FGitSourceControlModule::Get().GetProvider().GetFilesInCache() : InCommand.Files; - const bool bRevertAll = InCommand.Files.Num() < 1; + const bool bRevertAll = InCommand.Files.Num() == 0; if (bRevertAll) { TArray Params; @@ -517,10 +536,23 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) { TArray FilesToRemove; TArray FilesToCheckout; - GroupFileCommandsForRevert(InCommand.Files, FilesToRemove, FilesToCheckout); + TArray FilesToReset; + TArray FilesToDelete; + GroupFileCommandsForRevert(InCommand.Files, FilesToRemove, FilesToCheckout, FilesToReset, FilesToDelete); - // We've missed performing an operation on some files. - ensure(FilesToRemove.Num() + FilesToCheckout.Num() == InCommand.Files.Num()); + // Verify we haven't missed performing an operation on any file passed on for revert + ensure(FilesToRemove.Num() + FilesToCheckout.Num() + FilesToReset.Num() == InCommand.Files.Num()); + + for (const FString& FileName : FilesToDelete) + { + bool RequireExists = true; + bool EvenReadOnly = true; + IFileManager::Get().Delete(*FileName, RequireExists, EvenReadOnly); + } + if (FilesToReset.Num() > 0) + { + InCommand.bCommandSuccessful &= GitSourceControlUtils::RunCommand(TEXT("reset"), InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, FGitSourceControlModule::GetEmptyStringArray(), FilesToReset, InCommand.ResultInfo.InfoMessages, InCommand.ResultInfo.ErrorMessages); + } if (FilesToRemove.Num() > 0) { // "Added" files that have been deleted needs to be removed from revision control @@ -536,12 +568,14 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) } } + // This is all the files we "asked" to revert, in the case of InCommand.Files everything *should* be changed + // in the case where we didn't pass in any files, we ran a revert on the repository root, so we should refresh everything + const TArray& RequestedReverts = bRevertAll ? FGitSourceControlModule::Get().GetProvider().GetFilesInCache() : InCommand.Files; if (InCommand.bUsingGitLfsLocking) { // unlock files: execute the LFS command on relative filenames - // (unlock only locked files, that is, not Added files) TArray LockedFiles; - GitSourceControlUtils::GetLockedFiles(AllFilesToRevert, LockedFiles); + GitSourceControlUtils::GetLockedFiles(RequestedReverts, LockedFiles); if (LockedFiles.Num() > 0) { const TArray& RelativeFiles = GitSourceControlUtils::RelativeFilenames(LockedFiles, InCommand.PathToGitRoot); @@ -559,7 +593,7 @@ bool FGitRevertWorker::Execute(FGitSourceControlCommand& InCommand) // now update the status of our files TMap UpdatedStates; - bool bSuccess = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, AllFilesToRevert, InCommand.ResultInfo.ErrorMessages, UpdatedStates); + bool bSuccess = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, RequestedReverts, InCommand.ResultInfo.ErrorMessages, UpdatedStates); if (bSuccess) { GitSourceControlUtils::CollectNewStates(UpdatedStates, States); @@ -640,8 +674,7 @@ bool FGitFetchWorker::Execute(FGitSourceControlCommand& InCommand) if (Operation->bUpdateStatus) { // Now update the status of all our files - const TArray ProjectDirs {FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()),FPaths::ConvertRelativePathToFull(FPaths::ProjectConfigDir()), - FPaths::ConvertRelativePathToFull(FPaths::GetProjectFilePath())}; + const TArray ProjectDirs { FString(FPlatformProcess::BaseDir()) }; TMap UpdatedStates; InCommand.bCommandSuccessful = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, ProjectDirs, InCommand.ResultInfo.ErrorMessages, UpdatedStates); From a734bbfa5f882e8dc6a95b38e185fa20dc05b79c Mon Sep 17 00:00:00 2001 From: Darcy Abell Date: Fri, 3 Jan 2025 13:53:47 +1100 Subject: [PATCH 4/4] Undo accidental change to checkout worker --- Source/GitSourceControl/Private/GitSourceControlOperations.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp index 98079a98..1bd6d4b6 100644 --- a/Source/GitSourceControl/Private/GitSourceControlOperations.cpp +++ b/Source/GitSourceControl/Private/GitSourceControlOperations.cpp @@ -674,7 +674,8 @@ bool FGitFetchWorker::Execute(FGitSourceControlCommand& InCommand) if (Operation->bUpdateStatus) { // Now update the status of all our files - const TArray ProjectDirs { FString(FPlatformProcess::BaseDir()) }; + const TArray ProjectDirs {FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()),FPaths::ConvertRelativePathToFull(FPaths::ProjectConfigDir()), + FPaths::ConvertRelativePathToFull(FPaths::GetProjectFilePath())}; TMap UpdatedStates; InCommand.bCommandSuccessful = GitSourceControlUtils::RunUpdateStatus(InCommand.PathToGitBinary, InCommand.PathToRepositoryRoot, InCommand.bUsingGitLfsLocking, ProjectDirs, InCommand.ResultInfo.ErrorMessages, UpdatedStates);