Binlog forward compatibility and deserialisation-less reading support #1
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
- Also moved it to the bottom, because it is now the largest section
…de, and Username containing Unicode chars. dotnet#9229
by review cleanup Co-authored-by: Forgind <12969783+Forgind@users.noreply.github.com>
…001.1 Microsoft.Net.Compilers.Toolset From Version 4.8.0-3.23474.3 -> To Version 4.8.0-3.23501.1
…6.8.0.122 NuGet.Build.Tasks From Version 6.8.0-rc.117 -> To Version 6.8.0-rc.122
… content items from reference project to match thebehavior in VS tooling.
…: Build ID 8477267 (dotnet#9296)
Add `[Required]` to parameters.
- `XmlPeek` Task
- Change `Query` parameter.
- Remove redundant `Dispose` that was flagged by the analyzer.
- Change XmlPeek.XmlInput class from `Internal` to `private sealed` and change access of some members
- Minor cleanup changes
- `XmlPoke` Task
- Change `Query` parameter.
- Change `XmlInputPath` parameter.
- Minor cleanup changes
- XmlPeek_Tests class
- Add new `PeekWithNoParameters` test
- XmlPoke_Tests class
- Remove `PokeMissingParams` test
- The test was defined as a `[Fact]` and used a for loop to test 4 distinct cases
- The test expected `ArgumentNullException` to be thrown
- Add 4 new tests, one for each of the four cases:
- `PokeWithNoParameters`
- `PokeWithMissingRequiredQuery`
- `PokeWithMissingRequiredXmlInputPath`
- `PokeWithRequiredParameters` (completes the replacement of `PokeMissingParams` but might be a redundant test)
Fixes dotnet#9140.
…81-9d7e-61a32415f962 [main] Update dependencies from dotnet/roslyn nuget/nuget.client
On windows there can be problems with Tools that do not support Unicode
The insertion release definition extends this when we try to insert, and
the VS release will extend it indefinitely for public builds.
Fixes a build warning in our official builds
##[warning]DropRetentionDays not set. Defaulting to 10 years (3650
days). Please reduce drop retention period to 30 days if possible.
JanKrivanek
force-pushed
the
proto/binlog-events-offsets
branch
from
c4db2fe
to
6c21a54
Compare
RoslynTools.MSBuild 17.7.2 caused two CG alerts when doing signing validation. dotnet/arcade#14055 and its linked PRs have more context.
Fixes policheck:Error Changes Made Skip the non en-us locale resource files. Skip the file that contains the specified entity names in the deprecated folder Change country to country/region based on https://policheck.microsoft.com/Pages/TermInfo.aspx?LCID=9&TermID=79570 Testing Test with MSBuild pipeline build https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8509007&view=logs&j=7d9eef18-6720-5c1f-4d30-89d7b76728e9&t=c5a86041-9185-53e8-42a2-1cadc4486f0d&l=5251. There are no active results now.
* Mention unification in RAR found-conflicts message
Consider a message like
```
warning MSB3277: Found conflicts between different versions of "System.Runtime.InteropServices.RuntimeInformation" that could not be resolved.
There was a conflict between "System.Runtime.InteropServices.RuntimeInformation, Version=4.0.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "System.Runtime.InteropServices.RuntimeInformation, Version=4.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a".
"System.Runtime.InteropServices.RuntimeInformation, Version=4.0.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was chosen because it was primary and "System.Runtime.InteropServices.RuntimeInformation, Version=4.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was not.
References which depend on "System.Runtime.InteropServices.RuntimeInformation, Version=4.0.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" [C:\VisualStudio\VS17PrevPublic\Common7\IDE\PublicAssemblies\System.Runtime.InteropServices.RuntimeInformation.dll].
C:\VisualStudio\VS17PrevPublic\Common7\IDE\PublicAssemblies\System.Runtime.InteropServices.RuntimeInformation.dll
Project file item includes which caused reference "C:\VisualStudio\VS17PrevPublic\Common7\IDE\PublicAssemblies\System.Runtime.InteropServices.RuntimeInformation.dll".
System.Runtime.InteropServices.RuntimeInformation
References which depend on "System.Runtime.InteropServices.RuntimeInformation, Version=4.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" [].
C:\Users\user\.nuget\packages\azure.core\1.25.0\lib\net461\Azure.Core.dll
Project file item includes which caused reference "C:\Users\user\.nuget\packages\azure.core\1.25.0\lib\net461\Azure.Core.dll".
C:\Users\user\.nuget\packages\azure.core\1.25.0\lib\net461\Azure.Core.dll
C:\Users\user\.nuget\packages\azure.identity\1.8.0\lib\netstandard2.0\Azure.Identity.dll
C:\Users\user\.nuget\packages\azure.security.keyvault.secrets\4.4.0\lib\netstandard2.0\Azure.Security.KeyVault.Secrets.dll
C:\Users\user\.nuget\packages\nuget.services.keyvault\2.111.0\lib\net472\NuGet.Services.KeyVault.dll
C:\Users\user\.nuget\packages\nuget.services.configuration\2.111.0\lib\net472\NuGet.Services.Configuration.dll
```
What the message _means_ is that the first reference is the winner, and
what was chosen there will require unification for all the other
assemblies listed in the second part of the message. But what it says is
that the list of assemblies in the second part of the message depend on
the second version, which is not necessarily true--in fact, that's the
list of references that _can be unified_ to that version.
This isn't a full fix for dotnet#4757 but hopefully makes the message a bit
less misleading.
* Update tests
* fixup! Update tests
* Correct success This is not yet a problem but only because neither /preprocess nor /targets are supported for solution files. The root of the problem is if someone chooses to specify both /preprocess and /targets. If /preprocess fails but /targets succeeds, it currently will erroneously display success. This fixes that. As I said, that scenario doesn't currently exist but only because /targets cannot succeed unless /preprocess succeeded, but that is not guaranteed going forward. Notably, if /preprocess is extended to support solution files before /targets is, this will become an issue. * Make isTargets not run if !success --------- Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Fixes dotnet#7330 (plus one subtask of dotnet#8329) Changes Made Based on Add ability to create temp mapped drive for integration tests dotnet#8366 fixes to enable other Drive enumeration integration tests with a dummy folder in windows Remove one test data https://github.com/dotnet/msbuild/blob/fecef0fdffe59ba8b0251701a23be48bbd552726/src/Build.OM.UnitTests/Instance/ProjectItemInstance_Tests.cs#L1010-L1012C45 since there is no warning when inlude is not null and exclude with enumerating wildcards. The related logical code is msbuild/src/Build/Utilities/EngineFileUtilities.cs Line 339 in fecef0f private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec) . There is no condition satisfied. Associate unix Enumeration Tests long time run with issue Unix drive enumeration imports not expanded? dotnet#8373
Preliminary design proposal for the Packages Sourcing feature
Fixes dotnet#8762 Context Catch the exceptions when extensionsPathPropValue is null or importExpandedWithDefaultPath is empty. In NET Framework, Path.* function also throws exceptions if the path contains invalid characters Changes Made Catch the exception. Testing FallbackImportWithInvalidProjectValue Notes
.NET (Core) stuff is implied by ManagedDesktop now, and we require only "some .NET Framework SDK", so no need to specify explicitly as one will be delivered by ManagedDesktop. We do still target .NET Framework 3.5, though . . .
…004.4 (dotnet#9314) Microsoft.Net.Compilers.Toolset From Version 4.8.0-3.23501.1 -> To Version 4.8.0-3.23504.4 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…ngTargetExceptions (dotnet#9253) Fixes dotnet#9245 Context The test was disabled to unblock PR CI. Changes Made Increased the relevant timeout. Testing The test is reliably passing now. Notes This turned out to be an issue with the sleep command we use on Windows. In some cases PowerShell can take a super long time to start. I have been able to reproduce locally by enabling Fusion logging. Thread times of the powershell process: image We spend almost 10 seconds just loading assemblies, so the timeout of 10 seconds for the entire build was not enough. I don't have a full understanding of the mechanism that slows down PowerShell this much. At this point I'm happy we were able to confirm it's not a product issue, although I'm wondering if there is a better and more light-weight sleep command we could use on Windows instead (e.g. ping 127.0.0.1 -n <seconds>). Reviewers please opine. EDIT: In my trace, file system operations block extensively with wdfilter.sys on the stack, so the likely explanation for the issue appearing all of a sudden is a Defender update.
…o-main [automated] Merge branch 'vs17.9' => 'main'
Right justify target and duration in terminallogger
…net#9511) Prebuilt detection no longer detects Microsoft.SourceBuild.Intermediates as prebuilts due to dotnet/arcade#13935. Addresses dotnet/source-build#3010
The devdiv build machine images appear to have started setting the environment variable `NUGET_PACKAGES`, which caused a mismatch between the location where `drop.app` was restored (repo-local location) and where it was used from (environment-variable defined machine location), causing build failures. Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com>
…n.CurrentVersion.targets (dotnet#9520) dotnet#9387 introduced improved hashing for [`[MSBuild]::StableStringHash`](https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-stablestringhash), that however broke internal functionality relying on the hash to be stable between versions (despite documented otherwise). ### Changes Made * Reverted all the hashing changes * Kept the UTD marker change (fixing dotnet#9346) ### Note Longer term fix: dotnet#9519
NuGet's Central Package Management (CPM) allows users to specify package versions in a central location. However, sometimes you may need to override the centrally defined version for a particular package so you specify the `VersionOverride` attribute documented [here](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management#overriding-package-versions). Fixes AB#1929252.
…o-main [automated] Merge branch 'vs17.9' => 'main'
* Add to change wave 17.10 * VersionSwitch test changed from Theory to Fact to remove overlap with other test
…otnet#9517) * Update dependencies from https://github.com/nuget/nuget.client build 6.9.0.50 NuGet.Build.Tasks From Version 6.9.0-preview.1.45 -> To Version 6.9.0-preview.1.50 * Update dependencies from https://github.com/dotnet/roslyn build 20231208.9 Microsoft.Net.Compilers.Toolset From Version 4.9.0-3.23602.1 -> To Version 4.9.0-3.23608.9 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…o-main [automated] Merge branch 'vs17.9' => 'main'
* Change Extended args api back to be compatible with 17.8 * Bump version * Revert "Bump version" --------- Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>
* Version 17.10 (dotnet#9499) * Remove U2D marker dependency on ProjectGuid property * Revert "Version 17.10" --------- Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com>
Fixes dotnet#9513 Context Creation of CODEOWNERS file as per request in issue. Having a code owners file would also be generally useful when opening PRs in this repo, as it can sometimes be difficult and time consuming to determine who should be included as a reviewer. Changes Made Added @dotnet/source-build-internal as code owners for SB files Added @dotnet/kitten as code owner for some of files and folders. Testing No testing
…SBuild.exe only) (dotnet#9439) Fixes dotnet#9303 ### Context After a new version of `VS.Redist.Common.Net.Core.SDK.MSBuildExtensions` is inserted into VS, a native image for `Microsoft.DotNet.MSBuildSdkResolver` will be generated, both for devenv.exe and MSBuild.exe (see dotnet/installer#17732). We currently load SDK resolvers using `Assembly.LoadFrom` on .NET Framework, which disqualifies it from using native images even if they existed. This PR makes us use the native image. ### Changes Made Added a code path to use `Assembly.Load` to load resolver assemblies. The call is made such that if the assembly cannot be found by simple name, it falls back to loading by path into the load-from context, just like today. The new code path is enabled only for `Microsoft.DotNet.MSBuildSdkResolver` under a change-wave check. ### Testing Experimental insertions. ### Notes Using `qualifyAssembly` in the app config has the advantage of keeping everything _field-configurable_, i.e. in the unlikely case that a custom build environment will ship with a different version of the resolver, it will be possible to compensate for that by tweaking the config file. The disadvantage is that the same `qualifyAssembly` will need to be added to devenv.exe.config because .pkgdef doesn't support this kind of entry, to my best knowledge. It should be a one-time change, though, because [we have frozen the version of `Microsoft.DotNet.MSBuildSdkResolver` to 8.0.100.0](dotnet/sdk#36733).
…otnet#9533) This reverts commit 67d742f. This workaround no more needed.
* Update dependencies from https://github.com/dotnet/arcade build 20231130.1 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.XUnitExtensions From Version 8.0.0-beta.23463.1 -> To Version 8.0.0-beta.23580.1 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23426.1 -> To Version 1.0.0-beta.23475.1 (parent: Microsoft.DotNet.Arcade.Sdk * Bump minimum MSBuild and xcopy version for post-build --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Fixes dotnet#5881 Context Implementation of proposed ITaskItem[] SourceFolders parameter. Changes Made Within Copy.cs: Added ITaskItem[] SourceFolders parameter Modified ValidateInputs() to support the new parameter Modified InitializeDestinationFiles() Previously this method iterated SourceFiles to create DestinationFiles items given the DestinationFolder. The Copy task actually only operates on SourceFiles and DestinationFiles. Extended the method to: Use FileMatcher::GetFiles() for each directory in SourceFolders Create items in both SourceFiles and DestinationFiles for each file found Added additional unit tests in Copy_Tests.cs. Added new error MSB3894 Copy.IncompatibleParameters for the case where SourceFolders and DestinationFiles are both present. This is very close to the existing MSB3022 Copy.ExactlyOneTypeOfDestination but it seemed prudent not to re-use MSB3022. Testing Tested on Windows 11 and macOS 12 Ran full suite of unit tests from the command line Ran some sample project files Notes This implementation conforms to the proposed design with the exception that it does not copy empty directories. This implementation leverages the existing FileMatcher::GetFiles() which recurses the directory tree using multiple threads. GetFiles() returns a list of files. Empty directories are not identified. The use of FileMatcher could be replaced. The Copy task could implement its own support for recursing a directory tree. The wildcard matching logic in FileMatcher is not relevant for the Copy task.
Context
We still have checks for a change wave that's already out of rotation.
Changes Made
[MSBuild]::AreFeaturesEnabled('17.0') is now unconditionally true, let's remove the checks from Microsoft.Common.CurrentVersion.targets.
…o-main [automated] Merge branch 'vs17.9' => 'main'
Fixes dotnet#9131 Context As described on the issue, muti-targeted builds did not import the .user file on the outer build. This change makes the outer build import the .user file. Changes Made Added import reference to .user file in Microsoft.Common.CrossTargeting.targets . Testing Test is in SDK repo (dotnet/sdk#37192)
…tnet#9446) Related to dotnet#9303 ### Context We load `NuGet.Frameworks` lazily by path when the project being evaluated calls one of a few property functions that need it. All standard .NET projects use such functionality so this load is executed pretty much always. Since we're loading the assembly by path, the loader is unable to take advantage of the native image and we spend 50-100 ms JITting. ### Changes Made Improved MSBuild.exe startup (or, more precisely, first evaluation) by 50-100 ms by loading `NuGet.Frameworks` into a separate AppDomain using `Assembly.Load`. The AppDomain is set up with the right binding redirects for the loader to load the assembly by strong name and use its native image. ### Testing Experimental insertions. ### Notes There are several other assemblies that require non-trivial JITting on startup. The reason why we're tackling `NuGet.Frameworks` here are: - The assembly has no dependencies other than Framework assemblies, so the AD setup is relatively simple. - The functionality provided by this assembly has simple signatures so the cross-domain call cost is negligible. - A compatible native image for this assembly already exists. Other notes: - There is no impact on VS or any other MSBuild host because creating ADs may be risky and disruptive. We're changing only MSBuild.exe. - This PR is about the `NuGet.Frameworks` copy that lives in `{VS-installation}\Common7\IDE\CommonExtensions\Microsoft\NuGet`. The MSBuild process may also load SDK-specific copies from paths like `C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\tools\net472` which are out of scope here. This is what the AD looks like in SOS: ``` Domain 2: 000001dff97c7e70 Stage: OPEN Name: NuGetFrameworkWrapper Assembly: 000001dff6bac490 [C:\windows\Microsoft.Net\assembly\GAC_64\mscorlib\v4.0_4.0.0.0__b77a5c561934e089\mscorlib.dll] Assembly: 000001dff9732960 [C:\src\msbuild\artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\Microsoft.Build.dll] Assembly: 000001dff97e2410 [C:\src\msbuild\artifacts\bin\bootstrap\net472\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.Frameworks.dll] Assembly: 000001dff8de7250 [C:\windows\Microsoft.Net\assembly\GAC_MSIL\System.Core\v4.0_4.0.0.0__b77a5c561934e089\System.Core.dll] Assembly: 000001dff8dd6100 [C:\windows\Microsoft.Net\assembly\GAC_MSIL\System\v4.0_4.0.0.0__b77a5c561934e089\System.dll] Assembly: 000001dff8e1d9a0 [C:\windows\Microsoft.Net\assembly\GAC_MSIL\System.Xml\v4.0_4.0.0.0__b77a5c561934e089\System.Xml.dll] ```
We were hard-coded to an ancient version of NuGet.exe, but we should be fine taking the latest stable, which is default for NuGetToolInstaller.
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#9261
Context
dotnet#9261
This builds on top of dotnet#9219 PR - but creating it as separate PR for easier review and for separate discussion. HJowever in the end it'd be preferrable to deliver those together - to require a single binlog contract increment
Changes
Measurements
OrchardCore build with debug version of MSBuild via
dotnet build </bl>. Mean value of 5 runs used.MSBuild version names used in following tables:
File Size (bytes)
The increase is up to ~2.7% in case files are not embedded into binlog (
ProjectImports=None, not a default)Build speed
Redacting speed
Future improvements
This change helps grace handling of under-reading of data for deserialized events (the assumption is newer version added data); but over-reading still leads hard error. Over-reading can be handled more gracefully as well (by leveraging SubStreams and break reading once the deserialization code is beyond the expected size of the serialized event)