Skip to content

Rebuild support for OutputKind#52340

Merged
jaredpar merged 3 commits intodotnet:mainfrom
jaredpar:output
Apr 7, 2021
Merged

Rebuild support for OutputKind#52340
jaredpar merged 3 commits intodotnet:mainfrom
jaredpar:output

Conversation

@jaredpar
Copy link
Copy Markdown
Member

@jaredpar jaredpar commented Apr 1, 2021

The OutputKind value was not properly round tripping through the PDB. Instead
we were only distinguishing between roughly exe and dll outputs. The compiler
though knows about six different output kinds and that both impacts the manner
in which compilation takes place as well as what values are encoded into the
PDB (impacts the Subsystem value for example). Properly round tripping the
output kind here.

This builds off #52321 and will be kept in draft until that PR is complete.

@jaredpar jaredpar added Area-Compilers Feature - Rebuild Compiler ability to verify provenance of code via rebuild operations labels Apr 1, 2021
@jaredpar jaredpar force-pushed the output branch 2 times, most recently from df6b6ab to d47668f Compare April 1, 2021 20:37
@jaredpar jaredpar marked this pull request as ready for review April 6, 2021 13:04
@jaredpar jaredpar requested review from a team as code owners April 6, 2021 13:04
@jaredpar
Copy link
Copy Markdown
Member Author

jaredpar commented Apr 6, 2021

@dotnet/roslyn-compiler PTAL

The `OutputKind` value was not properly round tripping through the PDB. Instead
we were only distinguishing between roughly exe and dll outputs. The compiler
though knows about six different output kinds and that both impacts the manner
in which compilation takes place as well as what values are encoded into the
PDB (impacts the `Subsystem` value for example). Properly round tripping the
output kind here.
Comment thread src/Compilers/Core/RebuildTest/CompilationOptionsReaderTests.cs Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am a little nervous that we are going to find out about n-factorial eventually

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not gotta lie, I screwed this up once and was pleasantly surprised at how well test explorer scaled to my massive mistake

@jaredpar jaredpar enabled auto-merge (squash) April 7, 2021 00:57
@jaredpar jaredpar disabled auto-merge April 7, 2021 13:06
@jaredpar jaredpar merged commit 95bcec2 into dotnet:main Apr 7, 2021
@ghost ghost added this to the Next milestone Apr 7, 2021
@jaredpar jaredpar deleted the output branch April 7, 2021 13:06
public const string OptionInfer = "option-infer";
public const string OptionExplicit = "option-explicit";
public const string OptionCompareText = "option-compare-text";
public const string OutputKind = "output-kind";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

output [](start = 42, length = 6)

Can't the exact kind be inferred from the headers? It's not ideal when we duplicate information that's already there.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was unable to find a way to do this. If you can see a way I'm happy to flip to using that but there didn't seem to be the necessary info in the headers for all of the output kinds that we support.

Copy link
Copy Markdown
Member

@tmat tmat Apr 7, 2021

Choose a reason for hiding this comment

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

You're right, there is no way to distinguish winmdobj vs library. Everything else could be distinguished.

  • OutputKind.ConsoleApplication: Subsystem.WindowsCui
  • OutputKind.WindowsApplication: Subsystem.WindowsGui
  • OutputKind.WindowsRuntimeApplication: Subsystem.WindowsGui && DllCharacteristics.AppContainer
  • OutputKind.WindowsRuntimeMetadata: Characteristics.Dll && IsAssembly
  • OutputKind.DynamicallyLinkedLibrary: Characteristics.Dll && IsAssembly
  • OutputKind.NetModule: Characteristics.Dll && !IsAssembly

These two command lines result in the same output:

csc /target:winmdobj a.cs /out:1\a.dll /deterministic /subsystemversion:1.0
csc /target:library a.cs /out:2\a.dll /deterministic /subsystemversion:1.0

BTW: would be good to keep https://github.com/dotnet/roslyn/blob/main/docs/features/pdb-compilation-options.md#csharp-flags-that-can-be-derived-from-pdb-or-assembly up to date


In reply to: 608824123 [](ancestors = 608824123)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My bad. I forgot we had put so much detail into that doc. It's out of date for a lot of changes I made. I will send a PR to get this current.

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

Labels

Area-Compilers Feature - Rebuild Compiler ability to verify provenance of code via rebuild operations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants