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 configuration cache compatibility issues in gradle plugins #87567

Merged
merged 2 commits into from
Jun 13, 2022

Conversation

breskeby
Copy link
Contributor

@breskeby breskeby commented Jun 9, 2022

This fixes references to project that makes the plugin incompatible with Gradle
configuration cache

Related to #57918

This fixes references to project that makes the plugin incompatible with Gradle
configuration cache

Related to elastic#57918
@breskeby breskeby self-assigned this Jun 9, 2022
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v8.4.0 labels Jun 9, 2022
@breskeby breskeby marked this pull request as ready for review June 9, 2022 15:04
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

- Remove project references from extension properties
- Simplify build scripts by removing custom method for xpack project references
@breskeby breskeby marked this pull request as draft June 9, 2022 19:06
@breskeby
Copy link
Contributor Author

breskeby commented Jun 9, 2022

@pugnascotia I'll ping you again when done here. Sorry should have been a draft. I want to fix more related issues before merging

@breskeby breskeby changed the title Fix configuration cache compatibility issues in YamlRestCompatTestPlugin Fix configuration cache compatibility issues in gradle plugins Jun 13, 2022
@breskeby
Copy link
Contributor Author

@pugnascotia
I think this is ready now. There's more to fix around task listeners and LoggedExec, but that requires more rework that I don't want to mix with other stuff and keep it separated

@breskeby breskeby marked this pull request as ready for review June 13, 2022 07:18
Copy link
Contributor

@pugnascotia pugnascotia left a comment

Choose a reason for hiding this comment

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

Is the xpackProject utility redundant now, or is it going to become redundant? I'm curious about why we're moving away from it?

@breskeby
Copy link
Contributor Author

@pugnascotia the problem with xpackProject was that:

  1. using xpackProject in certain situations can break configure configuration cache compatibility as it uses a mutual project object under the hood that is discouraged to use in some use cases (e.g. at execution time)

  2. It always breaks compatibility with --configure-on-demand

  3. using xpackProject uses the project of the :x-pack project. referencing other project objects from other subproject should avoided where possible to decouple (sub project configurations). There's a good explanation of why we want to decouple our project configurations as much as possible here: https://docs.gradle.org/current/userguide/multi_project_configuration_and_execution.html#sec:decoupled_projects

  4. it adds little value over default out of the box gradle api (just use project(':x-pack:someProject') instead of xpackProject('someProject') Also in some occasions its even shorter. e.g. when this is used as xpackProject('someProject').path instead of just passing :x-pack:someProject

I'll try to put a bit more context in the PR description in the future to make the motivation behind these kind of changes more clear upfront

@breskeby breskeby merged commit 95d56cc into elastic:master Jun 13, 2022
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 >enhancement Team:Delivery Meta label for Delivery team v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants