Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.74.1 broke CI build on ubuntu #178

Open
mikes-gh opened this issue Apr 8, 2024 · 5 comments
Open

v1.74.1 broke CI build on ubuntu #178

mikes-gh opened this issue Apr 8, 2024 · 5 comments

Comments

@mikes-gh
Copy link

mikes-gh commented Apr 8, 2024

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
Copy link
Author

mikes-gh commented Apr 8, 2024

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

@sleeuwen
Copy link
Collaborator

sleeuwen commented Apr 9, 2024

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
Copy link
Collaborator

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 😛)

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:

  • The dotnet publish command is executed for the MudBlazor.Docs.WasmHost project
  • Early in the process, the Content ItemGroup is populated by MSBuild, it does not contain the MudBlazor.min.css file as it doesn't exist yet
  • As part of building the MudBlazor.Docs.WasmHost project, it needs to run the MudBlazor.Docs's CompileDocs target
    • The CompileDocs target runs dotnet run MudBlazor.Docs.Compiler in a separate process
    • As part of running the MudBlazor.Docs.Compiler project, it needs to build the MudBlazor project
    • The MudBlazor project gets build, which runs the Compile Sass task that generates the MudBlazor.min.css, and it adds the MudBlazor.min.css to the Content item group so it will be correctly included when publishing (note, this is still in the separate process that runs the MudBlazor.Docs.Compiler, it doesn't know about the MSBuild context that's building the MudBlazor.Docs.WasmHost project)
    • Everything runs fine and the Compiler does it's thing
  • Back in the process that was building the WasmHost project, this project also transitively depends on the MudBlazor project, so we build that
  • We hit the Compile Sass target, but now the .css file already exists, we run the sass command with the --update flag, and as nothing has changed in the source files, the output file doesn't change and the file is not updated, we don't receive the output line that it has written to the MudBlazor.min.css file and thus don't update the Content item group in MSBuild
  • The rest of the build continues and publishes, but as the MudBlazor.min.css file isn't in the Content item group, it does not get published in the output folder.

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
Copy link
Author

@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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants