Expose import graph to tasks#13680
Conversation
Tasks currently have no way to discover which files were imported during project evaluation. The data exists internally during evaluation but was never surfaced through any task-facing API.
This change adds an `EngineServices.ImportEdges` property that returns a structured list of import edges — each edge represents one `<Import>` element connecting an importing project file to the file it resolved to, optionally carrying the SDK name for SDK-resolved imports.
A new `ProjectImportEdge` readonly struct in `Microsoft.Build.Framework` carries `ImportedProjectPath`, `ImportingProjectPath`, and `SdkName`. Each imported file appears at most once (first occurrence in depth-first evaluation order), so the collection forms a tree rather than a DAG.
## Usage
Tasks can use pattern matching — the `null` default naturally handles older engines and out-of-proc nodes where the data was not serialized:
```csharp
if (BuildEngine is IBuildEngine10 { EngineServices.ImportEdges: IReadOnlyList<ProjectImportEdge> edges })
{
// use edges
}
```
## Opt-in serialization for out-of-proc nodes
On the in-process node the data is always available. On out-of-process worker nodes the data is only available if the project opts in, because serializing the import graph across nodes has a cost we don't want to impose on every build:
```xml
<PropertyGroup>
<MSBuildProvideImportGraph>true</MSBuildProvideImportGraph>
</PropertyGroup>
```
When this property is not set, serialization is skipped and `ImportEdges` returns `null` on the worker node. This means **zero additional IPC cost** for builds that don't need the import graph.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR exposes a structured project import graph to tasks by introducing a new Microsoft.Build.Framework.ProjectImportEdge type and plumbing evaluation-time import relationships through ProjectInstance into EngineServices.ImportEdges, with conditional serialization for out-of-proc nodes via MSBuildProvideImportGraph.
Changes:
- Added public
ProjectImportEdgereadonly record struct inMicrosoft.Build.Frameworkto represent individual<Import>relationships (including optional SDK name). - Added
EngineServices.ImportEdgestask-facing API and surfacedProjectInstance.ImportEdgesthrough the task host. - Implemented conditional
ProjectInstanceserialization of import edges for out-of-proc nodes and added unit tests validating population, deep copy, and serialization behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/ProjectImportEdge.cs | New public struct representing an import edge, with docs and ToString(). |
| src/Framework/EngineServices.cs | New ImportEdges virtual property on task-facing engine services. |
| src/Framework/Constants.cs | Added constant for MSBuildProvideImportGraph property name. |
| src/Build/Instance/ProjectInstance.cs | Captures import edges during evaluation; conditionally serializes them across nodes; deep copy support. |
| src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs | Surfaces ProjectInstance.ImportEdges via EngineServices implementation for tasks. |
| src/Build.UnitTests/Instance/ProjectInstance_ImportEdges_Tests.cs | Unit tests covering evaluation population, root exclusion, deep copy, and serialization opt-in/out. |
| #nullable enable | ||
| /// <summary> | ||
| /// Gets the import edges discovered during project evaluation, representing the graph of | ||
| /// <c><Import></c> relationships between project files. | ||
| /// </summary> | ||
| /// <value> | ||
| /// A read-only list of <see cref="ProjectImportEdge"/> values describing each import relationship, | ||
| /// or <see langword="null"/> if the import graph is not available on this node. | ||
| /// </value> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// The import graph is always available when the task runs on the in-process node. | ||
| /// For out-of-process nodes, set the MSBuild property <c>MSBuildProvideImportGraph</c> to <c>true</c> | ||
| /// in your project to opt in to serializing import graph data across nodes. | ||
| /// </para> | ||
| /// <para> | ||
| /// Tasks can use pattern matching to access this property: | ||
| /// <code> | ||
| /// if (BuildEngine is IBuildEngine10 { EngineServices.ImportEdges: IReadOnlyList<ProjectImportEdge> edges }) | ||
| /// { | ||
| /// // use edges | ||
| /// } | ||
| /// </code> | ||
| /// The pattern naturally handles older engines (where the property is absent) and | ||
| /// out-of-proc nodes where the data was not serialized (where the property returns <see langword="null"/>). | ||
| /// </para> | ||
| /// </remarks> | ||
| public virtual IReadOnlyList<ProjectImportEdge>? ImportEdges => null; | ||
| #nullable restore |
| public override bool IsOutOfProcRarNodeEnabled => _taskHost._host.BuildParameters.EnableRarNode; | ||
|
|
||
| /// <inheritdoc/> | ||
| public override IReadOnlyList<ProjectImportEdge> ImportEdges => |
| /// May be <see langword="null"/> on out-of-process nodes if the project did not opt in | ||
| /// to serializing import edges via the <c>MSBuildProvideImportGraph</c> property. | ||
| /// </remarks> | ||
| internal IReadOnlyList<ProjectImportEdge> ImportEdges { get; private set; } |
| bool hasImportEdges = false; | ||
|
|
||
| if (translator.Mode == TranslationDirection.WriteToStream) | ||
| { | ||
| hasImportEdges = ImportEdges is { Count: > 0 } | ||
| && string.Equals( | ||
| _properties?.GetProperty(Constants.MSBuildProvideImportGraphPropertyName)?.EvaluatedValue, | ||
| "true", | ||
| StringComparison.OrdinalIgnoreCase); | ||
| } |
| /// <param name="ImportedProjectPath">Full path of the imported project file.</param> | ||
| /// <param name="ImportingProjectPath">Full path of the project file that contains the <c><Import></c> element, or <see langword="null"/> if this is a direct import from the root project.</param> | ||
| /// <param name="SdkName">The SDK name if this import was resolved via an SDK reference (e.g. <c>"Microsoft.NET.Sdk"</c>); otherwise <see langword="null"/>.</param> | ||
| public readonly record struct ProjectImportEdge( | ||
| string ImportedProjectPath, | ||
| string? ImportingProjectPath, | ||
| string? SdkName = null) | ||
| { | ||
| /// <inheritdoc/> | ||
| public override string ToString() | ||
| { | ||
| string arrow = ImportingProjectPath is null | ||
| ? $"[root] -> {ImportedProjectPath}" | ||
| : $"{ImportingProjectPath} -> {ImportedProjectPath}"; | ||
|
|
| { | ||
| /// <summary> | ||
| /// Tests for the import edge feature: structured import graph data on ProjectInstance | ||
| /// exposed to tasks via EngineServices.GetImportEdges(). |
There was a problem hiding this comment.
Code Review — PR #13680 (ImportEdges on ProjectInstance / EngineServices)
| # | Dimension | Verdict |
|---|---|---|
| 8 | API Surface — EngineServices.Version |
🟠 1 MAJOR |
| 18 | Documentation — ImportingProjectPath null claim |
🟠 1 MAJOR |
| 22 | Correctness — deser null guard | 🟡 1 MODERATE |
| 16 | Idiomatic C# / Nullable Consistency | 🟡 1 MODERATE |
✅ 20/24 dimensions clean.
- API Surface —
Versionnot incremented for newImportEdgesmember; class contract requiresVersion3 - Documentation —
ImportingProjectPathdoc says "null if direct root import" but construction code never produces null; doc is wrong or the type annotation should be tightened - Correctness —
importedPathcan remainnullafterTranslate(ref importedPath)on a corrupted/mismatched stream, silently violating the non-nullablestring ImportedProjectPathcontract - Nullable — scoped
#nullable enable/#nullable restoreinEngineServices.cs; should be file-level
Investigated but not flagged:
- PublicAPI.Unshipped.txt — repo does not use
Microsoft.CodeAnalysis.PublicApiAnalyzers; no file to update. ✅ using Xunit.Abstractions— project uses xUnit v3 (Verify.XunitV3), whereITestOutputHelperlives in theXunitnamespace; no import needed. Confirmed by checking adjacent fileProjectInstance_Internal_Tests.cswhich follows the same pattern. ✅- IPC
Translate(ref string)null semantics — confirmed viaBinaryTranslator: on read,TranslateNullablereads one boolean; iffalse, returns early leaving the local unchanged. The writer-side invariant (paths are always non-null) makes this safe in the happy path, but the deserializer has no defensive guard (see Correctness finding above). ✅ (writer side),⚠️ (reader side — flagged) ImportingProjectPathnullability onResolvedImport— confirmed viaResolvedImport.cs:ImportingElementis documented as "Null if this is the top project"; theif (ImportingElement != null)guard meansContainingProject.FullPathis always set for any edge that is recorded. ✅OutOfProcTaskHostNode.EngineServicesImpl— does not overrideImportEdges; the defaultnullreturn is the correct fallback for the OOP task-host node (it has noProjectInstancereference). ✅MockEngine— does not overrideImportEdges; defaultnullis safe for tests that don't exercise this feature. ✅- Backwards compatibility —
ImportEdgesreturnsnullby default (notthrow NotImplementedException); this is consistent with the nullable-return design and does not break existing consumers. ✅ - Test quality — 6 tests covering population, deep copy, serialisation opt-in/opt-out, and
ToString. All well-named. ✅
Note
🔒 Integrity filter blocked 1 item
The following item were blocked because they don't meet the GitHub integrity level.
- #13680
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13680 · ● 18.8M
| /// out-of-proc nodes where the data was not serialized (where the property returns <see langword="null"/>). | ||
| /// </para> | ||
| /// </remarks> | ||
| public virtual IReadOnlyList<ProjectImportEdge>? ImportEdges => null; |
There was a problem hiding this comment.
[MAJOR] API Surface — EngineServices.Version not incremented for new member
The class XML doc comment on Version says:
"Must be incremented whenever new members are added. Derived classes should override the property to return the version actually being implemented."
IsOutOfProcRarNodeEnabled was Version2. Adding ImportEdges requires Version3. Without bumping the version, tasks that use the version number to gate capability detection will silently see Version2 even on an engine that fully implements ImportEdges.
Recommendation: Add a constant and bump Version:
/// <summary>Version 3 with ImportEdges.</summary>
public const int Version3 = 3;
// Update the base default and the TaskHost.EngineServicesImpl override:
public virtual int Version => Version3;TaskHost.EngineServicesImpl and any other concrete implementations that override ImportEdges should return Version3.
| /// </para> | ||
| /// </remarks> | ||
| public virtual IReadOnlyList<ProjectImportEdge>? ImportEdges => null; | ||
| #nullable restore |
There was a problem hiding this comment.
[MODERATE] Nullable Consistency — scoped #nullable enable/#nullable restore instead of file-level
MSBuild convention (see ProjectImportEdge.cs in this same PR) is to place #nullable enable at the top of the file. The scoped annotation here:
#nullable enable
public virtual IReadOnlyList<ProjectImportEdge>? ImportEdges => null;
#nullable restoreleaves the rest of the file (and any future additions) in the nullable-disabled context restored by #nullable restore. A new contributor adding a member below the block will not get nullable diagnostics.
Recommendation: Move #nullable enable to the top of the file (after the using directives) and remove the scoped #nullable restore. The only member affected by NRT in this file is ImportEdges — the other members use value types (bool) and don't need annotation changes.
| } | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
[MODERATE] Correctness — Null importedPath silently violates non-nullable contract
During deserialization the code initialises:
string importedPath = null;
translator.Translate(ref importedPath);
// ...
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));ITranslator.Translate(ref string) wraps the value in a nullable-marker protocol: on the read side, TranslateNullable reads one boolean; if false, it returns early and leaves the local unchanged (still null). So if the stream ever marks the path as null (corrupted packet, version skew, or crafted stream), importedPath stays null and gets silently passed to ProjectImportEdge whose first positional parameter is string ImportedProjectPath — non-nullable by declaration. This will produce an ImportEdges list whose entries have ImportedProjectPath == null, causing NREs in any caller that unconditionally dereferences it.
In practice the writer never emits a null ImportedProjectPath because resolvedImport.ImportedProject.FullPath is always populated. But the deserializer has no local guard.
Recommendation:
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
// Guard against a corrupted or version-mismatched stream.
if (importedPath is null)
{
continue; // or throw InvalidProjectFileException
}
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));| /// <param name="ImportedProjectPath">Full path of the imported project file.</param> | ||
| /// <param name="ImportingProjectPath">Full path of the project file that contains the <c><Import></c> element, or <see langword="null"/> if this is a direct import from the root project.</param> | ||
| /// <param name="SdkName">The SDK name if this import was resolved via an SDK reference (e.g. <c>"Microsoft.NET.Sdk"</c>); otherwise <see langword="null"/>.</param> | ||
| public readonly record struct ProjectImportEdge( |
There was a problem hiding this comment.
[MAJOR] Documentation Accuracy — ImportingProjectPath null claim is incorrect
The parameter doc says:
"or
nullif this is a direct import from the root project"
But tracing the construction path in ProjectInstance.CreateImportsSnapshot:
// Only enters the `if` when ImportingElement != null (i.e., NOT the outer project)
if (resolvedImport.ImportingElement != null)
{
importEdges.Add(new ProjectImportEdge(
importedPath,
resolvedImport.ImportingElement.ContainingProject.FullPath, // ← always non-null here
...));
}ResolvedImport.ImportingElement is documented as "Null if this is the top project" — meaning for the top project itself, not for its imports. When the top project imports a file, ImportingElement is the <Import> element in the top project, and ContainingProject.FullPath is the top project's path (non-null for any project loaded from disk).
Result: ImportingProjectPath is never null as produced by CreateImportsSnapshot. The null claim in the doc is misleading — it implies a behaviour that never occurs in practice.
Recommendation (two options):
- Change
string? ImportingProjectPathtostring ImportingProjectPath(non-nullable) and remove the doc claim about null, OR - Keep
string?but correct the doc to describe the actual case where null can occur (a programmatically-created project that has no file path, producing an emptyFullPath), and add a guard in the construction code.
| /// <para> | ||
| /// Tasks can use pattern matching to access this property: | ||
| /// <code> | ||
| /// if (BuildEngine is IBuildEngine10 { EngineServices.ImportEdges: IReadOnlyList<ProjectImportEdge> edges }) |
There was a problem hiding this comment.
🟡 Doc sample — minor correctness note
The pattern IBuildEngine10 { EngineServices.ImportEdges: IReadOnlyList<ProjectImportEdge> edges } uses extended property patterns (C# 10+). While syntactically valid, task authors targeting older language versions or using .NET Framework won't be able to use this syntax.
Consider also showing a more compatible alternative in the doc:
if (BuildEngine is IBuildEngine10 be && be.EngineServices.ImportEdges is { } edges)
{
// use edges
}| string importedPath = edge.ImportedProjectPath; | ||
| string importingPath = edge.ImportingProjectPath; | ||
| string sdkName = edge.SdkName; | ||
| translator.Translate(ref importedPath); | ||
| translator.Translate(ref importingPath); | ||
| translator.Translate(ref sdkName); |
There was a problem hiding this comment.
🟡 Nullable safety gap in deserialization
string importedPath = null;
string importingPath = null;
string sdkName = null;
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));importedPath is initialized to null and passed to Translate(ref string). Per BinaryTranslator.TranslateNullable: if the stream marker is false, the method returns early and importedPath remains null. This null is then passed to new ProjectImportEdge(importedPath, ...) where ImportedProjectPath is declared as non-nullable string.
While the writing path guarantees non-null values (they come from FullPath which is validated), corrupted or forward-incompatible streams could trigger this path.
Consider adding a defensive null check:
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
if (importedPath is not null)
{
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));
}Or at minimum suppress the nullable warning with importedPath! to document the invariant explicitly.
| /// <param name="SdkName">The SDK name if this import was resolved via an SDK reference (e.g. <c>"Microsoft.NET.Sdk"</c>); otherwise <see langword="null"/>.</param> | ||
| public readonly record struct ProjectImportEdge( |
There was a problem hiding this comment.
🟠 ImportingProjectPath nullability doc is inaccurate
The doc for ImportingProjectPath says:
"or
nullif this is a direct import from the root project"
But in CreateImportsSnapshot, edges are only created inside the if (resolvedImport.ImportingElement != null) guard — which explicitly excludes the root project entry. For every recorded edge, ImportingProjectPath is set to resolvedImport.ImportingElement.ContainingProject.FullPath, which is always non-null.
The ToString() method handles null, and the type is string?, but no code path actually produces a null ImportingProjectPath. This creates a misleading API contract.
Suggestion: Either:
- Tighten the type to
string(non-nullable) and remove the null doc/handling inToString(), or - Correct the doc to describe a realistic scenario where null would occur (e.g., "reserved for future use" or during deserialization of corrupted data)
|
|
||
| public virtual bool IsOutOfProcRarNodeEnabled => throw new NotImplementedException(); | ||
|
|
||
| #nullable enable |
There was a problem hiding this comment.
🟡 Scoped #nullable enable/#nullable restore inconsistency
This creates an isolated nullable island. The same PR adds ProjectImportEdge.cs with file-level #nullable enable — following the preferred convention for new code.
Consider enabling nullable for the entire file instead of using a scoped region. The existing members (LogsMessagesOfImportance, IsTaskInputLoggingEnabled, etc.) don't use nullable types so there's no annotation burden — you can safely add #nullable enable at file level and remove the #nullable restore.
| /// out-of-proc nodes where the data was not serialized (where the property returns <see langword="null"/>). | ||
| /// </para> | ||
| /// </remarks> | ||
| public virtual IReadOnlyList<ProjectImportEdge>? ImportEdges => null; |
There was a problem hiding this comment.
🟠 Missing Version3 constant — API contract violation
The EngineServices class contract (lines 32–35) explicitly states:
Must be incremented whenever new members are added. Derived classes should override the property to return the version actually being implemented.
Adding ImportEdges as a new public virtual member requires:
- A new
public const int Version3 = 3;constant - Updating
public virtual int Version => Version3; - Updating
TaskHost.EngineServicesImplto overrideVersionreturningVersion3
This is important for external EngineServices implementations that may check the version to determine which members are safe to call.
|
After discussion, we're going a different route. Closing in favour of #13681. |
Tasks currently have no way to discover which files were imported during project evaluation. The data exists internally during evaluation but was never surfaced through any task-facing API.
This change adds an
EngineServices.ImportEdgesproperty that returns a structured list of import edges — each edge represents one<Import>element connecting an importing project file to the file it resolved to, optionally carrying the SDK name for SDK-resolved imports.A new
ProjectImportEdgereadonly struct inMicrosoft.Build.FrameworkcarriesImportedProjectPath,ImportingProjectPath, andSdkName. Each imported file appears at most once (first occurrence in depth-first evaluation order), so the collection forms a tree rather than a DAG.Usage
Tasks can use pattern matching — the
nulldefault naturally handles older engines and out-of-proc nodes where the data was not serialized:Opt-in serialization for out-of-proc nodes
On the in-process node the data is always available. On out-of-process worker nodes the data is only available if the project opts in, because serializing the import graph across nodes has a cost we don't want to impose on every build:
When this property is not set, serialization is skipped and
ImportEdgesreturnsnullon the worker node. This means zero additional IPC cost for builds that don't need the import graph.