Skip to content

Conversation

@dsplaisted
Copy link
Member

Fixes #1234

The GenerateBuildRuntimeConfigurationFiles runs before _CheckForCompileOutputs, so that the FileWrites items it adds will be picked up by _CleanGetCurrentAndPriorFileWrites, and incremental builds and cleans will work correctly. (See some discussion of this on #381)

However, the CoreBuild target invokes the _CleanRecordFileWrites target when it errors:

  <Target
      Name="CoreBuild"
      DependsOnTargets="$(CoreBuildDependsOn)">

    <OnError ExecuteTargets="_TimeStampAfterCompile;PostBuildEvent" Condition="'$(RunPostBuildEvent)'=='Always' or '$(RunPostBuildEvent)'=='OnOutputUpdated'"/>
    <OnError ExecuteTargets="_CleanRecordFileWrites"/>

  </Target>

This means that even if there is a previous build error, we are trying to generate the runtime config files. In the case of #1234, the output path where we would write them hasn't been created, so the task throws an exception.

This PR adds a condition to only run the task if the folder it's going to write to exists. This feels like a band-aid solution though. Is there a better way to structure this which would add the correct FileWrites items when needed, but not even run the target if GetReferenceAssemblyPaths or some other target before-hand has failed?

@rainersigwald @AndyGerlicher @nguerrera

@nguerrera
Copy link
Contributor

Definitely a band aid. I think we should run earlier by a different mechanism. From an email thread about a related problem, beforetargets=copyfilestooutputdirectory instead of after build should do the trick.

Copy link
Contributor

@nguerrera nguerrera left a comment

Choose a reason for hiding this comment

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

We need to find a proper fix

@dsplaisted dsplaisted force-pushed the 1234-runtime-config-on-failure branch from 5b39a23 to 8952139 Compare May 26, 2017 06:30
Copy link
Contributor

@nguerrera nguerrera left a comment

Choose a reason for hiding this comment

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

Looks good now. Please update the description to match the new change.

@dsplaisted dsplaisted changed the title Don't try to generate runtime config files if output directory doesn't exist Don't generate deps or runtimeconfig files when build has failed but _CleanRecordFileWrites is being run May 26, 2017
@dsplaisted
Copy link
Member Author

@dotnet-bot

test Ubuntu16.04 Release
test Windows_NT Release
test Windows_NT_FullFramework Release

@dsplaisted
Copy link
Member Author

@MattGertz for approval

Customer scenario

Build targeting a version of .NET Framework where the reference assemblies aren't installed. This fixes an issue where an extra (and far more verbose error) would be reported, making it less obvious what the problem was.

Bugs this fixes:

#1234

Workarounds, if any

Read the error spew carefully

Risk

Low

Performance impact

Low to none - This just changes where in the build we do some work.

Is this a regression from a previous update?

No

Root cause analysis:

Some of our targets were running even when the build failed, which caused them to fail (since the output path didn't exist).

How was the bug found?

Dogfooding

@dsplaisted
Copy link
Member Author

@dotnet-bot test Ubuntu16.04 Release

@MattGertz
Copy link

(Just for bar context -- note that I would probably have punted on this after next week.)

@dsplaisted dsplaisted merged commit f8f29fa into dotnet:release/2.0.0 May 26, 2017
mmitche pushed a commit to mmitche/sdk that referenced this pull request Jun 5, 2020
…206.12 (dotnet#1257)

- Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.20106.12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants