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.

Sign up for GitHub

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

Jump to bottom

.NET Core WPF Build error on custom BaseIntermediateOutputPath #1718 #3120

Merged
merged 10 commits into from
Sep 17, 2020
Merged

.NET Core WPF Build error on custom BaseIntermediateOutputPath #1718 #3120

merged 10 commits into from
Sep 17, 2020
+389 −13

Conversation

ryalanms
Copy link
Member

@ryalanms ryalanms commented Jun 10, 2020 •
edited
Loading

Reviving the simplest of the proposed fixes for the BaseIntermediateOutputPath issue for comments. (.NET standard's Path.GetRelativePath is one of the alternatives.)

Building WPF applications using .NET Framework and .NET Core with various custom BaseIntermediateOutputPaths on different drives works as expected.

Note that PresentationBuildTasks.dll is built and consumed by both .NET Framework and .NET Core.

@ryalanms ryalanms requested a review from a team as a code owner June 10, 2020 23:25
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 10, 2020
@ryalanms ryalanms requested a review from rladuca June 10, 2020 23:27
vatsan-madhavan
vatsan-madhavan reviewed Jun 11, 2020
View reviewed changes
Copy link
Member

@vatsan-madhavan vatsan-madhavan left a comment

Choose a reason for hiding this comment

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

I did a quick scan and look good to me. I'll look over the changes again more carefully in a day or so.

...Microsoft.DotNet.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/MarkupCompiler.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
...Net.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/NETFrameworkGetRelativePath.cs Outdated Show resolved Hide resolved
@ryalanms
Copy link
Member Author

ryalanms commented Jul 2, 2020

@vatsan-madhavan and @lindexi: Any thoughts on PresentationBuildTasks.dll taking a dependency on .NET Standard instead and avoiding this new code?

@vatsan-madhavan
Copy link
Member

Any thoughts on PresentationBuildTasks.dll taking a dependency on .NET Standard instead and avoiding this new code?

I would be wary of doing this. This is an extensively used MSBuild task that must work robustly, and needs to load in VS, msbuild.exe and dotnet.exe reliably. The current way it builds (two outputs, net472, netcoreapp2.1) was carefully selected and tested, and the corresponding dependencies bundled with the SDK after much trial and error.

The change you are proposing can be done, but perhaps not as a reflexive workaround for an otherwise tractable problem in a PR. This will be a big architectural change and must be done carefully, with extensive testing and enough time (in previews) for reacting to errors (which you don't really have in .NET 5 at this point).

If/when you make the decision to enable cross-compilation on MacOS/Ubuntu etc., you'll have the opportunity to make this kind of change - at which time you'll also have the time to do it carefully keeping in mind packaging dependencies that need to be updated, additional testing, allocating time for ecosystem feedback etc.

@ryalanms ryalanms added this to the 5.0.0 milestone Aug 24, 2020
@ryalanms ryalanms merged commit 922458c into dotnet:master Sep 17, 2020
ryalanms added a commit that referenced this pull request Sep 18, 2020
@ryalanms
.NET Core WPF Build error on custom BaseIntermediateOutputPath #1718 (#…
8733ef2 
…3120)

* Use more robust relative path calculation in markup code generation
@ryalanms ryalanms mentioned this pull request Sep 18, 2020
Merged
@ryalanms ryalanms mentioned this pull request Oct 12, 2020
Merged
dotnet-maestro bot added a commit that referenced this pull request Oct 16, 2020
@dotnet-maestro
[master] Update dependencies from dotnet/winforms (#3647)
bc968b3 
[master] Update dependencies from dotnet/winforms
- Coherency Updates:
  - Microsoft.Win32.Registry: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.CodeDom: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Configuration.ConfigurationManager: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Diagnostics.EventLog: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.DirectoryServices: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Drawing.Common: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Reflection.MetadataLoadContext: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.AccessControl: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Cryptography.Xml: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Permissions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Principal.Windows: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Windows.Extensions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.Platforms: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.IO.Packaging: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.ILDAsm: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.ILAsm: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Resources.Extensions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Internal: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Ref: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)

 - .NET Core WPF Build error on custom BaseIntermediateOutputPath #1718 (#3120)

* Use more robust relative path calculation in markup code generation

 - Do not reflow lines after measure

 - disconnect HostVisual on its own thread

 - Fix three scrolling hangs

 - remove useless Assert

 - handle re-entrant request to close ToolTip

 - Move StackTrace to error branch

 - FixedPage SOM bugs

 - Don't add null item peers

 - Use correct lock object for predefined packages

 - repair bad copy/paste

 - fix build break

 - remove AppContext switch

 - DataGrid.Copy: fail silently if clipboard is locked

 - update_intellisense_artifacts

 - Remove or replace Policheck violations in code comments (#3606)

 - Merge pull request #3542 from dotnet/rc2-askmode-BaseIntermediateOutputPath

Ask-Mode: [release/5.0-rc2] Custom intermediate output paths shouldn't break markup compilation

 - Merge pull request #3564 from SamBent/VSPFreeze

Vsp freeze

 - Merge pull request #3565 from SamBent/AutomationPeerNull

Avoid null item AutomationPeers

 - Merge pull request #3566 from SamBent/TextReflow

Avoid reflow of shaped text

 - Merge pull request #3567 from SamBent/HostVisualThreading

HostVisual threading

 - Merge pull request #3568 from SamBent/PopupReentrancy

Reentrancy when closing ToolTip

 - Merge pull request #3570 from SamBent/ClearTypeAntiAliasing

ClearType anti-aliasing

 - Merge pull request #3574 from SamBent/NoisyStackTrace

Move StackTrace to error branch

 - Merge pull request #3575 from SamBent/FixedPageOverlap

FixedPage SOM bugs

 - Merge pull request #3576 from SamBent/LockedClipboard

DataGrid.Copy: fail silently if clipboard is locked

 - Merge pull request #3577 from SamBent/PredefinedPackageSynch

App resource threading issue

 - Revert "Merge pull request #3542 from dotnet/rc2-askmode-BaseIntermediateOutputPath"

This reverts commit 3d4e91c, reversing
changes made to ab840c4.

 - Merge branch 'release/5.0' of https://github.com/dotnet/wpf into release/5.0

 - update intellisense version

 - missed a version

 - 1228498 [ wpf ][ PoliCheck ] - Defect : Term "nuked"

 - Merge pull request #3637 from dotnet/revertPath

[release/5.0] Revert path change to markup compiler

 - Merge pull request #3642 from dotnet/policheck2

1228498 [ wpf ][ PoliCheck ] - Defect : Term "nuked"

 - Update arcade

 - Merge remote-tracking branch 'upstream/release/5.0' into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Update wpf-int dependencies

 - Replace null comparisons on non-nullable types that now cause compilation errors, due to the .NET SDK update.

 - PR feedback

 - PR feedback

 - Merge pull request #3640 from AdamYoblick/release/5.0

Update intellisense version

 - Merge remote-tracking branch 'upstream/release/5.0' into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Merge branch 'darc-master-f9327a78-0358-4132-8625-923a71a6b121' of https://github.com/dotnet/wpf into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Suppress obsolete API warnings. These must be fixed for 6.0.0.

 - Merge pull request #3659 from dotnet/fixdarcupdate

Temporarily suppress obsolete API errors to get WPF master building

 - Merge branch 'master' into darc-master-f9327a78-0358-4132-8625-923a71a6b121
@hymccord hymccord mentioned this pull request Nov 2, 2020
Closed
@ryalanms ryalanms mentioned this pull request Nov 24, 2020
Merged
This was referenced Jan 14, 2021
Merged
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lindexi lindexi lindexi left review comments

@Nirmal4G Nirmal4G Nirmal4G left review comments

@fabiant3 fabiant3 fabiant3 approved these changes

@vatsan-madhavan vatsan-madhavan vatsan-madhavan approved these changes

@SamBent SamBent Awaiting requested review from SamBent

@rladuca rladuca Awaiting requested review from rladuca

Assignees
No one assigned
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@ryalanms @vatsan-madhavan @Nirmal4G @lindexi @fabiant3