Merge main to 17.0 #6352
Merged
Merge main to 17.0 #6352
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
Co-authored-by: Matt Mitchell <mmitche@microsoft.com> Co-authored-by: Ben Villalobos <4691428+BenVillalobos@users.noreply.github.com>
Handle unsupported paths in ProjectInSolution.AbsolutePath (#6238)
Add an option to log global properties, properties and items on ProjectEvaluationFinishedEventArgs instead of ProjectStartedEventArgs. This option is currently only turned on by the BinaryLogger. This has several advantages. Currently only the projects that are built by the central node log their properties and items (properties are translated across nodes only if a special flag is set, and items are never translated). This resulted in properties and items not being available for projects built on other nodes. Now we log them after every evaluation and translate across nodes if needed. Together with the fact that we now log EvaluationId for each ProjectStarted, we can now recover properties and items for all project started events. This is the main purpose of this PR - to not lose properties and items like we currently do. We will still not log for project results that are satisfied by cache, because we don't keep track of evaluation for these. Presumably it will have already been logged previously. In addition, if more than one project are built from the same evaluation, we do not duplicate properties and items, only logging them once. This results in logging more information, but storing it more efficiently. Together with string and dictionary deduplication we see very significant savings in binlog size and some reduction in build time. This change has several large parts: 1. add a way to enumerate evaluation properties and items directly at the end of Evaluate() for PropertyDictionary<ProjectPropertyInstance> and ItemDictionary<ProjectItemInstance> 2. manual translation logic for ProjectEvaluationStarted and ProjectEvaluationFinished (instead of relying on TranslateDotNet/BinaryFormatter) 3. reading and writing ProjectEvaluationFinished GlobalProperties, Properties and Items in BuildEventArgsReader/Writer (used by BinaryLogger) 4. adding IEventSource4 with IncludeEvaluationPropertiesAndItems, to propagate this setting across nodes and threading it through the LoggingService 5. update the ParallelConsoleLogger and SerialConsoleLogger to print the new data, if present 6. tests One controversial design decision here is storing a reference to live evaluation data in ProjectEvaluationFinishedEventArgs. It does not make a snapshot of the data to avoid very significant allocations. It does take the lock on the PropertyDictionary<T>/ItemDictionary<T> when enumerating, because logging is asynchronous and the logging consumer (BinaryLogger) will enumerate the data potentially after the build has already started and the data is being mutated. I did see exceptions when enumerating without the lock. We had the same problem when the data was logged on ProjectStartedEventArgs though. In addition, there's a slight risk of logging not the exact data as it was at the end of evaluation, but the mutated data after some target has modified it. However given that the previous behavior was to not log anything for out-of-proc projects, and given the very significant allocation reduction, I think it's worth it. To mitigate, we could capture a snapshot at the end of evaluation, so we don't hold a reference to live data. This won't need a lock to enumerate. Ideally we also rely on the immutable collections to avoid allocations, but I didn't see an easy way to do that currently. We can investigate this in a future change. For items, it doesn't concatenate items of different types into a single large item stream, but keeps multiple lists, one per item type, to reflect the internal representation. Not flattening item types results in savings because we don't have to mention the item type for each item. This change increments the BinaryLogger file format to 12, to serialize GlobalProperties, Properties and Items on ProjectEvaluationFinishedEventArgs. It also stores items more efficiently, without having to know the number of item types in advance and enumerate in a single pass. It writes the item type and all items of that type, and it writes 0 to signal there are no more item types. It also no longer writes the Message for args as it can be recovered upon reading. New EnumerateProperties() and EnumerateItems() methods are added to Utilities, to consolidate the logic to enumerate the new data structures in a single location, used by packet translation logic, binary logger and the console loggers. Perf wise, I'm seeing no significant change on binlog size for small builds (it's a wash, because we log properties/items for all projects now, but they are no longer duplicated). For large projects I expect very significant savings though, as ProjectStarted is the most heavy-weight event in large binlogs. Build performance with /bl on small-ish builds is improved 27 s -> 24 s for single-core and 18 s -> 17 s for parallel. No observable change without /bl. Fixes #5316 Fixes #3616
Change relative path calculation
…5.9.1.8 (#6327) NuGet.Build.Tasks From Version 5.9.0-rc.7122 -> To Version 5.9.1-rc.8 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Improve doc comments
Fixes #2795 and indirectly fixes https://developercommunity.visualstudio.com/t/copytooutputdirectorypreservenewest-ignored-inside/1332219?from=email&viewtype=all#T-ND1363347 Context There's currently no way to include items in a project such that: Visual studio sees them in a specific folder (via <Link>). They are published to a user-defined path (currently controlled via <Link>) Changes Made Modify the AssignTargetPath task to return early if TargetPath metadata is already set on a particular item. Testing Need to add one test covering this. Tested locally with bootstrapped MSBuild on command line Tested locally with a boostrapped msbuild on internal VS Here's the repro I'm using: <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <TargetFramework>netstandard2.0</TargetFramework> </PropertyGroup> <ItemGroup> <Content Include="Files\**"> <Link>Files\%(Filename)%(Extension)</Link> <TargetPath>%(Filename)%(Extension)</TargetPath> <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> </Content> </ItemGroup> </Project> Notes The other way of solving this problem has to do with Microsoft.Common.CurrentVersion.targets. We modify it so that the AssignTargetPath task look something like this: <AssignTargetPath Files="@(Content)" RootFolder="$(MSBuildProjectDirectory)" Condition="'%(Content.TargetPath)'==''"> <Output TaskParameter="AssignedFiles" ItemName="ContentWithTargetPath" /> </AssignTargetPath> <ItemGroup> <ContentWithTargetPath Include="@(Content)" Condition="'%(Content.TargetPath)'!=''"/> </ItemGroup> This seems less efficient to me. AssignTargetPath is also called for all None, Content, and EmbeddedResource files. So if we go this batching route and want None or EmbeddedResource to have this feature, we'd need to batch those as well.
…6285) Fixes #2281 Context This change allows the WriteCodeFragment task to define assembly attributes that require parameters that are not of type System.String. For example, CSLCompliantAttribute can be generated with a parameter of true instead of "true". Changes Made Additional metadata can be defined on an AssemblyAttribute that specifies how to treat the parameters specified in the metadata. There are three different ways that the parameters can be treated. Infer the Type Without specifying any additional metadata, attributes that are defined in the mscorlib assembly (i.e. types that can be loaded via System.Type.GetType(string)) will have their parameter types inferred by finding the constructor where the parameter count matches the number of parameters specified in the metadata. For example, this: <ItemGroup> <AssemblyAttribute Include="CLSCompliantAttribute"> <_Parameter1>true</_Parameter1> </AssemblyAttribute> </ItemGroup> Will produce the code: [assembly: CLSCompliantAttribute(true)] For backward-compatibility, if the attribute cannot be found, or no matching constructor is found, the parameter is treated as a string. Declare the Type An additional metadata item can be used to specify the full name of the parameter type. To do this, add a metadata item that has the same name as the parameter with "_TypeName" appended to the end. For example, this: <ItemGroup> <AssemblyAttribute Include="TestAttribute"> <_Parameter1>True</_Parameter1> <_Parameter1_TypeName>System.Boolean</_Parameter1_TypeName> </AssemblyAttribute> </ItemGroup> Will produce the code: [assembly: TestAttribute(true)] This also works with named parameters: <ItemGroup> <AssemblyAttribute Include="TestAttribute"> <Foo>42</IdentifyLevel> <Foo_TypeName>System.Int32</Foo_TypeName> </AssemblyAttribute> </ItemGroup> [assembly: TestAttribute(42)] All types that can be used as attribute parameters are supported, except for arrays. For backward-compatibility, if a metadata item ends with "_TypeName", but there is no metadata item for the parameter with that name, then it will be treated as another named property. For example, this: <ItemGroup> <AssemblyAttribute Include="TestAttribute"> <Foo_TypeName>System.Int32</Foo_TypeName> </AssemblyAttribute> </ItemGroup> Would produce the code: [assembly: TestAttribute(Foo_TypeName="System.Int32")] Specify the Exact Code For cases where declaring the type is insufficient (such as when the parameter is an array), you can specify the exact that that will be generated for the parameter by adding metadata that has the same name as the parameter with "_IsLiteral" appended to the end. For example, this: <ItemGroup> <AssemblyAttribute Include="TestAttribute"> <_Parameter1>new int[] { 1, 3, 5 } /* odd numbers */</_Parameter1> <_Parameter1_IsLiteral>true</_Parameter1_IsLiteral> </AssemblyAttribute> </ItemGroup> Will produce the code: [assembly: TestAttribute(new int[] { 1, 3, 5 } /* odd numbers */)] The limitation with this is that the code you provide is language-specific. For example, the literal value in the metadata above will only work in C#. If you used that same metadata in a VB.NET project, you would receive a compiler error. This works with both positional and named parameters. As with the ..._TypeName metadata, if an ..._IsLiteral metadata does not have a corresponding parameter name, it will be treated as a named parameter for backward-compatibility. Mixed Parameter Behavior Because the additional metadata only applies to a specific parameter, you can choose to treat different parameters in different ways. For example, you can infer/use the default behavior for one parameter, specify the type for the second parameter and use a literal value for the third. For example: <ItemGroup> <AssemblyAttribute Include="TestAttribute"> <_Parameter1>This is a string</_Parameter1> <_Parameter2>42></Parameter2> <_Parameter2_TypeName>System.Int32</_Parameter2_TypeName> <_Parameter3>new int[] { 1 }</_Parameter3> <_Parameter3_IsLiteral>true</_Parameter3_IsLiteral> </AssemblyAttribute> </ItemGroup>
…#6312) Fixes #6281 Context BuildRequestDataFlags and ProjectLoadSettings are set during /t:restore in a best effort to run the Restore target in hopes that it will correct the potentially bad state that a project is in. Visual Studio also sets ProjectLoadSettings.IgnoreMissingImports so that an unresolved MSBuild project SDK doesn't prevent loading of a bad project so it can give the user an error and let them edit the file. However, this means that from the command-line an unresolved SDK doesn't fail /t:restore. This is because the missing "import" is ignored and non-existent targets are ignored so the build succeeds. Changes Made Introduced two new BuildRequestDataFlags: SkipNonexistentNonTopLevelTargets - A new flag to be used in this context to tell the build to ignore non-existent targets but not top level ones. In this case we're specifying /t:restore so if the Restore target doesn't exist, that should be an error. Only other targets that are trying to run are ignored (like InitialTargets, Before/AfterTargets, etc). FailOnUnresolvedSdk - We still need to ignore missing imports and I can't introduce a new flag to split the implementation now since Visual Studio sets ProjectLoadSettings.IgnoreMissingImports as a way to ignore unresolved SDKs. So this flag tells the evaluator to fail on an unresolved SDK but to continue ignoring other missing imports.
… build successfully when using exec (#6223) Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1264667 Context Users with profile names that contain parens in them will never be able to succesfully build a program that calls Exec with any command. This is because exec does the following: Generate a cmd file in the users temp directory Calls cmd /Q /D /C <path-to-temp-file> The problem with this is that running cmd /C "some command" does not work if the command has any parens in it. It needs to be escaped like so: ^( and ^). Changes Made When user sets EscapeSpecialCharacters in the exec task (boolean parameter), we escape characters that need to be escaped when calling cmd /c. We preserve the original functionality of always removing spaces and escaping '^' Added under changewave 16.10 Added documentation for it.
Also add a line about how to build Release.
Some loggers depended on ProjectStartedEventArgs.GlobalProperties being not null and set. It will take a long time to move them to ProjectEvaluationFinished (needs to bump MSBuild dependency to 16.10). For now log GlobalProperties in both places (ProjectStarted and ProjectEvaluationFinished). Hopefully the deduplication will save us from any significant increase in binlog size. Fixes #6341
Fix some URLs from Microsoft/msbuild -> dotnet/msbuild Also changes some branches from master to main.
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.
Since 17.0 is flowing to its internal home now, let's keep it up to date.