Handle unsupported paths in ProjectInSolution.AbsolutePath (#6238)

[jeffkl]

Customer Impact

Trying to build a solution with MSBuild.exe that contains a web site project causes MSBuild.exe to crash. This only affects the command line but includes dotnet build. It requires a relative path in the solution file and we've received 2 customer reports so far.

In those cases, we would return "C:\foo\http://localhost:8080" as an absolute path and then crash msbuild.

Testing

New unit test added. We will reach out to manual test team and compat test team to find out if website projects are included in their matrix.

Risk

Low, new code path has a try...catch and falls back to previous behavior.

Code Reviewers

@Forgind @cdmihai @benvillalobos

Description of fix

Previously a property in the MSBuild API would return a string containing an "absolute path" but did not expand path segments like ..\. We changed the code to call Path.GetFullPath() to expand these segments but did not realize that in some contexts, Visual Studio stores URLs in field that we were expecting to see a relative path. URLs contain characters that Path.GetFullPath() will throw a NotSupportedException for.

To fix this, we're not calling Path.GetFullPath() if the specified path is a URL and placed the call to Path.GetFullPath() inside a try...catch to fall back to previous behavior if that call fails

[jeffkl]

I think you need to do a merge commit on this? Otherwise we'll get a new commit? Where's @rainersigwald when you need him!? 😆

Maybe @cdmihai knows? We're trying to just move a commit from main to a servicing branch in a way that makes it easy to tell what commits are in what branch. So I cherry-picked the merged commit into a branch based on the servicing branch. Is that right?

[Forgind]

I think you need to do a merge commit on this? Otherwise we'll get a new commit? Where's @rainersigwald Rainer Sigwald FTE when you need him!? 😆

Yes, that is correct. Ideally, we would have merged into 16.9 first and let it flow automatically to master. This is messier, but we can have a merge commit into vs16.9 and let the merge commit flow to master or squash this, reset master to not have the commit, and take the squashed commit in master. The latter is cleanest, but the former is what I'd go for.

Also, since this is a servicing request, someone will have to fill out the servicing template. Would you mind doing that?

[jeffkl]

Also, since this is a servicing request, someone will have to fill out the servicing template. Would you mind doing that?

Sure thing, I just have no idea how to do that anymore, do you have any pointers?

[benvillalobos]

@jeffkl https://dev.azure.com/devdiv/DevDiv/_wiki/wikis/DevDiv.wiki/1969/Bug-Template-for-M2-and-QB-Modes?anchor=.net-framework-servicing-template

[Forgind]

That looks more in depth than is really necessary I'd just look at some of our prior servicing requests.

[jeffkl]

I've updated the PR comment and created an internal bug for ask mode just in case: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1293410/

[benvillalobos]

/cc: @marcpopMSFT

[marcpopMSFT]

@jeffkl A few questions. Does dotnet build work or dotnet msbuild build? Is the change needed in both the SDK and VS or just VS (noting that they are creating the builds this week for testing)? Will you be available to come to .net tactics on Thursday or did you want us to represent the fix?

[jeffkl]

@marcpopMSFT the code path is used by both dotnet build and msbuild.exe. However, its only causing issues for people with web site projects in their solution. The number is not very large, but its not zero either. I would prefer your team to represent the fix, I can try and be available to attend if needed so feel free to forward me the invite.

[marcpopMSFT]

Offline approval from the QB. I'm going to mark it as such and merge.