MSBuildFileSystemBase is partialy honoured when used to load imports

[shlomiassaf]

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:

msbuild/src/Build/Evaluation/Evaluator.cs

Line 2074 in a3b647d

fileMatcher: _evaluationContext.FileMatcher,

However, when loading the nested referenced project:

msbuild/src/Build/Evaluation/Evaluator.cs

Lines 2191 to 2198 in a3b647d

	importedProjectElement = ProjectRootElement.OpenProjectOrSolution(
	importFileUnescaped,
	new ReadOnlyConvertingDictionary<string, ProjectPropertyInstance, string>(
	_data.GlobalPropertiesDictionary,
	instance => ((IProperty)instance).EvaluatedValueEscaped),
	_data.ExplicitToolsVersion,
	_projectRootElementCache,
	explicitlyLoaded);

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:

               // 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.

[shlomiassaf]

As for use cases, we would like to implement a virtual file system that will load projects from a git commit so we can analyse the diff between 2 commits or between the working directory and any commit.

This is implemented but the missing bit is the inability to control loading of nested project within a csproj.

For example, we get to a point where the project wants to load Directory.Build.props from the csproj library.
In the working directory it's not there but in the commit it is.

The commit based file system will report Directory.Build.props is present but loading it will fail.

[benvillalobos]

This seems like a reasonable request. We would need to investigate performance implications here.

[shlomiassaf]

@benvillalobos Maybe one would need to opt-in for that, first phase, via parameter or something
So it will not get noticed or have an impact