Skip to content

Conversation

@ldematte
Copy link
Contributor

@ldematte ldematte commented Apr 8, 2025

Patchers transform specific classes in some "broken" dependencies to ensure they behave correctly (fixing a bug, disabling some undesired or dangerous behaviour, updating calls to deprecated or removed method overloads).

If we upgrade one of the dependencies we patch, we have a concerns that the patchers may not work against the classes in the new version.
This PR addresses this concern by introducing a check on the SHA256 digest of the class, to ensure we are operating on the same bytes the patcher was designed for; if the digest changes that means the class has been changed (e.g. for a dependency update). If that happens, we break the build process with a specific error, so we can double check that the patchers still work against the new classes.

Extracted from #126326

Relates to ES-11279

@ldematte ldematte added :Core/Infra/Core Core issues without another label >refactoring v9.1.0 labels Apr 8, 2025
@ldematte ldematte requested review from a team and DaveCTurner April 8, 2025 08:20
@ldematte ldematte requested a review from a team as a code owner April 8, 2025 08:20
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Apr 8, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@ldematte ldematte marked this pull request as ready for review April 8, 2025 13:24
@ldematte ldematte requested a review from DaveCTurner April 11, 2025 08:33
Copy link
Contributor

@mosche mosche left a comment

Choose a reason for hiding this comment

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

lgtm, just a tiny nit

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

It'd be slightly nicer to get all the hash mismatches at once, rather than having to update them one-by-one and re-run the build. But it's not that many, this LGTM too.

@ldematte
Copy link
Contributor Author

ldematte commented Apr 11, 2025

It'd be slightly nicer to get all the hash mismatches at once, rather than having to update them one-by-one and re-run the build

I like it, Done!

@ldematte ldematte enabled auto-merge (squash) April 11, 2025 13:12
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM still

@ldematte ldematte merged commit e4af657 into elastic:main Apr 11, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >refactoring Team:Core/Infra Meta label for core/infra team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants