Skip to content

[release/5.0] Add option to RazorPageGenerator for #line preprocessor directives (#27765) #29024

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

Merged
merged 1 commit into from
Jan 13, 2021

Conversation

dougbu
Copy link
Contributor

@dougbu dougbu commented Jan 4, 2021

  • cherry-picked from 1a241fc in 'master' branch

  • avoid new CS1504 errors because compiler can't find the ErrorPage.cshtml file

    • we override Arcade's $(MicrosoftCodeAnalysisCSharpVersion)
  • use new option in GeneratePage.ps1 script

    • script also did not know RazorSyntaxGenerator had moved to this repo
  • reflect latest RazorSyntaxGenerator processing

    • run GeneratePage.ps1
    • update README.md

nits:

  • quote all paths in GeneratePage.ps1 script
  • remove passive voice in README.md

…ssor directives (#27765)

- cherry-picked from 1a241fc in 'master' branch

- avoid new CS1504 errors because compiler can't find the ErrorPage.cshtml file
  - we override Arcade's `$(MicrosoftCodeAnalysisCSharpVersion)`
- use new option in GeneratePage.ps1 script
  - script also did not know `RazorSyntaxGenerator` had moved to this repo
- reflect latest `RazorSyntaxGenerator` processing
  - run GeneratePage.ps1
  - update README.md

nits:
- quote all paths in GeneratePage.ps1 script
- remove passive voice in README.md
@dougbu dougbu added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode labels Jan 4, 2021
@dougbu dougbu added this to the 5.0.3 milestone Jan 4, 2021
@dougbu dougbu requested a review from a team January 4, 2021 22:36
@dougbu dougbu requested a review from Tratcher as a code owner January 4, 2021 22:36
@dougbu
Copy link
Contributor Author

dougbu commented Jan 4, 2021

Suggesting tell-mode for 5.0.3. Sound right @Pilchie

@Pilchie
Copy link
Member

Pilchie commented Jan 4, 2021

Why tell mode? Can you elaborate on what your fixing, where it's used, etc? I'm not familiar with this bit of the repo.

@dougbu
Copy link
Contributor Author

dougbu commented Jan 4, 2021

Why tell mode?

Short answer

It's all build infrastructure. Well, not quite but…

More correct answer

The main changes are in a tool (RazorSyntaxGenerator) that does not ship and is only used manually. I am changing a single generated source file but only its contained #line pragmas and only to satisfy Roslyn.

Background

The change corrects build failures I've repeatedly seen locally but aren't yet happening on the CI.

The src/Shared/ErrorPage/ErrorPage.Designer.cs file is checked into the repo and compiled into a number of our projects. Newer versions of Roslyn validate the generated #line pragmas in this file and complain when the ErrorPage.cshtml file isn't found because it's in the src/Shared/ErrorPage/Views sub-directory. That is, builds fail because the #line pragmas are wrong.

We fixed this in 'master' but haven't done the same in 'release/5.0' yet. This will soon be necessary because the Roslyn compiler ships (in part) with VS and core-eng updates VS on the build agents.

The specific fix is to correct the tool used to generate the ErrorPage.Designer.cs file (RazorSyntaxGenerator) and then use it to fix the #line pragmas.

An alternate fix would be to move the generated file into the Views sub-directory and then change the references in consuming projects. I chose the current fix because it was somewhat more localized and gave me an opportunity to fix the "script also did not know RazorSyntaxGenerator had moved to this repo" bit.

@Pilchie
Copy link
Member

Pilchie commented Jan 4, 2021

This will soon be necessary because the Roslyn compiler ships (in part) with VS and core-eng updates VS on the build agents.

But we build these bits with dotnet build right? So they should be isolated from the VS version?

@dougbu
Copy link
Contributor Author

dougbu commented Jan 5, 2021

But we build these bits with dotnet build right? So they should be isolated from the VS version?

Yup but with emphasis on the word "should".

My deeper suspicion is this problem will primarily impact local builds of the 'release/5.0' branch after switching from 'master' (or branches based on either). That comes from dotnet build-server shutdown clearing the problem. Seems likely the problem is just that builds in 'release/5.0' end up using the newer Roslyn bits configured in 'master' because they're held in the VB/C# compiler server.

Note our build.ps1 and build.sh scripts disable node reuse. Unfortunately that affects dotnet msbuild processes, not the server process(es) used.

We enable the compiler server pretty much everywhere (in eng/Workarounds.props). That's identified as a workaround for dotnet/roslyn#27975 though it's probably more about the 5X performance degradation when not using the compiler server.

@Pilchie
Copy link
Member

Pilchie commented Jan 5, 2021

Got it, thanks for working through the details with me. I'm okay with Tell-Mode for this now.

@wtgodbe wtgodbe merged commit d546861 into release/5.0 Jan 13, 2021
@wtgodbe wtgodbe deleted the dougbu/back.port.error.page branch January 13, 2021 18:25
@dougbu
Copy link
Contributor Author

dougbu commented Jun 10, 2021

/backport to release/3.1

@github-actions
Copy link
Contributor

Started backporting to release/3.1: https://github.com/dotnet/aspnetcore/actions/runs/924024165

@github-actions
Copy link
Contributor

@dougbu backporting to release/3.1 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: [release/5.0] Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) - cherry-picked from 1a241fcfc2f8 in 'master' branch
Using index info to reconstruct a base tree...
A	src/Middleware/tools/RazorPageGenerator/Program.cs
M	src/Shared/ErrorPage/GeneratePage.ps1
M	src/Shared/ErrorPage/README.md
Falling back to patching base and 3-way merge...
Auto-merging src/Shared/ErrorPage/README.md
CONFLICT (content): Merge conflict in src/Shared/ErrorPage/README.md
Auto-merging src/Shared/ErrorPage/GeneratePage.ps1
CONFLICT (content): Merge conflict in src/Shared/ErrorPage/GeneratePage.ps1
CONFLICT (modify/delete): src/Middleware/tools/RazorPageGenerator/Program.cs deleted in HEAD and modified in [release/5.0] Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) - cherry-picked from 1a241fcfc2f8 in 'master' branch. Version [release/5.0] Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) - cherry-picked from 1a241fcfc2f8 in 'master' branch of src/Middleware/tools/RazorPageGenerator/Program.cs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 [release/5.0] Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) - cherry-picked from 1a241fcfc2f8 in 'master' branch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

dougbu added a commit that referenced this pull request Jun 10, 2021
- was "Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) (#29024)"
- cherry-picked from d546861 in 'release/5.0' branch
  - was cherry-picked from 1a241fc in 'main' branch

- avoid new CS1504 errors because compiler can't find the ErrorPage.cshtml file
  - problem seems specific to local builds of this branch
- no tooling changes because `RazorPageGenerator` is in aspnetcore-tooling for 3.1
- change amounts to running 'release/5.0' or 'main' version of tool in this branch
  - ErrorPage.cshtml files in three branches were identical at time of previous commits
    ('main' version of ErrorPage.cshtml recently for nullability)
dougbu added a commit that referenced this pull request Jun 10, 2021
- was "Add option to `RazorPageGenerator` for `#line` preprocessor directives (#27765) (#29024)"
- cherry-picked from d546861 in 'release/5.0' branch
  - was cherry-picked from 1a241fc in 'main' branch

- avoid new CS1504 errors because compiler can't find the ErrorPage.cshtml file
  - problem seems specific to local builds of this branch
- no tooling changes because `RazorPageGenerator` is in aspnetcore-tooling for 3.1
- change amounts to running 'release/5.0' or 'main' version of tool in this branch
  - ErrorPage.cshtml files in three branches were identical at time of previous commits
    ('main' version of ErrorPage.cshtml recently for nullability)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants