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

Enable Source Link in design-time build #1144

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Enable Source Link in design-time build #1144

merged 5 commits into from
Nov 16, 2023

Conversation

tmat
Copy link
Member

@tmat tmat commented Nov 16, 2023

Source Link, which is now included in the SDK and enabled by default, is causing Hot Reload to block stepping. Depending on timing of design-time builds this intermittently prevents the customer from debugging their application. Hot Reload blocks stepping due to differences in generated AssemlyInfo.cs files caused by Source Link targets.

Git commit SHA that is now included in AssemblyInformationalVersion attribute by default was not set in design-time build. This caused the content of *.AssemlyInfo.cs to differ between regular build and design-time build. If design-time build run after the debugging has started Hot Reload registered this difference as a change to be applied. Changing assembly level attributes is currently a rude edit and thus the change application is blocked, which in turns blocks stepping.

Fixes dotnet/sdk#36666

Customer Impact

Debugging projects is broken.

A workaround is to set EnableSourceControlManagerQueries to true globally (for all projects in the repo), e.g. add Directory.Build.props to the source root with:

<Project>
  <PropertyGroup>
    <EnableSourceControlManagerQueries>true</EnableSourceControlManagerQueries>
  </PropertyGroup>
</Project>

Regression?

  • Yes
  • No

7.0

Risk

  • High
  • Medium
  • Low

The change sets property that enables the same code path that's already executed in regular build to run in design-time build.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

The change looks fine to me but has implications for partner teams that I'd want signoff from.

Copy link

@tmeschter tmeschter left a comment

Choose a reason for hiding this comment

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

In general the set of compiler inputs and their contents should be the same in design-time builds and real builds. When we need to turn something off for DTBs we need to think hard about how that is going to impact the IDE. My advice is don't disable things in the DTB until you know they are expensive.

@tmat
Copy link
Member Author

tmat commented Nov 16, 2023

In general the set of compiler inputs and their contents should be the same in design-time builds and real builds. When we need to turn something off for DTBs we need to think hard about how that is going to impact the IDE. My advice is don't disable things in the DTB until you know they are expensive.

Agreed, in general, but I think we can safely exclude command line switches that only affect emit and do not affect EnC.

@tmeschter
Copy link

Agreed, in general, but I think we can safely exclude command line switches that only affect emit and do not affect EnC.

Sounds good to me.

@tmat
Copy link
Member Author

tmat commented Jan 2, 2024

Fixed in 8.0.200-preview.23604.9

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

Successfully merging this pull request may close these issues.

5 participants