Enlighten FormatUrl task for multithreaded mode

[jankratochvilcz]

Enlighten FormatUrl task for multithreaded mode

Parent: #11834

Context

FormatUrl is one of the remaining tasks listed in the migration epic (#11834) under "Other (either simple or with unknown issues)". The task itself is tiny (one method, ~3 lines), but its only cwd-dependent code path is buried two calls deep inside a shared ManifestUtilities helper:

FormatUrl.Execute
  → PathUtil.Format(InputUrl)
       → PathUtil.Resolve(path)
            → Path.GetFullPath(path)   // local relative path branch only

URL and UNC inputs are already cwd-independent; only local relative paths use Path.GetFullPath, which silently consumes process Environment.CurrentDirectory. Under multithreaded execution that's wrong — concurrent task instances may belong to different projects, and there is only one cwd per process.

PathUtil.Format has exactly one caller in the entire repo (FormatUrl), and PathUtil.Resolve is only invoked from inside PathUtil itself. Modifying their signatures therefore has zero compatibility risk — there is no shared consumer in GenerateApplicationManifest, GenerateDeploymentManifest, or anywhere else to keep stable.

Approach

Apply the PR #13267 (MSBuild/CallTarget) pattern: absolutize at the task boundary, then thread the already-absolute base directory through the existing helper.

1. Modify the signature of PathUtil.Format(string) to PathUtil.Format(string path, AbsolutePath baseDirectory). Also thread AbsolutePath baseDirectory through PathUtil.Resolve(string) so URL/UNC/local discrimination stays in one place — Resolve only consumes the base directory on the local-path branch; for URL/UNC inputs it ignores it.
2. Inside Resolve, replace the single Path.GetFullPath(path) call with new AbsolutePath(path, baseDirectory).GetCanonicalForm(). Using GetCanonicalForm() preserves the canonicalization that Path.GetFullPath performed today, including throwing on invalid path characters (SKILL Sin 6).
3. Mark FormatUrl with [MSBuildMultiThreadableTask] and implement IMultiThreadableTask with public TaskEnvironment TaskEnvironment { get; set; } (no default initializer — matches the built-in style used in Enlighten public versions of intrinsic tasks. #13267; the runtime default is supplied by TaskRouter).
4. FormatUrl.Execute calls PathUtil.Format(InputUrl, TaskEnvironment.ProjectDirectory). TaskEnvironment.ProjectDirectory is already typed as AbsolutePath and is guaranteed canonical by the engine (see PR Enlighten public versions of intrinsic tasks. #13267) — no GetAbsolutePath call is required.

Why no ChangeWave

For URL, UNC, and absolute local inputs, output is byte-identical to today. For relative local inputs the base directory changes from Environment.CurrentDirectory to TaskEnvironment.ProjectDirectory — but in legacy (non-MT) execution the two are equal, so observable behavior is unchanged. The semantic change only manifests under MT, which is the entire point of the migration.

If unforeseen behavioral diffs are found during review (e.g., differing canonicalization on edge inputs), gate the diff behind a new ChangeWave following the precedent set in PR #13069.

Test coverage assessment

Existing direct unit tests (src/Tasks.UnitTests/FormatUrl_Tests.cs, 10 cases)

NullTest, EmptyTest, NoInputTest, WhitespaceTestOnUnix, WhitespaceTestOnWindows, UncPathTest, LocalAbsolutePathTest, LocalUnixAbsolutePathTest, LocalWindowsAbsolutePathTest, LocalRelativePathTest, UrlLocalHostTest, UrlTest, UrlParentPathTest.

Coverage of input categories is good. Coverage of MT-specific concerns (project-relative resolution, concurrency) is zero.

Integration tests in this repo

None. FormatUrl is only consumed by Microsoft.Common.CurrentVersion.targets in the ClickOnce/publish flow (_DeploymentFormattedDeploymentUrl, _DeploymentFormattedSupportUrl, _DeploymentFormattedErrorReportUrl, _DeploymentFormattedApplicationUrl, _DeploymentFormattedComponentsUrl). End-to-end coverage of that flow lives in dotnet/sdk and the VS repo, not here. We will not add E2E publish tests in this repo as part of this issue.

Gaps to fill in this PR

• G1 — Concurrency test. Two FormatUrl instances with different ProjectDirectory values, same relative input, assert each result is rooted at the corresponding project dir. (SKILL red-team Phase 4.)
• G2 — Project-relative test. ProjectDirectory != Environment.CurrentDirectory, relative input resolves against ProjectDirectory. Documents the intentional semantic change.
• G3 — Inject TaskEnvironment into existing tests. Set t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest() in every existing test so LocalRelativePathTest's Environment.CurrentDirectory assumption stays valid (Fallback uses cwd as project dir).
• G4 — Add direct PathUtil.Format(string, AbsolutePath) tests covering one URL, one UNC, and one absolute local input — assert outputs are byte-identical to what the old single-arg signature produced (capture expected values from a current main-branch run before the refactor).

Acceptance criteria

• FormatUrl decorated [MSBuildMultiThreadableTask] and implements IMultiThreadableTask with a TaskEnvironment property (no default initializer — matches built-in style).
• PathUtil.Format and PathUtil.Resolve signatures both accept the project's AbsolutePath base directory; URL/UNC/local discrimination is preserved in Resolve as today.
• All existing FormatUrl_Tests pass on net472 and net10.0 with no behavior changes for null / empty / UNC / URL / absolute-local / localhost-URL / parent-segment URL inputs.
• WhitespaceTestOnWindows still throws ArgumentException (canonicalization preserved by GetCanonicalForm()).
• OutputUrl value is byte-identical to old behavior for URL, UNC, absolute-local, localhost-URL, and parent-segment URL inputs.
• OutputUrl never contains an absolutized-path leak — no AbsolutePath implicit conversion into the output property (SKILL Sin 1).
• Tests G1, G2, G3, G4 added and pass.
• .\build.cmd -v quiet succeeds; Tasks.UnitTests and Build.UnitTests pass.
• No new compiler warnings (warnings-as-errors in official build).
• No ChangeWave required; if any test required modification beyond G3, document why in the PR and gate the diff behind the next wave.
• multithreaded-task-migration SKILL sign-off checklist walked and passes.

Implementation order

1. Capture baseline outputs of PathUtil.Format for one URL, one UNC, one absolute local input from current main, then modify PathUtil.Format and PathUtil.Resolve signatures to accept AbsolutePath baseDirectory and replace Path.GetFullPath with new AbsolutePath(path, baseDirectory).GetCanonicalForm().
2. Apply attribute + interface to FormatUrl; route through the new signature using TaskEnvironment.ProjectDirectory.
3. Update existing FormatUrl_Tests to inject test TaskEnvironment (G3).
4. Add G1 (concurrency), G2 (project-relative), and G4 (direct PathUtil.Format baseline) tests.
5. Run full Tasks.UnitTests and Build.UnitTests; verify clean.
6. Run repo build with -v quiet to ensure no new warnings.

Risks / open questions

• ClickOnce flow under MT is not exercised by this repo's tests. Coordination with dotnet/sdk recommended before MT mode ships generally, but not blocking for this PR — the task itself is correct in isolation once migrated.

References

• Parent epic: Migrate Tasks in the MSbuild repo to the new Task type #11834
• Pattern PRs:
  • Enlighten public versions of intrinsic tasks. #13267 — Enlighten public versions of intrinsic tasks (absolutize-at-boundary pattern)
  • Enlighten FindUnderPath and AssignTargetPath tasks #13069 — Enlighten FindUnderPath and AssignTargetPath (ChangeWave usage)
  • Enlighten more tasks that require no change #13045 — Enlighten more tasks that require no change (attribute-only baseline)
  • Enlighten GetFrameworkPath and GetFrameworkSdkPath. #13282 — Enlighten GetFrameworkPath and GetFrameworkSdkPath (locking shared static state)
  • Enlighten RAR task #13319 — Enlighten RAR (heavyweight thread-TaskEnvironment-through-helpers pattern; for reference only)
• SKILL: .github/skills/multithreaded-task-migration/SKILL.md