search

koenvzeijl / AspNetCore.SassCompiler

Sass Compiler Library for .NET Core 3.1/5.x/6.x/7.x without node, using dart-sass as a compiler
https://www.nuget.org/packages/AspNetCore.SassCompiler/
MIT License
220 stars 26 forks source link

v1.74.1 broke CI build on ubuntu #178

Open mikes-gh opened 7 months ago

mikes-gh commented 7 months ago

Describe the bug It seems the latest update breaks our CI build. The CSS is just not there or in the wrong place (not sure which) but I see nothing in the logs either. Building locally on macOS seems fine too. I suspect the linux64 runtime.

To Reproduce Steps to reproduce the behaviour:

  1. Use v1.74.1 the CSS is missing after deploying of website https://github.com/MudBlazor/MudBlazor/actions/runs/8602958141
  2. Revert to v1.72.0 everything is fine again https://github.com/MudBlazor/MudBlazor/actions/runs/8606643719

Expected behaviour CSS is deployed to the website.

mikes-gh commented 7 months ago

Happy to help debug on a PR on our site. Just need to know how to log some more detailed info

sleeuwen commented 7 months ago

Hi @mikes-gh , thanks for reporting the issue, sadly I don't have time to check this today but it might be helpful if you could run a build/publish with diagnostic logging enabled (add -v:diag to the dotnet publish command), that should show a target "CompileSass" and why it it might be skipped or if it's not skipped it shows the exact sass commands which are executed.

I think I should have time to check this myself tomorrow.

sleeuwen commented 7 months ago

Hi @mikes-gh ,

I've had some time to debug this and was able to reproduce the same issue you were having in the CI build with a local docker container. I believe I found the problem and why it only occurs on the latest version. (sorry for the long write-up, this took a bit of time to figure out, and I found it interesting, so here is a full explanation of how I came to this conclusion :stuck_out_tongue:)

First some background, we had recently received an issue that when using dotnet watch run, it would constantly reload the .css files without there being any changes to the file (#176). I found out that it was an issue with passing an absolute path to the dart-sass compiler and the --update flag, that combination resulted in the dart-sass compiler to always update the output file, even if there were no changes in the file. I had filed an issue in the dart-sass repository for this, which they have fixed and is now included in the 1.74.1 version.

So what does this have to do with the issue you're having? With the diagnostic logging enabled, I noticed that the Compile Sass task for the MudBlazor project ran, but it didn't return any outputs. This means that the sass command we executed didn't change any files (which can happen if the output file is already up-to-date, as we use the --update flag). But it was a clean project so the .css definitely wasn't there before running dotnet publish (I always ran git clean -xdf to go back to a clean state without any artifacts, before running the dotnet publish command you use in the CI build). Eventually I noticed that the MudBlazor.min.css file that is missing in the new builds was created before the Compile Sass target was started. Looking through the msbuild logs there weren't any logs at the time it was created, but the first thing logged after the time the MudBlazor.min.css file was generated was: Done building target "CompileDocs" in project "MudBlazor.Docs.csproj".. So I had a look what is this "CompileDocs" task it's refering to, and found that this is a custom target that you use, which runs dotnet run --configuration release --project "$(SolutionDir)MudBlazor.Docs.Compiler/MudBlazor.Docs.Compiler.csproj" (if the Debug dll doesn't exist, this is probably the reason why it works for you locally as I'm guessing the Debug dll does exist for you while in the pipeline it doesn't, because that's a clean build). When I looked at the MudBlazor.Docs.Compiler project, I found that it has a project reference to the MudBlazor project.

So, this is what happens:

So, (TLDR?) With the previous 1.72.0, the --update flag didn't work correctly with absolute paths. This meant that when it ran the Compile Sass target the second time (in the process that builds the WasmHost project, first time was in the dotnet run MudBlazor.Docs.Compiler process), it would still update the file even though it didn't contain any changes and it wrote to standard output that it compiled the MudBlazor.min.css file. This meant that we added the MudBlazor.min.css file to the Content item group of MSBuild and it was included in the publish output as the .NET SDK looks at the Content ItemGroup as well. After updating to the 1.74.1 version, the bug was fixed so it no longer updates the output .css file. This in turn means it doesn't log to standard output that it compiled the .css file, thus we don't add it to the Content ItemGroup anymore and it isn't picked up by the .NET SDK.

I'm not sure there is something we are able to do about this, as we rely on the output of the sass command to tell which files are generated, and as you're allowed to supply a folder as well as a file we don't want to have the logic of what file would get generated in what case within the library and would have to keep that in sync with the sass project. Maybe now that you know why the problem occurs there is something you can change to avoid this problem?

One way I could think of is, there are a few MSBuild properties that we use to pass the required information to the Compile Sass task, two of which are SassCompilerAppsettingsJson and SassCompilerSassCompilerJson, it might be possible to override those properties from the command line when running the docs compiler so that they point to invalid files and the Compile Sass task doesn't compile anything in the sub-process. The SassCompilerBuildCommand property is another one we use that has to be set, so maybe you can set it to an empty value but I'm not sure if that could work as we could also just always overwrite the content of that property. Another option might be if we would add a property with which you can just directly disable the Compile Sass target (we already disable it in design-time builds).

I hope this helps you at least with determining a way to fix the problem, if we can help with getting a solution working let us know.

mikes-gh commented 7 months ago

@sleeuwen I appreciate your in depth review of the situation. I will need to read it a few times to get the full picture. Since this is a multi project solution the msbuild order of events is not straight forward. Is there any way of overriding the --update flag

sleeuwen commented 7 months ago

That is not possible no, we add the --update flag always after any custom arguments as only with the --update flag it will write to standard output what files it had compiled. If we don't add the flag it doesn't write anything to standard output so then we also don't know which files were created.