Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[main] Update dependencies from dotnet/runtime #16425

Merged
merged 4 commits into from
Mar 24, 2021

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Mar 19, 2021

This pull request updates the following dependencies

From https://github.com/dotnet/runtime

  • Subscription: aa69f164-2492-460a-3914-08d8e9750bf8
  • Build: 20210319.6
  • Date Produced: 3/19/2021 6:08 PM
  • Commit: 76954b4ed7b5cd48ace16fefb1209fe56779b247
  • Branch: refs/heads/main

…0318.9

Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.CodeDom , System.Security.Cryptography.ProtectedData , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 , System.Text.Encoding.CodePages
 From Version 6.0.0-preview.3.21167.1 -> To Version 6.0.0-preview.3.21168.9
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@marek-safar
Copy link
Contributor

I removed ios from the standard list as asking everyone to make Process conditional is a pretty bad breaking change.

/cc @terrajobst @jeffhandley

@sfoslund
Copy link
Member

@marek-safar looks like we're getting the same error after the change. It might be because iOS is still marked as a supported platform in the build we're using for stage 0.

@marek-safar
Copy link
Contributor

platform in the build we're using for stage 0

yep, I think that's the case but not sure how to skip that

@sfoslund
Copy link
Member

You might have to make that change to remove iOS from supported platforms as a separate PR, get it to flow through and produce a build, then use that build as stage 0 here. It would take a day or two but I'm not sure how to get a passing build otherwise.

@danmoseley
Copy link
Member

Is it only Process.Start? If so another option could be to remove [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] from just that method for now.

@marek-safar are you taking care of this?

@marek-safar
Copy link
Contributor

another option could be to remove [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] from just that method for now.

That's not an option.

@marek-safar are you taking care of this?

I'd like to hear from @jeffhandley or @terrajobst as they own the spec which includes ios everywhere.

…0319.6

Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.CodeDom , System.Security.Cryptography.ProtectedData , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 , System.Text.Encoding.CodePages
 From Version 6.0.0-preview.3.21167.1 -> To Version 6.0.0-preview.3.21169.6
@jeffhandley
Copy link
Member

jeffhandley commented Mar 23, 2021

You might have to make that change to remove iOS from supported platforms as a separate PR, get it to flow through and produce a build, then use that build as stage 0 here. It would take a day or two but I'm not sure how to get a passing build otherwise.

The other way to get a passing build here will be to update Directory.Build.targets (or another appropriate spot) to explicitly remove iOS (and Android) from the supported platforms for the SDK build.

<ItemGroup>
    <SupportedPlatform Remove="iOS" />
    <SupportedPlatform Remove="Android" />
</ItemGroup>

I expect that would get the build unblocked here, but we also need to reconsider whether or not it makes sense for iOS and Android to be in the default set. Similar to how we add Browser in only for Blazor projects, I suspect we'll want to move iOS and Android out of the default set and into project types where it makes sense--or make them opt-in. I'm not yet sure what that conclusion will be though.

Edit: Changed from Directory.Build.props to Directory.Build.targets so that the ordering is correct, overriding the default

@jeffhandley
Copy link
Member

The failures I'm now seeing (so far) are the following:

    Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAHelloWorldProject.It_publishes_with_a_publish_profile(selfContained: False, useAppHost: False) [FAIL]
      System.IO.IOException : Unable to delete directory C:\h\w\BD7C09AC\t\dotnetSdkTests\f5tk2iw3.3is\It_publishes_---653DE87C
      ---- System.IO.IOException : The process cannot access the file '\\?\C:\h\w\BD7C09AC\t\dotnetSdkTests\f5tk2iw3.3is\It_publishes_---653DE87C\ConsoleWithPublishProfile' because it is being used by another process.
      Stack Trace:
        /_/src/Tests/Microsoft.NET.TestFramework/TestDirectory.cs(51,0): at Microsoft.NET.TestFramework.TestDirectory.EnsureExistsAndEmpty(String path, String sdkVersion)
        /_/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs(60,0): at Microsoft.NET.TestFramework.TestAssetsManager.CreateTestProject(TestProject testProject, String callingMethod, String identifier, String targetExtension)
        /_/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAHelloWorldProject.cs(582,0): at Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAHelloWorldProject.It_publishes_with_a_publish_profile(Nullable`1 selfContained, Nullable`1 useAppHost)
        ----- Inner Stack Trace -----
           at System.IO.FileSystem.RemoveDirectoryRecursive(String fullPath, WIN32_FIND_DATA& findData, Boolean topLevel)
           at System.IO.FileSystem.RemoveDirectory(String fullPath, Boolean recursive)
        /_/src/Tests/Microsoft.NET.TestFramework/TestDirectory.cs(47,0): at Microsoft.NET.TestFramework.TestDirectory.EnsureExistsAndEmpty(String path, String sdkVersion)

@danmoseley
Copy link
Member

Two tests end up with the same output directory again.

    Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAClickOnceProject.It_publishes_with_a_publish_profile(publishSingleFile: False) [FAIL]
      Expected command to pass but it did not.
      File Name: C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\MSBuild\Current\Bin\msbuild.exe
      Arguments: /t:Publish C:\h\w\BD7C09AC\t\dotnetSdkTests\f5tk2iw3.3is\It_publishes_---653DE87C\ConsoleWithPublishProfile\ConsoleWithPublishProfile.csproj /restore /p:PublishProfile=test


    Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAHelloWorldProject.It_publishes_with_a_publish_profile(selfContained: False, useAppHost: False) [FAIL]
      System.IO.IOException : Unable to delete directory C:\h\w\BD7C09AC\t\dotnetSdkTests\f5tk2iw3.3is\It_publishes_---653DE87C
      ---- System.IO.IOException : The process cannot access the file '\\?\C:\h\w\BD7C09AC\t\dotnetSdkTests\f5tk2iw3.3is\It_publishes_---653DE87C\ConsoleWithPublishProfile' because it is being used by another process.

hmm

@danmoseley
Copy link
Member

@wli3 this is because your fix 2638bba which added

         public static string GetTestDestinationDirectoryPath(
             string testProjectName,
-            string callingMethod,
+            string callingMethodAndFileName,

updated this caller to add that filename

GetTestDestinationDirectoryPath(testProjectName, callingMethod + "_" + fileName, identifier);

but not these callers
https://github.com/dotnet/sdk/blob/7af705de81b69cfaa0804a92b61f60295ac7606d/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs#L61-L60
https://github.com/dotnet/sdk/blob/7af705de81b69cfaa0804a92b61f60295ac7606d/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs#L92-L91

How do you want to fix this?

@danmoseley
Copy link
Member

danmoseley commented Mar 23, 2021

Options

what do you prefer? I suggest option 3.

@wli3
Copy link

wli3 commented Mar 23, 2021

@sfoslund could you help work on this since you are the domestic cat this week? I think we can just take approach give up on trying to encode every test to a deterministic test path. The structure of the tests makes this too complicated. Instead just use random bytes . But make sure the test will print out the temp folder as part of the output. So we can debug it later.

@sfoslund
Copy link
Member

Sure, I created an issue to track since there's no need to block on this: #16490

@jiangzeng01
Copy link

It looks like there is issue dotnet/installer#10013 related to this which fails all WinForms/WPF apps using latest 6.0.100-preview.3.21173.8 SDK, if anyone knows when the issue dotnet/installer#10013 will be fixed?

@dotnet-maestro dotnet-maestro bot merged commit 364ebeb into main Mar 24, 2021
@dotnet-maestro dotnet-maestro bot deleted the darc-main-fde1fa60-b409-421b-9c16-9e3dc0eb6115 branch March 24, 2021 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants