Skip to content

Update the logic of custom culture support - #11607

Merged
YuliiaKovalova merged 13 commits into
dotnet:vs17.14from
YuliiaKovalova:dev/ykovalova/fix_custom_culture
Apr 2, 2025
Merged

Update the logic of custom culture support #11607
YuliiaKovalova merged 13 commits into
dotnet:vs17.14from
YuliiaKovalova:dev/ykovalova/fix_custom_culture

Conversation

@YuliiaKovalova

@YuliiaKovalova YuliiaKovalova commented Mar 19, 2025

Copy link
Copy Markdown
Member

Fixes #11446

Context

This PR introduces a configurable approach to MSBuild's custom culture resource handling. It adds property EnableCustomCulture that enables custom culture resource generation and a property to exclude specific culture directories from being treated as culture-specific resources.

After this change #11000, MSBuild automatically treats directories with names that look like culture codes as culture-specific resource directories, even when they might be serving a different purpose (e.g., hash-based directories or directories with short technical names that coincidentally match a culture code). This can cause unexpected behavior in build processes and create unwanted resource assemblies.

Changes Made

A two-part approach was implemented:

  • Added the EnableCustomCulture project property that has to be set as true to enable custom culture resource handling.

  • Exclusion Property: Added the <NonCultureResourceDirectories> MSBuild property that accepts a semicolon-delimited list of directory names to exclude from custom culture processing, allowing fine-grained control.

Testing

Manual + added e2e tests.

Note

  • I removed unused method in test class GenerateTableWithAssemblyFromTheGlobalLocation
  • Update full bootstrap folder content population for windows core scenarios - add the bits from the latest local VS detected on the machine.

@YuliiaKovalova YuliiaKovalova self-assigned this Mar 19, 2025
@YuliiaKovalova
YuliiaKovalova force-pushed the dev/ykovalova/fix_custom_culture branch 2 times, most recently from 0d652d8 to ee7ff76 Compare March 24, 2025 17:04
@YuliiaKovalova
YuliiaKovalova requested a review from a team as a code owner March 24, 2025 18:05
@YuliiaKovalova

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@YuliiaKovalova
YuliiaKovalova force-pushed the dev/ykovalova/fix_custom_culture branch from 8c4fa0f to 3501934 Compare March 25, 2025 09:46
@YuliiaKovalova

Copy link
Copy Markdown
Member Author

bootstrap changes were validated here: #11623

Comment thread src/MSBuild.Bootstrap.Utils/Tasks/LocateVisualStudioTask.cs Outdated
Comment thread eng/BootStrapMsBuild.targets Outdated
Comment thread src/MSBuild.Bootstrap.Utils/Tasks/LocateVisualStudioTask.cs Outdated
Comment thread src/MSBuild.Bootstrap.Utils/Tasks/LocateVisualStudioTask.cs Outdated

@JanProvaznik JanProvaznik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall seems reasonable, left some questions and comments

Comment thread src/Framework/Traits.cs Outdated
Comment thread src/Tasks/AssemblyDependency/ReferenceTable.cs Outdated
Comment thread src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs
Comment thread documentation/specs/custom-cultures.md
Comment thread documentation/specs/custom-cultures.md Outdated
Comment thread src/Tasks/CreateVisualBasicManifestResourceName.cs
Comment thread src/Tasks.UnitTests/ResolveAssemblyReference_CustomCultureTests.cs Outdated
Comment thread documentation/specs/custom-cultures.md
Comment thread eng/BootStrapMsBuild.targets
Comment thread src/BuildCheck.UnitTests/EndToEndTests.cs Outdated
Comment thread documentation/specs/custom-cultures.md Outdated
@YuliiaKovalova
YuliiaKovalova changed the base branch from main to vs17.14 April 1, 2025 18:22
@YuliiaKovalova
YuliiaKovalova force-pushed the dev/ykovalova/fix_custom_culture branch from 143c564 to a50210f Compare April 1, 2025 18:47
Comment thread documentation/wiki/ChangeWaves.md
Comment thread eng/BootStrapMsBuild.targets
Comment thread src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj Outdated
Comment thread src/Tasks/Microsoft.CSharp.CurrentVersion.targets
Comment thread src/Tasks.UnitTests/ResolveAssemblyReference_CustomCultureTests.cs Outdated
@YuliiaKovalova
YuliiaKovalova enabled auto-merge (squash) April 2, 2025 16:26
@YuliiaKovalova
YuliiaKovalova merged commit e436966 into dotnet:vs17.14 Apr 2, 2025
JanProvaznik added a commit to JanProvaznik/msbuild that referenced this pull request Jul 29, 2026
Wave 17.14 is out of rotation as of MSBuild 18.10, so remove the opt-out.
The only remaining live gate was SolutionFile.FullPath, which now always
throws ArgumentNullException for a null or empty path.

The .slnx-parser and RAR custom-culture features that originally shipped
under this wave were reverted/replaced earlier (see dotnet#11607), so only stale
Wave17_14 comments remained in Create{CSharp,VisualBasic}ManifestResourceName;
those are cleaned up as well.

Fixes dotnet#13266

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d2ae1d52-af0e-4c11-9189-1994e36e142b
JanProvaznik added a commit that referenced this pull request Jul 30, 2026
Fixes #13264
Fixes #13265
Fixes #13266

### Context

Change waves 17.10, 17.12 and 17.14 are long out of rotation, and 18.3
rotates out with them now that the repo is on 18.10. Per
[ChangeWaves-Dev.md](https://github.com/dotnet/msbuild/blob/main/documentation/wiki/ChangeWaves-Dev.md#change-wave-end-of-lifespan-procedure),
retiring a wave means deleting the `Version` field, removing the
`AreFeaturesEnabled` conditions so the feature becomes standard
behavior, dropping the tests that pinned the opted-out behavior, and
cleaning up the dead code left behind.

After this the rotation is 18.4 – 18.10, and the "will be removed in the
release accompanying .NET 11" bucket in `ChangeWaves.md` is empty, so it
is dropped.

### Changes Made

One commit per wave, plus one prerequisite bootstrap fix:

**`Overlay freshly-built MSBuild files into the bootstrap SDK's Current
folder`**

`BootstrapNetCore` overlays our build output onto the acquired SDK at
`sdk\<ver>\`, but the .NET SDK layout keeps some of MSBuild's own
props/targets under `sdk\<ver>\Current\`, because they are imported
through `$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\...` and
`$(MSBuildToolsVersion)` is `Current` there. `Microsoft.Common.props` is
the notable case: in an SDK layout it exists **only** under `Current\`,
and `Microsoft.NET.Sdk`'s `Sdk.props` imports it from that path for
every SDK-style project.

Our build output has no `Current\` subdirectory, so the overlay never
replaced it. That means every bootstrap build has silently been
evaluating the `Microsoft.Common.props` belonging to whatever older
MSBuild the acquired SDK shipped with, and changes to
`src\Tasks\Microsoft.Common.props` never took effect in bootstrap-based
tests.

Retiring 17.10 made that visible: the stale props still evaluated
`$([MSBuild]::AreFeaturesEnabled('17.10'))`, which trips the Debug
assert in `ChangeWaves` and took down 54 BuildCheck end-to-end tests.
Fixing the overlay addresses the root cause and leaves the assert doing
exactly the job it was added for.

**`Phase out ChangeWave 17.10`** — makes unconditional:
NuGet.Frameworks/AppDomain assembly loading in MSBuild.exe and VS,
process-wide SDK resolver caching (`CachingSdkResolverLoader`) plus the
.NET-only default-resolver fast path, target switch unquoting, `Link`
metadata on `Resource` items, `-version` trailing newline, `Traits`
refresh on environment change, `Exec` not trimming leading whitespace
and not exporting `LANG`/`LC_ALL` on Unix, `ToolTask` console-code-page
encoding, skipping project extension imports during restore, and
property reassignment tracking. Also deletes `StableStringHashLegacy`
and the `NoInlining` guard on `StableStringHash` that only existed to
keep StringTools out of the opted-out path.

**`Phase out ChangeWave 17.12`** — makes unconditional:
`TaskParameterEventArgs` for scalar task parameters and output
properties (the legacy textual form and `OutputPropertyLogMessage` path
are gone), `InvariantCulture` for `Convert.ToString` during evaluation,
`BuildRequestDataFlags` compatibility in `ResultsCache`, emitting
evaluation props/items when any sink asks, `ProjectStartedEventArgs`
returning empty collections instead of `null`, binlog `{}` wildcard file
names, and loading `Microsoft.DotNet.MSBuildSdkResolver` into the
default load context.

**`Phase out ChangeWave 17.14`** — the only live gate left was
`SolutionFile.FullPath`, which now always throws `ArgumentNullException`
for a null/empty path. The .slnx-parser and RAR custom-culture features
originally under this wave were already reverted/replaced (see #11607),
so only stale `Wave17_14` comments remained in
`Create{CSharp,VisualBasic}ManifestResourceName`.

**`Phase out ChangeWave 18.3`** — `WriteLinesToFile` always writes
atomically (temp file + replace with retry); the non-transactional path
and the quarantined test asserting the old data-losing behavior are
deleted.

### Testing

`build.cmd -test` on Windows (net11.0 x64 + net472 x86). Everything
passes except
`PortableTasks_Tests.TestDesktopMSBuildShouldRunPortableTask`, which
shells out to `msbuild` and fails for lack of a Visual Studio install on
this machine, not because of these changes.

### Notes

Behavior only changes for builds that were **opting out** via
`MSBUILDDISABLEFEATURESFROMVERSION`; the default (all waves enabled)
code paths make exactly the same decisions as before. Setting
`MSBUILDDISABLEFEATURESFROMVERSION` to a retired wave now clamps to
`18.4` and warns MSB4272, which is the documented out-of-rotation
behavior.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d2ae1d52-af0e-4c11-9189-1994e36e142b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RAR discovers new resource DLLs that aren't culture-related after custom-culture change

3 participants