#rb Sebastian.Nordgren
#rnx
#jira UE-158934
#preflight 62c829272823f28cf273a5aa
### Perforce Connection
- The test for bRequireWorkspace was inverted, we only want to require a workspace if the workspace is NOT optional.
- Combined the two if statements checking if we should try and auto assign a workspace to reduce indentation.
- Changed use of Len() == 0 to IsEmpty() for consistency
- Do not take the workspace name from the default environment if bRequireWorkspace is false
### PerforceSourceControlProvider
- When switching workspaces use EConnectionOptions::WorkspaceOptional when establishing the connection since we are providing the workspace name.
- Remove the check, the entire point of ::EnsureValidConnection is that it can change the workspace requested.
### PerforceSourceControloOperations
- Running FConnect command with perforce does not actually connect, but rather validates the clientspec. This needed to be changed to not give an error if the clientspec is empty.
- I'd rather allow FConnect to work for connections without a clientspec rather than avoid calling it at a higher level in case a different source control provider would require FConnect to be called at this point. Not calling it requires too much knowledge of the target source control provider to be safe.
### Future Work
- Currently the optional workspace flag is used to indicate that the workspace is being set for the connection manually and we don't want to auto evalute the workspace name. However we are really confusing two different issues here 1) having no client 2) having a specific client. In the second case we might want to allow defaulting to a valid workspace. So overall it would be better to have a flag indicating that it is okay to have no clientspec name and another flag indicating that we want to fallback to a valid name if the one we provided was not valid.
[CL 21008487 by paul chipchase in ue5-main branch]
#rb Trivial
#jira UE-155232 - Capture/Profile performance data for P4.
#preflight 62b1e14c405a18a061f2d594
[CL 20758715 by Patrick Laflamme in ue5-main branch]
#rb trivial
#rnx
#preflight 626f823667326112684d0c24
- Because the commands do not return data in tagged format it was being written directly to stdout, this would cause odd log messages stating that a workspace had been create/deleted that would be displayed outside of our normal logging format.
[CL 20006887 by paul chipchase in ue5-main branch]
#rb martin.ridgers, wouter.burgers
#rnx
#preflight 626be80153253f874bcbee3c
- Some commands do not return tagged output even when we set that as our preference and will print the output to stdout as we are not currently collecting the standard output messages.
-- We could collect this output and return as info to the caller but that would involve adding more parameters which I am loath to do at the moment. At some point I plan to replace all of the FPerforceConnection::Run input and output parameters with single data structures and unify the different connection types at which point adding support for returning all info output would make sense.
- To supress this output we would normally pass -q as a global-opt for the command, which in the case of the cpp API requires us to call ::SetQuiet on the client users but for that we need to be able to pass in the request to FPerforceConnection::Run.
-- Note that calling ::SetQuiet is pretty much the same as overriding ClientUser::OuputInfo
- At this point we had two bool parameters in ::Run already, so when adding a third we might as well convert this to a bitfield of flags.
- It seems that the former parameter bInAllowRetry was never hooked to anything since the last large source conde API refactor near a decade a go, so that could just be removed.
- I felt that the former parameter 'bInStandardDebugOutput' was not clearly named, in the end if it was true we would log the full p4 command (and timing of the command in VeryVerbose mode) and in almost all cases this was set to true. So I flipped the flag so it is not set to opt out of logging a command.
- I removed one overload of FPerforceConnection::Run by setting the flags to have a default value. This should help discourage anyone from adding more parameters to the method.
[CL 20006796 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 6256abb45f20a0a34d9d62e9
- This way of invoking an operation was missing a call to FPerforceSourceControlCommand::ReturnResults which will pass the info/error messages from the command to the operation that the caller passed into the provider in the first place.
- This will allow the caller to gain access to any messages generated during the operation.
[CL 19737642 by paul chipchase in ue5-main branch]
#jira UE-145489 - Perforce source control doesn't update a specific set of P4 change list states
#rb Paul.Chipchase
#preflight 62335a876666d7e7539c378f
[CL 19422016 by Patrick Laflamme in ue5-main branch]
#rb Per.Larsson
#rnx
#preflight 6232edad736af8e082347531
- When executing FCheckOut, passing in a valid ISourceControlChangelist to ISourceControlProvider::Execute will cause the checked out files to be added to the given changelist.
- FDelete now both have the same functionality, based on the FCheckOut code.
- Note that we follow the existing convention that a changelist provided on the command line always overrides a changelist passed into ::Execute
- This code exists in some form or other in most of the workers, we probably should move it to a single function somewhere to avoid potential divergence.
[CL 19418324 by paul chipchase in ue5-main branch]
#rb Sebastian.Nordgren
#rnx
#preflight 62309ccae65a7e65d686d1dc
#robomerge FNNC
- The behaviour for the settings for the default provider (that takes it's settings from the command line) should be OverrideExisting not OverrideAll to maintain the original behaviour. This was a mistake due to misreading the original code. As it was, setting one setting via the command line would clear any previously set or saved settings.
- When parsing the cmdline we used the form "KeyName=" as the key, as FParse::Value does not strip the = automatically, but when we were looking up the values to override settings we were just using "KeyName" which would not match. The lambda ::ParseCmdLineSetting now adds = to any key passed in when parsing, so there is no longer a need for the caller to add it. This helps the keys match and work correctly.
[CL 19386795 by paul chipchase in ue5-main branch]
#rb Patrick.Laflamme
#jira UE-145721
#rnx
#preflight 622f0358752729a2ad448e16
- The problem we have is if a p4 depot path is used when requesting filestates (to see if the file exists in p4 for example) it will be treated as a networkpath due to it starting with // so calling the filesystem to check if it is a directory or not can get very expensive.
- Other operations just check the end of the path for '/' or '\' which is what this operation used to do, but that was changed at some point to check the file system, presumably to fix another problem and as we don't really know how 3rd party code is using this system we cannot just change the behavior without risking breaking existing code bases.
- This fix allows the caller to opt in to the old behavior of just checking the end of the path to determine if it is a directory or not.
- This fix is not very good, future work for a better fix will be tracked in the attached jira.
[CL 19371141 by paul chipchase in ue5-main branch]
