Escape output item specs for ResolvePackageAssets task. #3112
Conversation
|
@nkolev92 So why does ContentFilesExample use %2B in contentFiles, when the common pattern is an actual + in lib?
Looking at this msbuild code has me thinking there are probably a lot of bugs around files and directories with names like that across the stack. |
src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
At a minimum, I'd like to make sure we don't slow down the incremental case (which I think we can fix by putting the escaped value in the cache and not escaping on read from the cache).
I'm also very curious to hear @rainersigwald's take because I'm guessing there have been a bunch of bugs on what happens to ItemSpec's on get. Is there a recommended pattern around that in general?
Finally, I'd like to understand if the ContentFileExample is representative of a real-world use case. Why is that package using %2B instead of + like all the other packages I've seen?
|
Escaping is indeed a bug farm, even inside the engine. I think it makes sense to escape before passing to the item constructor, and that it makes sense to do that once, not for every incremental build. I also agree that NuGet's behavior that spawned this is surprising. Sorta related: dotnet/msbuild#2178 |
This commit fixes the ResolvePackageAssets task to escape the output item specs. This allows assets in packages to contain characters that might get unescaped by MSBuild. Fixes dotnet#3069.
peterhuene
force-pushed
the
fix-resolve-package-assets
branch
from
d83d3f8
to
165ff97
Compare
….3 (dotnet#3112) - Microsoft.NET.Sdk - 3.1.100-preview1.19505.3
This commit fixes the ResolvePackageAssets task to escape the output
item specs. This allows assets in packages to contain characters that
might get unescaped by MSBuild.
Fixes #3069.