Sync with Cake args, smarter defaults

jnm2 commented 7 years ago

All scripts should continue running the same as they have, but this opens some new doors. I think I found a really cool solution for existing headaches. Would you mind taking a look and seeing if these things are doable?

Smarter defaults

If no target is specified, sends nothing to cake.exe instead of sending --target="Default".
- If no configuration is specified, sends nothing to cake.exe instead of sending --configuration="Release".
- If no verbosity is specified, sends nothing to cake.exe instead of sending --verbosity="Verbose".
This allows you to set your own defaults by using Argument("Target", "something-other-than-Default"), Argument("Configuration", "something-other-than-Release"), Argument("Verbosity", "something-other-than-Verbose") in your build scripts.

Also, it prevents this kind of error from showing up if you use = which the bootstrapper does not recognize but cake.exe does, e.g. -configuration=Debug:

Multiple arguments with the same name (configuration).

Fixes configuration validation

~~Retains tab completion~~ without preventing you from using configurations other than Debug and Release.

Syncs with Cake arguments

Adds -showdescription
Rotates the aliases -DryRun and -WhatIf to match cake.exe
Updates .PARAMETER descriptions to match cake.exe
Sorts parameters to match cake.exe

dnfclas commented 7 years ago

@jnm2, Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request. Thanks, .NET Foundation Pull Request Bot

gep13 commented 7 years ago

@jnm2 I personally don't have an issue with the changes in this PR, however, we wouldn't be able to merge this in until the same changes are made to the build.sh file. These bootstrappers should function in the same way.

@cake-build/cake-team what are your thoughts on this change?

patriksvensson commented 7 years ago

I'm OK with the change 👍

devlead commented 7 years ago

This fails on PowerShelll 2.0 https://ci.appveyor.com/project/cakebuild/resources/build/1.0.42#L26

PowerShell.exe : ArgumentCompleter : Cannot find the type for custom attribute 'ArgumentComplete
At line:5 char:1
+ PowerShell.exe -Version 2.0 -File .\build.ps1
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (ArgumentComplet...rgumentComplete:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

r'. Make sure that the assembly that contains this type is loaded.
At C:\projects\resources\build.ps1:46 char:23
+     [ArgumentCompleter <<<< ({ $wordToComplete = $args[2]; @('Debug', 'Releas
e') | Where-Object { $_ -like "$wordToComplete*" } })]
    + CategoryInfo          : InvalidOperation: (ArgumentCompleter:Token) [],  
   ParentContainsErrorRecordException
    + FullyQualifiedErrorId : CustomAttributeTypeNotFound

jnm2 commented 7 years ago

Let me see if I can address both issues.

jnm2 commented 7 years ago

@gep13 @patriksvensson @devlead @agc93 What is your opinion on https://github.com/cake-build/resources/pull/30/commits/2e5ca59aaf1fb9cd3261c8a41bc4c712d5350bba? As far as I can tell this is the only possible way to support PowerShell v2 without going back to ValidateSet.

jnm2 commented 7 years ago

I updated the bash script to handle parameters the same as the powershell script. Because completion and validation isn't a thing in the bash script, it doesn't need to process the arguments at all before Cake receives them, except for the --script parameter which Cake does not support.

jnm2 commented 7 years ago

I'm still curious what your thoughts are. That could easily be overkill. However, I'd rather not add back the ValidateSet because it actively prevents me from working with other configurations.

jnm2 commented 7 years ago

Hey, what's the status here? I'd like to get at least part of this through.

devlead commented 7 years ago

@jnm2 we are currently refactoring Cake heavily, i.e. removing the mono scripting engine, bumping framework version etc. That's why we're currently hesitant to make bootstrappers more complex. With these big changes we're currently evaluating and testing which versions runtimes/platforms/environment we can officially support.

Bootstrappers have been a source of many support issues for us, so long term we'll likely make them less complex and leave more of the work to cake.exe/dll. In a way that would also make the xplat experience more consistent.

The default bootstrappers main purpose to get people up and running as easy as possible, that's why we do encourage people modify them as much as they want to fit their needs. We'll get back to you post next release and once we've had the chance to discuss this more internally. Thanks for your patience.

jnm2 commented 7 years ago

Thanks, that is the kind of feedback I wanted to hear. If I remove the autocomplete addition, that makes this PR lower the complexity of the bootstrapper overall from what it was. I will do that.

jnm2 commented 7 years ago

I've updated the PR to leave the complexity lower than it started, still addressing the crucial aspect of removing the buggy defaults.

jnm2 commented 7 years ago

Updated!

devlead commented 7 years ago

@jnm2 could you please rebase against latest develop and we'll do a final review.

jnm2 commented 7 years ago

Done.

devlead commented 7 years ago

@jnm2 your changes have been merged, thanks for your contribution 👍

jnm2 commented 7 years ago

Thank you all! This is much appreciated.

cake-build / resources