Respect DebugSymbols=false Fixes #41364 #41384
Conversation
Otherwise we could end up passing contradictory parameters to the compiler.
ghost
added
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <DebugType Condition="'$(DebugSymbols)' == 'false'">None</DebugType> |
There was a problem hiding this comment.
I was undecided as to what I should do if DebugType already has a value. Open to suggestions.
There was a problem hiding this comment.
I think DebugSymbols should take precedence. Honestly its very opinionated but the philosophy of the property / the whole point it exists is to disable the pdbs, so if it doesn't work when the other property is set it feels not so useful.
Furthermore, DebugSymbols has been around longer than DebugType. DebugSymbols is at least 8 years old.
MicrosoftDocs/visualstudio-docs@381d67e
nagilson
left a comment
nagilson
left a comment
There was a problem hiding this comment.
The change looks good, but it would be nice to have a test to show this behavior and prevent regression.
This is also technically a breaking change, so we should be careful about that. Added the breaking-change doc label.
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <DebugType Condition="'$(DebugSymbols)' == 'false'">None</DebugType> |
There was a problem hiding this comment.
I think DebugSymbols should take precedence. Honestly its very opinionated but the philosophy of the property / the whole point it exists is to disable the pdbs, so if it doesn't work when the other property is set it feels not so useful.
Furthermore, DebugSymbols has been around longer than DebugType. DebugSymbols is at least 8 years old.
MicrosoftDocs/visualstudio-docs@381d67e
nagilson
added
breaking-change
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
It's a user-visible change in behavior, but I believe this is going back to how things were before .NET 8, so I believe it's a fix for a regression rather than a new breaking change. I didn't actually check that, though; I'm just going off of what the bug says. |
|
That's true, but it's certainly possible someone started relying on this behavior in .NET 8, setup an automated pipeline, and did not notice they had DebugSymbols=false. Then they would be pretty upset when they eventually find out that the pdb is no longer generated. I think this is the right change and it's good to have, but for almost any behavioral change in the SDK, someone is going to get broken by it 🥹 |
|
@dotnet-policy-service rerun |
What's that based on @Forgind? I don't think the behavior is that new. |
jkotas said the documentation didn't match .NET 8 behavior, which suggests that it changed a bit before .NET 8. To be clear, I wasn't saying it changed in .NET 8 or even .NET 7; I'm not really sure when 'before' .NET 8 it changed to this behavior. Because I didn't know, we ultimately decided to classify this as a standard breaking change and retarget to 9.0 accordingly. |
3 participants
Fixes #41364
DebugSymbols=false is a parameter that ultimately gets passed to the compiler, disabling pdb generation. There's also DebugType, which, when set to None, is supposed to disable generating pdbs. When DebugSymbols is set but not DebugType, the SDK sees DebugType as not set and sets it to portable, leading to pdb generation even though DebugSymbols was set to false. This is unexpected, so this makes DebugType automatically None when DebugSymbols was false.