[main] Update dependencies from dotnet/arcade #2719
Conversation
![dotnet-maestro[bot]](https://avatars.githubusercontent.com/in/16706?s=60&v=4){kind=small}
…331.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22181.2
…406.10 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22206.10
…415.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22215.2
…422.4 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22222.4
…425.6 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22225.6
|
This is failing because it's trying to update the repo to use .NET 7 SDK/runtime. I must admit I don't know if we should do that yet. I don't know enough about these automated PRs - can we just overwrite the versions but keep the rest of the update (to latest Arcade)? |
|
Nope, the newest arcade has a dependency on .net 7, so we’ll need to find the issue with .net 7 and fix it |
|
It's not an "issue" - we simply need to update couple of places so that everything is 7 - not hard. |
|
We're well over half way into .NET 7, it makes sense to self host on it at this point. |
…505.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.ApiCompat From Version 7.0.0-beta.22171.2 -> To Version 7.0.0-beta.22255.2
Some small refactoring of the build files to avoid having to update multiple places with the new TFM. Note that not all can be updated as they are used verbatim in the NuGet package, so can't rely on repo-only properties. Also currently I left the source code to repeat these. Eventually we might investigate generating `.cs` files in the msbuild to "inject" the TFM and other constants from the MSBuild to the compiled code. For the Roslyn tests, I hardcoded a new 7.0.0-preview.2 ref package reference, but this feels really weird - note that so far we've been testing against 6.0.0-preview.5 version. Ideally there would be some way to deduce the ref package version from the currently used SDK, and use that in the tests. The formatting changes are induced by running `lint`. I assume this is because of the SDK version change as well, but I don't know for sure.
…s://github.com/dotnet/linker into darc-main-3a65fa7f-262c-4578-97fd-670249162fc8
…-4578-97fd-670249162fc8
| @@ -0,0 +1,5 @@ | |||
| // <auto-generated/> | |||
There was a problem hiding this comment.
I spent couple of minutes trying to figure out why this got generated here, but no luck.
The theory is: since this project has source generators enabled and redirected to a different path, it's "weird". So I tried to recreate this in a simple repro project (console app using RegEx class), but no luck, in it the source generator didn't produce any file like this.
Not worth blocking the PR on this.
There was a problem hiding this comment.
To repro this you have to set:
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<CompilerGeneratedFilesOutputPath>generated</CompilerGeneratedFilesOutputPath>
I observed it in a simple console app using preview 2, even for apps that don't use regex. Looks like it's already fixed in preview 3 so we can just remove it when we upgrade to the next preview.
| @@ -13,7 +13,7 @@ Copyright (c) .NET Foundation. All rights reserved. | |||
|
|
|||
| <PropertyGroup> | |||
| <UsingILLinkTasksSdk>true</UsingILLinkTasksSdk> | |||
| <ILLinkTasksAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\net6.0\ILLink.Tasks.dll</ILLinkTasksAssembly> | |||
| <ILLinkTasksAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\net7.0\ILLink.Tasks.dll</ILLinkTasksAssembly> | |||
There was a problem hiding this comment.
This can't use a property - since this file is injected into the nuget package as-is.
| /// This allows tools to understand which methods are unsafe to call when compiling ahead of time. | ||
| /// </remarks> | ||
| [AttributeUsage (AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] | ||
| public sealed class RequiresDynamicCodeAttribute : Attribute |
There was a problem hiding this comment.
Some tests are failing because RequiresDynamicCode doesn't allow class targets, (I tihnk because dotnet/runtime#67778 hasn't made it into this version). I think we need to keep this for now.
There was a problem hiding this comment.
We can't really keep it because then there's two of them and the compiler doesn't know which one to pick.
I think we should basically bump the SDK to Preview3 in this change - ahead of Arcade. It should not cause much trouble - I just didn't get to to it yet today.
If should also resolve the RegEx problem as you mention below.
| @@ -0,0 +1,5 @@ | |||
| // <auto-generated/> | |||
There was a problem hiding this comment.
To repro this you have to set:
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<CompilerGeneratedFilesOutputPath>generated</CompilerGeneratedFilesOutputPath>
I observed it in a simple console app using preview 2, even for apps that don't use regex. Looks like it's already fixed in preview 3 so we can just remove it when we upgrade to the next preview.
|
Preview 3 doesn't have the RDC changes -- might have to wait for P4 for that. |
|
Let's wait for P4. I verified that updating to the latest P4 build everything seems to work. I don't want to upgrade the repo to a nightly build though... and P4 should be released soon anyway. |
dotnet-maestro
bot
deleted the
darc-main-3a65fa7f-262c-4578-97fd-670249162fc8
branch
@vitek-karas, dotnet/runtime is on the publicly released preview-3 SDK: https://github.com/dotnet/runtime/blob/2fe067d05c25310072a4cea3fb4d8549f2bf8ab3/global.json#L3 The linker dependency update in dotnet/runtime is now failing because you are moving faster here than we do in dotnet/runtime which must be avoided. Can we downgrade the SDK used in this repo to the publicly released preview-3 one? |
|
I hope we don't actually need to downgrade the SDK - seems like we just need to pick a lower runtime version for Mono.Linker.csproj. Can we keep building it for for net6.0, or target an earlier net7.0 preview there instead @vitek-karas? |
|
We can't really downgrade since we need a new attribute from framework which is only in P4. |
|
Sounds good, thanks. |
[main] Update dependencies from dotnet/arcade - Update the linker repo to .NET 7 Some small refactoring of the build files to avoid having to update multiple places with the new TFM. Note that not all can be updated as they are used verbatim in the NuGet package, so can't rely on repo-only properties. Also currently I left the source code to repeat these. Eventually we might investigate generating `.cs` files in the msbuild to "inject" the TFM and other constants from the MSBuild to the compiled code. For the Roslyn tests, I hardcoded a new 7.0.0-preview.2 ref package reference, but this feels really weird - note that so far we've been testing against 6.0.0-preview.5 version. Ideally there would be some way to deduce the ref package version from the currently used SDK, and use that in the tests. The formatting changes are induced by running `lint`. I assume this is because of the SDK version change as well, but I don't know for sure. - Merge branch 'darc-main-3a65fa7f-262c-4578-97fd-670249162fc8' of https://github.com/dotnet/linker into darc-main-3a65fa7f-262c-4578-97fd-670249162fc8 - Merge remote-tracking branch 'mono/main' into darc-main-3a65fa7f-262c-4578-97fd-670249162fc8 - Update to preview 3 SDK - Update to .NET 7 Preview 4 which should have the necessary changes. - Formatting Commit migrated from dotnet/linker@9d7304a
This pull request updates the following dependencies
From https://github.com/dotnet/arcade