Adds support for generating Mono named NETCore.App Runtime Packs #34980

steveisok · 2020-04-15T01:34:12Z

In the platforms where there will be both Mono and CoreCLR, we should provide uniquely named runtime packs for easier identification. This is currently scoped to desktop mono, but could be applied to other scenarios as needed.

Refer to #34193 for the discussion that drove this PR.

In the platforms where there will be both Mono and CoreCLR, we should provide uniquely named runtime packs for easier identification. dotnet#34193 contains the discussion that drove this PR.

src/installer/pkg/projects/netcoreapp/pkg/Microsoft.NETCore.App.Runtime.pkgproj

safern · 2020-04-15T01:37:45Z

Do we want to start generating mono desktop runtime packs already? Or is this just a step towards that?

src/installer/pkg/projects/netcoreapp/pkg/Microsoft.NETCore.App.Runtime.pkgproj

steveisok · 2020-04-15T01:59:02Z

Do we want to start generating mono desktop runtime packs already? Or is this just a step towards that?

Yes, I think so. This will be to support Xamarin.Mac. Also, the benchmark team was relying on runtime nuget packages in order to get mono into their runs. Since they were recently disabled, I'm intending for the runtime pack to be a substitute.

eng/pipelines/installer/jobs/base-job.yml

Co-Authored-By: Alexander Köplinger <[email protected]>

…k/runtime into enable-custom-named-runtimepack

…s chosen

src/installer/pkg/projects/Directory.Build.targets

…to pick up RuntimeFlavor earlier

…s that need worked through

steveisok · 2020-04-16T23:54:20Z

Removing linux & windows installers. Will file separate issues.

Linux (base-job needs modified)

##[error]Unhandled: Not found SourceFolder: /datadisks/disk1/workspace/_work/1/s/artifacts/obj/linux-x64.Release/sharedFrameworkPublish

Win (mono needs to write file version & other attributes to coreclr.dll)

.nuget\packages\microsoft.dotnet.build.tasks.sharedframework.sdk\5.0.0-beta.20201.2\targets\framework.dependency.targets(201,5): Missing FileVersion in 1 shared framework files:

steveisok · 2020-04-17T12:14:58Z

Official build passed https://dev.azure.com/dnceng/internal/_build/results?buildId=604886&view=results

dagood · 2020-04-17T15:01:24Z

I don't know what's expected since I haven't been keeping up with the infra, but this look strange:

In Installer Build and Test coreclr OSX_x64 Release, runtime.osx-x64.runtime.native.System.IO.Ports.5.0.0-preview.4.20216.15.nupkg is uploaded to the artifacts:

Copying /Users/runner/runners/2.166.2/work/1/s/artifacts/packages/Release/Shipping/runtime.osx-x64.runtime.native.System.IO.Ports.5.0.0-preview.4.20216.15.nupkg to /Users/runner/runners/2.166.2/work/1/a/UnsignedArtifacts/OSX_x64/Shipping/runtime.osx-x64.runtime.native.System.IO.Ports.5.0.0-preview.4.20216.15.nupkg

https://dev.azure.com/dnceng/internal/_build/results?buildId=604886&view=logs&j=b729f9b8-8cfd-5258-0750-b059b4fd2770&t=1d60dfb5-561e-5f67-3db2-39f0e5ea44ff&l=55

The same nupkg (well, filename) is uploaded during Installer Build and Test mono OSX_x64 Release:

Copying /Users/runner/runners/2.166.2/work/1/s/artifacts/packages/Release/Shipping/runtime.osx-x64.runtime.native.System.IO.Ports.5.0.0-preview.4.20216.15.nupkg to /Users/runner/runners/2.166.2/work/1/a/UnsignedArtifacts/OSX_x64/Shipping/runtime.osx-x64.runtime.native.System.IO.Ports.5.0.0-preview.4.20216.15.nupkg

https://dev.azure.com/dnceng/internal/_build/results?buildId=604886&view=logs&j=04abcb85-da1e-56d8-8bdb-e7a8fd8e385f&t=85712f9a-2311-57a9-bee8-a904ee29ec2f&l=13

That makes it a race, but don't know if the contents are different. This may be present in more of the build legs, don't know if this is really a problem with this PR.

It's weird to me that this Libraries native package is being uploaded at this point in the build, at all.

steveisok · 2020-04-17T15:07:06Z

I saw that too and thought about trying to remove it. Should it hold this PR up though?

dagood

Setup changes look reasonable to me. Up to infra team if the race issue needs more attention in this specific PR.

safern · 2020-04-17T15:48:54Z

Good catch @dagood. So the package contents should be the same as for this new leg the libraries assets are build against coreclr since I don’t see @steveisok adding a configuration for OSX with mono configuration in libraries.

However I think we should do that and probably skip package generation in libraries when RuntimeFlavor == mono. cc @ericstj thoughts?

ViktorHofer · 2020-04-17T15:57:38Z

eng/Subsets.props

+    <RuntimeFlavor Condition="'$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'Android' or '$(TargetOS)' == 'WebAssembly'">Mono</RuntimeFlavor>
+    <RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime')))">Mono</RuntimeFlavor>
+    <RuntimeFlavor Condition="'$(RuntimeFlavor)' == ''">CoreCLR</RuntimeFlavor>
+


nit: Please close this PropertyGroup and open a new one.

can that happen in a follow up PR?

ViktorHofer · 2020-04-17T15:57:50Z

eng/Subsets.props

  </PropertyGroup>

+  <!-- Init _subSet here in to allow RuntimeFlavor to be set as early as possible -->


Suggested change

Same - can that happen in a follow up PR?

of course, those were just nits.

dagood · 2020-04-17T16:04:48Z

eng/Subsets.props

+
+  <PropertyGroup>
+    <RuntimeFlavor Condition="'$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'Android' or '$(TargetOS)' == 'WebAssembly'">Mono</RuntimeFlavor>
+    <RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime')))">Mono</RuntimeFlavor>


Lost some + in the move, it looks like, if I understand the pattern right:

Suggested change

<RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime')))">Mono</RuntimeFlavor>

<RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime+'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime+')))">Mono</RuntimeFlavor>

I would put it on multiple lines for easier reading/review, but this is subjective and I'm sure some people are annoyed at me doing it all the time 😄:

<RuntimeFlavor Condition=" '$(RuntimeFlavor)' == '' and ( $(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime')) ) and ( !$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime')) )">Mono</RuntimeFlavor>

@steveisok I edited my message on Teams and added the missing pluses :D You probably took mine before the edit :D

FWIW, seems like this is a nit as well to me--I think it would only cause a problem if we added a clr.runtimefoobar subset (and... some probably complicated scenario about what the subset's meant to do).

Co-authored-by: Steve Pfister <[email protected]>

Adds support for generating Mono named NETCoreApp Runtime Packs

57e1d7e

In the platforms where there will be both Mono and CoreCLR, we should provide uniquely named runtime packs for easier identification. dotnet#34193 contains the discussion that drove this PR.

Dotnet-GitSync-Bot added the area-Setup label Apr 15, 2020

steveisok requested review from dagood and akoeplinger April 15, 2020 01:34

safern reviewed Apr 15, 2020

View reviewed changes

src/installer/pkg/projects/netcoreapp/pkg/Microsoft.NETCore.App.Runtime.pkgproj Outdated Show resolved Hide resolved

safern reviewed Apr 15, 2020

View reviewed changes

src/installer/pkg/projects/netcoreapp/pkg/Microsoft.NETCore.App.Runtime.pkgproj Show resolved Hide resolved

dagood reviewed Apr 15, 2020

View reviewed changes

src/installer/pkg/projects/netcoreapp/pkg/Microsoft.NETCore.App.Runtime.pkgproj Outdated Show resolved Hide resolved

dagood requested a review from NikolaMilosavljevic April 15, 2020 01:44

Tweaks

67144c5

Steve Pfister added 3 commits April 14, 2020 22:18

Add additional platforms to the installer build

a4e79df

Added runtime flavor to job name in official build

3403d21

Syntax

f8a20b6

akoeplinger reviewed Apr 15, 2020

View reviewed changes

eng/pipelines/installer/jobs/base-job.yml Outdated Show resolved Hide resolved

steveisok and others added 8 commits April 15, 2020 16:26

Update eng/pipelines/installer/jobs/base-job.yml

2fffa1d

Co-Authored-By: Alexander Köplinger <[email protected]>

Disabled crossgen for desktop mono installer and addressed feedback

47c7107

Merge branch 'enable-custom-named-runtimepack' of github.com:steveiso…

3d0823f

…k/runtime into enable-custom-named-runtimepack

Removed extra crossgen removal and renamed target

4d03a34

Tweak to get things going

da55a57

Little more yml tweaking

a8803f0

Make sure RuntimeFlavor is being set when just the installer subset i…

0f5b218

…s chosen

Extra yml tweaks

49d4216

jaredpar mentioned this pull request Apr 16, 2020

Restore failing on our projects #35057

Closed

safern reviewed Apr 16, 2020

View reviewed changes

src/installer/pkg/projects/Directory.Build.targets Outdated Show resolved Hide resolved

Steve Pfister added 2 commits April 16, 2020 18:00

Shifted building runtime packs only for all of mono. Altered subsets …

d301875

…to pick up RuntimeFlavor earlier

Removing linux and windows from the mono installer as there are error…

4752408

…s that need worked through

Made log name unique to the runtime flavor

305b0cc

akoeplinger approved these changes Apr 17, 2020

View reviewed changes

steveisok mentioned this pull request Apr 17, 2020

Follow up task for named runtime pack functionality #35107

Closed

2 tasks

dagood approved these changes Apr 17, 2020

View reviewed changes

steveisok requested a review from a team April 17, 2020 15:11

ViktorHofer reviewed Apr 17, 2020

View reviewed changes

dagood reviewed Apr 17, 2020

View reviewed changes

safern approved these changes Apr 17, 2020

View reviewed changes

safern mentioned this pull request Apr 17, 2020

Mdhwang/reduce mobile checks #35105

Merged

steveisok merged commit 73a10cc into dotnet:master Apr 17, 2020

steveisok pushed a commit to steveisok/runtime that referenced this pull request Apr 17, 2020

Fixes nits identified in dotnet#34980

c013c36

This was referenced Apr 17, 2020

Update RidSpecificPackProperties in framework.packing.targets dotnet/arcade#5287

Merged

[installer] Add RuntimeSpecificFrameworkSuffix property #35124

Merged

ViktorHofer pushed a commit that referenced this pull request Apr 18, 2020

Fixes nits identified in #34980 (#35121)

381754d

Co-authored-by: Steve Pfister <[email protected]>

ghost locked as resolved and limited conversation to collaborators Dec 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for generating Mono named NETCore.App Runtime Packs #34980

Adds support for generating Mono named NETCore.App Runtime Packs #34980

steveisok commented Apr 15, 2020

safern commented Apr 15, 2020

steveisok commented Apr 15, 2020

steveisok commented Apr 16, 2020

steveisok commented Apr 17, 2020

dagood commented Apr 17, 2020

steveisok commented Apr 17, 2020

dagood left a comment

safern commented Apr 17, 2020

ViktorHofer Apr 17, 2020

steveisok Apr 17, 2020

ViktorHofer Apr 17, 2020

steveisok Apr 17, 2020

ViktorHofer Apr 17, 2020

dagood Apr 17, 2020

ViktorHofer Apr 17, 2020

dagood Apr 17, 2020 •

edited

Loading

		</PropertyGroup>

		<!-- Init _subSet here in to allow RuntimeFlavor to be set as early as possible -->

	<RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime')))">Mono</RuntimeFlavor>
	<RuntimeFlavor Condition="'$(RuntimeFlavor)' == '' and ($(_subset.Contains('+mono+')) or $(_subset.Contains('+mono.runtime+'))) and (!$(_subset.Contains('+clr+')) and !$(_subset.Contains('+clr.runtime+')))">Mono</RuntimeFlavor>

Adds support for generating Mono named NETCore.App Runtime Packs #34980

Adds support for generating Mono named NETCore.App Runtime Packs #34980

Conversation

steveisok commented Apr 15, 2020

safern commented Apr 15, 2020

steveisok commented Apr 15, 2020

steveisok commented Apr 16, 2020

steveisok commented Apr 17, 2020

dagood commented Apr 17, 2020

steveisok commented Apr 17, 2020

dagood left a comment

Choose a reason for hiding this comment

safern commented Apr 17, 2020

ViktorHofer Apr 17, 2020

Choose a reason for hiding this comment

steveisok Apr 17, 2020

Choose a reason for hiding this comment

ViktorHofer Apr 17, 2020

Choose a reason for hiding this comment

steveisok Apr 17, 2020

Choose a reason for hiding this comment

ViktorHofer Apr 17, 2020

Choose a reason for hiding this comment

dagood Apr 17, 2020

Choose a reason for hiding this comment

ViktorHofer Apr 17, 2020

Choose a reason for hiding this comment

dagood Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

dagood Apr 17, 2020 •

edited

Loading