Import Localisation commandlet when not using -DisableSCCSubmit is not checking in the checked out localisation files

Import Localisation commandlet when not using -DisableSCCSubmit is not checking in the checked out localisation files.

If we run a GatherText commandlet with this command line: PROJECT -run=GatherText -config=“Config/Localization/Game_Import.ini” -EnableSCC

And Game_Import.ini looking like this:

-------------------

[CommonSettings]

SourcePath=Saved/Temp/Game/Game

DestinationPath=Content/Localization/Game

NativeCulture=en

CulturesToGenerate=en

CulturesToGenerate=es

CulturesToGenerate=fr

CulturesToGenerate=en-US

ManifestName=Game.manifest

ArchiveName=Game.archive

PortableObjectName=Game.po

[GatherTextStep0]

CommandletClass=InternationalizationExport

bImportLoc=true

LocalizedTextCollapseMode=ELocalizedTextCollapseMode::IdenticalTextIdAndSource

POFormat=EPortableObjectFormat::Unreal

-------------------

The result is the expected modified .archive files checked out, but not checked in, they are just left checked out.

As we are not specifying -DisableSCCSubmit, the expected behaviour should be for the commandlet to check those files in, so it can be used in automated processes.

Looking into the code, the reason this is happening is because in UInternationalizationExportCommandlet::Main (called from UGatherTextCommandlet::Execute) this is called:

PortableObjectPipeline::ImportAll(LocTextHelper, SourcePath, Filename, TextCollapseMode, POFormat, bUseCultureDirectory)

Inside there is this:

if (LocFileNotifies)
{
	LocFileNotifies->BeginParallelTasks();
}
 
// Parallel where this is eventually called: ImportPortableObject -> ... -> FLocTextHelper::SaveArchiveImpl -> FLocFileSCCNotifies::PreFileWrite -> FLocalizationSCC::CheckOutFile
 
// In FlocalizationSCC::CheckOutFile, because ParallelTaskCount > 0, the files are not added to FLocalizationSCC::CheckedOutFiles but to FLocalizationSCC::DeferredCheckedOutFiles.
 
if (LocFileNotifies)
{
	LocFileNotifies->EndParallelTasks();
}

In FLocalizationSCC::EndParallelTasks, the files in DeferredCheckedOutFiles list are actually checked out, but after being checked out they are not added in FLocalizationSCC::CheckedOutFiles list and because of this, in UGatherTextCommandlet::Execute, when it is doing this at the end of the process:

if (CommandletSourceControlInfo.IsValid() && !bDisableSubmit)
{
	FText SCCErrorStr;
	if (CommandletSourceControlInfo->CheckinFiles(GetChangelistDescription(GatherTextConfigPaths), SCCErrorStr))
	{
		UE_LOG(LogGatherTextCommandlet, Log, TEXT("Submitted Localization files."));
	}
	
///

And:

bool FLocalizationSCC::CheckinFiles(const FText& InChangeDescription, FText& OutError)
{
	checkf(IsInGameThread(), TEXT("FLocalizationSCC::CheckinFiles must be called on the game-thread"));
	checkf(ParallelTasksCount == 0, TEXT("FLocalizationSCC::CheckinFiles was called while parallel tasks are running"));
 
	if (CheckedOutFiles.IsEmpty())
	{
		return true;
	}
 
///

As you can see, if CheckedOutFiles is empty, nothing will be checked in.

With this, it is not as straight forward to call this commandlet in nightly automated processes for example.

In order to fix this, in FLocalizationSCC::EndParallelTasks, we have done this change:

// Try and apply the deferred check-out requests
if (!USourceControlHelpers::CheckOutOrAddFiles(DeferredCheckedOutFilesArray))
{
	//...
}
else
{
	for (int32 Index = 0; Index < DeferredCheckedOutFilesArray.Num(); ++Index)
	{
		FString& DeferredCheckedOutFile = DeferredCheckedOutFilesArray[Index];
		CheckedOutFiles.Add(MoveTemp(DeferredCheckedOutFile));
	}
}

Would this be an acceptable change to bring into the engine?

Thank you!

Steps to Reproduce
Run GatherText commandlet with these parameters:

PROJECT -run=GatherText -config=“Config/Localization/Game_Import.ini” -EnableSCC

(note that -DisableSCCSubmit is NOT specified, so checking in the changes automatically is expected).

Note that the set of files that are checked out (.archive files) are left checked out and are not checked in as expected.

Thank you for your reply and glad looks good for you.

I hope it helps.

Thank you,

Jose

Hi,

This fix looks reasonable to me, I’ve set up a code review to run it by the author of the parallel scheduler to make sure there aren’t any concerns before we check that in.

Best,

Cody

Hi,

This should now be submitted at CL#49084985. Thanks for the report!

Best,

Cody