Project-system: Add a way to associate inputs and outputs in FastUpToDate

[pranavkm]

Visual Studio Version: 16.4 Preview 2.0

Summary:

Linked ASP.NET Core issue: https://github.com/aspnet/AspNetCore/issues/13204

ASP.NET Core projects produce two compiled outputs: .cs files (et al) produce an App.dll, .cshtml files produce an App.Views.dll. When a cshtml file changes, App.dll remains unchanged, but a newer App.Views.dll needs to be compiled.

To allow a build to occur when cshtml files change, ASP.NET Core projects (via the Microsoft.NET.Sdk.Razor SDK), add cshtml files to UpToCheckInput. Fast UpToDateCheck is now aware of these files and rebuild when these files change. However doing so also leaves FastUpToDateCheck in an incosistent state - the input .cshtml file is always newer than the output MyApp.dll and consequently fast up to date check is broken until a user touches a cs file.

Feature request:
The feature request was to have a way to associate inputs and outputs in FUTD. By default, everything would be associated with the default UpToDateCheckBuilt. However, Inputs would have a way to state that they specifically affect certain outputs. When FUTD runs, it correlates these and uses this to perform the dirty check.

[drewnoakes]

This seems like a good idea. Let's design the feature.

We could add new optional metadata to the existing UpToDateCheckInput item. For example:

<UpToDateCheckInput Include="Home.cshtml" ProducesOutputFile="App.Views.dll" />

How this might behave:

1. The ProducesOutputFile metadata, when present, would mean the fast up-to-date check would only compare that input with the specified output.
2. If this metadata is omitted, the input is compared against all outputs.
3. If the metadata is the specified, but the referenced file does not exist, then the project is considered out of date.

Open questions:

1. Should ProducesOutputFile allow a semicolon separated list of paths?

[tmeschter]

@drewnoakes What if we generalized it a bit and instead of specifying the output file on the UpToDateCheckInput, we associated both UpToDateCheckInputs and UpToDateCheckOutputs with an "up-to-date set"? For example:

<UpToDateCheckInput Include="Home.cshtml" Set="Views" />
<UpToDateCheckOutput Include="App.Views.dll" Set="Views" />

This way we don't have to know the names of the output files when the input items are created, and we can easily associated multiple outputs with a group of inputs.

[tmeschter]

Also, can we think of any other scenarios where we might be able to use this functionality?

[pranavkm]

We could add new optional metadata to the existing UpToDateCheckInput item. For example:

How does this work with the items in UpToDateCheckBuilt? The RazorSDK would likely need to continue specifying a value for it to remain compatible with older versions of VS. @tmeschter's design sounds pretty neat in this regard, it's just an additional attribute specified as part of inputs and outputs.

Also, can we think of any other scenarios where we might be able to use this functionality?

/cc @nguerrera \ @rainersigwald in case you're aware of this.

[nguerrera]

Other scenarios: I am actually hitting this in SDK build. I had to add some extra up-to-date check inputs for things that we copy post-build. But then it's stuck out of date. I could tell the project system, that the outputs of these things are just where they get copied to, not the dll that might not need to be recompiled.

[nguerrera]

Generally, should check on anything that is content with copy to ouptut directory = preserve newest, now that I think of it.

[tmeschter]

@nguerrera As an aside, why do those files need to be copied as part of build? Would it make more sense to copy them as part of publish instead of build? (This is where I admit I don't really understand what publish is meant to do.)

[nguerrera]

There is often no publish. In my case and many others, the build is the only step.

[nguerrera]

I am genuinely curious what happens with:

<ItemGroup>
  <Content Include="Foo.txt" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

And then I change foo.txt without changing a cs file. It should build, the dll will be up to date, then copy the file. Then it should be fast up to date after the build. This would be a common scenario. That construct dates back ages and ages. If that works (the sdk case was much more convoluted, due to custom build steps so vanilla copied content might actually work already) then I would be curious how it works and if that should be generalized to align with this. If it does not work, it could be a long standing pain point that this feature could be used to address.

[nguerrera]

For the SDK, the content in question are the build targets that go with the msbuld tasks in the dll. With incorrect update handling, we had to touch a cs file for tests to see our targets changes. With my fix, it is considered not up to date from the time I touch a targets file to the time I touch a cs and build.

[drewnoakes]

@tmeschter I really like the idea of pooling inputs/outputs into sets. I started looking into the implementation between conference sessions yesterday and not only will it be easier to use when authoring projects, the implementation within the check itself should be easier to reason about.

@nguerrera I checked this:

And then I change foo.txt without changing a cs file. It should build, the dll will be up to date, then copy the file. Then it should be fast up to date after the build. This would be a common scenario. That construct dates back ages and ages. If that works (the sdk case was much more convoluted, due to custom build steps so vanilla copied content might actually work already) then I would be curious how it works and if that should be generalized to align with this.

When Foo.txt is copied to the output directory, the destination file ends up with the same timestamp as the source file, so thereafter the project is up to date as you would expect:

1>FastUpToDate: Checking PreserveNewest file 'ConsoleApp6\Foo.txt': (ConsoleApp6)
1>FastUpToDate:     Source 24/10/2019 6:57:31 AM: 'ConsoleApp6\Foo.txt'. (ConsoleApp6)
1>FastUpToDate:     Destination 24/10/2019 6:57:31 AM: 'ConsoleApp6\bin\Debug\netcoreapp3.1\Foo.txt'. (ConsoleApp6)
1>FastUpToDate: Project is up to date. (ConsoleApp6)

If you have other cases where this is not happening (perhaps the output is a transformation of the input), are you able to maintain the timestamp?

[drewnoakes]

Using sets will also make the up-to-date log output easier to read.

I will allow an item to belong to multiple sets (Set1;Set2).

We should decide what happens when a set has only inputs or only outputs. This seems like an invalid state. Can anyone think of a reason we might want this? If not, perhaps we consider such states as not up-to-date and log a warning message.

[nguerrera]

It looks like it's comparing the preservenewest files independently and not as regular up to date inputs to be compared to all up to date outputs. So these are special cased? Should they be handled as sets too with this design?

In the SDK case, we had our own target calling copy so the files were ignored from up to date. When I added them to up to date input, they get compared against the dll, not the destination of their copy. This would allow me to express that.

[rainersigwald]

The sets idea sounds good to me.

In MSBuild targets, Outputs must be specified if Inputs are, and having only-inputs FUTD sets seems like a similar error I can't think of a reason for. Outputs-only targets don't ever get skipped as up to date, so an output-only FUTD set would be equivalent to but worse than setting DisableFastUpToDateCheck=true. So I think I'm on board with a warning for either case.

[drewnoakes]

I've made good progress with this and will get a PR in tomorrow. Some notes.

@pranavkm

How does this work with the items in UpToDateCheckBuilt?

The following three item types may have a Set property:

• UpToDateCheckInput
• UpToDateCheckOutput
• UpToDateCheckBuilt

*.cshtml are currently represented as UpToDateCheckInput items, while the *.Views.dll file is currently represented as UpToDateCheckBuilt. Both will need Set metadata.

I have a fix for this issue working locally. I'm seeing correct behaviour for a test web application with local changes to the up-to-date check and the following in the test project:

<Target Name="U2D" BeforeTargets="Compile">
  <ItemGroup>
    <UpToDateCheckInput Update="**\*.cshtml" Set="Views" />
    <UpToDateCheckBuilt Update="**\*.Views.dll" Set="Views" />
  </ItemGroup>
</Target>

I assume you'll be able to set this Set metadata in the Microsoft.NET.Sdk.Web SDK.

Note that it's safe to add Set to items loaded by earlier versions of the fast up-to-date check as the property will just be ignored, so we don't need to be too precise about synchronising roll out here.

---

@nguerrera

It looks like it's comparing the preservenewest files independently and not as regular up to date inputs to be compared to all up to date outputs. So these are special cased? Should they be handled as sets too with this design?

Yes they're handled separately, and can apply to any item type. I'm currently only enabling sets on the three item types mentioned above. Do you have a use case for other item types?

In the SDK case, we had our own target calling copy so the files were ignored from up to date. When I added them to up to date input, they get compared against the dll, not the destination of their copy. This would allow me to express that.

You can represent custom copied files with:

<UpToDateCheckBuilt Include="Source\Path" Original="Destination\Path" />

The presence of the Original metadata means it's special cased and not compared against any other input. Only the source and destination are compared for such items. Would that work better for you?

[drewnoakes]

Some abridged logs to show things working.

Build after changing `Index.cshtml:

1>FastUpToDate: Comparing timestamps of inputs and outputs in set 'Views': (WebApplication14)
1>FastUpToDate: Adding UpToDateCheckBuilt outputs in set 'Views': (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\obj\Debug\netcoreapp3.0\WebApplication14.Views.dll' (WebApplication14)
1>FastUpToDate: Adding UpToDateCheckInput inputs in set 'Views': (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Index.cshtml' (WebApplication14)
1>FastUpToDate: Input 'C:\repos\WebApplication14\Pages\Index.cshtml' is newer (29/10/2019 11:00:17 PM) than earliest output 'C:\repos\WebApplication14\obj\Debug\netcoreapp3.0\WebApplication14.Views.dll' (29/10/2019 10:56:53 PM), not up to date. (WebApplication14)
1>FastUpToDate: Up to date check completed in 48.2 ms (WebApplication14)
1>------ Build started: Project: WebApplication14, Configuration: Debug Any CPU ------
1>WebApplication14 -> C:\repos\WebApplication14\bin\Debug\netcoreapp3.0\WebApplication14.dll
1>WebApplication14 -> C:\repos\WebApplication14\bin\Debug\netcoreapp3.0\WebApplication14.Views.dll
========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

Building again immediately:

1>FastUpToDate: Adding UpToDateCheckBuilt outputs in set 'Views': (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\obj\Debug\netcoreapp3.0\WebApplication14.Views.dll' (WebApplication14)
1>FastUpToDate: Adding UpToDateCheckInput inputs in set 'Views': (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Index.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Privacy.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\_ViewImports.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\_ViewStart.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Shared\_ValidationScriptsPartial.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Error.cshtml' (WebApplication14)
1>FastUpToDate:     'C:\repos\WebApplication14\Pages\Shared\_Layout.cshtml' (WebApplication14)
1>FastUpToDate: In set 'Views', no inputs are newer than earliest output 'C:\repos\WebApplication14\obj\Debug\netcoreapp3.0\WebApplication14.Views.dll' (29/10/2019 10:56:53 PM). (WebApplication14)
1>FastUpToDate: Project is up to date. (WebApplication14)
1>FastUpToDate: Up to date check completed in 127.7 ms (WebApplication14)
========== Build: 0 succeeded, 0 failed, 1 up-to-date, 0 skipped ==========

[nguerrera]

Oh, Original metadata looks like what I was after, thanks. I guess that means I don’t have a second use case for this feature.

[nguerrera]

@rainersigwald was Update in targets finally fixed or does @drewnoakes code above get lucky?

[rainersigwald]

It's getting lucky :( (microsoft/msbuild#2835 for context)

@drewnoakes use something like this instead:

<Target Name="U2D" BeforeTargets="Compile">
  <ItemGroup>
    <UpToDateCheckInput Condition=" '%(Extension)' == '.cshtml' " Set="Views" />
    <UpToDateCheckBuilt Condition="$([System.String]::Copy(%(Identity)).EndsWith('.Views.dll'))" Set="Views" />
  </ItemGroup>
</Target>

(I know the String.Copy thing there is a horror: microsoft/msbuild#1155)

[pranavkm]

I assume you'll be able to set this Set metadata in the Microsoft.NET.Sdk.Web SDK.

Yup, that should be doable. I'd be happy to try out a local build with your changes to verify the changes on our end end-to-end. But logs looks great.

Out of curiosity, what's UpToDateCheckOutput? I thought @tmeschter mis-remembered UpToDateCheckBuilt in the earlier comment, but it's clearly a thing. Should we be setting it in our SDK?

[drewnoakes]

@rainersigwald thanks for the fix. Thankfully the horror will be contained within a local test project.

@pranavkm once the PR is up you'll be able to F5 this repo to get the change into the Exp hive for further testing.

I asked @panopticoncentral about the difference between UpToDateCheckOutput and UpToDateCheckBuilt and the conclusion was that it's historical. As part of the PR I'm preparing I'll update our docs to improve our guidance on these features. I'll be sure to include that information too, and suspect we can effectively deprecate UpToDateCheckOutput.

[drewnoakes]

PR #5609 is now available with the fix for this issue..

I'd be happy to try out a local build with your changes to verify the changes on our end end-to-end.

@pranavkm you can pull down my branch to test against.