Fix AddDependency when dependent on other recipes
#3942
Conversation
The `AddDependency#onlyIfUsing` option currently only works if the source files already in the initial state (before any other recipes were applied) already contained references to what is specified in the `onlyIfUsing` option. Thus, if another recipe were to modify the code such that it adds a reference to code matching `onlyIfUsing`, this edit on its own would not lead to the dependency being added. The problem is that `AddDependency` only runs in one cycle and thus uses the scanning phase to decide if a dependency needs to be added. To properly describe the use case described above, it should however request a second cycle in some scenarios.
| @@ -116,6 +116,8 @@ public class AddDependency extends ScanningRecipe<AddDependency.Scanned> { | |||
| @Nullable | |||
| Boolean acceptTransitive; | |||
|
|
|||
| transient boolean requiresAnotherCycle = true; | |||
There was a problem hiding this comment.
In a first iteration I was planning on having this transient field (initially false) which the recipe in the edit phase sets to true if it detects any source files which match onlyIfUsing so that a second cycle gets run.
A more proper fix would be a dedicated API. One idea would be to add an ExecutionContext parameter to the existing Recipe#causesAnotherCycle() method, which would allow the recipe to store something in the ExecutionContext. Another idea would be to add a new API to RecipeRunCycle (can be obtained via ExecutionContext#getCycleDetails()) or directory on ExecutionContext. I am thinking something like a requestAnotherCycle(). But the existing Recipe#causesAnotherCycle() would likely have to remain for declarative recipes.
Depending on the approach, some additional changes may be required to the recipe scheduler, as it currently expects the recipe requesting another cycle to also have performed changes in the current cycle. This may not always be the case.
|
In the past the detail on |
|
@shanman190 I would have to look at the history of the recipe, but assuming it was a "scanning recipe" before 8.0, it would still have picked up on changes performed by earlier recipes, as the scanning recipes worked entirely different back then. But it would be worth it to verify this. |
|
Ahh, right right. Change type would put the type in use, then the |
The
AddDependency#onlyIfUsingoption currently only works if the source files already in the initial state (before any other recipes were applied) already contained references to what is specified in theonlyIfUsingoption. Thus, if another recipe were to modify the code such that it adds a reference to code matchingonlyIfUsing, this edit on its own would not lead to the dependency being added.The problem is that
AddDependencyonly runs in one cycle and thus uses the scanning phase to decide if a dependency needs to be added. To properly describe the use case described above, it should however request a second cycle in some scenarios.As for the implementation the idea would be for the recipe to in the edit phase continue to check for uses of the
onlyIfUsingregardless of whether it already found anything or not as soon as the build script for the respective project / source set has been processed. If a match is found it would indicate this to the recipe scheduler via a new API and could then also immediately stop scanning for more uses.