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

Packaging: Remove classpath ordering hack #23596

Merged
merged 2 commits into from
Mar 21, 2017

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Mar 15, 2017

After the removal of the joda time hack we used to have, we can cleanup
the codebase handling in security, jarhell and plugins to be more picky
about uniqueness. This was originally in #18959 which was never merged.

closes #18959

After the removal of the joda time hack we used to have, we can cleanup
the codebase handling in security, jarhell and plugins to be more picky
about uniqueness. This was originally in elastic#18959 which was never merged.

closes elastic#18959
@rjernst rjernst added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v5.4.0 v6.0.0-alpha1 labels Mar 15, 2017
@jasontedor jasontedor self-requested a review March 18, 2017 03:27
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM. I left two minor comments, treat them as you wish.

@@ -114,7 +112,7 @@ public void testDirAndJar() throws Exception {
public void testWithinSingleJar() throws Exception {
// the java api for zip file does not allow creating duplicate entries (good!) so
// this bogus jar had to be constructed with ant
Copy link
Member

Choose a reason for hiding this comment

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

This comment is out of date; we produce these with https://github.com/jasontedor/duplicate-classes/ now (the previous mechanism for building the bogus jars produced invalid jars which blew up animal-sniffer).

@@ -198,7 +203,7 @@ public static void checkJarHell(URL urls[]) throws URISyntaxException, IOExcepti
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
String entry = root.relativize(file).toString();
if (entry.endsWith(".class")) {
// normalize with the os separator
// normalize with the os separator, remove '.class'
entry = entry.replace(sep, ".").substring(0, entry.length() - 6);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if replacing entry.length() - 6 with entry.length() - ".class".length() would be clearer?

@rjernst rjernst merged commit f8453ac into elastic:master Mar 21, 2017
@rjernst rjernst deleted the plugin_joda_cleanup branch March 21, 2017 19:12
@rjernst
Copy link
Member Author

rjernst commented Mar 21, 2017

@jasontedor I addressed your comments before merging. Thanks for the review.

rjernst added a commit that referenced this pull request Mar 21, 2017
After the removal of the joda time hack we used to have, we can cleanup
the codebase handling in security, jarhell and plugins to be more picky
about uniqueness. This was originally in #18959 which was never merged.

closes #18959
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 22, 2017
* master:
  [API] change wait_for_completion defaults according to docs (elastic#23672)
  Share XContent rendering code in terms aggs (elastic#23680)
  Update ingest-node.asciidoc
  [DOCS] Update the docs about the fact that global ordinals for _parent field are loaded eagerly instead of lazily by default.
  Build: remove progress logger hack for gradle 2.13 (elastic#23679)
  Test: Add dump of integ test cluster logs on failure (elastic#23688)
  Plugins: Add plugin cli specific exit codes (elastic#23599)
  Plugins: Output better error message when existing plugin is incompatible (elastic#23562)
  Reindex: wait for cleanup before responding (elastic#23677)
  Packaging: Remove classpath ordering hack (elastic#23596)
  Docs: Add note about updating plugins requiring removal and reinstallation (elastic#23597)
  Build: Make plugin list for smoke tester dynamic (elastic#23601)
  [TEST] Propertly cleans up failing restore test
@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/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >non-issue Team:Delivery Meta label for Delivery team v5.4.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants