Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass TargetRid and SourceBuildNonPortable to the native scripts #74504

Merged
merged 22 commits into from
Sep 28, 2022
Merged

Pass TargetRid and SourceBuildNonPortable to the native scripts #74504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
45a8ea8
Pass SourceBuild TargetRid and SourceBuildPortable args through the n…
tmds Aug 26, 2022
9d2af1d
Rename -packagerid to -distrorid.
tmds Aug 31, 2022
a619594
Fix init-distro-rid.sh.
tmds Aug 31, 2022
599ef82
Use OutputRid instead of PackageRid.
tmds Aug 31, 2022
f39f890
Drop unnecessary Condition.
tmds Aug 31, 2022
3da8276
Try passing --distrorid as OutputRid.
tmds Aug 31, 2022
2fdda4a
NativeExport: use PackageRID for AppHostRuntimeIdentifier.
tmds Aug 31, 2022
d89911f
Rename --distrorid to --outputrid.
tmds Aug 31, 2022
165206c
Fix SourceBuild.props.
tmds Sep 1, 2022
3499ee1
Undo changes to init-distro-rid.sh.
tmds Sep 1, 2022
b1b93c2
Let source-build leg build 'banana-x64' rid.
tmds Sep 1, 2022
cd699f0
Merge branch 'main' into sb_portable
tmds Sep 1, 2022
8b7f3f9
SourceBuild.props: don't force a RuntimeOS.
tmds Sep 2, 2022
9244703
SourceBuild.props: set AdditionalRuntimeIdentifierParent.
tmds Sep 2, 2022
77958f9
Build banana.24-x64 instead.
tmds Sep 2, 2022
2b3a641
Fix SourceBuild.props.
tmds Sep 2, 2022
763dc66
SourceBuild.props: try fix AdditionalRuntimeIdentifier* usage.
tmds Sep 2, 2022
a9da4af
source-build.yml: add runtimeOS parameter.
tmds Sep 2, 2022
502c373
Merge branch 'main' into sb_portable
tmds Sep 19, 2022
0b228a7
SourceBuild.props: derive RuntimeOS from NETCoreSdkRuntimeIdentifier.
tmds Sep 27, 2022
5aa074a
Undo using NETCoreSdkRuntimeIdentifier.
tmds Sep 27, 2022
1965c29
Update eng/pipelines/common/global-build-job.yml
tmds Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions eng/SourceBuild.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,43 @@
<SourceBuildPortable>true</SourceBuildPortable>
<SourceBuildPortable Condition="'$(SourceBuildNonPortable)' == 'true'">false</SourceBuildPortable>

<!-- If TargetRid not specified, detect RID based on portability. -->
<!-- TargetRid names what gets built. -->
<TargetRid Condition="'$(TargetRid)' == '' and '$(SourceBuildNonPortable)' == 'true'">$([System.Runtime.InteropServices.RuntimeInformation]::RuntimeIdentifier)</TargetRid>
<TargetRid Condition="'$(TargetRid)' == ''">$(__DistroRid)</TargetRid>

<!-- Split e.g. 'fedora.33-x64' into 'fedora.33' and 'x64'. -->
<_targetRidPlatformIndex>$(TargetRid.LastIndexOf('-'))</_targetRidPlatformIndex>
<TargetRidWithoutPlatform>$(TargetRid.Substring(0, $(_targetRidPlatformIndex)))</TargetRidWithoutPlatform>
<TargetRidPlatform>$(TargetRid.Substring($(_targetRidPlatformIndex)).TrimStart('-'))</TargetRidPlatform>
<TargetArch>$(TargetRid.Substring($(_targetRidPlatformIndex)).TrimStart('-'))</TargetArch>

<!-- RuntimeOS is the build host rid OS. -->
<RuntimeOS>$(TargetRid.Substring(0, $(_targetRidPlatformIndex)))</RuntimeOS>

<!-- BaseOS is an expected known rid in the graph that TargetRid is compatible with.
It's used to add TargetRid in the graph if the parent can't be detected. -->
<BaseOS>$(RuntimeOS)</BaseOS>
Copy link
Member Author

Choose a reason for hiding this comment

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

TargetRid, RuntimeOS, and BaseOS should ripple down from source-build.

TargetRid names what is built.. The build will add this rid to graph. If it can't find a parent (for example, alpine.4.15-x64 finds alpine-x64 as the parent) then BaseOS is used as a parent. For example, if Orange Linux is a musl based version of Linux, BaseOS should be linux-musl.

RuntimeOS is the rid of the prebuilt SDK and artifacts that get used during the build. source-build/installer should set this to match the bootstrap SDK's NETCoreSdkRuntimeIdentifier.


<LogVerbosity Condition="'$(LogVerbosity)' == ''">minimal</LogVerbosity>
</PropertyGroup>

<Target Name="GetRuntimeSourceBuildCommandConfiguration"
BeforeTargets="GetSourceBuildCommandConfiguration">
<PropertyGroup>
<InnerBuildArgs>$(InnerBuildArgs) --arch $(TargetRidPlatform)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --arch $(TargetArch)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --configuration $(Configuration)</InnerBuildArgs>
<InnerBuildArgs Condition="'$(SourceBuildNonPortable)' == 'true'">$(InnerBuildArgs) --allconfigurations</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --verbosity $(LogVerbosity)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --nodereuse false</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --warnAsError false</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:PackageRid=$(TargetRid)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --outputrid $(TargetRid)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) --portablebuild $(SourceBuildPortable)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:NoPgoOptimize=true</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:KeepNativeSymbols=true</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:RuntimeOS=$(TargetRidWithoutPlatform)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:PortableBuild=$(SourceBuildPortable)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:RuntimeOS=$(RuntimeOS)</InnerBuildArgs>
<InnerBuildArgs Condition="'$(OfficialBuildId)' != ''">$(InnerBuildArgs) /p:OfficialBuildId=$(OfficialBuildId)</InnerBuildArgs>
<InnerBuildArgs Condition="'$(ContinuousIntegrationBuild)' != ''">$(InnerBuildArgs) /p:ContinuousIntegrationBuild=$(ContinuousIntegrationBuild)</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:BuildDebPackage=false</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:EnableNgenOptimization=false</InnerBuildArgs>
<InnerBuildArgs>$(InnerBuildArgs) /p:AdditionalRuntimeIdentifierParent=$(BaseOS)</InnerBuildArgs>
</PropertyGroup>
</Target>

Expand Down
10 changes: 10 additions & 0 deletions eng/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ usage()
echo " --os Target operating system: windows, Linux, FreeBSD, OSX, MacCatalyst, tvOS,"
echo " tvOSSimulator, iOS, iOSSimulator, Android, Browser, NetBSD, illumos or Solaris."
echo " [Default: Your machine's OS.]"
echo " --outputrid <rid> Optional argument that overrides the target rid name."
echo " --projects <value> Project or solution file(s) to build."
echo " --runtimeConfiguration (-rc) Runtime build configuration: Debug, Release or Checked."
echo " Checked is exclusive to the CLR runtime. It is the same as Debug, except code is"
Expand Down Expand Up @@ -424,6 +425,15 @@ while [[ $# > 0 ]]; do
shift 1
;;

-outputrid)
if [ -z ${2+x} ]; then
echo "No value for outputrid is supplied. See help (--help) for supported values." 1>&2
exit 1
fi
arguments="$arguments /p:OutputRid=$(echo "$2" | tr "[:upper:]" "[:lower:]")"
shift 2
;;

-portablebuild)
if [ -z ${2+x} ]; then
echo "No value for portablebuild is supplied. See help (--help) for supported values." 1>&2
Expand Down
6 changes: 6 additions & 0 deletions eng/common/templates/steps/source-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ steps:
targetRidArgs='/p:TargetRid=${{ parameters.platform.targetRID }}'
Copy link
Member

Choose a reason for hiding this comment

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

this changes eng/common so we need to make sure it is updated in dotnet/arcade or it'll get overwritten with the next arcade update

Copy link
Member Author

Choose a reason for hiding this comment

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

I've created dotnet/arcade#10782.

fi

runtimeOsArgs=
if [ '${{ parameters.platform.runtimeOS }}' != '' ]; then
runtimeOsArgs='/p:RuntimeOS=${{ parameters.platform.runtimeOS }}'
fi

publishArgs=
if [ '${{ parameters.platform.skipPublishValidation }}' != 'true' ]; then
publishArgs='--publish'
Expand All @@ -75,6 +80,7 @@ steps:
$internalRuntimeDownloadArgs \
$internalRestoreArgs \
$targetRidArgs \
$runtimeOsArgs \
/p:SourceBuildNonPortable=${{ parameters.platform.nonPortable }} \
/p:ArcadeBuildFromSource=true
displayName: Build
Expand Down
16 changes: 16 additions & 0 deletions eng/native/build-commons.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ usage()
echo "-gccx.y: optional argument to build using gcc version x.y."
echo "-ninja: target ninja instead of GNU make"
echo "-numproc: set the number of build processes."
echo "-outputrid: optional argument that overrides the target rid name."
echo "-portablebuild: pass -portablebuild=false to force a non-portable build."
echo "-skipconfigure: skip build configuration."
echo "-keepnativesymbols: keep native/unmanaged debug symbols."
Expand All @@ -232,6 +233,7 @@ __TargetArch=$arch
__TargetOS=$os
__HostOS=$os
__BuildOS=$os
__OutputRid=''

# Get the number of processors available to the scheduler
platform="$(uname)"
Expand Down Expand Up @@ -392,6 +394,16 @@ while :; do
__TargetArch=wasm
;;

outputrid|-outputrid)
if [[ -n "$2" ]]; then
__OutputRid="$2"
shift
else
echo "ERROR: 'outputrid' requires a non-empty option argument"
exit 1
fi
;;

ppc64le|-ppc64le)
__TargetArch=ppc64le
;;
Expand Down Expand Up @@ -474,3 +486,7 @@ fi

# init the target distro name
initTargetDistroRid

if [ -z "$__OutputRid" ]; then
__OutputRid="$(echo $__DistroRid | tr '[:upper:]' '[:lower:]')"
fi
3 changes: 3 additions & 0 deletions eng/pipelines/common/global-build-job.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ jobs:
platform:
buildScript: $(_sclEnableCommand) $(Build.SourcesDirectory)$(dir)build$(scriptExt)
nonPortable: true
# Use a custom RID that isn't in the RID graph here to validate we don't break the usage of custom rids that aren't in the graph.
targetRID: banana.24-x64
tmds marked this conversation as resolved.
Show resolved Hide resolved
runtimeOS: linux

- ${{ if in(parameters.osGroup, 'OSX', 'iOS', 'tvOS', 'MacCatalyst') }}:
- script: $(Build.SourcesDirectory)/eng/install-native-dependencies.sh ${{ parameters.osGroup }} ${{ parameters.archType }} azDO
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputPath>$(RuntimeBinDir)ilc/</OutputPath>
<RuntimeIdentifier>$(OutputRid)</RuntimeIdentifier>
<RuntimeIdentifier>$(PackageRID)</RuntimeIdentifier>
Copy link
Member Author

Choose a reason for hiding this comment

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

@MichalStrehovsky I think this may be breaking to the cross-compilation scenario.
OutputRid is linux-arm64 and PackageRID is linux-x64.

I see there is ILCompiler.csproj, and ILCompiler_crossarch.csproj.

This first, I guess, is used for creating a NuGet package (host: arm64 -> target: arm64)?
I think the latter is the one that is used on the build host (host: x64 -> target: arm64)?

Is that right?

I don't understand when/how ILCompiler gets packed. Where does this happen?

Copy link
Member

@am11 am11 Sep 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It gets packed by a special project file pkgproj src/installer/pkg/projects/Microsoft.DotNet.ILCompiler/Microsoft.DotNet.ILCompiler.pkgproj, which is ultimately using $(OutputRid) (via properties inheritance).

This change is correct. The reason we haven't gotten any complaints before is because we are not (yet) cross-compiling ILCompiler in a way that HostOS and TargetOS differ (the first part of RID...).

Copy link
Member Author

Choose a reason for hiding this comment

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

@am11 does Microsoft.DotNet.ILCompiler.pkgproj consume files built by ILCompiler.csproj?
If so, for what rid were these files built? OutputRid or PackageRID?

Copy link
Member

Choose a reason for hiding this comment

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

Microsoft.DotNet.ILCompiler.pkgproj consume files built by ILCompiler.csproj

It collects the files from various location and packs them together in a zip (.nupkg) file. In this case, PackageRID is only used to restore the packages when building the managed assembly (we don't have proeprocessor conditions based on RuntimeIdentifier, it's all MSIL or #if TARGET_{OS_or_ARCH} in the code). OutputRid is used in pkgprojs or passed to cmake to decide which bits to build and pack.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like we won't break anything then.

From what you said, I wonder: does ILCompiler.csproj need a RuntimeIdentifier?

Copy link
Member

Choose a reason for hiding this comment

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

does ILCompiler.csproj need a RuntimeIdentifier?

Good question. As I understood it; RuntimeIdentifer during the restore step is used to decide which runtime.{rid}.xyz for a given package to restore, if the package has native assets. Here, ILCompiler.props has multiple PackageReference and ProjectReference, it is likely that the resolved dependency graph of this project (under obj directory) has such package-with-native-assets which is requiring the RuntimeIdentifier to be specified.

<RunAnalyzers>true</RunAnalyzers>
</PropertyGroup>

Expand Down
2 changes: 1 addition & 1 deletion ...raries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/NativeExports.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<DnneGenRollForward>Major</DnneGenRollForward>
<!-- To integrate with DNNE's architecture calculation, we need to set the RID for this project. -->
<RuntimeIdentifier>$(OutputRid)</RuntimeIdentifier>
<AppHostRuntimeIdentifier>$(OutputRid)</AppHostRuntimeIdentifier>
<AppHostRuntimeIdentifier>$(PackageRID)</AppHostRuntimeIdentifier>
<_TargetsAppleOS Condition="'$(TargetOS)' == 'OSX' or '$(TargetOS)' == 'MacCatalyst' or
'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'iOSSimulator' or
'$(TargetOS)' == 'tvOSSimulator'">true</_TargetsAppleOS>
Expand Down
7 changes: 3 additions & 4 deletions src/native/corehost/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,13 @@ __LogsDir="$__RootBinDir/log"
__MsbuildDebugLogsDir="$__LogsDir/MsbuildDebugLogs"

# Set the remaining variables based upon the determined build configuration
__DistroRidLower="$(echo $__DistroRid | tr '[:upper:]' '[:lower:]')"
__BinDir="$__RootBinDir/bin/$__DistroRidLower.$__BuildType"
__IntermediatesDir="$__RootBinDir/obj/$__DistroRidLower.$__BuildType"
__BinDir="$__RootBinDir/bin/$__OutputRid.$__BuildType"
__IntermediatesDir="$__RootBinDir/obj/$__OutputRid.$__BuildType"

export __BinDir __IntermediatesDir __RuntimeFlavor

__CMakeArgs="-DCLI_CMAKE_HOST_VER=\"$__host_ver\" -DCLI_CMAKE_COMMON_HOST_VER=\"$__apphost_ver\" -DCLI_CMAKE_HOST_FXR_VER=\"$__fxr_ver\" $__CMakeArgs"
__CMakeArgs="-DCLI_CMAKE_HOST_POLICY_VER=\"$__policy_ver\" -DCLI_CMAKE_PKG_RID=\"$__DistroRid\" -DCLI_CMAKE_COMMIT_HASH=\"$__commit_hash\" $__CMakeArgs"
__CMakeArgs="-DCLI_CMAKE_HOST_POLICY_VER=\"$__policy_ver\" -DCLI_CMAKE_PKG_RID=\"$__OutputRid\" -DCLI_CMAKE_COMMIT_HASH=\"$__commit_hash\" $__CMakeArgs"
__CMakeArgs="-DRUNTIME_FLAVOR=\"$__RuntimeFlavor\" $__CMakeArgs"
__CMakeArgs="-DFEATURE_DISTRO_AGNOSTIC_SSL=$__PortableBuild $__CMakeArgs"

Expand Down
1 change: 1 addition & 0 deletions src/native/corehost/corehost.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
<BuildArgs Condition="'$(Ninja)' == 'true'">$(BuildArgs) -ninja</BuildArgs>
<BuildArgs>$(BuildArgs) -runtimeflavor $(RuntimeFlavor)</BuildArgs>
<BuildArgs Condition="'$(OfficialBuildId)' != ''">$(BuildArgs) /p:OfficialBuildId="$(OfficialBuildId)"</BuildArgs>
<BuildArgs>$(BuildArgs) -outputrid $(OutputRid)</BuildArgs>
</PropertyGroup>

<!--
Expand Down