Skip to content

Commit

Permalink
Remove workarounds in build scripts for netcoreapp and netfx versionl…
Browse files Browse the repository at this point in the history
…ess (dotnet#1841)

* Remove workarounds in build scripts for netcoreapp and netfx versionless

* Don't pass down netcoreapp in yml builds

* PR Feedback and condition passing -framework arg when not empty in yml files

* Condition TargetGroup global property in helix.yml

* PR Feedback to not use targetgroup for allconfigurations in sendtohelix

* Change from PackageTesting to BuildAllConfigurations
  • Loading branch information
safern authored Jan 27, 2020
2 parents 99e96a3 + afc2ace commit 0515878
Show file tree
Hide file tree
Showing 22 changed files with 73 additions and 72 deletions.
20 changes: 10 additions & 10 deletions docs/coding-guidelines/project-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ once before you can iterate and work on a given library project.
- Restore external dependencies
- CoreCLR - Copy to `bin\runtime\$(BuildConfiguration)`
- Netstandard Library - Copy to `bin\ref\netstandard`
- NetFx targeting pack - Copy to `bin\ref\netfx`
- NetFx targeting pack - Copy to `bin\ref\net472`
- Build targeting pack
- Build src\ref.builds which builds all references assembly projects. For reference assembly project information see [ref](#ref)
- Build product
Expand Down Expand Up @@ -37,7 +37,7 @@ Below is a list of all the various options we pivot the project builds on:
## Individual build properties
The following are the properties associated with each build pivot

- `$(TargetGroup) -> netstandard | netcoreapp | netfx`
- `$(TargetGroup) -> netstandard2.1 | netcoreapp5.0 | net472`
//**CONSIDER**: naming netcoreappcorert something shorter maybe just corert.
- `$(OSGroup) -> Windows | Linux | OSX | FreeBSD | [defaults to running OS when empty]`
- `$(ConfigurationGroup) -> Release | [defaults to Debug when empty]`
Expand Down Expand Up @@ -77,9 +77,9 @@ All supported targets with unique windows/unix build for netcoreapp:
```
<PropertyGroup>
<BuildConfigurations>
netcoreapp-Windows_NT;
netcoreapp-Unix;
netfx-Windows_NT;
$(NetCoreAppCurrent)-Windows_NT;
$(NetCoreAppCurrent)-Unix;
$(NetFrameworkCurrent)-Windows_NT;
</BuildConfigurations>
<PropertyGroup>
```
Expand All @@ -90,12 +90,12 @@ Placeholder build configurations can be added to the `<BuildConfigurations>` pro
Placeholder build configurations start with _ prefix.

Example:
When we have a project that has a `netstandard` build configuration that means that this project is compatible with any build configuration. So if we do a vertical build for `netfx` this project will be built as part of the vertical because `netfx` is compatible with `netstandard`. This means that in the runtime and testhost binaries the netstandard implementation will be included, and we will test against those assets instead of testing against the framework inbox asset. In order to tell the build system to not include this project as part of the `netfx` vertical we need to add a placeholder configuration:
When we have a project that has a `netstandard` build configuration that means that this project is compatible with any build configuration. So if we do a vertical build for `net472` this project will be built as part of the vertical because `net472` is compatible with `netstandard2.0`. This means that in the runtime and testhost binaries the netstandard implementation will be included, and we will test against those assets instead of testing against the framework inbox asset. In order to tell the build system to not include this project as part of the `net472` vertical we need to add a placeholder configuration:
```
<PropertyGroup>
<BuildConfigurations>
netstandard;
_netfx;
netstandard2.0;
_net472;
</BuildConfigurations>
</PropertyGroup>
```
Expand All @@ -120,8 +120,8 @@ TODO: Link to the target framework and OS fallbacks when they are available.
Temporary versions are at https://github.com/dotnet/corefx/blob/dev/eng/src/Tools/GenerateProps/osgroups.props and https://github.com/dotnet/corefx/blob/dev/eng/src/Tools/GenerateProps/targetgroups.props

## Supported full build configurations
- .NET Core latest on current OS (default) -> `netcoreapp-[RunningOS]`
- .NET Framework latest -> `netfx-Windows_NT`
- .NET Core latest on current OS (default) -> `$(NetCoreAppCurrent)-[RunningOS]`
- .NET Framework latest -> `$(NetFrameworkCurrent)-Windows_NT`

## Project configurations for VS
For each unique configuration needed for a given library project a configuration entry separated by a ';' should be added to the project so it can be selected and built in VS and also clearly identify the various configurations.<BR/>
Expand Down
2 changes: 1 addition & 1 deletion docs/project/library-servicing.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Note that it's possible that somebody else has already incremented the package v

## Determine AssemblyVersion

Each library has a property called `AssemblyVersion` which will be in either the library `.csproj` or in `Directory.Build.Props` (or dir.props). For servcing events you will want to increment the revision by 1 (e.g `4.0.0.0` -> `4.0.0.1`) in the library's `Directory.Build.Props` (or dir.props) if the library ships in its own package and has an asset that is applicable to .NET Framework. To determine if it applies to .NET Framework you should check to see if there are any `netstandard` or `netfx`/`net4x` configurations in the ref and/or src project, if there are then it has assets that apply.
Each library has a property called `AssemblyVersion` which will be in either the library `.csproj` or in `Directory.Build.Props` (or dir.props). For servcing events you will want to increment the revision by 1 (e.g `4.0.0.0` -> `4.0.0.1`) in the library's `Directory.Build.Props` (or dir.props) if the library ships in its own package and has an asset that is applicable to .NET Framework. To determine if it applies to .NET Framework you should check to see if there are any `netstandard` or `net4x` configurations in the ref and/or src project, if there are then it has assets that apply.

The reason we need to increment the assembly version for things running on .NET Framework is because of the way binding works there. If there are two assemblies with the same assembly version the loader will essentially pick the first one it finds and use that version so applications don't have full control over using the later build with a particular fix included. This is worse if someone puts the older assembly in the GAC as the GAC will always win for matching assembly versions so an application couldn't load the newer one because it has the same assembly version.

Expand Down
2 changes: 1 addition & 1 deletion docs/workflow/building/libraries/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ The libraries build has two logical components, the native build which produces

The build configurations are generally defaulted based on where you are building (i.e. which OS or which architecture) but we have a few shortcuts for the individual properties that can be passed to the build scripts:

- `-framework|-f` identifies the target framework for the build. It defaults to `netcoreapp` but possible values include `netcoreapp` or `netfx`. (msbuild property `TargetGroup`)
- `-framework|-f` identifies the target framework for the build. It defaults to latest `netcoreapp` but possible values include `netcoreapp` (Which will map to latest netcoreapp) or `net472`. (msbuild property `TargetGroup`)
- `-os` identifies the OS for the build. It defaults to the OS you are running on but possible values include `Windows_NT`, `Unix`, `Linux`, or `OSX`. (msbuild property `OSGroup`)
- `-configuration|-c Debug|Release` controls the optimization level the compilers use for the build. It defaults to `Debug`. (msbuild property `ConfigurationGroup`)
- `-arch` identifies the architecture for the build. It defaults to `x64` but possible values include `x64`, `x86`, `arm`, or `arm64`. (msbuild property `ArchGroup`)
Expand Down
2 changes: 1 addition & 1 deletion docs/workflow/testing/libraries/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ Each test project can potentially have multiple build configurations. There are

```cmd
cd src\libraries\System.Runtime\tests
dotnet msbuild System.Runtime.Tests.csproj /p:TargetGroup=netfx
dotnet msbuild System.Runtime.Tests.csproj /p:TargetGroup=net472
```
5 changes: 2 additions & 3 deletions eng/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function Get-Help() {

Write-Host "Libraries settings:"
Write-Host " -vs Open the solution with VS for Test Explorer support. Path or solution name (ie -vs Microsoft.CSharp)"
Write-Host " -framework Build framework: netcoreapp or netfx (short: -f)"
Write-Host " -framework Build framework: netcoreapp5.0 or net472 (short: -f)"
Write-Host " -coverage Collect code coverage when testing"
Write-Host " -testscope Scope tests, allowed values: innerloop, outerloop, all"
Write-Host " -allconfigurations Build packages for all build configurations"
Expand Down Expand Up @@ -137,8 +137,7 @@ foreach ($argument in $PSBoundParameters.Keys)
"test" { $arguments += " -test" }
"configuration" { $configuration = (Get-Culture).TextInfo.ToTitleCase($($PSBoundParameters[$argument])); $arguments += " /p:ConfigurationGroup=$configuration -configuration $configuration" }
"runtimeConfiguration" { $arguments += " /p:RuntimeConfiguration=$((Get-Culture).TextInfo.ToTitleCase($($PSBoundParameters[$argument])))" }
# This should be removed after we have finalized our ci build pipeline.
"framework" { if ($PSBoundParameters[$argument].ToLowerInvariant() -eq 'netcoreapp') { $arguments += " /p:TargetGroup=netcoreapp5.0" } else { if ($PSBoundParameters[$argument].ToLowerInvariant() -eq 'netfx') { $arguments += " /p:TargetGroup=net472" } else { $arguments += " /p:TargetGroup=$($PSBoundParameters[$argument].ToLowerInvariant())"}}}
"framework" { $arguments += " /p:TargetGroup=$($PSBoundParameters[$argument].ToLowerInvariant())" }
"os" { $arguments += " /p:OSGroup=$($PSBoundParameters[$argument])" }
"allconfigurations" { $arguments += " /p:BuildAllConfigurations=true" }
"arch" { $arguments += " /p:ArchGroup=$($PSBoundParameters[$argument]) /p:TargetArchitecture=$($PSBoundParameters[$argument])" }
Expand Down
8 changes: 1 addition & 7 deletions eng/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ usage()
echo ""

echo "Libraries settings:"
echo " --framework Build framework: netcoreapp or netfx (short: -f)"
echo " --framework Build framework: netcoreapp or net472 (short: -f)"
echo " --coverage Collect code coverage when testing"
echo " --testscope Test scope, allowed values: innerloop, outerloop, all"
echo " --allconfigurations Build packages for all build configurations"
Expand Down Expand Up @@ -85,14 +85,8 @@ while [[ $# > 0 ]]; do
arguments="$arguments /p:ConfigurationGroup=$val -configuration $val"
shift 2
;;
# This should be removed after we have finalized our ci build pipeline.
-framework|-f)
val="$(echo "$2" | awk '{print tolower($0)}')"
if [ "$val" == "netcoreapp" ]; then
val=netcoreapp5.0
elif [ "$val" == "netfx" ]; then
val=net472
fi
arguments="$arguments /p:TargetGroup=$val"
shift 2
;;
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/coreclr/templates/build-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
dependsOn:
- ${{ format('coreclr_product_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}
- ${{ format('libraries_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}


${{ if eq(parameters.testGroup, 'innerloop') }}:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ jobs:
dependsOn:
- ${{ format('coreclr_product_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}
- ${{ format('libraries_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}

# Run all steps in the container.
# Note that the containers are defined in platform-matrix.yml
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/coreclr/templates/perf-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
dependsOn:
- ${{ format('coreclr_product_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}
- ${{ format('libraries_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}

${{ if eq(parameters.osGroup, 'Windows_NT') }}:
extraSetupParameters: -CoreRootDirectory $(Build.SourcesDirectory)\artifacts\tests\coreclr\${{ parameters.osGroup }}.${{ parameters.archType }}.Release\Tests\Core_Root -Architecture ${{ parameters.archType }}
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/coreclr/templates/run-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
- ${{ if ne(parameters.stagedBuild, true) }}:
- ${{ format('coreclr_product_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}
- ${{ format('libraries_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}

# Compute job name from template parameters
${{ if eq(parameters.testGroup, 'innerloop') }}:
Expand Down
4 changes: 2 additions & 2 deletions eng/pipelines/installer/jobs/base-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ jobs:
parameters.archType,
parameters.liveRuntimeBuildConfig) }}
- ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
- libraries_build_netcoreapp_${{ format('{0}{1}_{2}_{3}',
- libraries_build_${{ format('{0}{1}_{2}_{3}',
parameters.osGroup,
parameters.osSubgroup,
parameters.archType,
Expand All @@ -328,7 +328,7 @@ jobs:
- ${{ if eq(parameters.buildFullPlatformManifest, true) }}:
- ${{ each platform in parameters.platforms }}:
- ${{ parameters.runtimeFlavor }}_product_build_${{ platform }}_${{ parameters.liveRuntimeBuildConfig }}
- libraries_build_netcoreapp_${{ platform }}_${{ parameters.liveLibrariesBuildConfig }}
- libraries_build_${{ platform }}_${{ parameters.liveLibrariesBuildConfig }}

steps:

Expand Down
15 changes: 11 additions & 4 deletions eng/pipelines/libraries/base-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ parameters:
jobs:
- template: /eng/common/templates/job/job.yml
parameters:
name: ${{ format('libraries_{0}_{1}_{2}{3}_{4}_{5}', parameters.name, parameters.framework, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
${{ if eq(parameters.framework, 'netcoreapp') }}:
${{ if notIn(parameters.framework, 'allConfigurations', 'net472') }}:
displayName: ${{ format('Libraries {0} {1}{2} {3} {4}', parameters.displayName, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
${{ if ne(parameters.framework, 'netcoreapp') }}:
name: ${{ format('libraries_{0}_{1}{2}_{3}_{4}', parameters.name, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
${{ if in(parameters.framework, 'allConfigurations', 'net472') }}:
displayName: ${{ format('Libraries {0} {1} {2} {3} {4}', parameters.displayName, parameters.osGroup, parameters.framework, parameters.archType, parameters.buildConfig) }}
name: ${{ format('libraries_{0}_{1}_{2}{3}_{4}_{5}', parameters.name, parameters.framework, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}

enableTelemetry: ${{ parameters.isOfficialBuild }} # TODO: figure out if it's needed
container: ${{ parameters.container }}
Expand All @@ -41,10 +42,11 @@ jobs:
- _msbuildCommonParameters: ''
- _stripSymbolsArg: ''
- _runtimeOSArg: ''
- _finalFrameworkArg: -framework ${{ parameters.framework }}
- _finalFrameworkArg: ''
- _buildScript: $(_buildScriptFileName)$(scriptExt)
- _warnAsErrorArg: ''
- _testScopeArg: ''
- _extraHelixArguments: ''

- librariesBuildArtifactName: ${{ format('libraries_bin_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}

Expand All @@ -66,8 +68,13 @@ jobs:
- ${{ if eq(parameters.osGroup, 'WebAssembly') }}:
- _runtimeOSArg: -os ${{ parameters.osGroup }}

- ${{ if ne(parameters.framework, '') }}:
- _finalFrameworkArg: -framework ${{ parameters.framework }}
- _extraHelixArguments: /p:TargetGroup=${{ parameters.framework }}

- ${{ if eq(parameters.framework, 'allConfigurations') }}:
- _finalFrameworkArg: -allConfigurations
- _extraHelixArguments: /p:BuildAllConfigurations=true

- ${{ if eq(parameters.isOfficialAllConfigurations, true) }}:
- _skipTestHostCopy: true
Expand Down
7 changes: 4 additions & 3 deletions eng/pipelines/libraries/build-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ parameters:
osSubgroup: ''
archType: ''
crossrootfsDir: ''
framework: netcoreapp
framework: ''
isOfficialBuild: false
isOfficialAllConfigurations: false

Expand Down Expand Up @@ -120,8 +120,8 @@ jobs:
- task: CopyFiles@2
displayName: Prepare shared framework runtime folder to publish
inputs:
sourceFolder: $(Build.SourcesDirectory)/artifacts/bin/pkg/netcoreapp5.0/runtime # The hardcoded target framework should be removed when we drop the support for versionless targetframeworks from ci.
targetFolder: $(Build.ArtifactStagingDirectory)/artifacts/bin/pkg/netcoreapp5.0/runtime
sourceFolder: $(Build.SourcesDirectory)/artifacts/bin/pkg
targetFolder: $(Build.ArtifactStagingDirectory)/artifacts/bin/pkg

- task: CopyFiles@2
displayName: Prepare docs folder to publish
Expand Down Expand Up @@ -184,3 +184,4 @@ jobs:
testScope: ${{ parameters.testScope }}
creator: dotnet-bot
helixToken: ''
extraHelixArguments: $(_extraHelixArguments)
7 changes: 5 additions & 2 deletions eng/pipelines/libraries/build-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ parameters:
osGroup: ''
osSubgroup: ''
archType: ''
framework: netcoreapp
framework: ''
isOfficialBuild: false
liveRuntimeBuildConfig: ''
timeoutInMinutes: 150
Expand Down Expand Up @@ -34,7 +34,10 @@ jobs:
displayName: 'Test Build'

dependsOn:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', parameters.framework, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if notIn(parameters.framework, 'allConfigurations', 'net472') }}:
- ${{ format('libraries_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if in(parameters.framework, 'allConfigurations', 'net472') }}:
- ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', parameters.framework, parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}

variables:
- librariesTestsArtifactName: ${{ format('libraries_test_assets_{0}_{1}_{2}', parameters.osGroup, parameters.archType, parameters.buildConfig) }}
Expand Down
Loading

0 comments on commit 0515878

Please sign in to comment.