Open augustoproiete opened 3 years ago
Hi @augustoproiete, can I please take this issue (https://github.com/cake-build/cake/issues/3280) and also the closely related https://github.com/cake-build/cake/issues/3279 ?
Hey @FrankRay78 assigned this one to you. As for https://github.com/cake-build/cake/issues/3279 I reached out to someone that seemed to be making progress on it. If we don't hear from them in a few days, it's all yours (I'll need you to add a comment on https://github.com/cake-build/cake/issues/3279 before I can assign it to you because of how GitHub works)
nb. I've come across the following spectre issue (https://github.com/spectreconsole/spectre.console/issues/203) when working on this issue, although it's not a blocker it is relevant as I would expect command line flags to depart the 'remaining args' list once successfully parsed.
The cake runner command line arguments that are standalone (ie. do not require a cake build script) are highlighted in the following table, first column answered as No
Requires [SCRIPT] | Option | Description |
---|---|---|
No | -h, --help | Prints help information |
Yes | --bootstrap | Download/install modules defined by #module directives, but do not run build |
Yes | --skip-bootstrap | Skips bootstrapping when running build |
Yes | -d, --debug | Launches script in debug mode |
Yes | -v, --verbosity |
Specifies the amount of information to be displayed. (Quiet, Minimal, Normal, Verbose, Diagnostic) |
Yes | --description | Shows description for each task |
Yes | --tree | Shows the task dependency tree |
Yes | --dryrun | Performs a dry run |
Yes | -e, --exclusive | Executes the target task without any dependencies |
No | --version | Displays version information |
No | --info | Displays additional information about Cake |
Yes | --invalidate-script-cache | Forces the script to be recompiled if caching is enabled |
Ok, I've done an amount of work on this and my findings so far are as follows....
I suspect the ultimate solution lies in checking the arguments passed into Cake.Main(string[] args)
for the presence of a cake build script. That will satisfy the issue proposal that states This issue intends to make the necessary updates to - only when a .cake file is provided as an argument
.
Then if a cake file has been specified, split the args in half by the cake file, ensuring the cake file name is in the first split. This could be done something similar to the following:
if (args != null && args.Length > 0)
{
var buildFile = args.FirstOrDefault(a => (a ?? "").Trim().ToLower().EndsWith(".cake"));
if (buildFile != null)
{
//The command line args explicitly contains a cake build file
int indexOfBuildFile = Array.IndexOf(args, buildFile); //nb. zero based
//Generate an array of args
var cakeRunnerArgs = args.Take(indexOfBuildFile + 1);
var cakeScriptArgs = args.Skip(indexOfBuildFile + 1);
The code could proceed as currently implemented but only with the cakeRunnerArgs
passed in.
At some point soon after and once everything has been initialised, the CommandContext.Remaining
arguments needs to be populated from cakeScriptArgs
so the remaining arguments are accessible from within the cake build script itself.
The required behaviour being asked for in this issue becomes quite complex when combinations of various command line arguments are considered, particularly pre- and post- the specified cake build script. To bottom this out fully, I'm listing some scenarios below to clarify what the required behaviour should be.
Cake.Tool version 2.2.0 along with the following cake build script (named frank.cake
) has been used to populate the 'Current Behaviour' column.
var animal = Argument<string>("animal", null);
var version = Argument<string>("version", null);
var tree = Argument<bool?>("tree", null);
Console.WriteLine($"Animal - {(animal ?? "null")}, Version - {(version ?? "null")}, Tree - {(tree == null ? "null" : tree.ToString())}");
Task("Empty")
.Does(() =>
{
});
Task("Default")
.IsDependentOn("Empty")
.Does(() =>
{
});
RunTarget("Default");
Command | Current Behaviour | Expected behaviour once this issue is complete |
---|---|---|
version argument | ||
dotnet cake --version |
2.2.0 | 2.2.0 |
dotnet cake --version frank.cake |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --version frank.cake --version=1.2.3 |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --version frank.cake --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --version frank.cake --animal=monkey |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake frank.cake --version |
2.2.0 | Animal - null, Version - null, Tree - null |
dotnet cake frank.cake --animal |
Animal - null, Version - null, Tree - null | Animal - null, Version - null, Tree - null |
dotnet cake frank.cake --version=1.2.3 |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - null, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - monkey, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --animal=monkey |
Animal - monkey, Version - null, Tree - null | Animal - monkey, Version - null, Tree - null |
tree argument (implicitly set) | ||
dotnet cake --tree |
Could not find script 'D:/Source/Repos/cake-temp/build.cake' | Could not find script 'D:/Source/Repos/cake-temp/build.cake' |
dotnet cake --tree frank.cake |
Flags cannot be assigned a value. --tree frank.cake |
Default └─Empty |
dotnet cake --tree frank.cake --version=1.2.3 |
Flags cannot be assigned a value. --tree frank.cake |
Default └─Empty |
dotnet cake --tree frank.cake --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --tree frank.cake |
Default └─Empty |
dotnet cake --tree frank.cake --animal=monkey |
Flags cannot be assigned a value. --tree frank.cake |
Default └─Empty |
dotnet cake frank.cake --tree |
Default └─Empty |
Animal - null, Version - null, Tree - null |
dotnet cake frank.cake --tree --version=1.2.3 |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - null, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --tree --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - monkey, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --tree --animal=monkey |
Default └─Empty |
Animal - monkey, Version - null, Tree - null |
tree argument (explicitly set) | ||
dotnet cake --tree=true |
Could not find script 'D:/Source/Repos/cake-temp/build.cake' | Could not find script 'D:/Source/Repos/cake-temp/build.cake' |
dotnet cake --tree=true frank.cake |
Default └─Empty |
Default └─Empty |
dotnet cake --tree=true frank.cake --version=1.2.3 |
Flags cannot be assigned a value. --version=1.2.3 |
Default └─Empty |
dotnet cake --tree=true frank.cake --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version=1.2.3 |
Default └─Empty |
dotnet cake --tree=true frank.cake --animal=monkey |
Default └─Empty |
Default └─Empty |
dotnet cake frank.cake --tree=true |
Default └─Empty |
Animal - null, Version - null, Tree - true |
dotnet cake frank.cake --tree=true --version=1.2.3 |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - null, Version - 1.2.3, Tree - true |
dotnet cake frank.cake --tree=true --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - monkey, Version - 1.2.3, Tree - true |
dotnet cake frank.cake --tree=true --animal=monkey |
Default └─Empty |
Animal - monkey, Version - null, Tree - true |
tree argument (implicitly set), version argument | ||
dotnet cake --tree --version |
2.2.0 | 2.2.0 |
dotnet cake --tree --version frank.cake |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --tree --version frank.cake --version=1.2.3 |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --tree --version frank.cake --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake --tree --version frank.cake --animal=monkey |
Flags cannot be assigned a value. --version frank.cake |
2.2.0 |
dotnet cake frank.cake --tree --version |
2.2.0 | Animal - null, Version - null, Tree - null |
dotnet cake frank.cake --tree --version=1.2.3 |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - null, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --tree --version=1.2.3 --animal=monkey |
Flags cannot be assigned a value. --version=1.2.3 |
Animal - monkey, Version - 1.2.3, Tree - null |
dotnet cake frank.cake --tree --animal=monkey |
Default └─Empty |
Animal - monkey, Version - null, Tree - null |
Hi @augustoproiete - please could yourself (or delegated project member) carefully read this comment and verify the expected behaviour in each row of the table is correct? I have included the actual cake build script used, along with the full dotnet cake ...
command I ran to generate the values in the 'Current Behaviour' column.
Apologies in advance for the level of detail I have gone to, but the more I looked at what was required for this issue, the more complicated it became when factoring in multiple command line arguments. I'm also unlikely to proceed any further with this issue until I've got confirmation that the expected behaviour outlined in the above table, is actually what is being asked for.
Additionally, implementing this issue as requested will break backwards compatibility!!! For example, see the table entry above for dotnet cake frank.cake --tree
. Users who currently execute this expecting the tree to show, will instead get the script executing.
I'm now close to being able to implement related issue https://github.com/cake-build/cake/issues/3279, see my most recent comment: https://github.com/cake-build/cake/issues/3279#issuecomment-1290598704
I suspect that issue 3279, once fully implemented, may be sufficient to fully satisfy this issue as cake users will be able to use the built-in cake options as remaining arguments (whilst avoiding need to use the --
separator), making the arguments and their values accessible from within their cake scripts.
If however, there remains a desire to strictly enforce all command line options before the cake build file as cake runner options, and all the command line options after the cake build file as remaining arguments, then to preserve status quo and avoid introducing breaking changes, I would suggest a new cake command is made available to the user.
For example, Cake.Commands.DefaultCommand
would remain as-is and continue to be the default command, but a new command called BuildCommand
would be introduced, being optional to use but enforcing the following schema:
dotnet cake build <cake runner options> <cake script> <remaining arguments>
ie.
dotnet cake build --debug -v:Normal build.cake --target=default --port=123 --mode=secure
I would be happy to write this new command if asked.
Documentation on the cake website project would probably need to be updated as well, here for example: https://cakebuild.net/docs/running-builds/runners/dotnet-tool
If however, there remains a desire to strictly enforce all command line options before the cake build file as cake runner options, and all the command line options after the cake build file as remaining arguments, then to preserve status quo and avoid introducing breaking changes, I would suggest a new cake command is made available to the user.
For example,
Cake.Commands.DefaultCommand
would remain as-is and continue to be the default command, but a new command calledBuildCommand
would be introduced, being optional to use but enforcing the following schema:
dotnet cake build <cake runner options> <cake script> <remaining arguments>
Hello @devlead, @augustoproiete, given that https://github.com/cake-build/cake/issues/3279 is very close to being resolved through PR https://github.com/cake-build/cake/pull/4142, do you have any thoughts as to this very closely related issue, in particular to my most recent comment (quoted just above for brevity)?
Hey @FrankRay78 I like your idea of implementing a new BuildCommand
that would allow us to ship this new behavior in a minor release of Cake as soon as it's ready, document it, and give some time for people to try it it, and eventually we can replace the DefaultCommand
with this new one, on a major release of Cake.
I also would suggest we update the DefaultCommand
to analyze the arguments and detect any Cake
-specific arguments passed after a .cake
file (if any), and emit a warning to the user, so that they can adjust their build scripts.
e.g.
dotnet cake --tree frank.cake --target=pack
OK / No warning
dotnet cake frank.cake --tree --target=pack
Warning: Cake will soon require Cake arguments to be passed before the cake build script file.
e.g. instead of
> dotnet cake frank.cake --tree --target=pack
should be
> dotnet cake --tree frank.cake --target=pack
Sounds good @augustoproiete, I hadn't thought about using a warning to prepare for the future transition. Can you assign this issue to me again and I will implement it?
Background
The Cake runner supports a number of arguments, such
--version
to display the version of the runner, or--debug
to launch the script in debug mode.However, this make it impossible for these names to be used as arguments in Cake build scripts. For example, if one wanted to use the argument
version
as an input to the build script:And then run Cake:
They would receive an error:
Flags cannot be assigned a value
Workaround
The current workaround is to use the "remaining arguments" feature when using one of these "reserved" argument names:
Proposal
This issue intends to make the necessary updates to - only when a
.cake
file is provided as an argument - to assume that any arguments after the.cake
file to be considered as "remaining arguments" instead of Cake runner arguments.In other words: Consider that all arguments before the
.cake
filename to be arguments targeted at the Cake runner, and all arguments after the.cake
filename to be targeted at the build script:dotnet cake --version
dotnet cake --version=1.2.3
Flags cannot be assigned a value
(unless #3279 is implemented)dotnet cake build.cake --version=1.2.3
version
argument to1.2.3
dotnet cake --version=1.2.3 build.cake
Flags cannot be assigned a value
(unless #3279 is implemented)This should work for all Cake runner arguments.
Related: