Skip to content

Local live builds not picking up runtime config #2093

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

Merged
safern merged 13 commits into from
Jan 31, 2020
Merged

Local live builds not picking up runtime config #2093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a962c75
Local live builds not picking up runtime config
jaredpar Jan 23, 2020
976b7ea
PR Feedback:
jaredpar Jan 23, 2020
95495dc
Remove unnecessary properties
jaredpar Jan 23, 2020
5740d55
Progress
jaredpar Jan 23, 2020
e641b28
Mix configurations
jaredpar Jan 23, 2020
9977e65
Undo MonoOsGorup delete
jaredpar Jan 24, 2020
46d25fe
Correct the import order
jaredpar Jan 24, 2020
9dbbce8
Publish binary logs
jaredpar Jan 25, 2020
3e634eb
Pass -ci as this is a CI build
jaredpar Jan 25, 2020
f0dbb6f
Progress
jaredpar Jan 27, 2020
84727f5
Test
jaredpar Jan 28, 2020
47f84a0
Set ArtifactsObjDir early enough for Mono
ViktorHofer Jan 28, 2020
0164e80
Don't overwrite the properties
jaredpar Jan 30, 2020
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
11 changes: 3 additions & 8 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
<RepositoryEngineeringDir>$([MSBuild]::NormalizeDirectory('$(RepoRoot)', 'eng'))</RepositoryEngineeringDir>
<ArtifactsDir>$([MSBuild]::NormalizeDirectory('$(RepoRoot)', 'artifacts'))</ArtifactsDir>
<ArtifactsBinDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'bin'))</ArtifactsBinDir>
<ArtifactsObjDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'obj'))</ArtifactsObjDir>
</PropertyGroup>

<!-- The TFMs to build and test against. -->
Expand All @@ -34,6 +35,8 @@
<NetFrameworkCurrent>net472</NetFrameworkCurrent>
</PropertyGroup>

<Import Project="$(RepositoryEngineeringDir)Configurations.props" />

<!--
Get ProjectToBuild and '<subset>ProjectToBuild' items. Using the items lets projects handle
$(Subset) automatically when creating project-to-project dependencies.
Expand All @@ -52,14 +55,6 @@
<CoreLibSharedDir>$([MSBuild]::NormalizeDirectory('$(LibrariesProjectRoot)', 'System.Private.CoreLib', 'src'))</CoreLibSharedDir>
</PropertyGroup>

<!-- Honor the generic RuntimeConfiguration property. -->
<PropertyGroup>
<CoreCLRConfiguration Condition="'$(CoreCLRConfiguration)' == ''">$(RuntimeConfiguration)</CoreCLRConfiguration>
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' != 'checked'">$(RuntimeConfiguration)</MonoConfiguration>
<!-- There's no checked configuration on Mono. -->
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' == 'checked'">Debug</MonoConfiguration>
</PropertyGroup>

<!-- Packaging properties -->
<PropertyGroup>
<!--
Expand Down
1 change: 1 addition & 0 deletions eng/Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
https://github.com/dotnet/arcade/issues/388
-->

<Import Project="$(MSBuildThisFileDirectory)Configurations.props" />
<Import Project="$(MSBuildThisFileDirectory)Subsets.props" />

<Target Name="CheckSpecifiedSubsetValidity"
Expand Down
11 changes: 11 additions & 0 deletions eng/Configurations.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project>
<!-- Honor the generic RuntimeConfiguration property. -->
<PropertyGroup>
<RuntimeConfiguration Condition="'$(RuntimeConfiguration)' == ''">$(ConfigurationGroup)</RuntimeConfiguration>
<RuntimeConfiguration Condition="'$(RuntimeConfiguration)' == ''">$(Configuration)</RuntimeConfiguration>
Copy link
Member

@ViktorHofer ViktorHofer Jan 28, 2020

Choose a reason for hiding this comment

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

This isn't necessary as ConfigurationGroup and Configuration are always passed in together. In future we will probably remove the former.

Copy link
Member Author

@jaredpar jaredpar Jan 28, 2020

Choose a reason for hiding this comment

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

Not necessarily. If you build the projects directly then ConfigurationGroup is not specified.

Also looking through the logs @jkoritzinsky and I observed places where $(ConfigurationGroup) was not set but $(Configuration) was. This change was inspired by that observation. It's deep in the restore graph where we encountered this.

Copy link
Member Author

@jaredpar jaredpar Jan 31, 2020

Choose a reason for hiding this comment

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

Concrete example where only $(Configuration) is passed to the build

https://teams.microsoft.com/l/message/19:b27b36ecd10a46398da76b02f0411de7@thread.skype/1580491671080?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=014ca51d-be57-47fa-9628-a15efcc3c376&parentMessageId=1580481868103&teamName=dotnet%2Fruntime%20repo&channelName=Infrastructure&createdTime=1580491671080

<CoreCLRConfiguration Condition="'$(CoreCLRConfiguration)' == ''">$(RuntimeConfiguration)</CoreCLRConfiguration>
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' != 'checked'">$(RuntimeConfiguration)</MonoConfiguration>
<!-- There's no checked configuration on Mono. -->
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' == 'checked'">Debug</MonoConfiguration>
</PropertyGroup>
</Project>
14 changes: 3 additions & 11 deletions eng/Subsets.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,29 +102,21 @@
<ProjectToBuild Include="$(RepoToolsLocalDir)regenerate-readme-table.proj" />
</ItemGroup>

<!-- Honor the generic RuntimeConfiguration property. -->
<PropertyGroup>
<CoreCLRConfiguration Condition="'$(CoreCLRConfiguration)' == ''">$(RuntimeConfiguration)</CoreCLRConfiguration>
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' != 'checked'">$(RuntimeConfiguration)</MonoConfiguration>
<!-- There's no checked configuration on Mono. -->
<MonoConfiguration Condition="'$(MonoConfiguration)' == '' and '$(RuntimeConfiguration.ToLower())' == 'checked'">Debug</MonoConfiguration>
</PropertyGroup>

<ItemDefinitionGroup Condition="'$(CoreCLRConfiguration)' != ''">
<CoreClrProjectToBuild>
<Properties>Configuration=$(CoreCLRConfiguration)</Properties>
<AdditionalProperties>Configuration=$(CoreCLRConfiguration)</AdditionalProperties>
</CoreClrProjectToBuild>
</ItemDefinitionGroup>

<ItemDefinitionGroup Condition="'$(MonoConfiguration)' != ''">
<MonoProjectToBuild>
<Properties>Configuration=$(MonoConfiguration)</Properties>
<AdditionalProperties>Configuration=$(MonoConfiguration)</AdditionalProperties>
</MonoProjectToBuild>
</ItemDefinitionGroup>

<ItemDefinitionGroup Condition="'$(LibrariesConfiguration)' != ''">
<LibrariesProjectToBuild>
<Properties>Configuration=$(LibrariesConfiguration)</Properties>
<AdditionalProperties>Configuration=$(LibrariesConfiguration)</AdditionalProperties>
</LibrariesProjectToBuild>
</ItemDefinitionGroup>

Expand Down
2 changes: 0 additions & 2 deletions eng/liveBuilds.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

<PropertyGroup>
<CoreCLROSGroup Condition="'$(CoreCLROSGroup)' == ''">$(OSGroup)</CoreCLROSGroup>
<CoreCLRConfiguration Condition="'$(CoreCLRConfiguration)' == ''">$(Configuration)</CoreCLRConfiguration>
<MonoOSGroup Condition="'$(MonoOSGroup)' == ''">$(OSGroup)</MonoOSGroup>
<MonoConfiguration Condition="'$(MonoConfiguration)' == ''">$(Configuration)</MonoConfiguration>
<LibrariesOSGroup Condition="'$(LibrariesOSGroup)' == ''">$(OSGroup)</LibrariesOSGroup>
<LibrariesConfiguration Condition="'$(LibrariesConfiguration)' == ''">$(Configuration)</LibrariesConfiguration>

Expand Down
16 changes: 13 additions & 3 deletions eng/pipelines/common/global-build-job.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
parameters:
buildConfig: ''
runtimeConfig: ''
archType: ''
osGroup: ''
osSubgroup: ''
Expand All @@ -11,8 +12,8 @@ parameters:
jobs:
- template: /eng/common/templates/job/job.yml
parameters:
name: ${{ format('runtime_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
displayName: ${{ format('Runtime Build {0}{1} {2} {3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
name: ${{ format('build_{0}{1}_{2}_{3}_runtime{4}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig, parameters.runtimeConfig) }}
displayName: ${{ format('Build {0}{1} {2} {3} Runtime {4}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig, parameters.runtimeConfig) }}
dependsOn: checkout
pool: ${{ parameters.pool }}
container: ${{ parameters.container }}
Expand All @@ -35,5 +36,14 @@ jobs:
- template: /eng/pipelines/common/clone-checkout-bundle-step.yml

# Build
- script: $(Build.SourcesDirectory)$(dir)build$(scriptExt) -c ${{ parameters.buildConfig }} -arch ${{ parameters.archType }}
- script: $(Build.SourcesDirectory)$(dir)build$(scriptExt) -c ${{ parameters.buildConfig }} -runtimeConfiguration ${{ parameters.runtimeConfig }} -arch ${{ parameters.archType }} -ci
displayName: Build product

- task: PublishBuildArtifacts@1
Copy link
Member

@safern safern Jan 27, 2020

Choose a reason for hiding this comment

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

Instead of having to add this step, you can just pass down a parameter to the template we use which is job.yml from arcade. That parameter is enablePublishBuildArtifacts: true.

https://github.com/dotnet/arcade/blob/master/eng/common/templates/job/job.yml#L180

Note that you also need to declare a variable _BuildConfig: ${{ parameters.buildConfig }} because it uses it to find the logs:
https://github.com/dotnet/arcade/blob/master/eng/common/templates/job/job.yml#L184

Copy link
Member Author

@jaredpar jaredpar Jan 27, 2020

Choose a reason for hiding this comment

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

This job though deliberately builds multiple configurations though hence seems like it can't directly hook up to that infrastructure

Copy link
Member

@safern safern Jan 27, 2020

Choose a reason for hiding this comment

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

It can hook up to that infrastructure, the binlog will be created under the global buildConfig which is passed through the -configuration argument. The binlog name and location is calculated on the build script in arcade, which doesn't know about runtimeConfiguration. However if you want to produce binlogs for other steps in the coreclr build scripts, then we will definitely not be able to use this.

displayName: Publish Logs
inputs:
PathtoPublish: '$(Build.SourcesDirectory)/artifacts/log/'
PublishLocation: Container
ArtifactName: $(Agent.Os)_PublishBuildAssets
continueOnError: true
condition: always()
2 changes: 2 additions & 0 deletions eng/pipelines/global-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
- OSX_x64
jobParameters:
testGroup: innerloop
runtimeConfig: debug

#
# Build the combined runtime repo
Expand All @@ -58,3 +59,4 @@ jobs:
- Linux_x64
jobParameters:
testGroup: innerloop
runtimeConfig: release
3 changes: 2 additions & 1 deletion src/coreclr/build.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ if %__RestoreOptData% EQU 1 (
echo %__MsgPrefix%Restoring the OptimizationData Package
powershell -NoProfile -ExecutionPolicy ByPass -NoLogo -File "%__RepoRootDir%\eng\common\msbuild.ps1" /clp:nosummary %__ArcadeScriptArgs%^
%OptDataProjectFilePath% /t:Restore^
%__CommonMSBuildArgs% %__UnprocessedBuildArgs%
%__CommonMSBuildArgs% %__UnprocessedBuildArgs%^
/nodereuse:false
if not !errorlevel! == 0 (
echo %__ErrMsgPrefix%%__MsgPrefix%Error: Failed to restore the optimization data package.
set __exitCode=!errorlevel!
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ restore_optdata()
echo "Restoring the OptimizationData package"
"$__RepoRootDir/eng/common/msbuild.sh" /clp:nosummary $__ArcadeScriptArgs \
$OptDataProjectFilePath /t:Restore /m \
$__CommonMSBuildArgs $__UnprocessedBuildArgs
$__CommonMSBuildArgs $__UnprocessedBuildArgs \
/nodereuse:false
local exit_code="$?"
if [[ "$exit_code" != 0 ]]; then
echo "${__ErrMsgPrefix}Failed to restore the optimization data package."
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
<Project InitialTargets="AddSkipGetTargetFrameworkToProjectReferences">
<PropertyGroup>
<CoreCLROSGroup Condition="'$(CoreCLROSGroup)' == ''">$(_bc_OSGroup)</CoreCLROSGroup>
<CoreCLRConfiguration Condition="'$(CoreCLRConfiguration)' == ''">$(_bc_ConfigurationGroup)</CoreCLRConfiguration>
<MonoOSGroup Condition="'$(MonoCLROSGroup)' == ''">$(_bc_OSGroup)</MonoOSGroup>
<MonoConfiguration Condition="'$(MonoConfiguration)' == ''">$(_bc_ConfigurationGroup)</MonoConfiguration>
</PropertyGroup>

<Import Project="..\..\Directory.Build.targets" />
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/shims/manual/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
<!-- Use runtime assemblies for the TargetFramework, using build configuration OSGroup -->
<RefPath>$(ArtifactsBinDir)runtime/$(TargetFramework)-$(_bc_OSGroup)-$(_bc_ConfigurationGroup)-$(ArchGroup)/</RefPath>
<CoreCLROSGroup>$(_bc_OSGroup)</CoreCLROSGroup>
<CoreCLRConfiguration>$(_bc_ConfigurationGroup)</CoreCLRConfiguration>
<MonoOSGroup>$(_bc_OSGroup)</MonoOSGroup>
<MonoConfiguration>$(_bc_ConfigurationGroup)</MonoConfiguration>
</PropertyGroup>

<ItemGroup>
Expand Down