Port all managed product binaries to use SDK style projects by AaronRobinsonMSFT · Pull Request #24285 · dotnet/coreclr

AaronRobinsonMSFT · 2019-04-27T02:03:14Z

This PR is not complete. The optprof scenario will be broken. @jkoritzinsky suggestions are welcome.

Much of this work was ported from #23988. Note that this work does not upgrade to 3.0, but merely moves the product projects to SDK project types.

cc @RussKeldorph @jashook @jkoritzinsky @jeffschwMSFT

build.proj

jashook

Lgtm thank you for the work!

jashook · 2019-04-29T15:35:01Z

src/Directory.Build.targets

+    <BuiltBinary Include="$(TargetPath)" />
+  </ItemGroup>
+
+  <!-- Target used to consolidate all PDBs into a single location -->


Can we expand on this comment further?

I don't know what you would like to see. All product PDBs go into a specific location. The reasoning seems to be to help packaging. I don't really know what else to say other than the present comment. I don't want the litter the code with policy only functionality.

Could you clarify why we need custom logic to move symbols? I expected that arcade would put them in the right place if we use it as designed.

We are only using arcade to build assets, not rely on arcades output paths at the moment. That is a second pass.

src/System.Private.CoreLib/System.Private.CoreLib.csproj

jashook · 2019-04-29T15:47:35Z

src/tools/r2rdump/R2RDump.csproj

-<Project Sdk="Microsoft.NET.Sdk" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+<Project Sdk="Microsoft.NET.Sdk">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />


Can we add a condition on ArcadeBuild=="" on the import?

I don't understand why in this case. It was present before and is needed regardless of Arcade or not.

AaronRobinsonMSFT · 2019-04-29T17:36:59Z

cc @sbomer @echesakovMSFT

AaronRobinsonMSFT · 2019-04-30T03:16:15Z

There is something awry here. I am not observing these issues locally. Will merge with master tomorrow and see if I can reproduce locally.

AaronRobinsonMSFT · 2019-04-30T19:03:43Z

I was finally able to reproduce this locally. After rebasing and doing a clean build, the issue went away. Pushing a fully rebased branch.

sbomer

Thank you for doing this!

src/System.Private.CoreLib/System.Private.CoreLib.csproj

dir.props

src/Directory.Build.props

sbomer · 2019-04-30T16:54:45Z

src/Directory.Build.targets

+    <BuiltBinary Include="$(TargetPath)" />
+  </ItemGroup>
+
+  <!-- Target used to consolidate all PDBs into a single location -->


Could you clarify why we need custom logic to move symbols? I expected that arcade would put them in the right place if we use it as designed.

AaronRobinsonMSFT · 2019-05-01T02:19:03Z

@dotnet-bot test Windows_NT x64 Release CoreFX Tests please

AaronRobinsonMSFT · 2019-05-01T02:59:45Z

@dotnet-bot test Windows_NT x64 Release CoreFX Tests please

AaronRobinsonMSFT · 2019-05-01T03:09:15Z

@dotnet-bot test Ubuntu x64 Checked CoreFX Tests please

AaronRobinsonMSFT · 2019-05-01T03:16:36Z

The failures appear to be a a real issue in tests\src\Interop\PInvoke\SafeHandles\Interface\InterfaceTest.cs - see SH_MAIntf.

AaronRobinsonMSFT · 2019-05-01T03:38:11Z

@jkoritzinsky Any thoughts on why the SH_MAIntf test is consistently failing? I see the comment, but am unsure why this would all of sudden start working. I think I remember talking to you about this functionality, but really can't remember the specifics. I can accept that I am missing some #define or configuration during the build. Any thoughts? If not I will just debug it tomorrow, but if you know what setting I might be missing that would be appreciated. Thanks.

coreclr/tests/src/Interop/PInvoke/SafeHandles/Interface/InterfaceTest.cs

Lines 56 to 61 in 07b3afc

    
           //NOTE: SafeHandle is now ComVisible(false)...QIs for IDispatch or the class interface on a  
        
           //    type with a ComVisible(false) type in its hierarchy are no longer allowed; so calling  
        
           //    the DW ctor with a SH subclass causes an invalidoperationexception to be thrown since 
        
           //    the ctor QIs for IDispatch 
        
           Console.WriteLine("Testing SH_MAIntf..."); 
        
           Assert.Throws<InvalidOperationException>(() => SH_MAIntf(sh, shVal, shfld1Val, shfld2Val), "Did not throw InvalidOperationException!");

AaronRobinsonMSFT · 2019-05-01T04:51:18Z

I discovered the issue. There are missing assembly level attributes (e.g. ComVisible(false)).

AaronRobinsonMSFT · 2019-05-01T05:05:44Z

@jkotas I need to add the following attributes to SPCL because the built-in method for SDK projects doesn't permit supplying attributes that have constructors taking non-string arguments. I am adding a new file to this project called AssemblyAttributes.cs with these specific attributes:

[assembly:System.Runtime.InteropServices.ComVisible(false)]
[assembly:System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute(System.Runtime.InteropServices.DllImportSearchPath.AssemblyDirectory | System.Runtime.InteropServices.DllImportSearchPath.System32)]

Do you have any preference where this file is located or if I should place this in an existing file?

AaronRobinsonMSFT · 2019-05-01T05:17:22Z

Correction. I found existing assembly level attributes in SPCL and moved these attributes there as well.

coreclr/src/System.Private.CoreLib/src/System/Internal.cs

Lines 27 to 29 in 07b3afc

    
           [assembly: DefaultDependencyAttribute(LoadHint.Always)] 
        
           // mscorlib would like to have its literal strings frozen if possible 
        
           [assembly: System.Runtime.CompilerServices.StringFreezingAttribute()]

AaronRobinsonMSFT · 2019-05-01T06:09:34Z

The CoreFX AppDomainTests.AssemblyResolve_IsNotCalledForCoreLibResources test is probably failing due to something in this PR.

src/System.Private.CoreLib/src/System/Internal.cs

jkotas · 2019-05-01T07:28:13Z

test is probably failing due to something in this PR.

Are the assembly attributes same before / after the change? The resource culture lookup is controlled via another bunch of attributes.

AaronRobinsonMSFT · 2019-05-01T15:24:04Z

Are the assembly attributes same before / after the change? The resource culture lookup is controlled via another bunch of attributes.

@jkotas Good question. I am looking into that now. It seems that assembly attributes come from a myriad of places so it makes tracking them down a bit difficult in the build system. I will check that out though.

AaronRobinsonMSFT · 2019-05-01T15:39:01Z

@jkotas Looks like the following attributes are different between the two:

new vs old

- System.CLSCompliantAttribute
- System.Reflection.AssemblyMetadataAttribute
- System.Reflection.AssemblyDefaultAliasAttribute
+ System.Resources.NeutralResourcesLanguageAttribute

I would assume the last one is related. I am unsure if the others are legacy or needed at all anymore. The CLSCompliantAttribute is weird that it isn't present, but that could have simply been a default value whenever the assembly info was generated in the previous system. I will need to dig through some more code in build tools to understand what it was doing. Thanks for the suggestion.

AaronRobinsonMSFT · 2019-05-02T01:40:56Z

This change will update the AssemblyFileVersion for System.Private.CoreLib and is related to https://github.com/dotnet/coreclr/issues/22844.

AaronRobinsonMSFT · 2019-05-02T02:30:17Z

/azp help

azure-pipelines · 2019-05-02T02:30:22Z

Supported commands
     help:
          Get descriptions, examples and documentation about supported commands
          Example: help "command_name"
     list:
          List all pipelines for this repository using a comment.
          Example: "list"
     run:
          Run all pipelines or a specific pipeline for this repository using a comment. Use
          this command by itself to trigger all related pipelines, or specify a pipeline
          to run.
          Example: "run" or "run pipeline_name"
     where:
          Report back the Azure DevOps orgs that are related to this repository and org
          Example: "where"

See additional documentation.

AaronRobinsonMSFT · 2019-05-02T02:30:47Z

/azp list

azure-pipelines · 2019-05-02T02:30:52Z

CI/CD Pipelines for this repository:
coreclr-outerloop
coreclr-ci
dotnet-coreclr-nullablefeature

SPCL builds - dotnet.exe msbuild System.Private.CoreLib.csproj /p:ArcadeBuild=true

Packaging step is broken

All scenarios work on Windows. Consuming Arcade versioning logic by default.

Add back RestoreOptData task

location for existing attributes.

Collapse clr.coreclr.props and clr.defines.targets files into a single file

AaronRobinsonMSFT · 2019-05-04T02:09:45Z

/azp run coreclr-outerloop

azure-pipelines · 2019-05-04T02:09:58Z

Azure Pipelines successfully started running 1 pipeline(s).

…oreclr#24285) Convert managed product binary to use SDK project system. - Uses Arcade for versions strings - Overrides Arcade defined output paths - should change in the future Commit migrated from dotnet/coreclr@099177b

AaronRobinsonMSFT added area-Infrastructure-coreclr * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Apr 27, 2019

AaronRobinsonMSFT added this to the 3.0 milestone Apr 27, 2019

jashook reviewed Apr 29, 2019

View reviewed changes

build.proj Outdated Show resolved Hide resolved

jashook approved these changes Apr 29, 2019

View reviewed changes

jkotas mentioned this pull request Apr 30, 2019

[master] Update dependencies from dotnet/core-setup #24149

Merged

AaronRobinsonMSFT force-pushed the port_spcl_to_sdk branch 2 times, most recently from fa84439 to 91b3414 Compare April 30, 2019 19:02

sbomer approved these changes Apr 30, 2019

View reviewed changes

jkotas reviewed May 1, 2019

View reviewed changes

src/System.Private.CoreLib/src/System/Internal.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT force-pushed the port_spcl_to_sdk branch from 9510630 to bd3ee41 Compare May 3, 2019 18:53

AaronRobinsonMSFT added 21 commits May 3, 2019 18:28

[SDKSPCL] used for changes to think about

06b9c3a

SPCL builds - dotnet.exe msbuild System.Private.CoreLib.csproj /p:ArcadeBuild=true

SPCL and SOS.NET.Core build as expected

ce278f6

Packaging step is broken

SPCL and other managed product projects are converted.

87a6feb

All scenarios work on Windows. Consuming Arcade versioning logic by default.

Chain in R2RDump project

d4398c4

Temporarily bring back build.proj to support version file generation.

893da4c

Update bash script

c85b2f9

Review feedback

8fba51b

Add back RestoreOptData task

Specify valid TFM for optdata projects

2211a48

Fix copy/paste error

8340d9a

Release build issues.

74a17f7

Rebase against master and remove spurious MSBuild flags

db3a767

Feedback

faebe45

Adding logging to CMake for PGO data file discovery

77c911a

Set the default value for MSBuild RestoreDuringBuild property to true

97d89c5

Move assembly level attributes out of csproj for SPCL and put in central

4f7e178

location for existing attributes.

Remove legacy NGEN attributes.

6c410e7

Fix SPCL attributes

a44311b

Fix PublicKey for SPCL

856d2e1

Let Arcade define the FileVersion string

17f0e72

Collapse clr.coreclr.props and clr.defines.targets files into a single file

Remove unexpected constants from SPCL build

831e800

Update hardcoded resource string location

6e14e89

AaronRobinsonMSFT force-pushed the port_spcl_to_sdk branch from 2563d06 to 6e14e89 Compare May 4, 2019 01:29

AaronRobinsonMSFT merged commit 099177b into dotnet:master May 6, 2019

AaronRobinsonMSFT deleted the port_spcl_to_sdk branch May 6, 2019 17:19

gbalykov mentioned this pull request May 22, 2019

Fix armel tests build #24678

Merged

alpencolt mentioned this pull request Jan 31, 2020

[Linux/armel] Broken cross build dotnet/runtime#12653

Closed

Conversation

AaronRobinsonMSFT commented Apr 27, 2019

Uh oh!

Uh oh!

jashook left a comment

Choose a reason for hiding this comment

Uh oh!

jashook Apr 29, 2019

Choose a reason for hiding this comment

Uh oh!

AaronRobinsonMSFT Apr 29, 2019

Choose a reason for hiding this comment

Uh oh!

sbomer Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

AaronRobinsonMSFT Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jashook Apr 29, 2019

Choose a reason for hiding this comment

Uh oh!

AaronRobinsonMSFT Apr 29, 2019

Choose a reason for hiding this comment

Uh oh!

AaronRobinsonMSFT commented Apr 29, 2019

Uh oh!

AaronRobinsonMSFT commented Apr 30, 2019

Uh oh!

AaronRobinsonMSFT commented Apr 30, 2019

Uh oh!

sbomer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbomer Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

Uh oh!

jkotas commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 1, 2019

Uh oh!

AaronRobinsonMSFT commented May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronRobinsonMSFT commented May 2, 2019

Uh oh!

azure-pipelines bot commented May 2, 2019

Uh oh!

AaronRobinsonMSFT commented May 2, 2019

Uh oh!

azure-pipelines bot commented May 2, 2019

AaronRobinsonMSFT commented May 1, 2019 •

edited

Loading

AaronRobinsonMSFT commented May 1, 2019 •

edited

Loading

AaronRobinsonMSFT commented May 2, 2019 •

edited

Loading