Failed to resolve relative file include.

The scenario:

MyClient.csproj includes ../../../msbuild/openapi.targets
openapi.targets adds ../../openapi/directory/Section.json to Content (or None, doesn't matter).
The changes in Section.json is the trigger for the MyClient.csproj to be affected.

The problem is that it is not a trigger, due to collector.PredictionsPerNode.Value-collection doesn't resolve my file, but leaves it relative. It simply doesn't match the normalized file textually.

My repo is strict net8 and and my local dotnet --version resolves to 8.0.400 (the definition is 8.0.100 with rollForward latestFeature).

Running the Debug "Affected"-command with updated location to my repo I can reproduce this and see the relative definition in Values. The file exists at that relative location. Running it in debug for me required rollForward latestFeature so it is not identifical to the repo. Starting the Visual Studio from activate.ps1-initialized shell did not help Visual Studio.

Adding a .Select(Path.GetFullPath) when resolving the file comparison with the normalized file I get the expected result. However I feel that the problem itself is that the file is not resolved within the graph-tool to the collector correctly.

I've tried to reproduce the scenario in a test, but failed to do so. Could it be due to the MSBuildLocator.RegisterDefaults() resolves to 8.0.100 in the tests and probably 8.0.400 when running it in debug in VS or as a command inside my repo?
I've noticed that the DotnetAffected.Core.Tests.ProjectGraphExtensionsTests.FindNodesThatDependOn_ShouldFindDependenciesForAllProjects is very flaky. It often fails in net8 but I've seen it under net6 also.

Running tests I also see that the collector has each project added twice, one referencing the other and for instance Global property TargetFrameworks is only initialized in one of them.
When debugging my repo, each project is present once with at a glance sane graph, resolving dependencies back and forth.

Debugging tests within Visual Studio under net8 sometimes gives me errors such

One or more errors occurred. (Could not load file or assembly 'Microsoft.Build.Utilities.Core, Version=15.1.0.0, Culture=neutral,

but not always.

Any pointers on where to continue to look or better yet a fix for the problem I'm seeing here?

Hi @marcusber, thanks for reaching out.

The relative reference of *proj files should work OK, at least we are using it on production with relative references, albeit to other csproj/.props mostly, but does work OK.
However, we usually run dotnet-affected on linux (mostly in CI). This may be a windows only problem, which is why we have not detected it before.

The problem is that it is not a trigger, due to collector.PredictionsPerNode.Value-collection doesn't resolve my file, but leaves it relative. It simply doesn't match the normalized file textually.

Adding a .Select(Path.GetFullPath) when resolving the file comparison with the normalized file I get the expected result.

Adding the .Select(Path.GetfullPath) to the below expression fixes the issue?

dotnet-affected/src/DotnetAffected.Core/Prediction/PredictionChangedProjectsProvider.cs

Lines 77 to 79 in 6a83d2f

    
           // determine nodes depending on the changed file 
        
           var nodesWithFiles = collector.PredictionsPerNode 
        
               .Where(x => x.Value.Contains(file));

Apparently we are trying to compare the rooted file names below, interestingly enough, there's a comment about windows paths, which brings me to my suspicion.

dotnet-affected/src/DotnetAffected.Core/Prediction/PredictionChangedProjectsProvider.cs

Lines 70 to 73 in 6a83d2f

    
           // normalize paths so that they match on windows. 
        
           var normalizedFiles = files 
        
               .Where(f => !_fileExclusions.Any(f.EndsWith)) 
        
               .Select(Path.GetFullPath);

When you debug on VS, how does the normalizedFiles and the collector.PredictionsPerNode look like? are they both rooted?

However I feel that the problem itself is that the file is not resolved within the graph-tool to the collector correctly.

Sorry i don't follow you on this statement, would you kindly clarify?

I've tried to reproduce the scenario in a test, but failed to do so. Could it be due to the MSBuildLocator.RegisterDefaults() resolves to 8.0.100 in the tests and probably 8.0.400 when running it in debug in VS or as a command inside my repo?

Please share this test with me so I can take a better look 👍

I've noticed that the DotnetAffected.Core.Tests.ProjectGraphExtensionsTests.FindNodesThatDependOn_ShouldFindDependenciesForAllProjects is very flaky. It often fails in net8 but I've seen it under net6 also.

Kindly share the error you are receiving. Perhaps it is something that broke when we updated to net8. I've not seen the issue myself.

The activate.ps1 command uses dotnet-install to get the proper .net versions for this project so you don't need to install them on your workstation. If you already have them on your workstation, then the script is not required.

Thanks @marcusber. My time is currently a bit limited, but I am happy to help get to the bottom of this 👍

Thanks @leonardochaia for taking time to answer.

This usecase is where a local tool is used to first update the open-api-files via swagger and dump them in OpenApi/subdir/file.json (using affected to find out which projects that needs re-generation). Then the next command figures out which api-clients that needs to be regenerated, also using affected. The problem here is that the Item in the api-project does not match the physical path as per below. The tools for this are usually executed on Windows. The usecase here is not executed in CI (which run on Linux, hence the slash references, since msbuild/dotnet handles the transformation between slash and backslash very well under windows but can't do the opposit in Linux). So it is usally a mixture between backslash and slash in the paths (when running on windows), which we never had any problems with.

Adding the .Select(Path.GetfullPath) to the below expression fixes the issue?

dotnet-affected/src/DotnetAffected.Core/Prediction/PredictionChangedProjectsProvider.cs

Lines 77 to 79 in 6a83d2f

// determine nodes depending on the changed file

var nodesWithFiles = collector.PredictionsPerNode

.Where(x => x.Value.Contains(file));

That is correct.
However I was initially under the impression that the Values-items should be normalized from the PredictInputsAndOutputs(..), hence my comment about "graph-tool". It would make sense if it was.
What I actually get is C:\s\cs\v\repo\backend\msbuild\../../OpenApi/subdir/file.json from the Value-collection for the project in PredictionsPerNode. That is how it is included from my common .target-file. I've checked the relative reference a couple of times if it would be the case that it doesn't find the file and therefor not resolving it. But I can't see that it is the case.

When checking on the AddInputFile in ProjectInputFilesCollector I also get C:\s\cs\v\repo\backend\msbuild\../../OpenApi/subdir/file.json. If a change should be made, it may be there?

The comment from normalizedFiles is valid. The changed file before it is resolved was in this case C:\s\cs\v\repo\OpenApi/subdir/file.json and is then, by applying Path.GetFullPath, resolved as a valid file path, as per the previous patch.

I've noticed that the DotnetAffected.Core.Tests.ProjectGraphExtensionsTests.FindNodesThatDependOn_ShouldFindDependenciesForAllProjects is very flaky. It often fails in net8 but I've seen it under net6 also.

Kindly share the error you are receiving. Perhaps it is something that broke when we updated to net8. I've not seen the issue myself.

I'll raise a seperate issue with that later on and hopefully an example which fails more often. We did a brief investigation and it seems like you could provoke the error very often. Well, at least under Windows.

Again, thanks for taking time for this.

Here's a repo reproducing the problem. I verified that the json-files has the same outline as above (absolute with a relative addition).

Oh, forgot to mention. To make things a bit more complicated (hopefully not) the repo i located on my disk in a case sensitive part of the disk.

fsutil.exe file queryCaseSensitiveInfo c:\s\cs
Case sensitive attribute on directory c:\s\cs is enabled.

Hi @marcusber , cool! I like the use case 👍

Thank you so much for the repo reproducing the issue. I can confirm I can reproduce the issue on my system following the steps provided.

The predictions for the sample repo outputs:

Only the paths to the json files are not "FullPath"s. The issue appears to be in the colector:

dotnet-affected/src/DotnetAffected.Core/Prediction/ProjectInputFilesCollector.cs

Lines 23 to 36 in 6a83d2f

    
           public void AddInputFile(string path, ProjectInstance projectInstance, string predictorName) 
        
           { 
        
               // Make the path absolute if needed. 
        
               if (!Path.IsPathRooted(path)) 
        
               { 
        
                   path = Path.GetFullPath(Path.Combine(projectInstance.Directory, path)); 
        
               } 
        
               else if (!path.StartsWith(_repositoryPath)) 
        
               { 
        
                   // ignore files outside the project's directory 
        
                   return; 
        
               } 
        
               this.AllFiles.Add(path);

As you can see there, the Path.GetFullPath is only called when the path is not rooted.
Predicted paths that are already rooted, do not get the Path.GetFullPath applied.

If I change there line 36 to ensure all added paths are rooted:

this.AllFiles.Add(Path.GetFullPath(path));

dotnet-affected starts to recognize the json as changed file.

ps: we should also change line 28 not to use GetFullPath so we don't call it twice on the same path.

This is pretty much the fix that you suggested initially, only placed in the collector instead of the IChangedProjectsProvider.

The tricky part is now to create a unit test for this case.

Do you want to create the PR yourself or you want me to take it from here? (I will have the time to sit on this mid next week)
Ideally we merge the PR with this fix and the test, so we ensure it does not get broken in the future, but if the test gets a bit tricky we can do a point release with the fix so you/your team can have it sooner.

thank you so much @marcusber for bringing this up, for the reproduction repo and for your investigation which was in the right spot.

@marcusber , I know I said I wouldn't but I thought I'd give it a try and I managed to test it. PR incoming.

@leonardochaia Thank you very much! You are more than welcome to make the suggested fixes. Thank you!

hi @marcusber , just to let you know that v4.0.1 has been released, which includes the fix we merged.

Thank you, do let me know if you find any other troubles, improvements, etc.
Leo.

	// determine nodes depending on the changed file
	var nodesWithFiles = collector.PredictionsPerNode
	.Where(x => x.Value.Contains(file));

	// normalize paths so that they match on windows.
	var normalizedFiles = files
	.Where(f => !_fileExclusions.Any(f.EndsWith))
	.Select(Path.GetFullPath);

	public void AddInputFile(string path, ProjectInstance projectInstance, string predictorName)
	{
	// Make the path absolute if needed.
	if (!Path.IsPathRooted(path))
	{
	path = Path.GetFullPath(Path.Combine(projectInstance.Directory, path));
	}
	else if (!path.StartsWith(_repositoryPath))
	{
	// ignore files outside the project's directory
	return;
	}

	this.AllFiles.Add(path);