-
Notifications
You must be signed in to change notification settings - Fork 2.6k
First step to create nuget packages for ARM32/Linux #8421
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,21 +4,21 @@ | |
| <PropertyGroup> | ||
| <SkipPackageFileCheck>true</SkipPackageFileCheck> | ||
| <PackageTargetRuntime>ubuntu.14.04-$(PackagePlatform)</PackageTargetRuntime> | ||
| <!-- only build for x64 --> | ||
| <PackagePlatforms>x64;</PackagePlatforms> | ||
| <!-- build for x64, arm --> | ||
| <PackagePlatforms>x64;arm;</PackagePlatforms> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <NativeSplittableBinary Include="$(BinDir)libcoreclr.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)libcoreclrtraceptprovider.so" /> | ||
| <NativeSplittableBinary Condition="'$(PackagePlatform)' != 'arm'" Include="$(BinDir)libcoreclrtraceptprovider.so" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this conditional (are we not building it for Arm)?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This file is not build for ARM.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the traceprovider not built for Arm?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still waiting on this.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found why is it not built for ARM.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am building on the Raspberry PI 3 Raspian.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just made the two changes and completed a build successfully on Linux Mint - my changes are in https://github.com/gkhanna79/coreclr/tree/ArmLTTNG. If these look good, then @hseok-oh can you please incorporate them as
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Successfully build libcoreclrtraceptprovider.so by @gkhanna79's patch with my private rootfs (based on ubuntu-mate 16.04). But it fails with default rootfs (ubuntu 14.04) for ARM cross build, and fails in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #3879 (comment) IMHO, it's better excluding
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hseok-oh Please file an issue for building this for Arm to ensure we dont miss it. With that said, I think the rest of the PR LGTM to merge. |
||
| <NativeSplittableBinary Include="$(BinDir)libdbgshim.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)libmscordaccore.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)libmscordbi.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)libsos.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)libsosplugin.so" /> | ||
| <NativeSplittableBinary Include="$(BinDir)System.Globalization.Native.so" /> | ||
| <ArchitectureSpecificNativeFile Include="$(BinDir)sosdocsunix.txt" /> | ||
| <ArchitectureSpecificNativeFile Include="$(BinDir)mscorlib.ni.dll" /> | ||
| <ArchitectureSpecificNativeFile Include="$(BinDir)System.Private.CoreLib.ni.dll" /> | ||
| <ArchitectureSpecificNativeFile Condition="'$(PackagePlatform)'!='arm'" Include="$(BinDir)mscorlib.ni.dll" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume you are doing this because cross-compilation done on a non-Arm machine (e.g. x64) does not build a x86 (or x64) hosted crossgen that can generate Arm code,right? If it, i think it will really help to bring up the x86 Linux build as that will help build a x86-hosted crossgen that generate arm32 BI images and thus, alleviating the need for these.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly. We also think implementing x86-host/arm-target crossgen as future work.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @hseok-oh. Can you please file an issue for "Building cross-targeting components" for Linux Arm32 bringup? |
||
| <ArchitectureSpecificNativeFile Condition="'$(PackagePlatform)'!='arm'" Include="$(BinDir)System.Private.CoreLib.ni.dll" /> | ||
| <ArchitectureSpecificLibFile Include="$(BinDir)System.Private.CoreLib.dll" /> | ||
| <ArchitectureSpecificLibFile Include="$(BinDir)mscorlib.dll" /> | ||
| <ArchitectureSpecificLibFile Include="$(BinDir)SOS.NETCore.dll" /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this function, why can we not fix https://github.com/dotnet/coreclr/pull/8421/files#diff-0b83f9dedf40d7356e5ca147a077acb4R67 to use
$__BuildArchinstead of$__HostArch?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$__DistroRidis used in https://github.com/hseok-oh/coreclr/blob/5145193d4970eaf9a008014a7b02307611bd96ef/build.sh#L237 before nuget packaging, and this function need host environment information to use msbuild on .net core.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the above, I would suggest that you rename the current $__DistroRid to $__HostDistroRid and your proposed $__CrossDistroRid as $__DistroRid. This will maintain the existing uses and convey the correct meaning via $__DistroRid and also eliminate your changes in dir.props.