-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Include localPath in deps.json for runtime assets #50120
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.
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
base: main
Are you sure you want to change the base?
Conversation
… on DestinationSubDirectory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for including localPath
properties in deps.json files for runtime assets, enabling dependencies to be placed in custom subdirectories while maintaining proper runtime resolution.
- Extends dependency context generation to include local paths based on
DestinationSubDirectory
metadata - Updates
RuntimeFile
andResourceAssembly
constructors to accept local path parameters - Adds comprehensive tests for both package dependencies and project references with custom subdirectories
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs |
Adds new test methods to validate localPath generation and runtime behavior with custom subdirectories |
src/Tasks/Microsoft.NET.Build.Tasks/ReferenceInfo.cs |
Extends ReferenceInfo class to track DestinationSubPath for references |
src/Tasks/Microsoft.NET.Build.Tasks/GenerateDepsFile.cs |
Updates deps file generation to include local path information from task items |
src/Tasks/Microsoft.NET.Build.Tasks/DependencyContextBuilder.cs |
Modifies dependency context building to pass local path to RuntimeFile and ResourceAssembly constructors |
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/simple.dependencies.deps.json |
Updates test fixture to include expected localPath properties in deps.json |
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenADependencyContextBuilder.cs |
Adds unit tests for local path inclusion in dependency context building |
@@ -85,7 +85,7 @@ public class GenerateDepsFile : TaskBase | |||
// CopyLocal subset ot of @(ReferencePath), @(ReferenceDependencyPath) | |||
// Used to filter out non-runtime assemblies from deps file. Only project and direct references in this | |||
// set will be written to deps file as runtime dependencies. | |||
public string[] UserRuntimeAssemblies { get; set; } | |||
public ITaskItem[] UserRuntimeAssemblies { get; set; } = []; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing UserRuntimeAssemblies from string[] to ITaskItem[] is a breaking change that could affect consumers of this task. Consider maintaining backward compatibility by providing an overload or conversion mechanism.
// Backward compatibility for string[] property | |
[Obsolete("Use UserRuntimeAssemblies as ITaskItem[] instead. This property is for backward compatibility.")] | |
public string[] UserRuntimeAssembliesAsString | |
{ | |
get => UserRuntimeAssemblies?.Select(item => item.ItemSpec).ToArray(); | |
set => UserRuntimeAssemblies = value?.Select(s => new TaskItem(s)).ToArray(); | |
} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really know what the best thing to do here is. For usage via MSBuild targets, I think the passed strings convert to ITaskItem[] ("value1;value2"
is two items with the itemspec set those values and no additional metadata).
Is external usage of this task via C# expected?
src/Tasks/Microsoft.NET.Build.Tasks/DependencyContextBuilder.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…pp.cs Co-authored-by: Copilot <[email protected]>
Include
localPath
property in deps.json for runtime assetsDestinationSubDirectory
metadata + file nameRuntimeFile
andResourceAssembly
constructors that take a local pathThe tests in
GivenThatWeWantToBuildANetCoreApp
validate the generated .deps.json and the current behaviour when running the produced output. They also show adding a target in the test project to put dependencies in a custom subdirectory - until the runtime change to actually use thelocalPath
property (dotnet/runtime#118297) gets in and over to the sdk repo, putting dependencies in custom subdirectories will fail at run time.Resolves #48406
cc @dotnet/appmodel @AaronRobinsonMSFT