Skip to content
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

Fix project load when intermediate output path isn't known #10198

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

davidwengier
Copy link
Contributor

Not sure whether me or @DustinCampbell broke this, but hit this assert during debugging today. What was happening was:

  • WorkspaceProjectStateChangeDetector tried to create a ProjectKey for a Project that had no compilation output assembly path
  • ProjectKey calls FilePathNormalizer.GetNormalizedDirectoryName, passes in null, and gets back /
  • ProjectKey then calls FilePathNormalizer.NormalizeDirectory on that /, which exploded

This fixes the first bullet point, by checking if Roslyn knows about the intermediate output path before continuing. Since this is only checking for project existence, we don't actually need to go further.
This fixes the third point by protecting against a single / being written for an input that is only a single /
This ignores the second point because it doesn't really matter, though it is slightly wasteful to get a normalized directory name, and then normalize it.

@davidwengier davidwengier requested a review from a team as a code owner April 2, 2024 06:00
Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure which of us broke this, but I'm glad you found it! 😁

@ryzngard
Copy link
Contributor

ryzngard commented Apr 2, 2024

Well, this needs to be merged soon because if David didn't find it RPS sure did.

image

@DustinCampbell
Copy link
Member

@davidwengier's on holiday. Should we merge this for him?

@ryzngard ryzngard merged commit 96ebc95 into dotnet:main Apr 2, 2024
12 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 2, 2024
@ryzngard
Copy link
Contributor

ryzngard commented Apr 2, 2024

Gladly. We're getting close to a week out of Razor insertions

@davidwengier davidwengier deleted the FixProjectLoad branch April 2, 2024 19:12
@davidwengier
Copy link
Contributor Author

Thanks folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants