Skip to content

Fix system property resolution at configuration time#78669

Merged
breskeby merged 1 commit intoelastic:masterfrom
breskeby:fix-sysprop-for-configcache
Oct 5, 2021
Merged

Fix system property resolution at configuration time#78669
breskeby merged 1 commit intoelastic:masterfrom
breskeby:fix-sysprop-for-configcache

Conversation

@breskeby
Copy link
Copy Markdown
Contributor

@breskeby breskeby commented Oct 5, 2021

This fixes system property resolution when running the build with --configuration-cache

One step closer to make use of configuration cache

@breskeby breskeby self-assigned this Oct 5, 2021
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.16.0 v8.0.0 labels Oct 5, 2021
@breskeby breskeby marked this pull request as ready for review October 5, 2021 09:01
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@breskeby
Copy link
Copy Markdown
Contributor Author

breskeby commented Oct 5, 2021

This fixes system property resolution when running the build with --configuration-cache

One step closer to make use of configuration cache

.orElse("")
.forUseAtConfigurationTime()
.get();
.getOrElse("");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own education, why is this way better?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the forUseAtConfigurationTime() unfortunately isn't applied to the Provider returned I the orElse block, which makes the build fail when running with configuration-cache enabled. That behaviour is quite unexpected, at least for my understanding. I'm discussing the shortages of this api with the Gradle team

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

having said that the gradle team mentioned that forUseAtConfigurationTime will go away in the future

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

having said that the gradle team mentioned that forUseAtConfigurationTime will go away in the future

Is there going to be a replacement? Does that mean that you can't depend on things like system properties or environment variables at configuration time?

@breskeby breskeby merged commit 545eb1a into elastic:master Oct 5, 2021
@breskeby breskeby deleted the fix-sysprop-for-configcache branch October 5, 2021 09:42
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 78669

breskeby added a commit to breskeby/elasticsearch that referenced this pull request Oct 5, 2021
This fixes system property resolution when running the build with --configuration-cache

One step closer to make use of configuration cache
elasticsearchmachine pushed a commit that referenced this pull request Oct 5, 2021
* Resolve system properties in build scripts via provider factory (#76199)

This allows tracking system properties used in the build configuration and brings us
one step closer to be gradle configuration cache compliant.

* Fix system property resolution at configuration time (#78669)

This fixes system property resolution when running the build with --configuration-cache

One step closer to make use of configuration cache
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 v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants