nuke icon indicating copy to clipboard operation
nuke copied to clipboard

Invalid Parameter parsing when value contains "-" at start

Open wieslawsoltes opened this issue 5 years ago • 12 comments

./build.ps1 --target Pack --version-suffix "-build1234"
    [Parameter("version-suffix")]
    public string VersionSuffix { get; set; }

The VersionSuffix parameter is not set to -build1234.

wieslawsoltes avatar Dec 01 '18 14:12 wieslawsoltes

Please see https://github.com/nuke-build/web/blob/master/source/docs/authoring-builds/parameter-declaration.md. Lisp-casing is supported out of the box. But it’s probably still a bug.

matkoch avatar Dec 01 '18 14:12 matkoch

@matkoch Lisp-casing is working. For example this works: ./build.ps1 --target Pack --version-suffix "build1234"

wieslawsoltes avatar Dec 01 '18 14:12 wieslawsoltes

So it’s working now? A declaration like [Parameter] bool SomeThing will automatically allow the parameter to be passed as -SomeThing or —some-thing, and even mixed versions. If the doc is missing something, please tell me.

matkoch avatar Dec 01 '18 14:12 matkoch

@matkoch The issue is not with lisp-casing.

The issue is with parameter value:

  • Does not work: --version-suffix "-build1234"
  • Works: --version-suffix "build1234"

I think parameter parser is not parsing correctly the parameter value "-build1234" because it contains - at start.

wieslawsoltes avatar Dec 01 '18 14:12 wieslawsoltes

I am expecting the VersionSuffix property to have value equal to "-build1234" but I get null instead.

wieslawsoltes avatar Dec 01 '18 14:12 wieslawsoltes

Ah sorry, I missed that. With quotation it should surely work, that can probably be fixed. But I would recommend to pass the suffix without “-“ and implement the version along the lines like “VersionSuffix != null ? ... : ...”. Anyway, will fix that.

matkoch avatar Dec 01 '18 14:12 matkoch

@matkoch No problem.

I have added workaround for now:

        if (!string.IsNullOrWhiteSpace(VersionSuffix))
        {
            VersionSuffix = "-" + VersionSuffix;
        }

wieslawsoltes avatar Dec 01 '18 14:12 wieslawsoltes

Besides this being an issue, looking at https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-pack?tabs=netcore2x#examples, I really don’t think that VersionSuffix should include a dash prefix.

matkoch avatar Dec 01 '18 19:12 matkoch

@matkoch That is good point, I was following some early .NET Core documentation. Looks like there is better way. https://docs.microsoft.com/en-us/dotnet/core/tools/project-json-to-csproj#version

<PropertyGroup>
  <VersionPrefix>1.0.0</VersionPrefix>
  <VersionSuffix>alpha</VersionSuffix>
</PropertyGroup>

https://stackoverflow.com/a/42615574/282801

And as I remember there were some bugs along the way in build tooling.

wieslawsoltes avatar Dec 01 '18 19:12 wieslawsoltes

I think this issue is not trivial to solve, yet it's also not serious. I'm putting it into backlog.

matkoch avatar Dec 01 '18 23:12 matkoch

In fact it should be easy to solve. It's just that we can't use Environment.GetCommandLineArgs(), but have to use Environment.CommandLine as well and extract matching quotes. It should also be checked if this is not a bug in the runtime implementation.

matkoch avatar Oct 02 '20 01:10 matkoch

@matkoch I would like to try and resolve this but I was unsure if this is still an issue. This would also be my first time looking at this code and I was wondering if you had any recommendations on where I could start?

Cpcagle1 avatar Jul 14 '21 22:07 Cpcagle1

May i can try to get it?

GmausDev avatar Oct 10 '22 15:10 GmausDev

@wieslawsoltes Good day,

Could you please confirm whether this issue still persists. If not, Could you please close it.

If yes, Could you please add recent context of this issue.

Thanks in advance

GopinathRakkiyannan avatar Oct 16 '22 16:10 GopinathRakkiyannan