Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Gradle >=4.2 compatibility #27591

Merged
merged 7 commits into from
Nov 30, 2017

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Nov 29, 2017

Gradle 4.2 introduced a feature for safer handling of stale output files. Unfortunately, due to the way some of our tasks are written, this broke execution of our REST tests tasks. The reason for this is that the extract task (which extracts the ES distribution) would clean its output directory, and thereby also remove the empty cwd subdirectory which was created by the clean task, see Gradle logging output:

:modules:lang-painless:integTestCluster#extract
Putting task artifact state for task ':modules:lang-painless:integTestCluster#extract' into context took 0.0 secs.
Deleting stale output file: /home/ywelsch/dev/elasticsearch/modules/lang-painless/build/cluster/integTestCluster node0
Up-to-date check for task ':modules:lang-painless:integTestCluster#extract' took 0.0 secs. It is not up-to-date because:
  No history is available.
:modules:lang-painless:integTestCluster#extract (Thread[Task worker for ':' Thread 8,5,main]) completed. Took 0.351 secs.

Subsequent tasks (installing a plugin or running the node) would then fail because of the missing cwd directory.

The reason why Gradle would remove the directory is that the earlier running clean task would programmatically create the empty cwd directory, but not make Gradle aware of this fact, which would result in Gradle believing that it could just safely clean the directory.

This PR explicitly defines a task to create the cwd subdirectory, and marks the directory as output of the task, so that the subsequent extract task won't eagerly clean it. It thereby restores full compatibility of the build with Gradle 4.2 and above (tested on my CI).

In the future, we should revisit the way we write these tasks, there are way too many tasks that copy stuff into common output folders, which also hurts making the build incremental (see The case against overlapping outputs).

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @ywelsch!

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I am testing this on my CI first where I repeatedly run into this problem.

@ywelsch
Copy link
Contributor Author

ywelsch commented Nov 30, 2017

I've done more testing on this, which uncovered two smaller bugs:

  • I had wrongly defined the task dependency of the new task, fixed in 5e98e4b. This was uncovered by running gradle check without clean.
  • While the build worked with Gradle 4.2, there would still be an unrelated problem with Gradle 4.3. Gradle 4.3 does not seem to like the @Input annotation on the waitCondition closure of AntFixture anymore, as it now does some extended input/output validation. I've simply removed the annotation in a9a7abc.
    With these two fixes in place, I've run the build successfully with both Gradle 4.2 and 4.3.

@ywelsch ywelsch merged commit 2738c78 into elastic:master Nov 30, 2017
ywelsch added a commit that referenced this pull request Nov 30, 2017
Gradle 4.2 introduced a feature for safer handling of stale output files. Unfortunately, due to the way some of our tasks are written, this broke execution of our REST tests tasks. The reason for this is that the extract task (which extracts the ES distribution) would clean its output directory, and thereby also remove the empty cwd subdirectory which was created by the clean task. The reason why Gradle would remove the directory is that the earlier running clean task would programmatically create the empty cwd directory, but not make Gradle aware of this fact, which would result in Gradle believing that it could just safely clean the directory.

This commit explicitly defines a task to create the cwd subdirectory, and marks the directory as output of the task, so that the subsequent extract task won't eagerly clean it. It thereby restores full compatibility of the build with Gradle 4.2 and above.

This commit also removes the @input annotation on the waitCondition closure of AntFixture, which conflicted with the extended input/output validation of Gradle 4.3.
ywelsch added a commit that referenced this pull request Nov 30, 2017
Gradle 4.2 introduced a feature for safer handling of stale output files. Unfortunately, due to the way some of our tasks are written, this broke execution of our REST tests tasks. The reason for this is that the extract task (which extracts the ES distribution) would clean its output directory, and thereby also remove the empty cwd subdirectory which was created by the clean task. The reason why Gradle would remove the directory is that the earlier running clean task would programmatically create the empty cwd directory, but not make Gradle aware of this fact, which would result in Gradle believing that it could just safely clean the directory.

This commit explicitly defines a task to create the cwd subdirectory, and marks the directory as output of the task, so that the subsequent extract task won't eagerly clean it. It thereby restores full compatibility of the build with Gradle 4.2 and above.

This commit also removes the @input annotation on the waitCondition closure of AntFixture, which conflicted with the extended input/output validation of Gradle 4.3.
ywelsch added a commit that referenced this pull request Nov 30, 2017
Gradle 4.2 introduced a feature for safer handling of stale output files. Unfortunately, due to the way some of our tasks are written, this broke execution of our REST tests tasks. The reason for this is that the extract task (which extracts the ES distribution) would clean its output directory, and thereby also remove the empty cwd subdirectory which was created by the clean task. The reason why Gradle would remove the directory is that the earlier running clean task would programmatically create the empty cwd directory, but not make Gradle aware of this fact, which would result in Gradle believing that it could just safely clean the directory.

This commit explicitly defines a task to create the cwd subdirectory, and marks the directory as output of the task, so that the subsequent extract task won't eagerly clean it. It thereby restores full compatibility of the build with Gradle 4.2 and above.

This commit also removes the @input annotation on the waitCondition closure of AntFixture, which conflicted with the extended input/output validation of Gradle 4.3.
ywelsch added a commit that referenced this pull request Nov 30, 2017
Gradle 4.2 introduced a feature for safer handling of stale output files. Unfortunately, due to the way some of our tasks are written, this broke execution of our REST tests tasks. The reason for this is that the extract task (which extracts the ES distribution) would clean its output directory, and thereby also remove the empty cwd subdirectory which was created by the clean task. The reason why Gradle would remove the directory is that the earlier running clean task would programmatically create the empty cwd directory, but not make Gradle aware of this fact, which would result in Gradle believing that it could just safely clean the directory.

This commit explicitly defines a task to create the cwd subdirectory, and marks the directory as output of the task, so that the subsequent extract task won't eagerly clean it. It thereby restores full compatibility of the build with Gradle 4.2 and above.

This commit also removes the @input annotation on the waitCondition closure of AntFixture, which conflicted with the extended input/output validation of Gradle 4.3.
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v5.6.5 v6.0.1 v6.1.0 v6.2.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants