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

disable api compat for design time build #32157

Closed
wants to merge 1 commit into from
Closed

disable api compat for design time build #32157

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<RuntimeGraph>$(LibrariesProjectRoot)\OSGroups.json</RuntimeGraph>
<BuildTargetFramework>netcoreapp5.0</BuildTargetFramework>
<AdditionalBuildTargetFrameworks Condition="'$(BuildingInsideVisualStudio)' == 'true'">$(AdditionalBuildTargetFrameworks);$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-OSX;$(NetCoreAppCurrent)-Linux;$(NetCoreAppCurrent)-NetBSD;$(NetCoreAppCurrent)-FreeBSD</AdditionalBuildTargetFrameworks>
<RunApiCompat Condition="'$(DesignTimeBuild)' == 'true'">false</RunApiCompat>
Copy link
Member

@eerhardt eerhardt Feb 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking a bit more about this. Why is ApiCompat hooked into $(TargetsTriggeredByCompilation)? That doesn't seem like the ideal place for it to be hooked into the build. I would have expected it to be an AfterTargets="Build" or AfterTargets="CoreBuild", something along those lines.

If we are going to keep it hooked into $(TargetsTriggeredByCompilation) and if we didn't want to depend on the $(DesignTimeBuild) property, it looks like a different property we could check is $(SkipCompilerExecution). This is the property that tells CoreCompile to not actually compile the C# code during these design-time builds. It is paired with $(ProvideCommandLineArgs), which tells the Csc task to just output the full csc.exe command line args, which VS uses. It seems reasonable to not run ApiCompat when $(SkipCompilerExecution) == true.

Copy link
Member

Choose a reason for hiding this comment

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

Looking at https://github.com/dotnet/project-system/blob/master/docs/design-time-builds.md#determining-whether-a-target-is-running-in-a-design-time-build, it looks like depending on $(DesignTimeBuild) is acceptable.

QUESTION: Should this change also go into https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.ApiCompat/build/Microsoft.DotNet.ApiCompat.targets#L21? Does it ever make sense to run ApiCompat during a DesignTimeBuild? Reading that project-system doc:

Targets that dynamically change references, source files or compilation options must run during design-time builds to avoid unexpected behavior in Visual Studio. In contrast, if a target does not contribute these items, then it should actively avoid running in these builds to ensure design-time builds are as fast as possible.

I don't think ApiCompat changes references, source files or compilation options, right? So it would seem best to default it to off during a DesignTimeBuild.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @eerhardt on that. Let's move the fix over to arcade and change the defaults in ApiCompat.targets to not run on design time builds. I would not merge this in to not add unnecessary complexity.

Copy link
Member

Choose a reason for hiding this comment

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

Here's my Build Logging window when ApiCompat is running during DesignTimeBuild:

image

And when I shut it off:

image

Notice that the time went from 2 seconds down to under 0.1 seconds for the netstandard2.0 design time build. If you had a few projects in a single solution, doing ApiCompat would be taking a lot of time.

</PropertyGroup>

<Import Project="$(RuntimePropsFile)" Condition="Exists('$(RuntimePropsFile)')"/>
Expand Down