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 templates sourceName & port of template fixes from main #41536

Merged
merged 23 commits into from
May 16, 2022

Conversation

DamianEdwards
Copy link
Member

@DamianEdwards DamianEdwards commented May 5, 2022

This PR fixes two template issues that were introduced by the recent addition of the --use-program-main option, as well as another issue discovered during implementation of the fixes:

  1. With "Do not use top-level statements" checked, in Blazor WebAssembly App/ Blazor Server App/gRPC, the namespace is "Company.WebApplication1" rather than the solution name. #41529

    Description

    In the Blazor WebAssembly, Blazor Server, and gRPC project templates, if the --use-program-main option is specified, the created Program.cs file contains a namespace declaration that does not match the project name, but instead is always Company.WebApplication1.

    Customer Impact

    This doesn't result in any compiler warning or other build issues on its own, but the name is obviously wrong.

    Cause

    This is due to the namespaces not being updated for each specific project template when it was copy/pasted on authoring, as each project template uses a different placeholder namespace that is then replaced with the project name after creation.

    Discovered

    This issue was found internally during verification of 6.0.300. The fix has already been merged to main.

    Testing

    Automated tests have been updated to verify that the declared namespaces in the generated *.cs files start with the specified project name. The changes were also verified with manual testing.

    Regression

    This is not a regression as it's an issue in the implementation of a newly added feature.

  2. Create new Web API using dotnet new webapi with use-minimal-apis and use-program-main options set, you get a warning on build #41491

    Description

    In the Web API project template, if both the --use-program-main and --use-minimal-apis options are specified, the project ends up with two different entry points: one in Program.cs and another in Program.Main.cs.

    Customer Impact

    This results in a compiler warning when trying to build the created project:

    >dotnet build
    Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
    Copyright (C) Microsoft Corporation. All rights reserved.
    
      Determining projects to restore...
      All projects are up-to-date for restore.
    C:\Users\<user>\Solutions\Projects02\WebApi02\Program.Main.cs(5,24): warning CS7022: The entry point of the program is
    global code; ignoring 'Program.Main(string[])' entry point. [C:\Users\<user>\Solutions\Projects02\WebApi02\WebApi02.csp
    roj]
      WebApi02 -> C:\Users\<user>\Solutions\Projects02\WebApi02\bin\Debug\net6.0\WebApi02.dll
    
    Build succeeded.
    
    C:\Users\<user>\Solutions\Projects02\WebApi02\Program.Main.cs(5,24): warning CS7022: The entry point of the program is
    global code; ignoring 'Program.Main(string[])' entry point. [C:\Users\<user>\Solutions\Projects02\WebApi02\WebApi02.csp
    roj]
        1 Warning(s)
        0 Error(s)
    
    Time Elapsed 00:00:05.16
    

    Cause

    This is due to an authoring bug in the template config that doesn't correctly handle the case when both options are specified.

    Discovered

    This issue was found internally during verification of 6.0.300. The fix has already been merged to main.

    Testing

    Automated tests have been updated to verify that only the expected files are generated for all template option combinations, along with verifying that no compiler warnings are emitted when building newly created projects. The changes were also verified with manual testing.

    Regression

    This is not a regression as it's an issue in the implementation of a newly added feature.

  3. Web API template produces unbuildable project when --use-minimal-apis and any authentication (-au) option is supplied.

    Description

    In the Web API project template, if both the --use-minimal-apis and any cloud authentication (-au) option is supplied, the resultant project contains numerous code errors that result in build failures. This issue only manifests when using the templates from the command line (dotnet new) as Visual Studio uses its own mechanism for configuring authentication in new projects.

    Customer Impact

    New minimal API projects with cloud authentication options created from the command line cannot be built.

    Cause

    This is due to authoring mistakes in the template itself.

    Discovered

    These issues were discovered as part of reviewing the templates when implementing the fixes for the issues described above. Despite these mistakes being in the templates since the --use-minimal-apis option was added during 6.0.0, there were no customer reports. The fix has already been merged to main.

    Testing

    Automated tests have been updated to verify that the declared namespaces in the generated *.cs files start with the specified project name. The changes were also verified with manual testing.

    Regression

    This is not a regression as it's an issue in the implementation since the feature was added.

Addresses #41529 in 6.0.xxx
Fixes #41519

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 5, 2022
@ghost ghost added this to the 6.0.x milestone May 5, 2022
@ghost
Copy link

ghost commented May 5, 2022

Hi @DamianEdwards. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@DamianEdwards DamianEdwards changed the title Fix templates sourceName Fix templates sourceName & port of template fixes from main May 5, 2022
@DamianEdwards DamianEdwards marked this pull request as ready for review May 6, 2022 17:00
@DamianEdwards DamianEdwards requested review from a team, Tratcher and Pilchie as code owners May 6, 2022 17:01
@DamianEdwards DamianEdwards added the Servicing-consider Shiproom approval is required for the issue label May 6, 2022
@ghost
Copy link

ghost commented May 6, 2022

Hi @DamianEdwards. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@DamianEdwards DamianEdwards requested a review from HaoK May 6, 2022 18:20
@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels May 10, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.6 May 10, 2022
@DamianEdwards DamianEdwards requested a review from a team as a code owner May 10, 2022 21:07
- Updated test cases to cover more options combinations
- Fixed route pattern typo in minimal APIs case
- Added missing UseAuthorization call in minimal APIs + Windows Auth case
- Fail template tests if a compiler warning is output on build or publish
- Fix Web API template when call graph option is true
- Add arg constants & fix formatting

Fixes #41491
- Update template scripts to use correct package version when using locally
- Update template-baselines.json to cover new template options
Now that we fail if "warning" appears in the command output, we have to be sure to now issue and dotnet CLI commands that result in SDK warnings (in this case the change in net6.0 that -r must be accompanied with --self-contained or --no-self-contained).
@DamianEdwards DamianEdwards force-pushed the dedward/41529-template-namespace branch from 41c8d22 to e808b74 Compare May 12, 2022 18:11
@DamianEdwards
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@DamianEdwards
Copy link
Member Author

This has been approved by shiproom for 6.0.6 and is ready for review @HaoK @BrennanConroy @dougbu @wtgodbe

Comment on lines -218 to +220
var commonTestArgs = $"test {Options.Target} --diag:{diagLog} --logger:xunit --logger:\"console;verbosity=normal\" --blame \"CollectHangDump;TestTimeout=15m\"";
var commonTestArgs = $"test {Options.Target} --diag:{diagLog} --logger xunit --logger \"console;verbosity=normal\" " +
"--blame-crash --blame-hang-timeout 15m";
Copy link
Member

Choose a reason for hiding this comment

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

Does --blame-hang-timeout imply --blame-hang @nohwnd @MarcoRossignoli

Copy link
Member

Choose a reason for hiding this comment

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

Going through all my missed notifications. Yes it does. It enables --blame (creating the sequence xml file) and hang dumps with the default dump type (full), and non-default timeout (what you define for the --blame-hang-timeout). Same goes for ---blame-hang-dump-type, or --blame-crash-dump-type implying --blame-crash.

@mmitche
Copy link
Member

mmitche commented May 16, 2022

Reminder that code complete is today.

@dougbu dougbu merged commit 3418843 into release/6.0 May 16, 2022
@dougbu dougbu deleted the dedward/41529-template-namespace branch May 16, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants