Repeating Gradle Task Names Between Subprojects

[ata-nas]

Topic

We have some subprojects that have repeating gradle task names, for example in :server and in :suites we have tasks named createDockerImage.

To Discuss

• when in repo root and we run ./gradlew createDockerImage by default this will run the task both for :server and :suites
• overhead
• need-to-know solution
• potential dangers
• would it be reasonable to sometimes have duplicated task names between subprojects
• having very clear and strict rules about the approach for this topic

Discussion

DISCLAIMER: the task createDockerImage is given as an example since it is an already existing duplication as of today's latest commit in main c2a360a. It might not be the best task for this discussion, because there might be a point where we would want to have repeating task names especially if we would want to execute a given task on multiple projects w/o any qualifiers and if we are sure that these tasks are self contained and would not cause any side effects or silent bugs. And for example in the smoke-test.yaml that describes a gh action if we use this task which only creates docker images like so ./gradlew createDockerImage this will create a docker image which we will not use, but there is no bug technically (this is why this is not the best example, but I still use it simply because it exists currently), just redundant operations, but sometimes in some tasks, if we do not know about the repeating task name, we could introduce problems if used without qualifiers for example like ./gradlew :server:createDockerImage. This being said, we of course must look at the point of having or not having duplicated gradle task names from both perspectives, do we want or not want to have such effects as listed in this disclaimer and below as well.

This is a discussion we initially had with @mattp-swirldslabs and @AlfredoG87:
Currently we have some subprojects that have repeating gradle task names, for example in :server and in :suites we have tasks named createDockerImage. This means that when we are in repo root and we run ./gradlew createDockerImage by default this will run the task both for :server and :suites. Called without any qualifiers, any project that has a task named createDockerImage (or any other task duplicated task name), that task will be executed. This means that we need to use :server:createDockerImage if we want to execute the task only for :server which is a need-to-know solution and is not a good thing, since if we see the build.gradle.kts of :server and we see the createDockerImage task, we might not be aware that the same exists for other subproject. This could mislead us and we could misuse the running of the given (any duplicated) gradle task. If for example we have a task that would run a project, lets call it runProject for example, and we reuse it in a couple of subprojects, running ./gradlew runProject from repo root would attempt to run two projects at once which might be something that should never happen (especially when we have gh actions jobs that are related to our ci/cd process), but we could easily miss that this is happening, since there could be no problems visible initially. And so would it be better to simply name the tasks serverRunProject and suitesRunProject a better solution than the need to know solution of duplicating the task name and relying on the need-to-know nature of it? Also, when do we actually want to have duplicated task names and if we have such, is it possible to extract and reuse them instead of defining these tasks per project (if applicable)?

As far as overhead, running things that we should not run due to task name duplication and not having knowledge about it, should theoretically be only once, since gradle is cached (please correct me if I am wrong)?

This being said, here are a couple of questions I think we should discuss:

• Is the need-to-know solution a valid concern, or is the reliance on thorough documentation solving the issue and it is a fault of the user if they misuse?
• Is potential overhead a concern or not?
• What dangers could arise from the misuse and human error due to the need-to-know nature of the issue?
• When do we need and want to have duplicated task names and when we must avoid them OR we must always not have any duplicates OR we must always have duplicates?
• What rules could we make and impose in general in order to know what to do when we need to create new gradle tasks?

[georgi-l95]

IMO this should be fixed in the following manner:

• Keep the task naming as it is for the server, because it's the main focus in the repo, and it's from where those docker images are build.
• Change the naming for the suites tasks to indicate that running this task will build docker image, specifically for the suites uses.

This is for the problem at hand.
For future gradle tasks, I think that naming should indicite related subproject, except if it's about the server, because this is the focus.

[jsync-swirlds]

The "canonical gradle" approach is to encourage duplicate names if a particular set of tasks should ever be run together or have the same purpose, and use execution rules (e.g. something like .onlyIf(...Project.getProject() != Project.getRootProject()...)) to restrict running tasks that should not be run across projects.
The current example createDockerImage is good, as there is legitimate reason (in CI for instance) to have the top level task run all of the subtasks.
Further, in Gradle, the "standard" tasks (like compileTestJava) are always repeated in every subproject where they might be relevant, and this is by design.

As far as need-to-know, gradle provides inspection commands (e.g. gradle projects) for exactly this purpose, and these commands are also repeated in every subproject.

We should absolutely be thoughtful about task naming, and if tasks should not be repeated between projects, then name them separately (although including the subproject name is not good, because the subproject name is already part of the task name), or apply a restrictive execution rules (if the most appropriate name really is the same) so the task can only be run with the correct subproject prefix.

[AlfredoG87]

Agreed, I also think the "repeated" name is valid and should stay for this particular case.

[ata-nas]

@georgi-l95 @jsync-swirlds @AlfredoG87 thanks for the discussions. Just to be clear, when we first discussed the topic with @mattp-swirldslabs and @AlfredoG87, we did reach to the conclusion (correct me if I am wrong) that the specific task createDockerContainer is fine and should remain like that, since the tasks aim to build the runtime environment, much like the task build we expect that when we run it from root, we will have everything built and ready (this is why my big disclaimer that this specific task is not the ideal candidate to make an example of for the discussion I wanted to create, since it does not 'exactly' fit the criteria for the concerning type tasks).

However I do need to point out, that currently both tasks createDockerImage depend on tasks (again repeating names) that are called updateDockerEnv. Now both these tasks do indeed execute the same shell script and touch on a common resource the .env file that gets built. Since the tasks run asynchronously, I could imagine that this would cause some undesired results, especially if the updateDockerEnv tasks touch the same resource but put different specific for their purpose values there (currently should not be the case since they are run with the same parameters, but that could very easily change ad be overlooked), so I would imagine that something like this would be possible to happen (feel free to correct me if I am wrong):

1. ./gradlew createDockerImage - we call from root and the items below start to execute both for server and suites since we have both tasks named createDockerImage
2. :server:updateDockerEnv - executes because of step 1 since :server:createDockerImage depends on the update task
3. :suites:updateDockerEnv - executes because of step 1 since :suites:createDockerImage depends on the update task
4. :server:createDockerImage - it now uses whatever :suites:updateDockerEnv task has put in there
5. :suites:createDockerImage - get executed last

It is precisely things like these (but not limited to these) that worry me, not only when building, but running apps.
So my discussion was mainly aimed at that aspect of it.

[jsync-swirlds]

If we have multiple tasks in the build, including subprojects, that truly modify the same file, then that's a bug in the task design.

That said, it might be worth checking carefully what folders the tasks are running in. Gradle often runs each subproject task in a different build subfolder for exactly this reason, otherwise a java compile would interfere with every other java compile.

Have you looked at the build subfolders of the two subprojects? Usually that's where Gradle will run the individual tasks, and the file to be modified by updateDockerEnv should be local to those folders.

[jsync-swirlds]

@ata-nas,
We should bring this up in the daily tomorrow and make a final team decision, then document here and close the discussion.

[ata-nas]

Noted.

[jsync-swirlds]

Did we get a final decision on this?

[ata-nas]

@jsync-swirlds from our last discussion, we pointed out the following points about the topic that I believe we agreed on:

• it is encouraged to reuse task names on tasks that we design to be able to run in parallel and make sense to execute them on all (or more than one) subproject from repo root
• if we want to execute a task with repeated name only in a single place, we should use the fully qualified name of the task, e.g. ./gradlew :server:createDockerImage
• the tasks that we design must never leave the build root of the respective project, otherwise it will be nondeterministic and error prone due to race conditions if we reuse the resources produced by a task
• we should go through our repo to identify misuses and file issues to fix them

I believe these are the general guidelines that we discussed to follow, @mattp-swirldslabs @georgi-l95 @AlfredoG87 can you guys let us know your final verdict so we can close this discussion?

Also, I am not sure if people did not click the upvote here (we agreed on 4+ upvotes to consider the discussion as accepted) or someone disagrees with the discussed above which leads to the lack of upvote, @jsync-swirlds maybe we should be more diligent with following up the discussions and encouraging others to upvote/downvote and to maybe ping so we know they did or to state their thoughts on the matter 'I agree'/'I do not agree' otherwise these discussions take a lot of time to resolve, but yet again, they are not really gh issues at this stage yet.

But for this topic, which I believe touches on important guidelines, we should strongly consider to enforce it (even with 3/4 upvotes) since it could lead to wrongfully built and distributed packages in the end, for example as we touch in the same file both for a subproject aimed at production and one aimed more as a dev/testing environment (we are going outside the build root for the respective project), we could package dev/testing environment into production one, this is quite the issue.

[jsync-swirlds]

Team agreement is to follow the recommendation @ata-nas documented as follows:

• It is encouraged to reuse task names on tasks that we design to be able to run in parallel and make sense to execute them on all (or more than one) subproject from repo root
• If we want to execute a task with repeated name only in a single place, we should use the fully qualified name of the task, e.g. ./gradlew :server:createDockerImage
• The tasks that we design must never leave the build root of the respective project, otherwise it will be nondeterministic and error prone due to race conditions if we reuse the resources produced by a task
• We should go through our repo to identify misuses and file issues to fix them