Reqnroll icon indicating copy to clipboard operation
Reqnroll copied to clipboard

Incremental build support (take 2)

Open gasparnagy opened this issue 1 month ago • 6 comments

🤔 What's changed?

This is a PR that merges the essence of #793, #794 and #908

  • This declares the inputs and outputs in the MSBuild target for code generation so that the incremental build optimization can determine which feature files have changed and only run the generator for those files.
  • Enables deletion of code-behind files on clean (and also on rebuild as that calls clean anyway)
  • Removes our own code-level write optimization that would not work together with the input/output stuff (considered to replace with a solution that keeps the existing file equality check but touches the file if the check passed, but regarding the measurements it does not improve the things, what we win on non-writing we lose on reading & comparing)
  • Disables VS fast up-to-date check settings, because it seems to work anyway
  • Removed a lot of obsolete stuff that was needed for VS2019
  • Added system tests that verify incremental build and that project becomes up-to-date

What is not fixed:

  • In Visual Studio if you change a feature file and build then if you build again without changing anything than VS will invoke msbuild (instead of quickly reporting it to be up-to-date). The invoked msbuild will report up-to-date, but it takes a few seconds. If you build the 3rd time, it will be reported up-to-date by VS itself. This is a complicated issue related to VS Fast-up-to-date-checking (FUTDC) and cannot be solved as long as we save the code behind files into the project folder. But the impact is low anyway.

The changes were tested on both VS2026 and VS2022. Some tests have been done with non-SDK projects as well.

⚡️ What's your motivation?

Real incremental build support, faster builds

Fixes #906

🏷️ What kind of change is this?

  • :zap: New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • [x] I've changed the behaviour of the code
  • [x] Added tests
  • [x] Users should know about my change
    • [x] I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

gasparnagy avatar Nov 26 '25 14:11 gasparnagy

@markk-df Please check this PR, especially the Fast-Up-To-Date-Check aspect. As the feature files are correctly set as input for the UpdateFeatureFilesInProject target, the FUTDC seems to work. Or at least on the projects where I tried, I haven't seen issues. Is there a special way how this could be better verified?

Update: I have found how to diagnose FUTDC and I see the problem now. I will fix it tomorrow.

image

gasparnagy avatar Nov 26 '25 17:11 gasparnagy

Here is the test. I am not an expert of Reqnroll/SpecFlow files, so I cannot give the exact recipe. But the essence is - make a change to the .feature file that would not affect the generated .feature.cs file. In this way you are bumping the timestamp of the .feature file, but the generator is NOT going to overwrite the respective .feature.cs file, because of the nature of your change to the .feature file. Thus .feature.cs file timestamp remains old.

Does FUTDC work for you in this case?

markk-df avatar Nov 26 '25 19:11 markk-df

Actually, I can see you removed the generator check for the same content. So you always overwrite the .feature.cs file, even when it is unchanged. Is this a good change? I mean, I do not know how often people make feature file changes that do not translate to anything in the respective CS file. But if that happens often, then I am not sure it is a good change.

markk-df avatar Nov 26 '25 19:11 markk-df

Your change to add Inputs/Outputs to the UpdateFeatureFilesInProject target is correct, but I do not think it affects the FUTDC. It definitely affects msbuild when it builds the code in that it can skip this target. BUT, if the msbuild is called when nothing has changed - that means FUTDC has failed, because the whole purpose of it is skip msbuild when it thinks the code is up to date.

markk-df avatar Nov 26 '25 19:11 markk-df

Actually, I can see you removed the generator check for the same content. So you always overwrite the .feature.cs file, even when it is unchanged. Is this a good change? I mean, I do not know how often people make feature file changes that do not translate to anything in the respective CS file. But if that happens often, then I am not sure it is a good change.

The removed generator check was a backup solution, because at that time we were not able to setup the proper MsBuild-based up-to-date checking and therefore the generator task has been always processing all files. This was anyway inefficient, but to make it a bit better at least we decided no to update the file on the disk if the file is the same. So the goal was not to optimize the case when a feature file change does not need to be reflected in the generated file, but to optimize the unchanged files. Most of the feature file changes anyway require a different content.

The new input/output based up-to-date checking will not only decide whether the feature files are globally up-to-date, but it calculates the up-to-date status for every feature file individually, so our task will only receive those files that have been changed. With that the generator check is not really necessary anyway, but keeping them in would cause even more trouble if the feature file is modified in that special way that it does not trigger a change.

So in summary, we loose a little bit by not rewriting the files with the same content, but we win a lot by not even parsing and processing the unchanged files.

So I think this change will be fine.

Your change to add Inputs/Outputs to the UpdateFeatureFilesInProject target is correct, but I do not think it affects the FUTDC. It definitely affects msbuild when it builds the code in that it can skip this target. BUT, if the msbuild is called when nothing has changed - that means FUTDC has failed, because the whole purpose of it is skip msbuild when it thinks the code is up to date.

I have now spent 3+ hours on understanding FUTDC and trying out different combinations, and found the following:

There are two potential issues related to FUTDC:

  • Problem 1: If you ONLY change a feature file in VS then the build is not triggered (this is severe)
  • Problem 2: If you had feature file modifications and made a build then the subsequent build is not reported to be up-to-date (because the generated feature.cs files are newer then the last build start time). In this case the build will be triggered, but it will not do anything, because the MsBuild up-to-date check will catch it. So this second one is not that critical.

My findings:

  1. With the code of this PR this problem is not present. The reason is that the package registers the feature files using <AvailableItemName Include="ReqnrollFeatureFiles"/> as an "available item" and "available items" are considered as input by default for FUTDC. So in this case there is no need to use neither UpToDateCheckInput nor UpToDateCheckBuilt. There is a sample repo from a project system engineer that demonstrates this: https://github.com/drewnoakes/generate-code-sample

  2. What worries me a bit, that this registration is already there since v2.1.0, so I wonder how you observed the wrong behavior that you reported in #906. So that needs a more proper investigation, or maybe re-check once this PR is out.

  3. As far as I understand, the UpToDateCheckBuilt has to be used when you generate something that itself is a separate output and not something that will be used as a further input for the final .dll output. So I don't think that that would help and I don't understand how it helped in your case. Again, this is a bit worrying, we need to re-check.

  4. The "Problem 2" still exists and I could not find a way to overcome that. The mentioned sample repo suffers from the same issue, and I have posted a question about it. Once we get an answer we can react.

The best would be if you could have a reproducible steps that demonstrate the FUTDC issues you have seen, so that I could check if they appear with this PR as well. If this is not possible (there is no clear reproducible steps), then my suggestion is to merge this PR and see how it works in real. "On my machine" it seems to work at least... 😎

gasparnagy avatar Nov 27 '25 10:11 gasparnagy

Finally I got a solution for "Problem 2" as well, but it will only work once we save the generated files to the obj folder... https://github.com/dotnet/project-system/issues/9477#issuecomment-3585655742

gasparnagy avatar Nov 27 '25 12:11 gasparnagy