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

CSS is not included in pack of multi-targeted library #224

Open
mikes-gh opened this issue Jan 21, 2025 · 3 comments
Open

CSS is not included in pack of multi-targeted library #224

mikes-gh opened this issue Jan 21, 2025 · 3 comments

Comments

@mikes-gh
Copy link

mikes-gh commented Jan 21, 2025

Describe the bug
Please see #218 for context

That PR fixed a race condition that could lock input/output files when dart-sass was running concurrently.
For example when dotnet packing a multi-targeted build.
By adding the mutex it seems that although the race condition with regards to file locking is fixed, ocassionally the css was not included in dotnet pack. It appears depending on which framework was building the css it might not get included.
I am assuming this is due to the --update flag of dart sass which will result in no stdout when building the scss --> css a second time.

Also related #178

My theory is that this behaviour is to do with the logic that msbuild uses to pickup the staticwebassets for packing in a Blazor library.
Presumably the way in which the staticwebassets is determined is somehow tied to one of the frameworks in a multi-target library.

To test this theory I set our project to unconditionally add MudBlazor.min.css after SassCompiler had run.
See MudBlazor/MudBlazor#10643

To Reproduce
Steps to reproduce the behavior:

  1. Download HEAD of MudBlazor (uses SassCompiler 1.83.0 no mutex)

  2. Always includes MudBlazor.min.css in dotnet pack

  3. Update SassCompiler package to 1.83.1

  4. Sometimes omits MudBlazor.min.css in dotnet pack

  5. Download Build: Fix for missing MudBlazor.min.css in SassCompiler MudBlazor/MudBlazor#10643

  6. Always includes MudBlazor.min.css in dotnet pack

Expected behavior
Always includes MudBlazor.min.css in dotnet pack

Solution
Determine output css from sasscompiler.json. And always include in the Content Items. For folders this would require a bit of logic.
I can do a PR if required.

@mikes-gh mikes-gh changed the title CSS is not included in publish of multitargeted library CSS is not included in publish of multi-targeted library Jan 21, 2025
@mikes-gh
Copy link
Author

BTW doing a Remove Include on Content Items is actually better for incremental compilation support.
Since MSBuild does a hash of the files and the order of the files matters.
The way it is at the momement MSBuild still has to reevaluate on the second build.

@mikes-gh mikes-gh changed the title CSS is not included in publish of multi-targeted library CSS is not included in pack of multi-targeted library Jan 21, 2025
@adiessl
Copy link
Contributor

adiessl commented Jan 27, 2025

So what command is being used exactly? dotnet pack MudBlazor.csproj? This is run on a clean solution, so a restore and build happens in the same process? Does it make a difference if you run dotnet build MudBlazor.csproj followed by dotnet pack MudBlazor.csproj --no-build? Would you be able to provide a binary log (dotnet pack MudBlazor.csproj -bl, see https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Providing-Binary-Logs.md, make sure that it does not include secrets etc.) of a problematic build and one that is fine so we can compare them?

MudBlazor/MudBlazor#10643 was merged in the meantime, does that mean you are fine with this solution? It seems to me to be less of a hack than determining the CSS files to be included from the contents of sasscompiler.json (which might not even exist, as the configuration can also be done via appsettings.json).

@mikes-gh
Copy link
Author

mikes-gh commented Jan 27, 2025

So what command is being used exactly?

dotnet pack -c Release

MudBlazor/MudBlazor#10643

Had to be reverted as it caused duplicate asset errors
https://github.com/MudBlazor/MudBlazor/actions/runs/12968901739/job/36172389253#step:7:35

We are currently using 1.83.0 and living with the occasional file contention errors.

I dont have bin logs only normal diag build logs.
From these I have made my conclusions in the above post.

We have decided to fork in order to support multi-targeted build/publish.

We already have a working template in https://github.com/MudBlazor/JSCompiler

It seems to me to be less of a hack than determining the CSS files to be included from the contents of sasscompiler.json (which might not even exist, as the configuration can also be done via appsettings.json).

We dont beleive this to be a hack if done correctly.

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