Skip to content

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.


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).

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