Rewrite parser logic with C# #71
Conversation
|
@FabienTschanz Wow I bet this one is leaps and bounds better than the current parser but we really got to give some time to test it, in fact even if it works wonders I'd kinda consider it as "breaking change" as far as m365dsc goes, the same way that the powershell graph sdk team is making changes around WAM but also the module loading priority issues and we cannot update the graph version in m365dsc because of that. can this be put on hold at least for a couple of weeks? My solution depends 100% on DSCParser (and ReverseDSC as well) and whenever it deviates even if just so slightly everything breaks on my side. Not saying it should not go ahead, just asking for some time to test it, I'm waiting for a new M365DSC version so that I can cross a major version and can get rid of the several patches I have on my side and after that I'll check this one out. |
|
No problem with waiting from my side. I did test it with full exports of all workloads with the most complex and nested configurations you can achieve. This version also fixes quite some issues with indentation and deeply nested objects when using I have similar optimizations running and under development for M365DSC. All of the changes here are also in a private ADO repo which I'm developing to use for my company, and since I started developing this version around Christmas, it was getting better and better. Right now, I have no issues left to solve from my side. If you discover any issue once you start testing it, let me know how I can reproduce them and I will gladly try to fix them. Using the AST with its strongly typed code, I can debug the parsed configuration way easier than before and it also performs way faster. My 20k lines configuration is parsed in 4-6 seconds, depending on Windows PowerShell vs PowerShell 7. PowerShell 7 is a little bit slower because |
|
And with my automated export and report pipeline, I discovered so many issues with M365DSC (and the other modules) that I almost can't keep up with fixing them. I guess you saw the number of PRs I put up in the M365DSC repo, and I'm still not done. Quite some more things to come, including new development things. My priority is directed to making the modules faster and much more stable since we want to use them for our customers very soon. |
|
For sure, I actually also have an exporter script as part of my pipelines and after that another script which uses ConvertTo-DSCObject and whenever there's a misbehaving resource which breaks the pipeline my customers complain about it, just had one that reported a problem that actually is already fixed in M365DSC latest version but I still have my Frankenstein version so I had to apply yet another workaround and hate doing that so I just hope Nik releases a new version without this patch so I have time to look into it. Then I have yet another script with my own version of ConvertFrom-DSCObject (because I first need to convert Markdown policies to PowerShell objects) which gets all indentations right so they all look "right" comparing to what M365DSC exports :) |
|
I guess we're the masters of "patching" our stuff together so it just works 😆 |
|
I now rigorously tested this version and added an improved In my pipeline, I can convert all resources and compare them with the report cmdlets from Microsoft365DSC much faster than before. I know that the conversion works because it's able to detect changes in e.g. double nested CIM instances. In my example, that is from an Intune Remediation assignment where the time was updated. I also went ahead and did a conversion to and from DSC objects and outputted it back to a file. This implementation is better in every aspect and more precise due to being typed. If there is an issue with the parser, we know exactly where it fails and have the necessary debug information available to trace the issue down. If somebody wants to test it out, I can help you get there with building the module. |
|
@ricmestre Do you want to test this version? Nik is afaik looking internally with a couple of teams, feedback is very much appreciated to make it failproof. |
Utilities/Build.ps1
Outdated
| ) | ||
|
|
||
| # Verify .NET SDK is available | ||
| try { |
There was a problem hiding this comment.
this block doesn't look right, I have dotnet runtimes installed but no sdks at the moment on my work machine so running this command will get me the below printed in the screen, see "Using .NET SDK version:" at the bottom $dotnetVersion is $null. I think you should use dotnet --list-sdks here.
Also, it's an external command so try/catch exception won't work, you have to check if var $LASTEXITCODE is zero.
The command could not be loaded, possibly because:
* You intended to execute a .NET application:
The application '--version' does not exist or is not a managed .dll or .exe.
* You intended to execute a .NET SDK command:
No .NET SDKs were found.
Download a .NET SDK:
https://aka.ms/dotnet/download
Learn about SDK resolution:
https://aka.ms/dotnet/sdk-not-found
Using .NET SDK version:
There was a problem hiding this comment.
thinking a bit more, you probably will want to run Get-Command -Name dotnet -ErrorAction SilentlyContinue just to make sure that at least you have some runtime installed and only proceed to run dotnet --list-sdks after (since I don't have sdks installed I don't get any output here, but at least it doesn't fail and $LASTEXITCODE = 0)
There was a problem hiding this comment.
Thanks a lot for the feedback, appreciate it. Yeah, I only executed the build script on my main workstation where I have all of the necessary tools. I did not properly check the error routine on another machine which doesn't have the tools installed. Currently updating the build script to incorporate your feedback. Will push an updated version shortly.
| # Build the project | ||
| Write-Host "" | ||
| Write-Host "Building C# project $ProjectName..." -ForegroundColor Yellow | ||
| $buildResult = dotnet build $projectPath -c $Configuration --nologo 2>&1 |
There was a problem hiding this comment.
My build fails in both PS5.1 and PS7, below is the error as displayed in PS7 since it's shorter but in PS5.1 it complains exactly about the same and I have .NET 10 SDK installed.
PS C:\Users\serial\Documents\dscparser-fabien\DSCParser\Utilities> .\Build.ps1 -Configuration "Release"
Building DSCParser.CSharp...
Repository Root: C:\Users\serial\Documents\dscparser-fabien\DSCParser
Project Path: C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj
Configuration: Release
Cleaning previous build artifacts...
Building C# project DSCParser.CSharp...
Exception: C:\Users\serial\Documents\dscparser-fabien\DSCParser\Utilities\Build.ps1:113
Line |
113 | throw "Build failed with exit code $LASTEXITCODE. Build resul …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Build failed with exit code 1. Build result: Determining projects to restore... C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.Shared\DSCParser.Shared.csproj : error NU1100: Unable to resolve
| 'NETStandard.Library (>= 2.0.3)' for '.NETStandard,Version=v2.0'. [C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj]
| C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve 'NETStandard.Library (>= 2.0.3)' for '.NETStandard,Version=v2.0'.
| C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.Shared\DSCParser.Shared.csproj : error NU1100: Unable to resolve 'PowerShellStandard.Library (>= 5.1.1)' for '.NETStandard,Version=v2.0'.
| [C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj] C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to
| resolve 'Microsoft.CSharp (>= 4.7.0)' for '.NETStandard,Version=v2.0'. C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve
| 'Microsoft.Management.Infrastructure (>= 3.0.0)' for '.NETStandard,Version=v2.0'. C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve
| 'PowerShellStandard.Library (>= 5.1.1)' for '.NETStandard,Version=v2.0'. Failed to restore C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.Shared\DSCParser.Shared.csproj (in 374 ms). Failed to restore
| C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj (in 388 ms). Build FAILED. C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.Shared\DSCParser.Shared.csproj
| : error NU1100: Unable to resolve 'NETStandard.Library (>= 2.0.3)' for '.NETStandard,Version=v2.0'. [C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj]
| C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve 'NETStandard.Library (>= 2.0.3)' for '.NETStandard,Version=v2.0'.
| C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.Shared\DSCParser.Shared.csproj : error NU1100: Unable to resolve 'PowerShellStandard.Library (>= 5.1.1)' for '.NETStandard,Version=v2.0'.
| [C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj] C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to
| resolve 'Microsoft.CSharp (>= 4.7.0)' for '.NETStandard,Version=v2.0'. C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve
| 'Microsoft.Management.Infrastructure (>= 3.0.0)' for '.NETStandard,Version=v2.0'. C:\Users\serial\Documents\dscparser-fabien\DSCParser\src\DSCParser.CSharp\DSCParser.CSharp.csproj : error NU1100: Unable to resolve
| 'PowerShellStandard.Library (>= 5.1.1)' for '.NETStandard,Version=v2.0'. 0 Warning(s) 6 Error(s) Time Elapsed 00:00:02.24
There was a problem hiding this comment.
That's strange. I installed the Microsoft.DotNet.SDK.10 with winget and then I was able to build it without issues. I'll investigate a bit more, but I did only that on a fresh Windows 11 VM without issues.
There was a problem hiding this comment.
I installed the sdk through scoop initially but PS7 could not find it even after assigning $env:DOTNET_ROOT manually, so I removed it and then installed it through winget too, this way it's able to find dotnet without issues but then I get that error message in both PS5.1 and PS7 as mentioned earlier
There was a problem hiding this comment.
Really strange. I assume you removed the DOTNET_ROOT env variable after trying and restarted the shell? I'll check again on a new VM this evening.
There was a problem hiding this comment.
Of course, also because installing any application through winget is not added to $PATH automatically like scoop so you always have to start a new session or update $PATH yourself.
2 participants
This PR completely rewrites the parser logic with C# for improved type handling, memory handling and performance. It also gets rid of the CIM instance instantiation, which was previously used for checking the underlying resource state. This is now no longer a part of the module.
Some figures: The new logic is massively quicker to convert from and to dsc objects. Even without CIM instantiations, the parsing is between 2 and 5 times faster, and the moment CIM instantiations (with elevated privileges in PowerShell) are required, it cannot be compared anymore. The bigger the configuration to parse, the greater the performance benefits.