Skip to content

Embed untracked sources in all non-WPF projects#51353

Merged
RikkiGibson merged 2 commits intodotnet:masterfrom
RikkiGibson:embed-untracked
Feb 23, 2021
Merged

Embed untracked sources in all non-WPF projects#51353
RikkiGibson merged 2 commits intodotnet:masterfrom
RikkiGibson:embed-untracked

Conversation

@RikkiGibson
Copy link
Member

@RikkiGibson RikkiGibson commented Feb 20, 2021

Related to #51099
/cc @jaredpar @tmat

This is necessary for us to be able to eventually rebuild all non-WPF projects in Roslyn.

@RikkiGibson RikkiGibson requested review from a team as code owners February 20, 2021 01:00
@RikkiGibson RikkiGibson requested a review from a team February 20, 2021 01:00
@ghost ghost added the Area-Infrastructure label Feb 20, 2021
@tmat
Copy link
Member

tmat commented Feb 20, 2021

Can we set the value in a single place conditionally on UseWpf?

@RikkiGibson
Copy link
Member Author

RikkiGibson commented Feb 20, 2021 •
edited
Loading

I did attempt to use the following properties in Settings.props but found that it did not resolve the build error:

    <EmbedUntrackedSources Condition="$(UseWpf) == 'true'">false</EmbedUntrackedSources>
    <EmbedUntrackedSources Condition="$(EmbedUntrackedSources) == ''">true</EmbedUntrackedSources>

I got about 18 build errors similar to this:

C:\Users\rikki\src\roslyn\artifacts\obj\Microsoft.CodeAnalysis.EditorFeatures.Wpf\Debug\net472\InlineRename\Dashboard\Dashboard.g.cs(243,23): error CS1504: Source file 'InlineRename\ 
Dashboard\Dashboard.xaml' could not be opened -- Could not find file. [C:\Users\rikki\src\roslyn\src\EditorFeatures\Core.Wpf\Microsoft.CodeAnalysis.EditorFeatures.Wpf_ugmpovhr_wpftmp 
.csproj]

Because of this I just fell back to specifying the property in all the WPF projects.

@tmat
Copy link
Member

tmat commented Feb 21, 2021

Settings.props is imported before the project sets UseWpf. You'd need to set it in a .targets file.

@RikkiGibson
Copy link
Member Author

RikkiGibson commented Feb 22, 2021

d'oy, thanks for the msbuild refresher.

jaredpar
jaredpar reviewed Feb 23, 2021
View reviewed changes
eng/targets/Imports.targets
When markup compilation is involved, WPF generates .g.cs files via the GenerateTemporaryTargetAssembly task.
These do not always have the correct full-paths to their respective .xaml files annotated in them in the #line directives.
This can cause compile errors if these files are included in Source Link.
https://github.com/dotnet/wpf/issues/1718
Copy link
Member

@jaredpar jaredpar Feb 23, 2021

Choose a reason for hiding this comment

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

Believe this should be removed when we migrate to Arcade 6. How should we track removing this when that happens?

Copy link
Member Author

@RikkiGibson RikkiGibson Feb 23, 2021

Choose a reason for hiding this comment

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

I'm not sure why updating Arcade is related to this. It seems like a matter of fixing a WPF bug? Am I misunderstanding?

Copy link
Member

@jaredpar jaredpar Feb 23, 2021

Choose a reason for hiding this comment

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

It's because Arcade 6 has <EmbedUntrackedSources> set to true by default as well as has a work around for WPF Projects. Hence once we move to Arcade 6 this work around will be redundant with theirs

https://github.com/dotnet/arcade/blob/ca7fab569267ed3bc73360882d652d119aae5653/src/Microsoft.DotNet.Arcade.Sdk/tools/Workarounds.props#L28

Copy link
Member Author

@RikkiGibson RikkiGibson Feb 23, 2021

Choose a reason for hiding this comment

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

I'll just leave the linked issue open and update the description to remind us to remove this workaround.

@RikkiGibson RikkiGibson mentioned this pull request Feb 23, 2021
Closed
1 task
@RikkiGibson RikkiGibson merged commit a3753af into dotnet:master Feb 23, 2021
@ghost ghost added this to the Next milestone Feb 23, 2021
@RikkiGibson RikkiGibson deleted the embed-untracked branch February 23, 2021 05:54
@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaredpar jaredpar jaredpar approved these changes

@tmat tmat tmat approved these changes

Assignees

No one assigned

Labels

Area-Infrastructure

Projects

None yet

Milestone

16.10.P2

Development

Successfully merging this pull request may close these issues.

4 participants

@RikkiGibson @tmat @jaredpar @allisonchou