Updating JAR output path to avoid using long path.

mashhurs commented 9 months ago

Description

Embedding the plugin into core and faced an issue that jar path exceeds path limit or TarWriter we used when assembling. With this change, we will shorten the JAR path.

BEFORE

268 chars: logstash-8.13.0-SNAPSHOT/vendor/bundle/jruby/3.1.0/gems/logstash-filter-elastic_integration-0.1.2-java/vendor/jar-dependencies/co/elastic/logstash/plugins/filter/elasticintegration/logstash-filter-elastic_integration/0.1.2/logstash-filter-elastic_integration-0.1.2.jar

AFTER

225 chars: logstash-8.13.0-SNAPSHOT/vendor/bundle/jruby/3.1.0/gems/logstash-filter-elastic_integration-0.1.2-java/vendor/jar-dependencies/co/elastic/logstash-filter-elastic_integration/0.1.2/logstash-filter-elastic_integration-0.1.2.jar

yaauie commented 8 months ago

The actual changes here LGTM, but if we were to simply add the jar to the vendor directory without any superfluous nesting we may be able to simply use JRuby's Kernel#require_relative, which would shorten the jar's path to:

155 chars: logstash-8.13.0-SNAPSHOT/vendor/bundle/jruby/3.1.0/gems/logstash-filter-elastic_integration-0.1.2-java/vendor/logstash-filter-elastic_integration-0.1.2.jar

mashhurs commented 8 months ago

The actual changes here LGTM, but if we were to simply add the jar to the vendor directory without any superfluous nesting we may be able to simply use JRuby's Kernel#require_relative, which would shorten the jar's path to:

155 chars: logstash-8.13.0-SNAPSHOT/vendor/bundle/jruby/3.1.0/gems/logstash-filter-elastic_integration-0.1.2-java/vendor/logstash-filter-elastic_integration-0.1.2.jar

if we were to simply add the jar to the vendor: if we directly place under vendor, the require_jar method (in jar_dependencies.rb) requires three params (group id, artifact id and version). So, we need at least one path for group id. Maybe we can use elastic only? Like elastic - project - version as we organize packaging? How do you think?
- in a reality, there is not issue in most OS file systems if path is over 256 limit. Pushing TarWriter would make more sense. Unfortunately, there is an open issue lasting over years for this.
we may be able to simply use JRuby's Kernel#require_relative: ah this one I couldn't get. Do you mean instead TarWriter we use Kernel#require_relative first and zip?

jsvd commented 8 months ago

the minimal change LGTM too. I believe @yaauie's suggestion is to replace use of require_jar from jar_dependencies with a plain require or require_relative with the path to the jar, something like (pseudocode)

        jarRequiresFile.withWriter { w ->
            w << "# AUTOGENERATED BY THE GRADLE SCRIPT. DO NOT EDIT.\n\n"
            w << '########################################################################\n' +
                 '# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V.\n' +
                 '# under one or more contributor license agreements. Licensed under the\n' +
                 '# Elastic License 2.0; you may not use this file except in compliance\n' +
                 '# with the Elastic License 2.0.\n' +
                 '########################################################################\n'
            w << '\n'
            w << "require_relative(::File.join("..", "vendor", "#{project.name}-#{project.version}.jar")\n"
        }

elastic / logstash-filter-elastic_integration