[Static web assets] Account for HintPath when we compute the build assets #19873
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javiercn
commented
Aug 19, 2021
Comment on lines
203
to
206
| if (string.Equals(candidate.GetMetadata("ResolvedFrom"), "{HintPathFromItem}", StringComparison.Ordinal)) | ||
| { | ||
| candidate.SetMetadata("OriginalItemSpec", candidate.ItemSpec); | ||
| } |
There was a problem hiding this comment.
We need this to account for the fact that the assembly was resolved using HintPath, otherwise static web assets thinks the .csproj is the original representation of the item and will try to copy that to the output folder.
Will be adding some more unit tests in this area
* Fixes an issue with projects not working with reference assemblies using HintPath. * Fixes an issue where files without an extension where being included inside an additional folder with the file name on nuget packages. * Adds additional error logging for the brotli compression tool. * Adds additional login for the gzip compress task. * Adds editor config files in the blazor wasm and razor sdk folders to handle 'var' usage preferences in ASP.NET Core projects. * Performs a more selective filtering for static web assets candidates from the build candidates in blazor webassembly applications.
* Moves static web assets pack tests with the rest of the static web assets tests. * Adds multiple additional pack tests * Packages targeting multiple frameworks. * Pack with no build * Adds build and publish tests for libraries using Reference and HintPath * Add baseline test for publish with GenerateDocumentationFile=true * Adds tests to validate the criteria we use to select the source file to use for compressing static web assets during build
javiercn
force-pushed
the
javiercn/dotnet-sdk-swa-fixes
branch
from
2821a18
to
cd1f4d2
Compare
javiercn
commented
Aug 20, 2021
| } | ||
|
|
||
| [Fact] | ||
| public void Pack_StaticWebAssets_WithoutFileExtension_AreCorrectlyPacked() |
There was a problem hiding this comment.
GeneratePackageOnBuild doesn't matter (as the issue mentions)
javiercn
commented
Aug 20, 2021
| } | ||
|
|
||
| [Fact] | ||
| public void Build_Works_WithLibraryUsingHintPath() |
There was a problem hiding this comment.
javiercn
commented
Aug 20, 2021
| @@ -103,6 +103,54 @@ public void Publish_WithDefaultSettings_Works() | |||
| VerifyTypeGranularTrimming(blazorPublishDirectory); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void Publish_Works_WithLibraryUsingHintPath() | |||
There was a problem hiding this comment.
javiercn
commented
Aug 20, 2021
| @@ -147,6 +147,37 @@ public void StaticWebAssets_Publish_Hosted_Works() | |||
| intermediateOutputPath); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void StaticWebAssets_Publish_DoesNotIncludeXmlDocumentationFiles_AsAssets() | |||
There was a problem hiding this comment.
Just a test to validate dotnet/aspnetcore#32744
javiercn
commented
Aug 20, 2021
Comment on lines
+234
to
255
| var fromMonoPackage = string.Equals( | ||
| candidate.GetMetadata("NuGetPackageId"), | ||
| "Microsoft.NETCore.App.Runtime.Mono.browser-wasm", | ||
| StringComparison.Ordinal); | ||
|
|
||
| reason = extension switch | ||
| { | ||
| ".a" => "extension is .a is not supported.", | ||
| ".c" => "extension is .c is not supported.", | ||
| ".h" => "extension is .h is not supported.", | ||
| ".a" when fromMonoPackage => "extension is .a is not supported.", | ||
| ".c" when fromMonoPackage => "extension is .c is not supported.", | ||
| ".h" when fromMonoPackage => "extension is .h is not supported.", | ||
| // It is safe to filter out all XML files since we are not interested in any XML file from the list | ||
| // of ResolvedFilesToPublish to become a static web asset. Things like this include XML doc files and | ||
| // so on. | ||
| ".xml" => "it is a documentation file", | ||
| ".rsp" => "extension is .rsp is not supported.", | ||
| ".props" => "extension is .props is not supported.", | ||
| ".rsp" when fromMonoPackage => "extension is .rsp is not supported.", | ||
| ".props" when fromMonoPackage => "extension is .props is not supported.", | ||
| ".blat" when !timezoneSupport => "timezone support is not enabled.", | ||
| ".dat" when invariantGlobalization && fileName.StartsWith("icudt") => "invariant globalization is enabled", | ||
| ".json" when fileName == "emcc-props" => $"{fileName}{extension} is not used by Blazor", | ||
| ".json" when fromMonoPackage && fileName == "emcc-props" => $"{fileName}{extension} is not used by Blazor", | ||
| ".js" when fileName == "dotnet" => "dotnet.js is already processed by Blazor", | ||
| ".js" when assetType == "native" => $"{fileName}{extension} is not used by Blazor", | ||
| ".pdb" when !copySymbols => "copying symbols is disabled", |
There was a problem hiding this comment.
pranavkm
approved these changes
Aug 20, 2021
src/RazorSdk/Tasks/StaticWebAssets/ComputeStaticWebAssetsTargetPaths.cs
Outdated
Show resolved
Hide resolved
…tPaths.cs Co-authored-by: Pranav K <prkrishn@hotmail.com>
javiercn
commented
Aug 20, 2021
| @@ -295,7 +295,8 @@ Copyright (c) .NET Foundation. All rights reserved. | |||
|
|
|||
| <Target Name="ResolveStaticWebAssetsConfiguration"> | |||
| <PropertyGroup> | |||
| <_StaticWebAssetsManifestBase>$(BaseIntermediateOutputPath)$(Configuration)\$(TargetFramework.ToLowerInvariant())\</_StaticWebAssetsManifestBase> | |||
| <_StaticWebAssetsManifestBase>$(BaseIntermediateOutputPath)$(Configuration)\</_StaticWebAssetsManifestBase> | |||
| <_StaticWebAssetsManifestBase Condition="'$(AppendTargetFrameworkToOutputPath)' != 'false'">$(_StaticWebAssetsManifestBase)\$(TargetFramework.ToLowerInvariant())\</_StaticWebAssetsManifestBase> | |||
There was a problem hiding this comment.
javiercn
commented
Aug 20, 2021
| @@ -737,6 +737,46 @@ public void PublishProjectWithReferences_NoBuild_GeneratesPublishJsonManifestAnd | |||
| intermediateOutputPath); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void PublishProjectWithReferences_AppendTargetFrameworkToOutputPathFalse_GeneratesPublishJsonManifestAndCopiesPublishAssets() | |||
There was a problem hiding this comment.
javiercn
commented
Aug 20, 2021
src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.StaticWebAssets.targets
Outdated
Show resolved
Hide resolved
…argets Avoid double slashes
|
/backport to release/6.0.1xx-rc1 |
This was referenced Sep 7, 2021
|
Had this issue after moving a .net 5.0 blazor app to 6.0. Error went away after navigating to ..\obj\Debug and just an 'md net6.0' to create a net6.0 path. Simple missing directory check on the MS side. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes dotnet/aspnetcore#35349
Fixes dotnet/aspnetcore#32744
Fixes dotnet/aspnetcore#29561
Fixes dotnet/aspnetcore#35561