Skip to content

Conversation

@dotnet-maestro
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Nov 19, 2025

Note

This is a codeflow update. It may contain both source code changes from
the source repo
as well as dependency updates. Learn more here.

This pull request brings the following source code changes

From https://github.com/dotnet/razor

Diff the source with this PR branch
darc vmr diff --name-only https://github.com/dotnet/razor:a8b3ccb884e497811fa31ef385f9486211bf88c5..https://github.com/dotnet/dotnet:darc-release/10.0.2xx-ca97e402-82f6-41cb-a6c6-4fecaf98a68b

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@DustinCampbell
Copy link
Member

DustinCampbell commented Nov 24, 2025

@davidwengier: It looks like dotnet/razor@ded0f0e is the cause of this build break.

@DustinCampbell
Copy link
Member

‼️ Once the latest changes flow from dotnet/razor, this PR will need the changes in these commits:

  • Update RazorSdk to account for changes to ITagHelperFeature (9a7e708)
  • Update RazorSdk after DefaultTagHelperDescriptorProvider removal (dc17a09)

@dotnet-policy-service dotnet-policy-service bot requested a review from a team November 25, 2025 17:01
@DustinCampbell
Copy link
Member

‼️ Once the latest changes flow from dotnet/razor, this PR will need the changes in these commits:

  • Update RazorSdk to account for changes to ITagHelperFeature (9a7e708)
  • Update RazorSdk after DefaultTagHelperDescriptorProvider removal (dc17a09)

This is done.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@davidwengier
Copy link
Member

Build is green. I have no idea why.

Stopping the Razor analyzer from building made the build start to fail with null annotation compiler errors, and VSTHRD002 errors in tests, which didn't affect us before, and don't affect us in main. Not sure if anyone from VMR can explain, but it's nothing I know how to deal with, that's for sure. Hopefully none of the million random changes I made cause any other issues 🤷‍♂️

</PropertyGroup>

<ItemGroup Condition="'$(DotNetBuildSourceOnly)' != 'true'">
<ItemGroup Condition="'$(DotNetBuildSourceOnly)' != 'true' and '$(DotNetBuildFromVMR)' != 'true'">
Copy link
Member

Choose a reason for hiding this comment

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

This looks questionable...why do the analyzers not need to be built in the VMR?

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="5.0.0-2.25461.22" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="5.0.0-2.25461.22" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="$(MicrosoftCodeAnalysisVersionForAnalyzerTests)" />
Copy link
Member

Choose a reason for hiding this comment

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

This also looks questionable. Why is the tests property being used for the analyzers project?

@mmitche
Copy link
Member

mmitche commented Dec 19, 2025

The analyzer can only reference a version of MS.CA.CSharp as high as the build compiler, either from the SDK or IDE. The VMR build here is using 10.0.101 and so cannot using anything higher than a 5.0.0 version of Roslyn. So I don't think we can use the version from Version.Details.props.

Why would you not be able to reference a version of M.CA higher than the build compiler? I definitely can see that restriction at runtime, but at build time it doesn't make much sense to me.

@davidwengier
Copy link
Member

davidwengier commented Dec 19, 2025

The analyzer in question is one for developers working in our repo, not one we deploy to the general public of Razor users, so "runtime" in this case is "a build of Razor.slnx", hence, this build. What I can't explain is why stopping that analyzer building caused the introduction of other analyzer and compiler warnings that weren't present two days ago. I only took the analyzer out of the build because it seemed like having the pre-built was less desirable, based on comments above.

@dotnet-maestro
Copy link
Contributor Author

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dkurepa
Copy link
Member

dkurepa commented Dec 22, 2025

the build is green, can this be merged?

@dotnet-maestro
Copy link
Contributor Author

dotnet-maestro bot commented Jan 2, 2026

Note

PRs from original repository included in this codeflow update:

💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance.

@dotnet-maestro
Copy link
Contributor Author

dotnet-maestro bot commented Jan 6, 2026

@premun
Copy link
Member

premun commented Jan 7, 2026

@mmitche @DustinCampbell this is green again but I don't understand if we want to merge

@DustinCampbell
Copy link
Member

@mmitche @DustinCampbell this is green again but I don't understand if we want to merge

@davidwengier?

@davidwengier
Copy link
Member

No issues from me, I would love to merge this as I think (hope!) it unblocks some other things we have going on. @mmitche had concerns though, and definitely understands how things should work in the VMR better than me. I just threw spaghetti at the wall until the build went green :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

8 participants