Centralize build properties and clean up project files#2472
Draft
Sergio0694 wants to merge 15 commits into
Draft
Centralize build properties and clean up project files#2472Sergio0694 wants to merge 15 commits into
Sergio0694 wants to merge 15 commits into
Conversation
Add centralized global properties and stricter defaults in Directory.Build.props/targets: enable embedded debug symbols, set DotNetVersion/net10.0 and LangVersion=14.0, enforce warnings-as-errors in Release, enable strict/analysis levels and code-style enforcement, add common NoWarn entries (CS8500, NETSDK1229), generate documentation files. In Directory.Build.targets enable AOT/marshalling-friendly settings for net10.0 projects (IsAotCompatible for libraries, DisableRuntimeMarshalling, EmitSkipLocalsInitAttribute), NativeAOT publish optimizations (InvariantGlobalization, OptimizationPreference, IlcDehydrate=false, IlcResilient=false, ControlFlowGuard) and other build folder/FrameworkReference adjustments. Also cleaned up comments and formatting.
…d.props These build properties are now set centrally for all C# projects in src/Directory.Build.props, so the per-project copies are redundant: LangVersion, TreatWarningsAsErrors/CodeAnalysisTreatWarningsAsErrors, AnalysisLevel, AnalysisLevelStyle, EnforceCodeStyleInBuild, Features=strict, GenerateDocumentationFile, and the centralized NoWarn entries CS8500 and NETSDK1229. Removing the stray LangVersion overrides also drops the leftover 'preview' override in WinRT.Impl.Generator (a leftover from before C# 14.0 shipped), so every project now picks up the centralized LangVersion=14.0 instead of accidentally compiling with the preview language version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…y.Build.targets
These properties are now applied centrally to all net10.0 C# projects (and, for
the NativeAOT options, to all net10.0 projects that set PublishAot=true) in
src/Directory.Build.targets, so the per-project copies are redundant:
- IsAotCompatible, DisableRuntimeMarshalling, EmitSkipLocalsInitAttribute
- InvariantGlobalization, OptimizationPreference, StackTraceSupport,
UseSystemResourceKeys, ControlFlowGuard, IlcDehydrate, IlcResilient
The build tools keep their own PublishAot=true (which is what gates the
centralized NativeAOT options), so the generated binaries are unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Directory.Build.props now defines a single DotNetVersion property (net10.0) so
the .NET version used across the repo can be changed in one place. Point the
core projects' TargetFramework at it instead of hardcoding net10.0:
- The runtime, projection writer, generator core, source generator and the
five CLI build tools now use <TargetFramework>$(DotNetVersion)</TargetFramework>.
- WinRT.Internal uses $(DotNetVersion)-windows10.0.26100.1 (it keeps its
CsWinRT 3.0 Windows TFM revision).
The resolved framework is unchanged (net10.0), so build outputs land in the
same folders that existing HintPath/tool-directory references rely on.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Several project-level NoWarn suppressions no longer fire and can be dropped
(verified with Release --no-incremental builds, where warnings are errors):
- WinRT.Interop.Generator: remove IDE0028 and IDE0370. IDE0028 no longer
fires. IDE0370 ("suppression is unnecessary") was firing on a redundant
null-forgiving operator 'type.FullName!' in SignatureGenerator.cs; FullName
is a non-nullable string, so the '!' is removed and the suppression dropped.
- WinRT.Impl.Generator / WinRT.WinMD.Generator: remove IDE0028 (no longer fires).
- WinRT.Projection.Generator: drop IDE0028 from the NoWarn list (no longer
fires), keeping only the IL* trim/AOT suppressions for the Roslyn reference.
The Writer's IDE suppressions and the projection generator's IL* suppressions
are intentionally retained because they still fire.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The centralized properties added in Directory.Build.props/targets surfaced two
build breaks in projects that don't allow unsafe code (e.g. WinRT.Internal):
- EmitSkipLocalsInitAttribute was enabled for every net10.0 C# project, but the
C# compiler requires AllowUnsafeBlocks to use '[module: SkipLocalsInit]'
(CS0227). Condition the centralized property on AllowUnsafeBlocks so only
projects that opt into unsafe code emit it. This matches the pre-centralization
behavior (only the unsafe runtime/build-tool projects ever set it) and fixes
WinRT.Internal plus any other non-unsafe net10.0 project.
- GenerateDocumentationFile is now enabled everywhere, which makes the compiler
validate XML doc comments. ProjectionInternalAttribute had a <para> closed by
</remarks> (CS1570), which becomes an error under the centralized
Release warnings-as-errors. Add the missing </para>.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Every C# project in the repo allows unsafe code (the runtime and build tools rely heavily on pointers and other unsafe constructs for interop), so set AllowUnsafeBlocks=true once in the centralized Globals property group, right after LangVersion, and remove the now-redundant per-project copies (the nine core projects plus UnitTest, DiagnosticTests, BuildDeterminismComponent, WinRT.Host.Shim, and the FunctionalTests Directory.Build.props). The conditional override in the NonWinRT functional test (AllowUnsafeBlocks=false when TestUnsafeDisabled is set) is kept, since it still correctly overrides the centralized default for that test scenario. The previously added EmitSkipLocalsInitAttribute guard (gated on AllowUnsafeBlocks) keeps composing correctly: projects that opt out of unsafe code also won't emit SkipLocalsInit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The centralized analysis settings now apply to WinRT.Host.Shim, which surfaced
trim/AOT, documentation and code-style warnings (treated as errors by the
Authoring Directory.Build.props):
- IL2026/IL2075: this project is meant to be used for hosting scenarios with
CoreCLR, so trimming/AOT aren't supported by design. The centralized
IsAotCompatible default is now only applied when a project hasn't set it,
and WinRT.Host.Shim explicitly opts out (IsAotCompatible=false), which
disables the trim/AOT analyzers for it.
- CS1591: add XML docs for the public Shim type, GetActivationFactoryDelegate
and GetActivationFactory.
- IDE0073/IDE0008/IDE0047/IDE0046 (and the IDE0300/IDE0350/IDE0058 already
applied by dotnet format): add the file header, use explicit types, drop
redundant parentheses, and use a conditional expression.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…is rules
The centralized code-style enforcement now applies to WinRT.Generator.Tasks,
surfacing warnings that are treated as errors in Release:
- IDE0073: replace the '.NET Foundation' file header with the repo's
'Copyright (c) Microsoft Corporation.' header in the five task files.
- IDE0370: remove the redundant null-forgiving operator on the non-nullable
CsWinRTToolsDirectory property in the Path.Combine calls.
- IDE0005: remove unnecessary using directives (applied via dotnet format).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Perf/ResultsComparer is a local-only benchmark results comparer (not shipped and not part of cswinrt.slnx), so the repo's centralized strict analysis doesn't add value there. Suppress CS1591 (missing XML docs) and disable EnforceCodeStyleInBuild for it, with a comment, rather than churning a ported tool to satisfy the CsWinRT code-style rules. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Helpers.cs used 'AnalyzerConfigOptionsProvider?' parameters, but the project isn't in a nullable-enabled context, so under the centralized Release warnings-as-errors these produced CS8632 errors. The project is non-nullable, so the annotation has no effect on behavior; drop the '?' on both parameters. The pre-existing CS0246 in UnitTesting.cs (a stale 'using WinRT;' from the 2.x namespace) is left as-is, to be addressed separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The generated projection sources surface evaluation-only ([Experimental]) Windows Runtime APIs (e.g. Windows.Storage.Provider.StorageProviderShareLinkState, Windows.ApplicationModel.DataTransfer.TransferTarget), which produce CS8305 and fail under the projections' warnings-as-errors. These APIs are expected in a full SDK projection, so add a NoWarn for CS8305 in the shared Projections Directory.Build.props. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
Remove several obsolete/strict build properties and consolidate projection settings. This removes TreatWarningsAsErrors, DisableRuntimeMarshalling, CsWinRTAotOptimizerEnabled, EnableTrimAnalyzer, IsTrimmable and IsAotCompatible, adds a "Projection settings" comment, and moves SimulateCsWinRTNugetReference into the projections PropertyGroup for clearer organization.
Add CS1591 to the WarningsNotAsErrors list in src/Projections/Directory.Build.props so missing XML comment warnings (e.g. for CoreDragDropManager) are not treated as build errors. Also update the nearby comment block to document the CS1591 entry.
8a3fa34 to
1bf3b12
Compare
The centralized GenerateDocumentationFile=true applied to every project, which
enforced CS1591 (missing XML comment) on all public members of non-product
projects and on generated projection code (e.g. SourceGenerator2Test produced
~74k CS1591). We only care about XML docs for the actual product projects, which
are all named with a 'WinRT.' prefix, so gate GenerateDocumentationFile behind
MSBuildProjectName.StartsWith('WinRT.').
This removes the documentation warnings (CS1591/CS1573) from tests, samples,
benchmarks, the local projection projects, etc., while keeping docs enabled for
the product projects (which already build clean with them on).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Centralize the common MSBuild properties for all C# projects into
Directory.Build.props/Directory.Build.targets, then remove the now-redundant per-project copies, fix the C# language version that was accidentally pinned toprevieweverywhere, and point the core projects at a single shared$(DotNetVersion).Motivation
Properties such as
LangVersion,AllowUnsafeBlocks, warnings-as-errors, analysis levels, code-style enforcement, and the AOT/NativeAOT settings were copy-pasted across roughly a dozen project files, which is noisy and easy to drift. A strayLangVersion=previewinDirectory.Build.targets(imported after each project body) was also silently overriding every project back to the preview language version, even the ones that explicitly set14.0. Centralizing these settings removes the duplication, makes it possible to try a different .NET version or language version from one place, and fixes the accidentalpreviewlanguage version. While validating the result, the centralization also surfaced two latent build breaks that are fixed here.Changes
Centralized build configuration
src/Directory.Build.props: add aGlobalsproperty group (scoped to.csproj) that definesDotNetVersion(net10.0),LangVersion=14.0,AllowUnsafeBlocks=true, Release-onlyTreatWarningsAsErrors/CodeAnalysisTreatWarningsAsErrors,AnalysisLevel/AnalysisLevelStyle,EnforceCodeStyleInBuild,Features=strict,GenerateDocumentationFile, and the commonNoWarnentries (CS8500,NETSDK1229).src/Directory.Build.targets: apply the AOT/marshalling settings (IsAotCompatiblefor libraries,DisableRuntimeMarshalling,EmitSkipLocalsInitAttribute,TrimmerSingleWarn) to net10.0 projects, and the NativeAOT publish options (InvariantGlobalization,OptimizationPreference,StackTraceSupport,UseSystemResourceKeys,ControlFlowGuard,IlcDehydrate,IlcResilient) to net10.0PublishAotprojects.Project file cleanup
WinRT.Runtime2,WinRT.Generator.Core,WinRT.Projection.Writer,WinRT.SourceGenerator2,WinRT.Sdk.Projection,WinRT.Internal, and the five build tools (cswinrtinteropgen,cswinrtimplgen,cswinrtprojectiongen,cswinrtprojectionrefgen,cswinrtwinmdgen). This also drops the leftoverLangVersion=previewoverride inWinRT.Impl.Generator.AllowUnsafeBlocks=truecopies from the core projects plusUnitTest,DiagnosticTests,BuildDeterminismComponent,WinRT.Host.Shim, and theFunctionalTestsDirectory.Build.props. TheNonWinRTtest keeps its conditionalAllowUnsafeBlocks=false(whenTestUnsafeDisabled) since it still overrides the centralized default for that scenario.TargetFrameworkat$(DotNetVersion)(WinRT.Internaluses$(DotNetVersion)-windows10.0.26100.1). The resolved framework is unchanged (net10.0), so output folders and existingHintPath/tool-directory references are unaffected.Suppressions no longer necessary
NoWarnentries that no longer fire, verified withRelease --no-incrementalbuilds (where warnings are errors):IDE0028fromcswinrtinteropgen/cswinrtimplgen/cswinrtwinmdgen/cswinrtprojectiongen, andIDE0370fromcswinrtinteropgen.WinRT.Interop.Generator/Helpers/SignatureGenerator.cs:IDE0370was firing on a redundant null-forgiving operatortype.FullName!(FullNameis a non-nullablestring); removed the!so the suppression is genuinely unnecessary. The Writer's IDE suppressions and the projection generator'sIL*(Roslyn trim/AOT) suppressions are retained because they still apply.Build-break fixes surfaced by the centralization
src/Directory.Build.targets: only emit[module: SkipLocalsInit]for projects that setAllowUnsafeBlocks(the C# compiler requires unsafe code for[SkipLocalsInit]or it producesCS0227). This matches the pre-centralization behavior and unbreaks non-unsafe net10.0 projects such asWinRT.Internal. (WithAllowUnsafeBlocksnow centralized totrue, this guard continues to do the right thing for any project that opts back out.)src/WinRT.Internal/ProjectionInternalAttribute.cs: fix a malformed XML doc comment (a<para>closed by</remarks>,CS1570) that became an error onceGenerateDocumentationFilewas enabled under Release warnings-as-errors.Validation
All core projects plus
WinRT.Internalbuild green in Release (--no-incremental, warnings-as-errors active).WinRT.Sdk.Projectioncannot be restored locally (NU1102forMicrosoft.Windows.SDK.Contracts, which CI supplies via an explicitSdkPackageVersion); this is pre-existing and unrelated to these changes. Test and sample projectTargetFrameworkvalues were intentionally left untouched, so a CI Release run is worth doing to confirm nothing else relies on the previously per-project settings.