-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Fix hdfs-related IT tests for java24 #122044
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
3fc401d
Use hadoop patcher for hdfs-fixture too
ldematte d484793
Merge remote-tracking branch 'upstream/main' into fix-hdfs-tests-java24
ldematte 08f59d6
Merge remote-tracking branch 'upstream/main' into fix-hdfs-tests-java24
ldematte 6de0f88
Try to use gradle transformer, not working
ldematte 537cdd7
Removing patcher project
ldematte 1d389bf
[CI] Auto commit changes from spotless
bb497df
Patcher fix
ldematte 473ee30
Merge branch 'fix-hdfs-tests-java24' of github.com:ldematte/elasticse…
ldematte f715051
Merge remote-tracking branch 'upstream/main' into fix-hdfs-tests-java24
ldematte 4674a0b
Fix repository-hdfs gradle to perform transformation directly
ldematte af8fbd9
Add test classpaths
ldematte e478f43
Merge remote-tracking branch 'upstream/main' into fix-hdfs-tests-java24
ldematte c86d491
Unmuting
ldematte 4646fdc
Reorder gradle file
ldematte affaf73
Regex?
ldematte ec9e749
Merge branch 'main' into fix-hdfs-tests-java24
ldematte 791c56b
Add hdfs3 fixture too
ldematte cecb60b
Use regex to match artifact; add check that we patch all classes in t…
ldematte 256d104
Merge branch 'fix-hdfs-tests-java24' of github.com:ldematte/elasticse…
ldematte File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import org.gradle.api.file.ArchiveOperations | ||
|
|
||
| apply plugin: 'elasticsearch.java' | ||
|
|
||
| dependencies { | ||
| implementation 'org.ow2.asm:asm:9.7.1' | ||
| implementation 'org.ow2.asm:asm-tree:9.7.1' | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had to go, as some classes are not present in both jars; it's unfortunate, but I think acceptable. If it's not, should we pass the classes to patch via the command line, so it is configurable in each gradle
JavaExectask?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should pass the files to patch down, it should be part of the configuration of the task. we can keep how to actually patch the classes here, but the list of classes to patch be passed in.
However, why would the same class be present in two different jars? That's jarhell, which we should be rejecting already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have said both projects; the hdsf-fixture projects needs a subset of the patches, as it pulls in a different (smaller) set of dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still missing something. Why do dependencies matter? We should only be patching a given class once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The classes we need to patch are in the 2 project's dependencies: for
hadoop-client-api, they are in bothhadoop-commonandhadoop-auth;hdfs-fixturehowever depends only onhadoop-common(viahadoop-minicluster), so trying to patch classes inhadoop-authresult in an error (the class is not there).But like you said, this can be solved by making the patcher get a list of the class names.