Make SWIX builds more dependable #25751
Conversation
|
CC @dotnet/roslyn-infrastructure for review. |
|
Note: I deliberately put this into dev15.7.x instead of master. There is a change I have to make for dev15.7 which requires a new version of the SWIX package. Hence trying to stave off the massive set of SWIX warnings that would result from that change. |
|
@jaredpar Seems this breaks the build? |
Worked on my machine ... |
|
@jaredpar Your machine doesn't count. ;-) |
This should work around the problem with SWIX builds where they fail if more than one version of the SWIX NuGet package is installed on the machine.
The Swix dependency is linked via a "BeforeBuild" target. The targets file for SWIX unconditionally overrides this target. Hence the definition must be after.
|
@jasonmalinowski think I fixed it. This time i validated it on my machine. |
| <Project> | ||
| <Import Project="..\..\..\build\Targets\Settings.props" /> | ||
| <PropertyGroup> | ||
| <SwixBuildPath>$(NuGetPackageRoot)\microbuild.plugins.swixbuild\$(MicroBuildPluginsSwixBuildVersion)\</SwixBuildPath> |
There was a problem hiding this comment.
This version is (at least by name) a different version than what it was previously. Expected?
There was a problem hiding this comment.
Which name here? Everything should be the same as before.
There was a problem hiding this comment.
I get it now. Your explanation cleared things up.
|
@jaredpar Care to update the pull request with a few sentence summary of the state of the world today? I can see things changing, but I am at a loss to understand how this helps anything. |
|
@jasonmalinowski sure The state of the world before this change is that we indirectly imported the SWIX props and targets files. The direct import was for MicroBuild.Core which then imported SWIX through it's plugin model. The problem with the MicroBuild.Core plugin model is:
This makes our build unnecessarily fragile. Our code only needs the SWIX plugin in order to build today. Hence this changes our build to skip the fragile plugin step and just import the SWIX props / targets. |
There was a problem hiding this comment.
I hear @jinujoseph was looking to get rid of the squirrel...
You're the second person who said that in response to this PR. That may be bad for me. :) |
This fixes a signing regression caused be dotnet#25751. That changed us to directly call the SWIX build props / targets vs. getting them implicitly from the microbuild props / targets. One of the behaviors that the microbuild props / targets had that wasn't accounted for was the signing of the VSIX after they are constructed. Hence this lead to signing errors on insertion. This PR fixes that by making the following changes: 1. Adds SWIX VSIX to SignToolData.json so they are accounted for during normal batch signing. 1. Moves the SWIX build to the pre-sign portion of our build. 1. Removes the references from VSMAN -> SWIX. These references are there only to enforce ordering which is unnecessary. Having them remain risks that building the VSMAN will cause the SWIX to be re-built which could possibly invalidate signing.
This fixes a signing regression caused be #25751. That changed us to directly call the SWIX build props / targets vs. getting them implicitly from the microbuild props / targets. One of the behaviors that the microbuild props / targets had that wasn't accounted for was the signing of the VSIX after they are constructed. Hence this lead to signing errors on insertion. This PR fixes that by making the following changes: 1. Adds SWIX VSIX to SignToolData.json so they are accounted for during normal batch signing. 1. Moves the SWIX build to the pre-sign portion of our build. 1. Removes the references from VSMAN -> SWIX. These references are there only to enforce ordering which is unnecessary. Having them remain risks that building the VSMAN will cause the SWIX to be re-built which could possibly invalidate signing.
This fixes a signing regression caused be #25751. That changed us to directly call the SWIX build props / targets vs. getting them implicitly from the microbuild props / targets. One of the behaviors that the microbuild props / targets had that wasn't accounted for was the signing of the VSIX after they are constructed. Hence this lead to signing errors on insertion. This PR fixes that by making the following changes: 1. Adds SWIX VSIX to SignToolData.json so they are accounted for during normal batch signing. 1. Moves the SWIX build to the pre-sign portion of our build. 1. Removes the references from VSMAN -> SWIX. These references are there only to enforce ordering which is unnecessary. Having them remain risks that building the VSMAN will cause the SWIX to be re-built which could possibly invalidate signing.
This should work around the problem with SWIX builds where they fail if
more than one version of the SWIX NuGet package is installed on the
machine.
Ask Mode template not completed
Customer scenario
What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)
Bugs this fixes
(either VSO or GitHub links)
Workarounds, if any
Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW
Risk
This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code
Performance impact
(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")
Is this a regression from a previous update?
Root cause analysis
How did we miss it? What tests are we adding to guard against it in the future?
How was the bug found?
(E.g. customer reported it vs. ad hoc testing)
Test documentation updated?
If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.