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 build when using Razor source generator #6535

Merged

Conversation

ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented May 5, 2022

Fixes #4421
Fixes #5697

Description

Fixes build when using Razor source generator by adding RootNamespace to the list of property added to the generated project.

Without this PR, the Razor source generator would generate files in the wrong namespace (WpfBlazorRepro_5s0cnboz_wpftmp instead of WpfBlazorRepro).

Fixes incremental build where the RootNamespace is changing on each build and therefore is recognized as a fresh build (Not incremental).

Customer Impact

Without this PR, customers cannot build a WPF application with the Razor source generator (Maybe others?).

Regression

No. To my knowledge, it never worked.

Testing

Tested by building the repro project in the issue (https://github.com/Eilon/WPFXamlRazorGeneratorBug). This PR fixed the build.

Tested the repro here: #5697 (comment). This PR makes the repro build incrementally.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@ThomasGoulet73 ThomasGoulet73 requested a review from a team as a code owner May 5, 2022 03:37
@ghost ghost assigned ThomasGoulet73 May 5, 2022
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label May 5, 2022
@ghost ghost requested review from dipeshmsft, singhashish-wpf and SamBent May 5, 2022 03:38
@ghost ghost added the Community Contribution A label for all community Contributions label May 5, 2022
@singhashish-wpf
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s).

@Nirmal4G
Copy link
Contributor

We really need to rethink the whole GenerateTemporaryTargetAssembly using a different solution like source generators. Instead of adding every single property over time. First was the project name, then file, then output paths and now namespace!

@ThomasGoulet73 ThomasGoulet73 force-pushed the fix-razor-source-generator-build branch from 64e8247 to 24aa88a Compare August 1, 2023 02:44
@ThomasGoulet73
Copy link
Contributor Author

I rebased my changes on main and validated that this PR fixes #5697. I will link my PR to this issue.

@ThomasGoulet73
Copy link
Contributor Author

@Nirmal4G I agree with you that this build process is not optimal. This PR is more like a patch instead of fixing the problem at its root which is that the build process is weird. Unfortunately, this is the best I can do without changing a bunch of things since the build process requires BAML compilation which requires a built assembly so that it can output resources to add to said assembly. For now this requires 2 build but I have changes locally that instead of triggering a whole build it just adds the resources to the assembly in the first build using Cecil but it is not PR-ready.

@rchauhan18 rchauhan18 merged commit 4a8ae45 into dotnet:main Aug 21, 2023
8 checks passed
@rchauhan18
Copy link
Contributor

@ThomasGoulet73, thanks for your contribution

@ThomasGoulet73 ThomasGoulet73 deleted the fix-razor-source-generator-build branch August 21, 2023 13:16
@ThomasGoulet73
Copy link
Contributor Author

Thank you @rchauhan18

@ghost ghost locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
5 participants