Fix toolset package failing to build from clean state in VS #78251
Conversation
ghost
added
Area-Infrastructure
untriaged
jjonescz
changed the title
| <Copy SourceFiles="@(_FileWithPath)" | ||
| DestinationFiles="$(BaseOutputPath)$(Configuration)\%(_FileWithPath.TargetPath)" | ||
| SkipUnchangedFiles="true" /> | ||
| SkipUnchangedFiles="true" | ||
| ContinueOnError="true" /> |
There was a problem hiding this comment.
Should we instead condition on Condition="'$(DesignTimeBuild)' != 'true'" - or does it actually need to run?
There was a problem hiding this comment.
Ah we don't want this running in design time builds either. That AfterTargets="Build" really needs to go since that's still going to cause issues here. I'm going to make another push to this PR.
There was a problem hiding this comment.
Condition="'$(DesignTimeBuild)' != 'true'" sounds like a good idea. I'm not sure why AfterTargets="Build" is bad (in this scenario), but feel free to improve.
There was a problem hiding this comment.
why AfterTargets="Build" is bad
It's easier to delete it than it is to convince ourselves it's correct in all situations. 😄 The problem is there's enough special types of builds that do strange things with project references that it's just not really safe to make the assumption. For example, you can also do a command line build passing a property to not build dependencies, and I'm not sure what that would do.
So yes, we could add a condition to remove DesignTimeBuilds, but if we just delete the AfterTargets="Build" we don't have to do anything in the first place. Less is more.
There was a problem hiding this comment.
There is one potential workflow change if you did like the ability to just do a build and see the files; that said we're adding I/O for copies for the 99.9% of the time people don't need that. If we wanted to refactor this a bit we could have an explicit target do the copy and you can do that.
There was a problem hiding this comment.
It's not just that I like the workflow, I've added it so it can be used for debugging, e.g.:
Line 103 in 489d016
That being said, there could probably be a property to opt in to it when needed.
We can depend on Build rather than be AfterTargets="Build" so that way we don't run when we don't expect to be.
|
I'm signing off, but since I did the last push maybe somebody else should sign off too. |
| @@ -61,7 +62,8 @@ | |||
| <TfmSpecificPackageFile Include="@(_FileWithPath)" PackagePath="%(_FileWithPath.TargetPath)" /> | |||
| </ItemGroup> | |||
|
|
|||
| <!-- Used for debugging parts of the toolset (e.g., build tasks). --> | |||
| <!-- Used for debugging parts of the toolset (e.g., build tasks). | |||
| Doesn't work in VS (where `<ProjectReference Targets="Publish" />` doesn't have any effect), hence we set `ContinueOnError="true"`. --> | |||
There was a problem hiding this comment.
D'oh I forgot to delete this comment.
| @@ -13,17 +13,18 @@ | |||
|
|
|||
| This package is primarily intended as a method for rapidly shipping hotfixes to customers. Using it as a long term solution for providing newer compilers on older MSBuild installations is explicitly not supported. That can and will break on a regular basis. | |||
|
|
|||
| The supported mechanism for providing new compilers in a build enviroment is updating to the newer .NET SDK or Visual Studio Build Tools SKU. | |||
| The supported mechanism for providing new compilers in a build environment is updating to the newer .NET SDK or Visual Studio Build Tools SKU. | |||
There was a problem hiding this comment.
Not that there was any doubt, but I did a git blame and yeah this was my fault.
Internal thread