Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

Update to Visual Studio MSBuild 17.0 #435

Open
wants to merge 285 commits into
base: xplat-master
Choose a base branch
from

Conversation

AptiviCEO
Copy link

This PR is to update Mono MSBuild to version 17.0 from my fork.

Any reviews are welcome.

benvillalobos and others added 30 commits May 27, 2021 23:47
Fixes # dotnet#5981

Summary
MSBuild didn't support customers using the x64 AL.exe tool by default when their projects targeted x64. dotnet#6207 implemented a fix to include x64 in the path when relevant, but this commit mistakenly forgot to update one property name which results in an empty parameter being passed. This results in the x86 version of AL.exe being the default choice.

This fix changes the name of the property to the correct one that is set just before AL is called.

Customer Impact
Customer's that want to compile .resx files using 64-bit AL.exe require a workaround to do so. This change will allow this workaround to become default behavior.

Testing
Customer did a manual fix here that manually set SdkToolsPathMaybeWithx64Architecture to the value that _ALExeToolPath would be and their build succeeded.

Risk
Low. The previous value is preserved when not on x64, and only appends the platform to the path to AL.exe when targeting x64.

Code Reviewers
Description of fix
Rename property passed into AL from SdkToolsPathMaybeWithx64Architecture to _ALExeToolPath, which is set a few lines above the AL call.

Context
dotnet#6207 introduced logic to fix the above issue. Unfortunately we need to update one location (the one that matters) to use that newly created variable. I missed this during the refactor in this commit

Changes Made
Update the variable used for SdkToolsPath when calling AL.

Testing
See the linked issue. The customer passing /p:SdkToolsPathMaybeWithx64Architecture="C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\x64\" (the value that _ALExeToolPath has) fixed their issue.

Notes
_ALExeToolPath is defined just above the AL call that this PR modifies, if you're wondering where it came from.
These should have been changed when we updated past .NET Core 2.1 but were missed.
Solution metaprojects are assembled in a confusing way that impacts what
targets can be hooked into via BeforeTargets/AfterTargets. Prior to this
change one couldn't hook before the build would attempt to traverse the
list of projects in the solution.

Extend the "list of targets we know we'll create shortly" list to
include the things that are in the `InitialTargets` list and
`GetSolutionConfigurationContents`.

Fixes dotnet#6452.
Changes Made
updated VS deploy script to copy more files needed during runtime
added option to break into the BuildManager via a new env var. This makes it super easy to debug msbuild running in devenv without having to copy over local bits
…entArgs (dotnet#6402)

We weren't logging TargetSkippedEventArgs in two cases: when the target was satisfied from cache (previously built), or when the outputs were up-to-date. We were logging simple messages. Switch to logging TargetSkippedEventArgs in these cases as well.

Add a new TargetSkipReason enum to indicate why the target was skipped. Store and serialize it for node packet translator and binary logger.

When logging a TargetSkippedEventArgs because a target was built previously it is also useful to find the original target invocation (e.g. to see the target outputs). Add OriginalBuildEventContext to TargetResult and ensure it is translated properly. Add OriginalBuildEventContext on TargetSkippedEventArgs and serialize that as well (both node packet translator and binary logger).

Note that if the target didn't build because the Condition was false, the OriginalBuildEventContext will be a project context, not a target context.

We have to increment the binlog file format version to 14 to add the two new fields.

Implement BuildEventContext.ToString() for easier debugging.

Add an internal setter for Importance on BuildMessageEventArgs.

We were missing unit-tests for node packet translation of TargetSkippedEventArgs. Add test.

Fixes dotnet#5475
We currently log the file in which a task is invoked, but we don't log the line and column, so if there are multiple tasks of the same name in a target, there's an ambiguity as to which task it is.

Pass the line and column information to TaskStartedEventArgs.

Fortunately we don't need to increment the binlog file format as we can piggy-back on the existing infrastructure for messages which can already log line and column if present. Reading older binlogs with the new MSBuild will work as the line and column flags won't be set, so no attempt to read anything extra. Reader newer binlogs with old MSBuild (same version but pre-this change) will also work as the line and column fields will be set and the existing infrastructure will read them, but nothing will consume that data.
…6.0.0.66 (dotnet#6486)

NuGet.Build.Tasks
 From Version 5.10.0-rc.7240 -> To Version 6.0.0-preview.1.66

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…ility constraints (dotnet#6475)

Progress towards dotnet#6068 

Context
MSBuildFileSystemBase was introduced as a replacement of IFileSystem in dotnet#5546. It was added as an abstract class so potentially with the same constraints as an interface:

Versioning constraints. We can't add a new public member to the class, unless we make it non-abstract and provide a default implementation of new members.
Usability constraints. It is not possible to use MSBuildFileSystemBase to intercept calls or override a subset of them because the default MSBuild-provided file system is not exposed.
Changes Made
To address the constraints above, I am making the class non-abstract and having it by default forward all calls to the default MSBuild file system. This way the caller may choose to override only a subset of methods (e.g. file enumeration and existence/metadata checks) leaving the rest as is (e.g. actual file reads).

As a result I was able to delete both IFileSystemAdapter.cs and MSBuildFileSystemAdapter.cs because they're not needed anymore.

The difference between IFileSystem methods and public MSBuildFileSystemBase methods was just one method having a different name. I have renamed the one on the internal interface, assuming that this does not cause any troubles with the old NuGet, as tested by TestOldNuget(). This enabled further simplification and reduce the number of the "adapter" call hops.

With the changes in this PR, we have:

An internal interface IFileSystem, kept around for compat reasons.
A public class MSBuildFileSystemBase, which implements IFileSystem with virtual methods. The class is meant to be derived from and its methods overridden.
If we find ourselves adding new methods to the contract, it won't break existing MSBuildFileSystemBase because the new calls will just call through to the default implementation. If the change we need to make requires an orchestrated behavior change, we can always add a virtual 'version' prop, overriding which the derived class tells us which version of the contract it supports.

Testing
Existing unit test coverage.

Notes
This change is meant to be non-breaking. I'm basically just making an abstract class non-abstract (would have to be done at some point anyways if we were to add a new method). The rest of the changes are internal.
…iguration.Interop (dotnet#6469)

Fixes AB#1329223

Context
We're hitting issues with MSBuildTaskHost importing the Microsoft.VisualStudio.Setup.Configuration.Interop assembly, which is not being ngen'd properly. Turns out we don't need to import this package at all, so I've removed the packagereference along with un-defining constants when on net3.5 to prevent compilation errors.

Testing
Tested a successful build with msbuildtaskhost.csproj

Notes
Also don't define FEATURE_VISUALSTUDIOSETUP for MSBuildTaskHost.csproj[automated] Merge branch 'vs16.11' => 'main'
Fixes dotnet#6085

Context
Fixes two issues:

If an official build is kicked off that had stale optprof, no optprof, or is a branch that never would have optptrof (think exp/), it would fail. We'd then have to re-run using a specific optprofdropname
When overriding the optprofdropname, we need to delete the IbcSourceBranchName. This is one less thing for the kitten (or any other dev launching pipeline builds of specific branches) to worry about!
Changes Made
Our official builds now take OptProfDropName as a pipeline parameter. We should leave the default value (which is default) if we don't plan to use a custom drop. When we do, paste your optprofdrop there and the script should do the rest.

No more manually clearing the IbcSourceBranch pipeline variable!

Testing
See https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=4724888&view=results
I launched a pipeline from branch bevillal/ci/quality-of-life.

I did not clear the IbcSourceBranchName pipeline variable, but did set the OptProfDrop pipeline parameter and it properly cleared SourceBranch. This fixes the issue with setting OptProfDrop and needing to clear that field manually.
Now see https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=4724898&view=results
I launched a build from the same branch and did NOT set optprofdrop. The build correctly pulls optprof data from main.
Adding these allows them to be provided in Visual Studio's completion.

ProduceReferenceAssemblies
UseWindowsForms
UseWPF
This pull request updates the following dependencies

From https://github.com/dotnet/arcade
Subscription: 93d865d2-823f-4e4d-e0b6-08d91b0a84f2
Build: 20210531.1
Date Produced: 5/31/2021 7:30 PM
Commit: c7d6bd607715f334cda90e01967bb0c02dee09be
Branch: refs/heads/main
Updates:
Microsoft.DotNet.Arcade.Sdk: from 6.0.0-beta.21227.1 to 6.0.0-beta.21281.1
This pull request updates the following dependencies

From https://github.com/dotnet/roslyn
Subscription: 9095e9e7-a620-4e16-c14f-08d91ef85541
Build: 20210527.15
Date Produced: 5/28/2021 12:17 AM
Commit: 0adf94c625f834f1c89d11700621302b658a50ea
Branch: refs/heads/release/dev17.0-preview1-vs-deps
Updates:
Microsoft.Net.Compilers.Toolset: from 3.9.0-2.20574.26 to 4.0.0-1.21277.15
This PR does the following:

Adds the local build infrastructure that lets ArPow (arcade-powered source-build) run in this repo. See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/onboarding/local-onboarding.md for more details about how it works.

To try it out locally, run this on Linux: ./build.sh -c Release --restore --build --pack /p:ArcadeBuildFromSource=true -bl

Implements source-build CI.

To make sure ArPow (arcade-powered source-build) keeps working in this repo, we need to add it to PR validation. We also need it to run in the official build to publish source-built artifacts that can be tested downstream.

See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/onboarding/ci-onboarding.md for ArPow CI onboarding info.

Incorporates the existing source-build patches into the repo.

Some background on source-build patches, for anyone who isn't familiar with previous pushes for patch incorporation:

A patch is essentially just a commit that has been extracted from Git into a .patch file that can be applied on demand. The effort to build .NET from source involves creating patches because repos make changes that are incompatible with source-build and need to be fixed up after the original released source code has already been finalized. When the original repo gets PRs over time for servicing, the PR changes sometimes conflict with the source-build patches, just like a merge conflict. The patch files need to be fixed up when this happens, which is a significant maintenance problem for the source-build team.

Several times, the source-build team has pushed for "patch incorporation". This means to merge the commit represented in the .patch file into the original repo's official branch. Doing so prevents patch merge conflicts, because there's no longer a patch to merge against. However, patches inevitably pile up again when getting subsequent servicing releases to work in source-build.

ArPow lets us end this maintenance-heavy process. By running source-build inside CI, patch merge conflicts will immediately block PR validation, so fixup can be handled in place, not solely by the source-build team. Running source-build in CI also means creating new patches won't be necessary except in exceptional circumstances.

See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/implementation-plan.md for more details on the ArPow implementation plan.

Fixes: dotnet/source-build#2068.
…to-main

[automated] Merge branch 'vs16.11' => 'main'
Context
The project cache was being queried serially. Oops.
This is because the monolithic BuildManager._syncLock was being held during the cache query, thus serializing all access.

Changes Made
Implements the 2nd option from the design doc: https://gist.github.com/cdmihai/0955cb217b2cbd66e18c89b20bf68319#2-reuse-how-the-buildmanager-offloads-parallel-work-to-build-nodes

Reverted code of ExecuteSubmission to what it was before project cache plugins.
Changed ExecuteSubmission to either issue a cache request or a build request (until now it only issues build requests)
The cache request is sent to the ProjectCacheService which submits it to the thread pool. This achieves parallelization of cache requests
Each cache request, on its own thread:
evaluates the project if necessary
does the cache query
When a cache request finishes in the ProjectCacheService it is posted back on the BuildManager work queue thread and is handled by either skipping the build or doing a real build.
Design time builds were a pain to get right this time. Previously design time builds were easy to deal with because the BuildManager detected them early enough. Now they get detected later in the project cache service. The build manager detects this and shuts the cache service off when design time builds are detected.

Testing
Added a parallel stress test. This should be a good test to both ensure the cache is queried in parallel and to stress test the concurrency in the engine.

Risk assessment
This should make the non project cache logic less risky than it was before, since I took the project cache logic out of BuildManager and moved it to the ProjectCacheService.
* Use dotnet certificate
…to-main

[automated] Merge branch 'vs16.11' => 'main'
It's taking longer than we initially hoped so moving it to get it off the critical path.
Fixes dotnet/source-build#2068

Context
This was something I discovered while trying to consume the Source-Build intermediate package produced by the first ArPow changes made in dotnet#6387

Changes Made
Set the SourceBuildManagedOnly property correctly.

How Tested
Verified the resulting Microsoft.SourceBuild.Intermediate.msbuild package is not named rid specific.
Fixes dotnet#6098.

The `SignFile` task is referenced by the SDK for both .NET Framework and Core/5+, but not actually included in the non-framework assemblies.

A simple test, applying `SignFile` to one of the DLLs in artifacts using a self-signed certificate, succeeded.
Added some more unit tests as per suggestions

Fixed unit test failing on linux

Removed unnecessary length check

Co-authored-by: Forgind <Forgind@users.noreply.github.com>
This pull request updates the following dependencies

From https://github.com/dotnet/arcade
Subscription: 93d865d2-823f-4e4d-e0b6-08d91b0a84f2
Build: 20210602.1
Date Produced: 6/2/2021 5:18 PM
Commit: 9945dc4ebbb511b027df34cb5ab579f6395d1dda
Branch: refs/heads/main
Updates:
Microsoft.DotNet.Arcade.Sdk: from 6.0.0-beta.21281.1 to 6.0.0-beta.21302.1
This pull request updates the following dependencies

From https://github.com/dotnet/roslyn
Subscription: 9095e9e7-a620-4e16-c14f-08d91ef85541
Build: 20210603.3
Date Produced: 6/3/2021 6:25 PM
Commit: 5dc3c06726b564dade27edac0236b93fe6babb25
Branch: refs/heads/release/dev17.0-preview1-vs-deps
Updates:
Microsoft.Net.Compilers.Toolset: from 4.0.0-1.21277.15 to 4.0.0-1.21303.3
marcin-krystianc and others added 28 commits September 20, 2021 18:33
We want to use static for CoreClrAssemblyLoader so each unique SDK resolver assembly is loaded into memory and JITted only once.

Fixes dotnet#6842 (comment)

### Context

We use static for the `CoreClrAssemblyLoader` field so each unique SDK resolver assembly is loaded into memory and JITted only once. All subsequent load requests will return assembly from the assembly loader's cache instead of loading it again from disk. This change increases the performance of SDK resolution and at the same time avoids leaking memory due to loading the same SDK resolver assembly multiple times and never unloading it.

### Changes Made

The `CoreClrAssemblyLoader` field in the `SdkResolverLoader` class was changed from non-static to static member. The same instance of `CoreClrAssemblyLoader` will be used by all instances of `SdkResolverLoader`. It is consistent now with other uses of `CoreClrAssemblyLoader` in msbuild.

### Testing

Tested manually using repro from dotnet#5037 (comment)

### Notes

Alternative approach would be to use collectible `CoreClrAssemblyLoader` / `AssemblyLoadContext` - that would fix the leak as well but it would be less performant as it wouldn't benefit from re-using already loaded and JITed assemblies.
Resolves dotnet#6858

### Context

Add new properties introduced in https://github.com/dotnet/designs/blob/main/accepted/2021/winforms/streamline-application-bootstrap.md#msbuild-properties

### Test

Tested by updating the local version in C:\Program Files\Microsoft Visual Studio\2022\Preview\Xml\Schemas\1033\MSBuild and verifying in VS
Skip Updating CopyComplete Marker When Not Necessary
Context
Upgrading version of System.Net.Http package to the latest one, that is 4.3.4.

Changes Made
We have indirect references to System.Net.Http with lesser versions.
I added a direct reference to our .csproj files in order to overwrite the version we take.

Also, in couple of projects we have an assembly reference to the System.Net.Http, which I replaced by a package reference.

Testing
Unit tests & DDRITs.
Context
The `DebugType` property has a set of expected values, and is not just a plain text property.
Currently completion on the `DebugType` property offers no guidance.

Changes Made
This change adds those values so they appear in completion. It also documents that `pdbonly` is equivalent to `full` in recent compilers.
6732: Default to sha2 digest for clickonce manifest
Works around an issue with the Validate step when signing packages.
This defaults to looking for a nearby dotnet.exe (if you're on core) and uses that instead, falling back to <name>.exe only on failure. This should handle any case in which you find MSBuild in an sdk installation.
Microsoft.DotNet.Arcade.Sdk
 From Version 5.0.0-beta.21464.1 -> To Version 5.0.0-beta.21505.11
Fixes dotnet#6917 by ensuring that the copy-marker file is _always_ added
to the FileWrites item if the copy-referenced-assemblies target runs
so that IncrementalClean never sees it as an 'orphan' file and then
deletes it.
These assemblies have moved and versioned in Visual Studio 2022.

Update of the workaround for dotnet#1675 in dotnet#4139.
* Final branding for 17.0 GA

* Move release branding to versionprefix line
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.