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

.net 8 P4 Brotli compress task not functioning as in previous previews #48308

Closed
1 task done
MarkStega opened this issue May 18, 2023 · 15 comments
Closed
1 task done

.net 8 P4 Brotli compress task not functioning as in previous previews #48308

MarkStega opened this issue May 18, 2023 · 15 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Milestone

Comments

@MarkStega
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have a Brotli compress task in a number of projects in the form of

		<!--Call the BlazorWebAssembly Task BrotliCompress and store the results in MyCompressedFiles-->
		<BrotliCompress FilesToCompress="@(MyStaticFiles)" OutputDirectory="$(IntermediateOutputPath)compress\" ToolAssembly="$(_BlazorWebAssemblySdkToolAssembly)" ToolExe="$(_RazorSdkDotNetHostFileName)" ToolPath="$(_RazorSdkDotNetHostDirectory)">
			<Output TaskParameter="CompressedFiles" ItemName="MyCompressedFiles" />
		</BrotliCompress>

This work through DN8P3 but is broken in DN8P4. It errors with

Error	MSB4044	The "BrotliCompress" task was not given a value for the required parameter "OutputDirectory".	ICEBG.Client	C:\Program Files\dotnet\sdk\8.0.100-preview.4.23260.5\Sdks\Microsoft.NET.Sdk.StaticWebAssets\targets\Microsoft.NET.Sdk.StaticWebAssets.Compression.targets	132	

I see the 'compress' directory does get created under the 'obj'

Expected Behavior

I expect that the task completes as it did previously.

Steps To Reproduce

https://github.com/MarkStega/ICEBG branch vNext

Just clone, checkout the vNext branch, and do a build from within VS (I 1m using 17.7 P1)

Exceptions (if any)

No response

.NET Version

8.0.100-preview.4.23260.5

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label May 18, 2023
@mkArtakMSFT mkArtakMSFT added this to the Discussions milestone May 18, 2023
@MackinnonBuck
Copy link
Member

Thanks for reaching out, @MarkStega.

The BrotliCompress task is an internal task not meant for public consumption, so breaking changes can be expected between releases.

However, we now have public APIs that can be used to mark assets for compression.

Could you please try replacing your code with the following?

<ItemGroup>
  <AssetToCompress Include="@(MyStaticFiles)" />
</ItemGroup>

@MackinnonBuck MackinnonBuck added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label May 18, 2023
@ghost
Copy link

ghost commented May 18, 2023

Hi @MarkStega. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@MarkStega
Copy link
Author

@MackinnonBuck

Thanks for the prompt reply. I had a whole section of csproj modeled after something I found on the .Net Foundation web site. The 'AssetToCompress' worked perfectly once and I was excited to be able to remove about 50 lines of 'cruft' from the custom csproj elements.

Unfortunately, after removing the unused code, the 'AssetToCompress' stopped functioning. Here is the current csproj section:

	<Target Name="GZBRCompression" AfterTargets="Build">

		<!--Choose what to compress-->
		<ItemGroup>
			<MyStaticFiles Include="$(ProjectDir)/wwwroot/**/*.css" />
			<MyStaticFiles Include="$(ProjectDir)/wwwroot/**/*.js" />
		</ItemGroup>

		<!--Log what we will do-->
		<Message Importance="High" Text="======= Brotli/GZip source has @(MyStaticFiles-&gt;Count()) static Files =======" />
		<Message Importance="High" Text=" Brotli/GZip source files: %(MyStaticFiles.OriginalItemSpec)%(MyStaticFiles.Extension)" />

		<!--Compress-->
		<ItemGroup>
			<AssetToCompress Include="@(MyStaticFiles)" />
		</ItemGroup>

		<!--Collect output from the compression-->
		<ItemGroup>
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.br" />
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.gz" />
		</ItemGroup>

		<!--Log what we have done-->
		<Message Importance="High" Text="======= Brotli/GZip compression resulted in @(MyCompressedFiles-&gt;Count()) static files =======" />
		<Message Importance="High" Text=" Brotli/GZip compressed files: %(MyCompressedFiles.OriginalItemSpec)%(MyCompressedFiles.Extension)" />

	</Target>

The logging shows:

5>ICEBG.Client -> C:\Solutions\OHI\ICEBG-P12.5\.artifacts\bin\ICEBG.Client\devserver\ICEBG.Client.dll
5>======= Brotli/GZip source has 4 static Files =======
5> Brotli/GZip source files: .css
5> Brotli/GZip source files: .js
5>======= Brotli/GZip compression resulted in 0 static files =======
5> Brotli/GZip compressed files:
5>Done building project "ICEBG.Client.csproj".

This shows the correct count of files to compress (2 .js and 3 .css). However, there are no brotli or gzip files being produced. Any thoughts?

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels May 19, 2023
@IvanJosipovic
Copy link

I also use this msbuild task and would love for this to be baked into the Blazor SDK. This way we can use the manual startup process documented here https://learn.microsoft.com/en-us/aspnet/core/blazor/host-and-deploy/webassembly?view=aspnetcore-7.0#compression and have the most size efficient static sites.

@MackinnonBuck
Copy link
Member

@MarkStega, defining AssetToCompress items doesn't actually perform the compression - it just marks assets for compression, which happens later in the build pipeline. This is why it appears as if no files are getting compressed - the check is happening too early. If you want to do something with the compressed files on disk, you can define a target that runs after GenerateComputedBuildStaticWebAssets. Hope this helps!

@MackinnonBuck
Copy link
Member

@IvanJosipovic, is there a reason why defining compressed assets via AssetToCompress doesn't work for your scenario?

@MarkStega
Copy link
Author

MarkStega commented May 19, 2023

@MackinnonBuck

Thanks for the hint, I changed my csproj to have:

	<Target Name="GZBRCompression" BeforeTargets="GenerateComputedBuildStaticWebAssets">

		<!--Choose what to compress-->
		<ItemGroup>
			<MyStaticFiles Include="$(ProjectDir)/wwwroot/**/*.css" />
			<MyStaticFiles Include="$(ProjectDir)/wwwroot/**/*.js" />
		</ItemGroup>

		<!--Log what we will do-->
		<Message Importance="High" Text="======= Brotli/GZip source has @(MyStaticFiles-&gt;Count()) uncompressed files =======" />
		<Message Importance="High" Text=" Brotli/GZip source files: %(MyStaticFiles.OriginalItemSpec)%(MyStaticFiles.Extension)" />

		<!--Compress-->
		<ItemGroup>
			<AssetToCompress Include="@(MyStaticFiles)" />
		</ItemGroup>

	</Target>
	
	<Target Name="ReportCompression" AfterTargets="GenerateComputedBuildStaticWebAssets">
		
		<!--Collect output from the compression-->
		<ItemGroup>
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.br" />
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.gz" />
		</ItemGroup>

		<!--Log what we have done-->
		<Message Importance="High" Text="======= Brotli/GZip compression resulted in @(MyCompressedFiles-&gt;Count()) compressed files =======" />
		<Message Importance="High" Text=" Brotli/GZip compressed files: %(MyCompressedFiles.OriginalItemSpec)%(MyCompressedFiles.Extension)" />

	</Target>

The log output shows:

1>======= Brotli/GZip source has 4 uncompressed files =======
1> Brotli/GZip source files: .css
1> Brotli/GZip source files: .js
1>======= Brotli/GZip compression resulted in 0 compressed files =======
1> Brotli/GZip compressed files:
1>ICEBG.Client -> C:\Solutions\OHI\ICEBG-P12.5\.artifacts\bin\ICEBG.Client\devserver\ICEBG.Client.dll
1>Done building project "ICEBG.Client.csproj".

Looking at the wwwroot directory there are no compressed files. Is there a better 'BeforeTargets for the prep stage?

@MackinnonBuck
Copy link
Member

@MarkStega Sorry, I just realized that your MyStaticFiles items define file name patterns rather than actual StaticWebAsset items.

Rather than using AssetToCompress, which expects "static web asset" items, could you instead try this?

<PropertyGroup>
  <CompressionIncludePatterns>$(CompressionIncludePatterns);$(ProjectDir)/wwwroot/**/*.css;$(ProjectDir)/wwwroot/**/*.js</CompressionIncludePatterns>
</PropertyGroup>

@MarkStega
Copy link
Author

@MackinnonBuck No luck:

	<Target Name="GZBRCompressionSetup" BeforeTargets="GenerateComputedBuildStaticWebAssets">

		<!--Choose what to compress-->
		<ItemGroup>
			<MyUncompressedFiles Include="$(ProjectDir)/wwwroot/**/*.css" />
			<MyUncompressedFiles Include="$(ProjectDir)/wwwroot/**/*.js" />
		</ItemGroup>

		<!--Log what we will do-->
		<Message Importance="High" Text="======= Brotli/GZip source has @(MyUncompressedFiles-&gt;Count()) uncompressed files =======" />
		<Message Importance="High" Text=" Brotli/GZip source files: %(MyUncompressedFiles.OriginalItemSpec)%(MyUncompressedFiles.Extension)" />

		<!--Compress specification-->
		<PropertyGroup>
			<CompressionIncludePatterns>$(CompressionIncludePatterns);$(ProjectDir)/wwwroot/**/*.css;$(ProjectDir)/wwwroot/**/*.js</CompressionIncludePatterns>
		</PropertyGroup>

	</Target>

	<Target Name="ReportCompression" AfterTargets="GenerateComputedBuildStaticWebAssets">

		<!--Collect output from the compression-->
		<ItemGroup>
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.br" />
			<MyCompressedFiles Include="$(ProjectDir)/wwwroot/**/*.gz" />
		</ItemGroup>

		<!--Log what we have done-->
		<Message Importance="High" Text="======= Brotli/GZip compression resulted in @(MyCompressedFiles-&gt;Count()) compressed files =======" />
		<Message Importance="High" Text=" Brotli/GZip compressed files: %(MyCompressedFiles.OriginalItemSpec)%(MyCompressedFiles.Extension)" />

	</Target>
------------------------------
1>======= Brotli/GZip source has 4 uncompressed files =======
1> Brotli/GZip source files: .css
1> Brotli/GZip source files: .js
1>======= Brotli/GZip compression resulted in 0 compressed files =======
1> Brotli/GZip compressed files:
1>ICEBG.Client -> C:\Solutions\OHI\ICEBG-P12.5\.artifacts\bin\ICEBG.Client\devserver\ICEBG.Client.dll
1>Done building project "ICEBG.Client.csproj".

@MackinnonBuck
Copy link
Member

@MarkStega Could you define the CompressionIncludePatterns property outside of a target? Its value is used in the ResolveCompressedFiles target, so you'll want to make sure the property is set before that runs.

Also just to confirm: This is a Blazor WebAssembly app you're working with?

If this still doesn't work, you could try generating a binlog and making sure the ResolveCompressedFiles and CompressFiles targets are running, and that CompressionIncludePatterns is up-to-date by the time ResolveCompressedFiles gets run.

@MarkStega
Copy link
Author

@MackinnonBuck

No, the particular csproj is a "Microsoft.NET.Sdk.Razor" project that produces a dll that can be used by either a server hosted or wasm hosted blazor app.

Moving the CompressionIncludePatterns outside of the targetdidn't help. Earlier in the same csproj is code to build the js & css files that are to be compressed. (As an aside, I wish that #38445 wasn't getting kicked down the road yet again. It was a nice hint of things to come in Net6, it got deferred at the end of Net 6, all of Net 7, and looks like the same will happen in Net 8)

Anyhow, since the property is moved out of the target and it fails I'll get the binlog and see what is going on and report back.

@MarkStega
Copy link
Author

@MackinnonBuck

I haven't turned on the high resolution mode yet but I can see that the _ResolveCompressedFiles and _CompressFile targets are executing before the GZBRCompressionSetup target.

I changed the 'BeforeTargets' to 'UpdateExistingPackageStaticWebAssets' which brought it to the correct location.

I'll turn on the detailed logs tomorrow and actually examine the values.

I did notice that the _CompressFiles target only does a BrotliCompress in this csproj while in the project that build the Blazor dll that both Brotli & GZip compression tasks are run.

@MackinnonBuck
Copy link
Member

@MarkStega

I'm thinking the issue is probably that there's something wrong with the include pattern. I just tried creating a RazorClassLibrary (from dotnet new razorclasslib) and added the following to the .csproj file:

<PropertyGroup>
  <CompressionIncludePatterns>$(CompressionIncludePatterns);*.png</CompressionIncludePatterns>
</PropertyGroup>

This resulted in background.png getting picked up for compression, and it showed up in the obj/Debug/net8.0/compressed directory.

You could try changing the include patterns to just **/*.css and **/*.js, since the patterns are already relative to wwwroot.

I did notice that the _CompressFiles target only does a BrotliCompress in this csproj while in the project that build the Blazor dll that both Brotli & GZip compression tasks are run.

Hm, not sure why only Brotli compression would run. The default behavior is that Gzip compression runs during build and both Gzip and Brotli run during publish. You can configure this via the BuildCompressionFormats and PublishCompressionFormats properties, e.g.:

<PropertyGroup>
  <BuildCompressionFormats>gzip</BuildCompressionFormats>
  <PublishCompressionFormats>gzip;brotli</PublishCompressionFormats>
</PropertyGroup>

@MarkStega
Copy link
Author

MarkStega commented May 20, 2023

@MackinnonBuck

The change to the pattern resulted in compressed files getting created and changing the BuildCompressionFormats to add brotli also worked.

However, the compressed files in the artifact 'compressed' directory have lost their original names and the results are not getting copied to wwwroot

:
explorer_2023-05-20_12-55-02

Also, what is getting compressed includes the files in 'staticwebassets'.

So it appears that trying to use the public APIs is the wrong approach for the build. The good news is that I can use the detailed log to understand the parameters to the internal gzip & brotli tasks so I can revert to a custom build step (unless you have a better idea).

CentralNode_devenv_PID=20484_x64_BuildManager_Default.zip

[EDIT] I may have spoken too soon about the public apis not being useful. I published the server project (that uses ICEBG.Client) and the publish directory for content (C:\Solutions\OHI\ICEBG-P12.5.artifacts\publish\ICEBG.Web.UserInterface\devserver\wwwroot_content\ICEBG.Client) and I see that properly named btotli & gzip files exist.

So a series of questions:

  1. Why bother to compress in a build step when all it results in are a series of files in the 'compressed' folder?

  2. Also, in _content, there are three directories that correspond to three nuget packages that were included in ICEBG.Client and they indeed have the 'raw' js & css from those packages. None of these files have corresponding brotli/gzip versions.
    Is there another step that I need to add in the build process to get those compressed?

  3. Why does the build step compress the js/css that come from the nuget packages while the publish does not?

  4. Are you wishing that you didn't get to interact on this issue :-) ?

Published _content directory:
explorer_2023-05-21_07-39-23

Published _content\ICEBG.Client\css directory:
explorer_2023-05-21_07-39-36

Published _Content\Material.Blazor directory:
explorer_2023-05-21_07-39-51

@ghost
Copy link

ghost commented Jul 20, 2023

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Jul 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

5 participants
@MarkStega @IvanJosipovic @MackinnonBuck @mkArtakMSFT and others