-
Notifications
You must be signed in to change notification settings - Fork 847
Extract logic and hardcoded values about compiler's TFM into a single source of truth #19251
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.
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
base: main
Are you sure you want to change the base?
Conversation
- Create eng/productTfm.txt as single source of truth for product TFM (net10.0) - Update Directory.Build.props to read TFM from file and derive major version - Update FSharpBuild.Directory.Build.targets to emit fsProductTfm and fsProductTfmMajorVersion constants in generated buildproperties.fs
…mework) in MSBuild files - Update UseLocalCompiler.Directory.Build.props to use TFM property - Add TFM property reading to buildtools/checkpackages/Directory.Build.props - Update checkpackages fsproj files to use the property Part of TFM centralization effort - Sprint 2
- eng/Build.ps1: Read TFM from productTfm.txt for coreclrTargetFramework, bootstrapTfm, and fsharpNetCoreProductTfm variables - eng/build.sh: Read TFM using cat and tr -d to strip whitespace - eng/build-utils.ps1: Read TFM from productTfm.txt for fsharpNetCoreProductTfm This removes hardcoded 'net10.0' from these script files, enabling centralized TFM management through eng/productTfm.txt.
- eng/Build.ps1: Read TFM from productTfm.txt for coreclrTargetFramework, bootstrapTfm, and fsharpNetCoreProductTfm variables - eng/build.sh: Read TFM using cat and tr -d to strip whitespace - eng/build-utils.ps1: Read TFM from productTfm.txt for fsharpNetCoreProductTfm This removes hardcoded 'net10.0' from these script files, enabling centralized TFM management through eng/productTfm.txt.
Replace hardcoded TFM list in toolingCompatibleVersions with dynamically
computed list based on FSharp.BuildProperties.fsProductTfmMajorVersion.
- Generate net{N}.0 from major version down to 5
- Append legacy netcoreapp/netstandard versions
- List automatically updates when eng/productTfm.txt changes
Replace hardcoded TFM list in toolingCompatibleVersions with dynamically
computed list based on FSharp.BuildProperties.fsProductTfmMajorVersion.
- Generate net{N}.0 from major version down to 5
- Append legacy netcoreapp/netstandard versions
- List automatically updates when eng/productTfm.txt changes
- TestFramework.fs: Add productTfm that reads from eng/productTfm.txt - TestFramework.fs: Update dotnetArchitecture to use productTfm - CompilerAssert.fs: Use TestFramework.productTfm in runtimeconfig generation - ProjectGeneration.fs: Use TestFramework.productTfm in fsproj template - Utilities.fs: Use TestFramework.productTfm for target framework
- TestFramework.fs: Add productTfm that reads from eng/productTfm.txt - TestFramework.fs: Update dotnetArchitecture to use productTfm - CompilerAssert.fs: Use TestFramework.productTfm in runtimeconfig generation - ProjectGeneration.fs: Use TestFramework.productTfm in fsproj template - Utilities.fs: Use TestFramework.productTfm for target framework
- Update tests/fsharp/single-test.fs to use productTfm (3 occurrences) - Update DependencyManagerInteractiveTests.fs to use TestFramework.productTfm (18 occurrences) - Both test projects build successfully with 0 errors - Remaining net10.0 in tests/ are only in utility scripts, MSBuild config, and comments
- Update tests/fsharp/single-test.fs to use productTfm (3 occurrences) - Update DependencyManagerInteractiveTests.fs to use TestFramework.productTfm (18 occurrences) - Both test projects build successfully with 0 errors - Remaining net10.0 in tests/ are only in utility scripts, MSBuild config, and comments
- tests/ILVerify/ilverify.ps1: Use $default_tfm instead of hardcoded net10.0 - EndToEndBuildTests fsproj files: Use $(FSharpNetCoreProductDefaultTargetFramework) - BasicProvider/*.fsproj - ComboProvider/*.fsproj - DesignTimeProviderPackaging/*.fsproj
- tests/ILVerify/ilverify.ps1: Use $default_tfm instead of hardcoded net10.0 - EndToEndBuildTests fsproj files: Use $(FSharpNetCoreProductDefaultTargetFramework) - BasicProvider/*.fsproj - ComboProvider/*.fsproj - DesignTimeProviderPackaging/*.fsproj
- TestBasicProvider.cmd: Read TFM from productTfm.txt - TestComboProvider.cmd: Read TFM from productTfm.txt - Equality.fsproj: Use $(FSharpNetCoreProductDefaultTargetFramework) - FSharpMetadataResource_Trimming_Test.fsproj: Use MSBuild property - SelfContained_Trimming_Test.fsproj: Use MSBuild property - StaticLinkedFSharpCore_Trimming_Test.fsproj: Use MSBuild property - FSharpCoreVersionTest.props: Read TFM from productTfm.txt - identifierAnalysisByType.fsx: Document as dev utility (F# scripts require hardcoded path) All functional test code now uses centralized TFM source.
- TestBasicProvider.cmd: Read TFM from productTfm.txt - TestComboProvider.cmd: Read TFM from productTfm.txt - Equality.fsproj: Use $(FSharpNetCoreProductDefaultTargetFramework) - FSharpMetadataResource_Trimming_Test.fsproj: Use MSBuild property - SelfContained_Trimming_Test.fsproj: Use MSBuild property - StaticLinkedFSharpCore_Trimming_Test.fsproj: Use MSBuild property - FSharpCoreVersionTest.props: Read TFM from productTfm.txt - identifierAnalysisByType.fsx: Document as dev utility (F# scripts require hardcoded path) All functional test code now uses centralized TFM source.
- Add eng/productTfm.txt as single source of truth for product TFM - Update Directory.Build.props to read TFM from file - Update FSharpBuild.Directory.Build.targets to generate F# constants - Update UseLocalCompiler.Directory.Build.props to use TFM property - Update buildtools/checkpackages to use TFM property - Update eng/Build.ps1, eng/build.sh, eng/build-utils.ps1, eng/test-determinism.ps1 - Update eng/templates/regression-test-jobs.yml - Update CompilerLocation.fs to use generated constants - Update TestFramework.fs with productTfm constant - Update test files to use centralized TFM - Add release notes
- Add eng/productTfm.txt as single source of truth for product TFM - Update Directory.Build.props to read TFM from file - Update FSharpBuild.Directory.Build.targets to generate F# constants - Update UseLocalCompiler.Directory.Build.props to use TFM property - Update buildtools/checkpackages to use TFM property - Update eng/Build.ps1, eng/build.sh, eng/build-utils.ps1, eng/test-determinism.ps1 - Update eng/templates/regression-test-jobs.yml - Update CompilerLocation.fs to use generated constants - Update TestFramework.fs with productTfm constant - Update test files to use centralized TFM - Add release notes
…into extract-tfm-logic
- Add eng/productTfm.txt as single source of truth for product TFM (net10.0) - Expose via MSBuild: $(FSharpNetCoreProductDefaultTargetFramework), $(FSharpNetCoreProductMajorVersion) - Expose via F# build properties: fsProductTfm, fsProductTfmMajorVersion - Expose via test framework: TestFramework.productTfm - Update build scripts (bash/powershell) to read TFM dynamically - Update test projects to use centralized TFM value - Update ILVerify to read product TFM from eng/productTfm.txt
…into extract-tfm-logic
❗ Release notes required
Warning No PR link found in some release notes, please consider adding it.
|
- Exclude fsProductTfm and fsProductTfmMajorVersion from FSharp.Core's buildproperties.fs since the 'int' type isn't defined yet during FSharp.Core compilation (bootstrapping issue) - Fix array yield syntax in CompilerLocation.fs to use yield! for combining arrays instead of yield with semicolon-separated values
…ProductDefaultTargetFramework
… artifact The UseLocalCompiler.Directory.Build.props now reads FSharpNetCoreProductDefaultTargetFramework from productTfm.txt. For regression tests, this file needs to be co-located with the props file. Changes: - UseLocalCompiler.Directory.Build.props: Add logic to read productTfm.txt from either co-located path (regression tests) or eng/ subdirectory (local usage) - azure-pipelines-PR.yml: Stage both UseLocalCompiler.Directory.Build.props and eng/productTfm.txt together before publishing as UseLocalCompilerProps artifact This fixes MSB4057 errors in FSharpPlus regression tests where the target '_GetRestoreSettingsPerFramework' was not found due to undefined TFM property.
… TFM Create eng/TargetFrameworks.props with: - FSharpNetCoreProductDefaultTargetFramework=net10.0 (defensive, only if not set) - FSharpNetCoreProductMajorVersion derived via regex This is the first step in migrating from productTfm.txt to a proper MSBuild property file. The txt file is kept temporarily for coexistence during the migration.
Replace File.ReadAllText(productTfm.txt) with FSharp.BuildProperties.fsProductTfm which is auto-generated during build from MSBuild properties.
Update all 4 PowerShell scripts to use dotnet msbuild --getProperty: via eng/common/dotnet.ps1 instead of Get-Content productTfm.txt. Files updated: - eng/Build.ps1 - eng/build-utils.ps1 - eng/test-determinism.ps1 - tests/ILVerify/ilverify.ps1 This is part of the migration from productTfm.txt to the MSBuild-based eng/TargetFrameworks.props as the single source of truth for the product TFM.
Replace `cat productTfm.txt` with `dotnet msbuild --getProperty` to read FSharpNetCoreProductDefaultTargetFramework from eng/TargetFrameworks.props. This is part of the migration from productTfm.txt to the MSBuild-centric approach for the single source of truth for the product TFM.
Replace set /p reading from productTfm.txt with dotnet msbuild --getProperty:FSharpNetCoreProductDefaultTargetFramework in both TestBasicProvider.cmd and TestComboProvider.cmd.
- azure-pipelines-PR.yml: Copy TargetFrameworks.props instead of productTfm.txt - regression-test-jobs.yml: Use --getProperty to read TFM from staged props file
- Remove eng/productTfm.txt (replaced by TargetFrameworks.props) - Update release notes to reference TargetFrameworks.props - Update identifierAnalysisByType.fsx comment to reference new location
…ding productTfm.txt Phase 2 of the productTfm.txt migration: Replace File.ReadAllText calls with Import of eng/TargetFrameworks.props as the single source of truth for TFM. Files updated: - Directory.Build.props - UseLocalCompiler.Directory.Build.props - buildtools/checkpackages/Directory.Build.props - tests/AheadOfTime/Directory.Build.props - tests/fsharp/SDKTests/tests/FSharpCoreVersionTest.props
ViktorHofer
left a comment
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.
This looks good. I'm surprised that there are so many places in scripts and even in YML that need the TFM. That's a bit unusual but is fine.
One important feedback: Whenever you call into dotnet msbuild, make sure that you pass global properties in as well. Otherwise conditions on DotNetBuildSourceOnly=true are wrong and the TFM would be wrong.
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
|
Remove / change this block now? Lines 72 to 87 in 1870673
|
|
@ViktorHofer : Had exactly the same in mind , done :) |
|
Is NetCurrent available in that new props file? The value comes from the Arcade SDK. |
Fixes #19204
Architecture: Single Source of Truth
What changed
eng/TargetFrameworks.propsas the single source of truth for the product TFM, including source-build override (DotNetBuildSourceOnly=true→$(NetCurrent))dotnet msbuild --getProperty:FSharpNetCoreProductDefaultTargetFrameworkinstead of hardcoded valuesbuild.shforwards/p:DotNetBuildSourceOnlyto ensure correct TFM resolution in source-build/VMR scenariosDirectory.Build.propssimplified — the<Choose>block for source-build override is now handled byTargetFrameworks.propsFSharp.BuildPropertiesmodule instead of reading text fileseng/productTfm.txt(superseded by props file)