common
common copied to clipboard
matkoch
Added support for logger parameters to MSBuild.
Added ConsoleLogger and FileLogger to MSBuildSettings, allowing fluent API specification of parameters for file loggers and the console logger when running MSBuild.
Example:
MSBuild(s => s
.SetTargetPath(Solution)
.SetTargets("Rebuild")
.SetConfiguration(Configuration)
.AddFileLogger(l => l
.SetLogFile("D:\\mylog.log")
.DisableAppend()
.SetVerbosity(MSBuildVerbosity.Normal)
.EnableShowTimestamp()
.EnablePerformanceSummary()
)
.SetMaxCpuCount(Environment.ProcessorCount)
.SetNodeReuse(IsLocalBuild));
Fixes nuke-build/nuke#314
I have no idea what the check failure on macOS is about unfortunately. Is there something I did wrong?
FYI, it will take a while until I can get into this. In the next release, I plan to do some work around CI system integration. Only after that iteration, I will get to merge this (unless it would look all perfect and shiny).
No worries. I'll try to come up with a suggestion on how this could be handled by code generation in the meantime, and post a suggestion once I come up with something I feel good about.
I have now spent some time thinking about how code generation in Nuke could be extended to support this scenario, and I've come up with the following five enhancements that I think would make sense and could be useful also outside of this limited case.
I would be more than happy to implement some or all of these and submit a new PR if you like. But first of course I would like your input on these ideas @matkoch, when you've got the time to review them:
1. Conditional Formatting
This is a feature that allows the selection of the format of an argument based on the value of that argument, similar to a switch statement in C#. This could be useful outside of this particular feature for any CLI tool that has mutually exclusive parameters to turn a feature on or off. For example -EnableMPLogging and -DisableMPLogging. Based on how the configuration API in Nuke is generated, it would be nice to have a simple property named MPLogging, which would generate the configurator methods EnableMPLogging() and DisableMPLogging(), instead of having two properties, that would generate the kind of weird looking EnableEnableMPLogging(), DisableEnableMPLogging() etc.
The idea is to extend the "format" specifiers of the properties in json specifications to allow a complex format specifiecation in addition to a simple string.
The various format alternatives would be specified in an array like the following:
[
{
"case": true,
"then": "/ShowSummary"
},
{
"case": false,
"then": "/NoSummary"
},
{
"case": null,
"then": "/UseNull"
},
{
"default": "/DoNothing"
}
]
The value after a case could be a bool value, null or a string. If no case matches, then the default is used if specified, otherwise nothing is output.
Alternative 1 - Array format
The format accepts also an array of cases, and a new property switchOn is introduced to indicate the value to switch on.
The advantage of this variant is that the format specifier is kept fairly compact, while allowing to decide which value to switch on. (The default would be {value} unless specified, but {key} could also be specified here if desired).
Example:
{
"name": "ShowSummary",
"type": "bool",
"switchOn": "{value}",
"format": [
{
"case": true,
"then": "/ShowSummary"
},
{
"case": false,
"then": "/NoSummary"
},
{
"default": "/Dummy"
}
]
}
Alternative 2 - Object format
In this alternative the format specifier is an object instead of an array, encapsulating all information about the format specifier inside that object. The on property here decides which value to switch on in the subsequent cases. Again, the default if not specified would be {value}.
The advantage of this variant is that the format is more encapsulated than in alternative 1 above, in that we no longer have the "switchOn" property a level above the format, but instead everything related to the format is inside that object. It also allows for easier extension of other types of formatting in the future, by just adding a new keyword in addition to switch.
A possible disadvantage is that it's a bit more verbose to write.
{
"name": "ShowSummary",
"type": "bool",
"format": {
"switch": {
"on": "{value}",
"cases": [
{
"case": true,
"then": "/ShowSummary"
},
{
"case": false,
"then": "/NoSummary"
},
{
"default": "/Dummy"
}
]
}
}
}
Personally I am in favor of the 2:nd alternative, object format here.
2. Index of array item in format specifier
Introduce the {index} format specifier to include the index of the item in the list. In the MSBuild scenario we need this index to be 1-based, but in general there could be scenarios where one would want this 0-based instead, so also introduce a way to select the base of the index, or include this in the format specifier.
Alternative 1 - indexBase property
Add the property indexBase with possible values of either 0 or 1to the Property type. This would change the behavior of the index specifier to be either 1-based or 0-based.
Example:
{
"name": "FileLoggers",
"type": "List<string>",
"indexBase": 1,
"format": "/flp{index}:{value}"
}
Alternative 2 - Include base in specifier
The specifier could include the base, which may be a bit more concise. For example {index0} and {index1}, or {index(0)} and {index(1)} . {index+1} is another option to produce a one-based index if {index} is the zero based index, and the one that probably is the easiest to understand.
This is the option I would personally prefer.
3. MaxItems specifier on array Properties
In the case of MSBuild, at most 9 file loggers are supported. I'd like to introduce a maxItems property to the Property class, to allow setting a limit to the number of items that could be added to the list without having an error generated.
Example:
{
"name": "FileLoggers",
"type": "List<string>",
"maxItems": 9,
"format": "/flp{index}:{value}"
}
4. ToString() generation in DataClasses
To allow for a dataclass to produce a single value for a property, as is the case of the file logger parameters in MSBuild, we could add the option to generate a ToString() method on a data class that takes the format of each item, just as if they were normal arguments, and joins them together with a separator. We should also allow for custom ToString() overloads in the partial class (which is the same as simply not generating a ToString() method I guess)
Suggest adding the toStringMethod property to a data class, allowing the specification of a separator.
Note: If any property today contains a
format, an override ofConfigureArgumentsis generated which leads to a compile time error. We probably should not do this, but instead generate just theToStringin this case.
Example (containing a bunch of the previous suggestions as well).
"dataClasses": [
{
"name": "MSBuildFileLogger",
"toStringMethod": {
"separator": ";"
},
"properties": [
{
"name": "Summary",
"type": "bool",
"format": [
{
"when": false,
"then": "NoSummary"
},
{
"when": true,
"then": "ShowSummary"
}
]
},
{
"name": "PerformanceSummary",
"type": "bool",
"format": "PerformanceSummary"
},
{
"name": "LogFile",
"type": "string",
"format": "LogFile={value}"
}
]
}
]
5. Additional extension method generation
Add support for generating fluent type configuration methods on single properties as well (Methods accepting a configurator function that is). In the MSBuild example we have a property of type List<MSBuildFileLogger> and we want to have the following extension methods generated:
public static MSBuildSettings AddFileLogger(this MSBuildSettings toolSettings, params Func<MSBuildFileLogger, MSBuildFileLogger>[] configurators)
{
var instance = new MSBuildFileLogger();
return toolSettings.AddFileLogger(configurators.Select(configurator => configurator(instance)));
}
public static MSBuildSettings SetFileLogger(this MSBuildXSettings toolSettings, params Func<MSBuildFileLogger, MSBuildFileLogger>[] configurators)
{
var instance = new MSBuildFileLogger();
return toolSettings.SetFileLogger(configurators.Select(configurator => configurator(instance)));
}
Suggest adding support for the extensionMethods property also to individual properties to have these generated.
Example:
{
"name": "FileLoggers",
"type": "List<MSBuildFileLogger>",
"extensionMethods": true,
"format": ...
}
Would also be possible to add in support for binary logging as well? Or I can do that as a different PR if that makes more sense.
@david-driscoll That should be doable as well. We just need to come to an agreement on how to implement the support for this with the code generation first I think. Probably no point in implementing it before that.