Skip to content

Enlighten GenerateTrustInfo for multithreaded mode #13622

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.

Sign up for GitHub

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

Open
jankratochvilcz wants to merge 2 commits into
base: main
Choose a base branch
from
+19 −10
Open

Enlighten GenerateTrustInfo for multithreaded mode #13622

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24b3582
Enlighten GenerateTrustInfo for multithreaded mode
Apr 27, 2026
ceea4bb
Address GenerateTrustInfo review feedback: guard empty BaseManifest.I…
Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/Tasks/GenerateTrustInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ namespace Microsoft.Build.Tasks
/// This task generates the application trust from the base manifest
/// and the TargetZone and ExcludedPermissions properties.
/// </summary>
public sealed class GenerateTrustInfo : TaskExtension, IGenerateTrustInfoTaskContract
[MSBuildMultiThreadableTask]
public sealed class GenerateTrustInfo : TaskExtension, IGenerateTrustInfoTaskContract, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;

private const string Custom = "Custom";

public ITaskItem BaseManifest { get; set; }
Expand Down Expand Up @@ -50,16 +53,20 @@ public override bool Execute()
}

// Read trust-info from app.manifest
if (BaseManifest != null && FileSystems.Default.FileExists(BaseManifest.ItemSpec))
if (BaseManifest != null && !string.IsNullOrEmpty(BaseManifest.ItemSpec))
{
try
{
trustInfo.ReadManifest(BaseManifest.ItemSpec);
}
catch (Exception ex)
AbsolutePath baseManifestPath = TaskEnvironment.GetAbsolutePath(BaseManifest.ItemSpec);
if (FileSystems.Default.FileExists(baseManifestPath))
{
Log.LogErrorWithCodeFromResources("GenerateManifest.ReadInputManifestFailed", BaseManifest.ItemSpec, ex.Message);
return false;
try
{
trustInfo.ReadManifest(baseManifestPath);
}
catch (Exception ex)
{
Log.LogErrorWithCodeFromResources("GenerateManifest.ReadInputManifestFailed", BaseManifest.ItemSpec, ex.Message);
return false;
}
}
}

Expand Down Expand Up @@ -99,14 +106,16 @@ public override bool Execute()
}

// Write trust-info back to a stand-alone trust file
trustInfo.Write(TrustInfoFile.ItemSpec);
AbsolutePath trustInfoFilePath = TaskEnvironment.GetAbsolutePath(TrustInfoFile.ItemSpec);
trustInfo.Write(trustInfoFilePath);
Comment on lines 108 to +110
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

This change is specifically meant to make BaseManifest/TrustInfoFile path handling deterministic under multithreaded task isolation. There don’t appear to be unit tests covering GenerateTrustInfo with a non-default TaskEnvironment (e.g., project directory different from process CWD); adding one would help prevent regressions in relative-path resolution.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@jankratochvilcz jankratochvilcz Apr 27, 2026

Choose a reason for hiding this comment

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

The relative-path correctness here flows through the same TaskEnvironment.GetAbsolutePath path that's exercised by the engine-level multithreaded driver tests and by other migrations of similar shape (~10+ tasks now using the same pattern for relative path resolution). Adding a per-task multithreaded fixture would duplicate that infrastructure.

The behavior under the multithreaded driver is: GetAbsolutePath resolves against the task's project directory rather than process CWD — the resolution itself is the contract validated by TaskEnvironment unit tests. The functional path-resolution behavior here is identical to other migrated tasks (SignFile, ResolveKeySource in the same series), all relying on the same code path.

Happy to add coverage if reviewers feel strongly, but my read is the regression risk is low and the test cost is high.


return true;
}
}

#else

[MSBuildMultiThreadableTask]
public sealed class GenerateTrustInfo : TaskRequiresFramework, IGenerateTrustInfoTaskContract
{
public GenerateTrustInfo()
Expand Down
Loading