Fix non-portable coreclr arm64 build #63121

gbalykov · 2021-12-24T13:28:33Z

Without this change arm64 coreclr build for tizen breaks with:

/home/runtime/.dotnet/sdk/6.0.100/Microsoft.Common.CurrentVersion.targets(5100,5): error MSB3030: Could not copy the file "/home/runtime/.packages/runtime.tizen.6.5.0-arm64.microsoft.dotnet.ilcompiler/7.0.0-alpha.1.21612.2/tools/libobjwriter.so" because it was not found. [/home/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj]

Build command:

ROOTFS_DIR=`pwd`/.tools/rootfs/arm64 ./build.sh --portablebuild false --cross --clang10 --arch arm64 --runtimeConfiguration Release --librariesConfiguration Release --subset clr+libs.native /p:EnableSourceLink=false

This is related to #62563 and #63035.

cc @alpencolt

ghost · 2021-12-24T13:28:39Z

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Without this change arm64 coreclr build for tizen breaks with:

/home/runtime/.dotnet/sdk/6.0.100/Microsoft.Common.CurrentVersion.targets(5100,5): error MSB3030: Could not copy the file "/home/runtime/.packages/runtime.tizen.6.5.0-arm64.microsoft.dotnet.ilcompiler/7.0.0-alpha.1.21612.2/tools/libobjwriter.so" because it was not found. [/home/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj]

Build command:

ROOTFS_DIR=`pwd`/.tools/rootfs/arm64 ./build.sh --portablebuild false --cross --clang10 --arch arm64 --runtimeConfiguration Release --librariesConfiguration Release --subset clr+libs.native /p:EnableSourceLink=false

This is related to #62563 and #63035.

cc @alpencolt

Author:	gbalykov
Assignees:	-
Labels:	`area-Infrastructure-coreclr`
Milestone:	-

gbalykov · 2021-12-24T13:29:09Z

@MichalStrehovsky could you, please, take a look?

am11 · 2021-12-24T14:13:17Z

@gbalykov, if this project has same packaging plan as crossgen2 (i.e. only portable platform packages are produced), then I think a better / scalable solution is to replace these lines:

runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj

Lines 24 to 30 in 32700f0

    
               <RuntimeIdentifier>$(__DistroRid)</RuntimeIdentifier> 
        
               <!-- DistroRid is injected through environment variables in build scripts. Provide reasonable default 
        
                    when the build is invoked directly, e.g. building inside Visual Studio --> 
        
               <RuntimeIdentifier Condition="'$(RuntimeIdentifier)' == '' and '$(TargetsWindows)' == 'true'">win-$(TargetArchitecture)</RuntimeIdentifier> 
        
               <RuntimeIdentifier Condition="'$(RuntimeIdentifier)' == '' and '$(TargetsLinux)' == 'true'">linux-$(TargetArchitecture)</RuntimeIdentifier> 
        
               <RuntimeIdentifier Condition="'$(RuntimeIdentifier)' == '' and '$(TargetsOSX)' == 'true'">osx-$(TargetArchitecture)</RuntimeIdentifier>

with a single:

<RuntimeIdentifier>$(Crossgen2PackageRID)</RuntimeIdentifier>

and keep it enabled on all platforms.

MichalStrehovsky

This looks good. If @am11's suggestion works for you, that one also looks good.

gbalykov · 2021-12-27T09:29:07Z

I've updated this PR to @am11 solution, I think it's a better one, but I've also renamed Crossgen2PackageRID to PortablePackageRID. This approach also fixes build.

am11 · 2021-12-27T11:04:22Z

SourceBuild's failure is related, others are networking issues. This means it's not on the same packaging plan as crossgen2. We can try using MicrosoftNetCoreIlasmPackageRuntimeId instead.

gbalykov · 2022-01-04T15:55:30Z

@am11 approach with MicrosoftNetCoreIlasmPackageRuntimeId seemed to work

am11 · 2022-01-04T16:51:01Z

Assuming it works with both armel and arm64 Tizen builds, looks good, thanks!
Perhaps we can add a property with a neutral name and use that one in ILCompiler.csproj (while keeping the existing one; currently used by source-build):

--- Directory.Build.props
+++ Directory.Build.props

-    <!-- There are no non-portable builds for Ilasm/Ildasm -->
-    <MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(PortableBuild)' != 'true' and '$(_portableOS)' == 'linux'">linux-$(_hostArch)</MicrosoftNetCoreIlasmPackageRuntimeId>
-    <MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(MicrosoftNetCoreIlasmPackageRuntimeId)' == ''">$(_toolRuntimeRID)</MicrosoftNetCoreIlasmPackageRuntimeId>

+    <!-- There are no non-portable builds for Ilasm, Ildasm, ILC etc. -->
+    <ToolsRID Condition="'$(PortableBuild)' != 'true' and '$(_portableOS)' == 'linux'">linux-$(_hostArch)</ToolsRID>
+    <ToolsRID Condition="'$(ToolsRID)' == ''">$(_toolRuntimeRID)</ToolsRID>
+    <MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(MicrosoftNetCoreIlasmPackageRuntimeId)' == ''">$(ToolsRID)</MicrosoftNetCoreIlasmPackageRuntimeId>

@gbalykov, (not related to this PR but) one interesting thing I noticed from 63122 in your build steps is that in case of libs/libs.tests, you don't pass -portablebuild=false. Is there a particular reason for that? If not, please do pass that argument to all relevant commands and then we can simplify the RID calculation a little bit by deleting this hacky line:

runtime/Directory.Build.props

Line 151 in ae2e360

<_runtimeOS Condition="$(_runtimeOS.StartsWith('tizen'))">linux</_runtimeOS>

gbalykov · 2022-01-04T20:34:24Z

@am11 thanks for ToolsRID name suggestion, I'll update PR.

Regarding portable build of libs. There were build problems with non-portable build of libs for tizen, which I first met with .net 5 (if I remember correctly). This is related to the fact that we do not ship tizen nuget packages. So, dotnet restore failed. Now I do not see any problems when libs are also built with --portablebuild false (both with and without the line that sets _runtimeOS for tizen). However, this same error with not found nuget packages still shows up for coreclr tests build with portablebuild=false, so we use two step build of clr tests too: native part as non-portable, managed part as portable.

am11 · 2022-01-04T21:08:51Z

Directory.Build.props

While you are at it, could you please rename the (private) property _toolRuntimeRID -> _toolsRID as well? It is only defined and used in this file. (word "runtime" is redundant)

gbalykov · 2022-01-09T16:52:15Z

@am11 I've updated PR, could you, please, take a look?

am11

LGTM, Thank you!
Linux failure is #61359 and OSX timeout is unrelated as well.

am11 · 2022-01-09T22:40:04Z

Directory.Build.props

Suggested change

<MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(MicrosoftNetCoreIlasmPackageRuntimeId)' == ''">$(ToolsRID)</MicrosoftNetCoreIlasmPackageRuntimeId>

<MicrosoftNetCoreIlasmPackageRuntimeId>$(ToolsRID)</MicrosoftNetCoreIlasmPackageRuntimeId>

Done, thanks

gbalykov · 2022-01-11T08:20:32Z

Thanks!

ghost added area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member labels Dec 24, 2021

gbalykov mentioned this pull request Dec 24, 2021

Libraries tests 7.0 (master) cross build failed for tizen arm64 #63122

Closed

MichalStrehovsky approved these changes Dec 27, 2021

View reviewed changes

gbalykov force-pushed the fix-non-portable-arm64-build branch from 0a3ba44 to 656540a Compare December 27, 2021 09:27

gbalykov force-pushed the fix-non-portable-arm64-build branch from 656540a to adc0270 Compare January 4, 2022 13:07

gbalykov force-pushed the fix-non-portable-arm64-build branch from adc0270 to 1662704 Compare January 4, 2022 20:52

am11 reviewed Jan 4, 2022

View reviewed changes

gbalykov force-pushed the fix-non-portable-arm64-build branch from 1662704 to c31c20b Compare January 9, 2022 16:44

am11 reviewed Jan 9, 2022

View reviewed changes

Fix non-portable arm64 build

620fe36

gbalykov force-pushed the fix-non-portable-arm64-build branch from c31c20b to 620fe36 Compare January 10, 2022 07:31

MichalStrehovsky approved these changes Jan 11, 2022

View reviewed changes

MichalStrehovsky merged commit 917d369 into dotnet:main Jan 11, 2022

ghost locked as resolved and limited conversation to collaborators Feb 10, 2022

	<MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(MicrosoftNetCoreIlasmPackageRuntimeId)' == ''">$(ToolsRID)</MicrosoftNetCoreIlasmPackageRuntimeId>
	<MicrosoftNetCoreIlasmPackageRuntimeId>$(ToolsRID)</MicrosoftNetCoreIlasmPackageRuntimeId>

Fix non-portable coreclr arm64 build #63121

Fix non-portable coreclr arm64 build #63121

Uh oh!

Conversation

gbalykov commented Dec 24, 2021

Uh oh!

ghost commented Dec 24, 2021

Uh oh!

gbalykov commented Dec 24, 2021

Uh oh!

am11 commented Dec 24, 2021

Uh oh!

MichalStrehovsky left a comment

Choose a reason for hiding this comment

Uh oh!

gbalykov commented Dec 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

am11 commented Dec 27, 2021

Uh oh!

gbalykov commented Jan 4, 2022

Uh oh!

am11 commented Jan 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gbalykov commented Jan 4, 2022

Uh oh!

am11 Jan 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbalykov commented Jan 9, 2022

Uh oh!

am11 left a comment

Choose a reason for hiding this comment

Uh oh!

am11 Jan 9, 2022

Choose a reason for hiding this comment

Uh oh!

gbalykov Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

gbalykov commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gbalykov commented Dec 27, 2021 •

edited

Loading

am11 commented Jan 4, 2022 •

edited

Loading

am11 Jan 4, 2022 •

edited

Loading