Fake.Tools.Octo creates invalid command line args

[CiaranMacCarthaigh]

Description

On Windows, when using Fake.Tools.Octo to push a package to Octopus, the command line arguments are not correctly constructed, causing Octopus to fail.

Repro steps

1. Ensure that you have dotnet-octo.exe in a valid path.

2. Call Fake.Tools.Octo.push with one package. An example is shown:

let setCommon (ps:Octo.Options) =
     { ps with ToolPath = @".octo\"  // Replace this with the correct path to the folder containing dotnet-octo.exe
               ToolName = "dotnet-octo.exe"
               Server = {
               ServerUrl = "octopusServer"
               ApiKey = "ABC123456" }
     }

Octo.push (fun options ->
    { options with
        Common = setCommon options.Common
        Packages = [ "valid-file-path" ]
    }
)

Expected behavior

Octopus correctly parses the command line options.

Actual behavior

Octopus cannot parse the command line options and returns an error:

Unrecognized command arguments: --package=valid-file-path

Related information

• Operating system: Windows 10.0.18363.628
• Branch: release/next
• Runtime: .NET Core 3.1
• Severity: major
• Version of FAKE: 5.19 (Fake.Tools.Octo version: 5.19.0)

Known workarounds

This issue does not appear in certain earlier versions of Fake.Tools.Octo (such as 5.12). I don't know which versions are affected, I haven't tried to reproduce them.

Probable cause and possible solution

From what I've read of the code, what happens is roughly:

1. Octo takes the packages and converts the collection into a string of the format:
   --package=valid-file-path. Each of them are separated by a space character. This is done in the function stringListParam. However, it also adds a space character before the first item. Unless the collection is empty, the first character in the resultant string is always a space.

2. Eventually, when preparing the command line to start the process, CmdLineParsing.windowsArgvToCommandLine will iterate through the arguments provided. Because this packages argument has a space as the first character, it wraps it in double quotations, to account for the space. The string it creates is then:

push " --package=valid-file-path" --server=octopusServer --apikey=ABC123456

which is invalid for Octopus.

If the string returned by stringListParam does not have this starting space then windowsArgvToCommandLine will return:

push --package=valid-file-path --server=octopusServer --apikey=ABC123456

which is valid for Octopus.

A possible solution is to Trim the string that comes out of stringListParam, to remove leading whitespace. This will only patch over the problem, for one package. Supplying more than one package will result in a command line like:

push "--package=valid-file-path-1 --package=valid-file-path-2 --package=valid-file-path-3" --server=octopusServer --apikey=ABC123456

which is also invalid.

[github-actions]

Welcome to the FAKE community! Thank you so much for creating your first issue and therefore improving the project!

[matthid]

Yes indeed, thanks for the analysis! That module needs some updates, it needs to use the latest arguments parsing APIs so things like this cannot happen anymore.
I hope someone can send a PR :)

[CiaranMacCarthaigh]

Are you referring to what's described in https://fake.build/apidocs/v5/fake-core-argumentsmodule.html and https://fake.build/core-process.html?

[matthid]

@CiaranMacCarthaigh Yes indeed.

[CiaranMacCarthaigh]

I'll have a crack at it.

[ids-pfinn]

@CiaranMacCarthaigh Pretty sure this was due to my PR and I think I fixed it here it is only a quick fix instead of porting tot he new arguments module. Sorry you ran into the bug!

[matthid]

I guess this has been solved.