dotnet / dotnet/msbuild

MSBuildFileSystemBase is partialy honoured when used to load imports

Open
#7,956 6 comments 2 reactions 0 assignees View on GitHub
Area: Evaluation Area: Performance bug triaged
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

### Issue Description

When creating a `Project` we can pass a custom file system implementation (`MSBuildFileSystemBase`) through the `EvaluationContext`

The file system is used internally, among other things, to check for file existence and load references.

When checking for file existence the custom file system is indeed used:

https://github.com/dotnet/msbuild/blob/a3b647d766676a735211a42aa0726d1d940ed13d/src/Build/Evaluation/Evaluator.cs#L2074

However, when loading the nested referenced project:

https://github.com/dotnet/msbuild/blob/a3b647d766676a735211a42aa0726d1d940ed13d/src/Build/Evaluation/Evaluator.cs#L2191-L2198

There's no use of the file system to load the project which will cause an exception as it will use the default file system.

Note that we use the custom filesystem to check if the file exists **but use a different file system** to load it!

### Expected Behavior

`Project` should use the `MSBuildFileSystemBase` provided to it in the `EvaluationContext` to access the file system.

### Actual Behavior

`Project` use the `MSBuildFileSystemBase` provided to it in the `EvaluationContext` to check if files exists but it will load the files using the default file systme.

### Analysis

It's clear that `ProjectRootElement` or any project element does not resolve imports so it does not care about the FS.

However, the evaluator should not load the project with a path, instead it should load using a provided `ProjectRootElement` directly. (I.E create the project from `ProjectRootElement` and not from the path)

> An alternative is to accept an handler to load it if it's not in the `projectRootElementCache`.

It is should be straight forward:

```cs
// fs: MSBuildFileSystemBase
var projectCollection = new ProjectCollection();

using var reader = new XmlTextReader(fs.GetFileStream(pathToFile, FileMode.Open, FileAccess.Read, FileShare.None));
var projectRootElement = ProjectRootElement.Create(reader, projectCollection);

// We must re-set the location so internal imports will follow the right path.
// The default in this cause is the main process directory (`PWD`)
projectRootElement.FullPath = pathToFile;

return Project.FromProjectRootElement(projectRootElement, new ProjectOptions
{
LoadSettings = ProjectLoadSettings.Default,
ProjectCollection = projectCollection,
EvaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared, fs),
});
```

### Versions & Configurations

Checked with 17.2 (net6). Code implies its the same form 16.9 (net5.0) and for `main` branch.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.