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

Use reproducible method of generating properties file for better caching #42539

Merged

Conversation

mark-vieira
Copy link
Contributor

The default Java implementation of Properties.store() is non-reproducible. First, because it stores properties internally in a HashTable so the final ordering will be non-deterministic and secondly because it injects a comment with a timestamp into the generated file. Both of these things break build cacheability since this file eventually ends up on the build classpath.

Here we use Gradle's WriteProperties task instead which solves both issues and ensures for the same set on inputs, reproducible output will be generated.

@mark-vieira mark-vieira added the :Delivery/Build Build or test infrastructure label May 24, 2019
@mark-vieira mark-vieira requested a review from rjernst May 24, 2019 18:24
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

processResources {
inputs.file(propsFile)
// We need to be explicit with the version because we add snapshot and qualifier to it based on properties
inputs.property("dynamic_elasticsearch_version", props.getProperty("elasticsearch"))
Copy link
Member

Choose a reason for hiding this comment

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

It seems we lost adding this property?

Copy link
Contributor Author

@mark-vieira mark-vieira May 24, 2019

Choose a reason for hiding this comment

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

It's no longer necessary as the generate task tracks all properties as an input.

properties(props) // final computed prop values are an input to generateVersionProperties task```

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

@mark-vieira mark-vieira merged commit 9772574 into elastic:master May 24, 2019
@mark-vieira mark-vieira deleted the build-cacheability-improvements branch May 24, 2019 20:47
mark-vieira added a commit that referenced this pull request May 24, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
@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 >non-issue Team:Delivery Meta label for Delivery team v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants