Default to transitively copying content items

[benvillalobos]

Fixes #1054

Context

IncrementalClean has the behavior of deleting content items from transitive references on rebuild of the main project. With the move to 17.0, we want to default to copying all transitive content items into the main output folder to avoid this behavior.

Changes Made

Under change wave 17.0, default MSBuildCopyContentTransitively to true.

Testing

Tested locally on https://github.com/rainersigwald/IncrementalCleanDemo

Notes

[rainersigwald]

@drewnoakes, this has incremental-build/FUTD implications. We'd really like the copy behavior to be consistent between full builds and incremental builds (see the linked issue; it's currently a mess). Fixing it will require some additional project traversals, though, so we'll do some building of projects that were skipped by FUTD (not significant building; should just be output collection). The set of files copied to a project's outputs will then be consistent but not easy to determine via evaluation alone, because it'll include transitive content (it does today . . . if the transitive projects actually built and weren't skipped).

[drewnoakes]

@drewnoakes, this has incremental-build/FUTD implications

I'm unclear on what this means. Some possible interpretations:

• This will have a performance impact, because if A depends on B and B is up-to-date, the build for A will do more work in B, or
• The FUTD check needs to change in order to schedule more projects for MSBuild to build, or
• Potential future work for the Project System to know about the project's output items becomes more expensive as they no longer come from evaluation alone

I am curious how how much perf impact this change may have. I can add <MSBuildCopyContentTransitively>true</MSBuildCopyContentTransitively> to OrchardCore and run my test suite to get numbers. What kinds of scenarios would be worth running here?

[benvillalobos]

@drewnoakes Any scenario with many transitive project references should work.

Also need to add modify the changewave doc here.

[rainersigwald]

• This will have a performance impact, because if A depends on B and B is up-to-date, the build for A will do more work in B, or

Close--it's only relevant for transitive dependencies. Suppose A depends on B depends on C and B and C are up to date. Before, C would not need to be evaluated or built for the A build--B would be evaluated and only a couple of cheap targets there would be built. Now, we'll have to evaluate and build a couple of cheap targets in the full closure of dependencies.

Note that this doesn't actually mean much by default for .NET SDK projects, because the SDK flattens ProjectReferences so they're always direct.

• The FUTD check needs to change in order to schedule more projects for MSBuild to build, or

It does not, fortunately. We can handle it behind the scenes.

• Potential future work for the Project System to know about the project's output items becomes more expensive as they no longer come from evaluation alone

This is plausible, confounded by the case that it's already true today (we're just ensuring that the output will be consistent rather than only apply if C built).

I can add <MSBuildCopyContentTransitively>true</MSBuildCopyContentTransitively> to OrchardCore and run my test suite to get numbers. What kinds of scenarios would be worth running here?

That would be awesome. I think the "everything up to date except a change in a unit test project" scenario would be worst hit--before that should have optimized away the transitives but now it won't.

[benvillalobos]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[drewnoakes]

I ran the scenarios and don't see any noticeable difference across three runs with/without MSBuildCopyContentTransitively.