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

Make sure to compile SCSS files only once for multi target projects #215

Closed

Conversation

adiessl
Copy link
Contributor

@adiessl adiessl commented Jan 2, 2025

This (hopefully) fixes #209.

See #209 (comment) for more details.

<ShouldRunTarget>false</ShouldRunTarget>
<!-- to prevent targets from being run extra times, enforce that only the outer
build of a multi-targeted project or a single-targeted build can run -->
<ShouldRunTarget Condition="'$(TargetFrameworks)' == '' OR $(IsOuterBuild)">true</ShouldRunTarget>
Copy link

@mikes-gh mikes-gh Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be interested to know what the value of $(TargetFrameworks) is when building the MudBlazor.Docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When building MudBlazor.Docs, the value of TargetFrameworks for MudBlazor is still net8.0;net9.0 and since there is no outer build in this case, the target is not run.

Copy link

@mikes-gh mikes-gh Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspected that. I wonder if the parent project sets the TargetFramework and whether Condition="'$(TargetFramework)' != ''" (note the singular) might work.

Copy link

@mikes-gh mikes-gh Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added

  <Target Name="ShowFramework" BeforeTargets="Build">
    <Message Importance="High" Text="Before build Target Framework: '$(TargetFramework)'" />
    <Message Importance="High" Text="Before build Target Frameworks: '$(TargetFrameworks)'" />
  </Target>

To the MudBlazor.csproj

building MudBlazor the library gives

  Determining projects to restore...
  All projects are up-to-date for restore.
  MudBlazor.Analyzers -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.Analyzers/bin/Debug/netstandard2.0/MudBlazor.Analyzers.dll
  MudBlazor.SourceGenerator -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.SourceGenerator/bin/Debug/netstandard2.0/MudBlazor.SourceGenerator.dll
  wwwroot/MudBlazor.min.js UpToDate
  wwwroot/MudBlazor.min.js UpToDate
  MudBlazor -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor/bin/Debug/net8.0/MudBlazor.dll
  Before build Target Framework: 'net8.0'
  Before build Target Frameworks: 'net8.0;net9.0'
  MudBlazor -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor/bin/Debug/net9.0/MudBlazor.dll
  Before build Target Framework: 'net9.0'
  Before build Target Frameworks: 'net8.0;net9.0'
  Before build Target Framework: ''                   <----- I think this is the outer build
  Before build Target Frameworks: 'net8.0;net9.0'

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:11.27

building MudBlazor.Docs gives

  Determining projects to restore...
  All projects are up-to-date for restore.
  MudBlazor.Analyzers -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.Analyzers/bin/Debug/netstandard2.0/MudBlazor.Analyzers.dll
  MudBlazor.SourceGenerator -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.SourceGenerator/bin/Debug/netstandard2.0/MudBlazor.SourceGenerator.dll
  wwwroot/MudBlazor.min.js UpToDate
  MudBlazor -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor/bin/Debug/net9.0/MudBlazor.dll
  Before build Target Framework: 'net9.0'
  Before build Target Frameworks: 'net8.0;net9.0'
  MudBlazor.Examples.Data -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.Examples.Data/bin/Debug/net9.0/MudBlazor.Examples.Data.dll
  Generating Docs
  XML Documentation Coverage for MudBlazor:
  
  Types:      337 of 421 (80%) types
  Properties: 2260 of 2772 (82%) properties
  Methods:    452 of 759 (60%) methods
  Fields:     280 of 398 (70%) fields
  Events:     171 of 184 (93%) events/EventCallback
  
  Docs.Compiler updated 0 generated files
  Docs.Compiler generated 0 new files
  Docs.Compiler completed in 2937 milliseconds.
  MudBlazor.Docs -> /Users/mike/Documents/Repos/MudBlazor/src/MudBlazor.Docs/bin/Debug/net9.0/MudBlazor.Docs.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:29.30

Which would lead me to think that there is a type in the original article and that it should be

<ShouldRunTarget Condition="'$(TargetFramework)' != '' OR $(IsOuterBuild)">true</ShouldRunTarget>

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not even sure the target framework needs to be checked tbh since if we are are doing multitargeting we change th BeforeTargets to DespatchToInnerBuilds but I probably need to do some more testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to

<ShouldRunTarget Condition="'$(TargetFramework)' != '' OR $(IsOuterBuild)">true</ShouldRunTarget>

and added

<Target Name="ShowFramework" BeforeTargets="Compile Sass">
  <Message Importance="High" Text="Before build Target Framework: '$(TargetFramework)'" />
  <Message Importance="High" Text="Before build Target Frameworks: '$(TargetFrameworks)'" />
</Target>

When I build the project MudBlazor.Docs I get the following output for the MudBlazor project:
image

This is for the project MudBlazor.Docs itself:
image

The condition from the blog post seems to be right after all.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you're right. It seems this solution doesn't work for project references since DispatchToInnerBuild doesnt happen when referencing the project. We could probably set the OuterBuild msbuild variable in the project reference but that seems pretty hacky

@mikes-gh
Copy link

mikes-gh commented Jan 4, 2025

dotnet/msbuild#1897 is an interesting article.

@adiessl
Copy link
Contributor Author

adiessl commented Jan 4, 2025

dotnet/msbuild#1897 is an interesting article.

I also found this yesterday and took it as inspiration for another implementation attempt for fixing our problem. Unfortunately this led to another kind of race condition. I will open another PR with the code later for documentation purposes.

@mikes-gh
Copy link

mikes-gh commented Jan 4, 2025

I'm now wondering whether serialising the sass compilation with a mutex as I first thought would not be a better bet and let msbuild do its thing.

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

Successfully merging this pull request may close these issues.

Using SassCompiler in a multitarget build causes file contention on Windows
3 participants