Add EmbedAllSources build task parameter #23656
Conversation
| @@ -98,6 +98,7 @@ | |||
| DelaySign="$(DelaySign)" | |||
| DisabledWarnings="$(NoWarn)" | |||
| DocumentationFile="@(DocFileItem)" | |||
| EmbedAllSources="$(EmbedAllSources)" | |||
There was a problem hiding this comment.
Does this make sense doing it this way? For the MSBuild experience, I assume you'd just write:
<ItemGroup>
<EmbeddedFiles Include="@(Compile)" />
</ItemGroup>
or something.
There was a problem hiding this comment.
The compiler already has /embed command line option. That's much more efficient since we avoid copying all Compile items.
| if (EmbeddedFiles != null) | ||
| // If EmbedAllSources is specified we can ignore files explicitly specified in EmbeddedFiles. | ||
| // They do not contribute to the set of files included in the compilation. | ||
| // They only designate which files that are being compiled should be also embedded. |
There was a problem hiding this comment.
How do I say "embed all sources and this one extra file like a XAML file or something?" not possible?
There was a problem hiding this comment.
Actually, I was mistaken. Fixing.
|
@tmat Does this ultimately appear as a CompilationOption or EmitOption? Language service threading is coming in another PR if appropriate? |
|
@jasonmalinowski There is no option. The parser populates the embedded files list like so: http://source.roslyn.io/#Microsoft.CodeAnalysis.CSharp/CommandLine/CSharpCommandLineParser.cs,1241 |
|
test failure: #21559 |
|
@dotnet/roslyn-compiler Please review. |
|
@dotnet/roslyn-compiler Ping |
tmat
force-pushed
the
EmbedAllSources
branch
from
0245006 to
da1002b
Compare
| @@ -98,6 +98,7 @@ | |||
| DelaySign="$(DelaySign)" | |||
| DisabledWarnings="$(NoWarn)" | |||
| DocumentationFile="@(DocFileItem)" | |||
| EmbedAllSources="$(EmbedAllSources)" | |||
There was a problem hiding this comment.
I believe it should say EmbedAllSource or EmbedAllSourceFiles.
There was a problem hiding this comment.
Yeah, EmbedAllSourceFiles would be probably somewhat better but F# already uses EmbedAllSources and it's good to be consistent across the board.
There was a problem hiding this comment.
(F# could introduce an alternative property, but then that would just add complexity that's not worth it).
|
@Pilchie for approval |
|
@Pilchie ping. |
|
For compiler changes, please get approval from @MeiChin-Tsai |
|
@MeiChin-Tsai Could you please take a look and approve? |
|
approved. |
Customer scenario
Expose
/embedcommand line option thru csc and vbc build tasks.Introduce
EmbedAllSourcesproperty to enables customers to embed all source files without writing custom targets.Bugs this fixes
#19127
Workarounds, if any
The customer can add
to their targets file.
Risk
Small.
Performance impact
None.
Is this a regression from a previous update?
No.
Root cause analysis
How was the bug found?
Customer.
Test documentation updated?