Skip to content

Conversation

@dotnet-maestro
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Jul 12, 2020

This pull request updates the following dependencies

From https://github.com/mono/linker

  • Subscription: bcca1ed9-0939-490e-f02f-08d75d5208ed
  • Build: 20200711.1
  • Date Produced: 7/11/2020 4:47 PM
  • Commit: 3b524e4aa109ce5c8d9db5d6447386a0714d0d2d
  • Branch: refs/heads/master
  • Updates:
    • Microsoft.NET.ILLink.Tasks: from 5.0.0-preview.3.20360.3 to 5.0.0-preview.3.20361.1

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

  • Subscription: 667d63c7-a2fc-425c-9020-08d76d41c6c3
  • Build: 20200710.1
  • Date Produced: 7/10/2020 3:06 PM
  • Commit: b34a70799faa67f13c2e72852e4f3af463ff4d6c
  • Branch: refs/heads/master
  • Updates:
    • System.ComponentModel.TypeConverter.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.Drawing.Common.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.IO.Compression.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.IO.Packaging.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.Net.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.Private.Runtime.UnicodeData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.Security.Cryptography.X509Certificates.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1
    • System.Windows.Extensions.TestData: from 5.0.0-beta.20319.2 to 5.0.0-beta.20360.1

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

  • Subscription: 724038b1-3479-4d3b-3cbd-08d824643728
  • Build: 20200709.7
  • Date Produced: 7/10/2020 12:50 AM
  • Commit: 88c2807979f8762a121f8e4b217191672d334822
  • Branch: refs/heads/maint/maint-67
  • Updates:
    • Microsoft.NETCore.Runtime.ICU.Transport: from 5.0.0-preview.8.20359.5 to 5.0.0-preview.8.20359.7

Microsoft.NET.ILLink.Tasks
 From Version 5.0.0-preview.3.20360.3 -> To Version 5.0.0-preview.3.20361.1
@dotnet-maestro
Copy link
Contributor Author

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

  • Standard Merge Policies No un-ignored checks.
  • ✔️ Standard Merge Policies Succeeded - No reviews have requested changes.
  • ✔️ Standard Merge Policies Succeeded - No version downgrade detected.

@Dotnet-GitSync-Bot
Copy link
Collaborator

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.

…ld 20200710.1

System.ComponentModel.TypeConverter.TestData , System.Drawing.Common.TestData , System.IO.Compression.TestData , System.IO.Packaging.TestData , System.Net.TestData , System.Private.Runtime.UnicodeData , System.Security.Cryptography.X509Certificates.TestData , System.Windows.Extensions.TestData
 From Version 5.0.0-beta.20319.2 -> To Version 5.0.0-beta.20360.1
@dotnet-maestro dotnet-maestro bot changed the title [master] Update dependencies from mono/linker [master] Update dependencies from mono/linker dotnet/runtime-assets Jul 12, 2020
@dotnet-maestro
Copy link
Contributor Author

dotnet-maestro bot commented Jul 12, 2020

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

  • Standard Merge Policies Unsuccessful checks: runtime (CoreCLR Pri0 Runtime Tests Run Windows_NT x64 checked), runtime (Build Browser wasm Release AllSubsets_Mono), runtime (CoreCLR Tools Unit Tests Linux x64 checked), runtime, runtime-live-build
  • ✔️ Standard Merge Policies Succeeded - No reviews have requested changes.
  • ✔️ Standard Merge Policies Succeeded - No version downgrade detected.

Microsoft.NETCore.Runtime.ICU.Transport
 From Version 5.0.0-preview.8.20359.5 -> To Version 5.0.0-preview.8.20359.7
@dotnet-maestro dotnet-maestro bot changed the title [master] Update dependencies from mono/linker dotnet/runtime-assets [master] Update dependencies from mono/linker dotnet/runtime-assets dotnet/icu Jul 12, 2020
@marek-safar marek-safar force-pushed the darc-master-23077c0f-7360-4e89-9561-4bdb8dfcff9b branch from ff306f4 to 08f7d91 Compare July 14, 2020 06:12
@marek-safar
Copy link
Contributor

Failures are unrelated, merging to unblock work on ICU and linker

@marek-safar marek-safar merged commit 018c8fb into master Jul 14, 2020
@marek-safar marek-safar deleted the darc-master-23077c0f-7360-4e89-9561-4bdb8dfcff9b branch July 14, 2020 16:35
@ericstj
Copy link
Member

ericstj commented Jul 15, 2020

cc @ViktorHofer @safern Don't we normally avoid updating our SDK to non-public builds? This is going to force folks to install a nightly build if they want VS support.

@safern
Copy link
Member

safern commented Jul 15, 2020

We usually bump the SDK on infrastructure rollouts. If we do need to bump it outside of the infra rollouts we need to send an email to the contributors and post it in some channel like gitter to let people know about the breaking change.

@safern
Copy link
Member

safern commented Jul 15, 2020

Don't we normally avoid updating our SDK to non-public builds?

No, if we need features to unblock something in our repo, it is fine to use daily build bits, they can be installed with dotnet-install scripts.

@ericstj
Copy link
Member

ericstj commented Jul 15, 2020

01f68c7

Bump SDK dependency to bring updated ILLink task

Do we actually rely on the SDK for that? I thought we download the package directly via

runtime/eng/Versions.props

Lines 118 to 119 in 059782e

<!-- ILLink -->
<MicrosoftNETILLinkTasksVersion>5.0.0-preview.3.20363.5</MicrosoftNETILLinkTasksVersion>

Maybe @eerhardt or @joperezr have some insight here.

if we need features to unblock something in our repo

True, but we prefer depending on those features from packages if we can since it's less disruptive to contributors.

@marek-safar
Copy link
Contributor

Do we actually rely on the SDK for that?

Yes, SDK has its own illink targets which are used here

@safern
Copy link
Member

safern commented Jul 15, 2020

Should we update our PackageReference to include build assets and not rely on the SDK? as @ericstj suggests?

ExcludeAssets="build"

@joperezr
Copy link
Member

The tricky part about the ILLinker is that the actual tool comes from one package, but the targets and task to invoke it come from the SDK. This makes it really tricky as we update the actual tool quite frequently but the SDK not so much and most of the fixes we sometimes rely on are on the targets/tasks like when we want to enable feature switches for example. This disconnect is one of the reasons why we have our own ILlink targets instead of using the default ones, since we want to control the way we call the linker as sometimes we need features the linker already has, but we are lacking SDK that can take advantage of those.

@safern
Copy link
Member

safern commented Jul 15, 2020

I see. Makes sense.

I think we should send out an email to let folks know about the SDK update outside an infrastructure rollout.

@eerhardt
Copy link
Member

The tricky part about the ILLinker is that the actual tool comes from one package, but the targets and task to invoke it come from the SDK.

Are we at a point where we can just use the linker exe that comes with the SDK? Do we actually need to be on the latest linker .exe in the runtime repo? It feels like keeping the linker exe and the linker targets in sync is a goal.

@marek-safar
Copy link
Contributor

Are we at a point where we can just use the linker exe that comes with the SDK

I don't think we are and probably won't be for some time. Would the alternative be to move SDK targets to linker package?

{
"sdk": {
"version": "5.0.100-preview.6.20310.4",
"version": "5.0.100-preview.8.20362.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is a breaking change that shouldn't have been made outside the monthly Infrastructure Rollout window.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marek-safar can you please send an email to the runtime-repo alias explaining why the SDK's min version was bumped and why it happened outside the batched rollout window? Thanks

@ericstj
Copy link
Member

ericstj commented Jul 16, 2020

Would the alternative be to move SDK targets to linker package?

Yeah. Could be linker package could be something else. Maybe it makes sense to follow the same model as roslyn. It has similar characteristics.

@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
@danmoseley danmoseley added area-codeflow for labeling automated codeflow and removed area-Infrastructure labels Jul 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-codeflow for labeling automated codeflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.