Uploaded image for project: 'Maven'
  1. Maven
  2. MNG-6115

Add Jansi native library search path to our start scripts to avoid extraction to temp file on each run

    Details

      Description

      By default Jansi extracts the native, shared library to temp which results in unnecessary file I/O and a huge amount of temp files when Maven is run very often. One can avoid that by adding -Dlibrary.jansi.path=${MAVEN_HOME}/lib/ext to the start script. The user has to copy the libjansi.so or jansi.dll once and will avoid the overhead.

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build maven-3.x #1567 (See https://builds.apache.org/job/maven-3.x/1567/)
          MNG-6115 prevent JAnsi from writing temp native files to lib/ext (hboutemy: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=181b0215aa1199e152db9d2c08b1a01436547805)

          • (add) apache-maven/src/lib/jansi-native/README.txt
          • (edit) apache-maven/src/bin/mvn
          • (edit) apache-maven/src/main/assembly/component.xml
          • (edit) apache-maven/pom.xml
          • (edit) apache-maven/src/bin/mvn.cmd
          • (edit) maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build maven-3.x #1567 (See https://builds.apache.org/job/maven-3.x/1567/ ) MNG-6115 prevent JAnsi from writing temp native files to lib/ext (hboutemy: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=181b0215aa1199e152db9d2c08b1a01436547805 ) (add) apache-maven/src/lib/jansi-native/README.txt (edit) apache-maven/src/bin/mvn (edit) apache-maven/src/main/assembly/component.xml (edit) apache-maven/pom.xml (edit) apache-maven/src/bin/mvn.cmd (edit) maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
          Hide
          hboutemy Hervé Boutemy added a comment -

          notice that:
          1. it works well for many people for more than 6 months: I'm pretty confident we won't wreck havoc
          2. if it causes issues for somebody (which for sure will be exceptional but may happen), the local fix is just to remove jansi-1.13.jar from lib/: no Jansi lib means no colour but no failure

          Show
          hboutemy Hervé Boutemy added a comment - notice that: 1. it works well for many people for more than 6 months: I'm pretty confident we won't wreck havoc 2. if it causes issues for somebody (which for sure will be exceptional but may happen), the local fix is just to remove jansi-1.13.jar from lib/ : no Jansi lib means no colour but no failure
          Hide
          michael-o Michael Osipov added a comment -

          Stephen Connolly, please test this on macOS. I think that the code won't work for you because binaries are univeral on macOS. The code ignores that fact.

          Show
          michael-o Michael Osipov added a comment - Stephen Connolly , please test this on macOS. I think that the code won't work for you because binaries are univeral on macOS. The code ignores that fact.
          Hide
          stephenc Stephen Connolly added a comment -

          I vote for Hervé's solution. I do not want to see extracting over and over. If upstream can provide a better solution in the longer term, well we can adopt that, but right now IMHO if we cannot fix the extraction over and over then we rip out colours from 3.5.0.

          Duplicating some code from HawtJNI is the lesser evil IMHO

          Show
          stephenc Stephen Connolly added a comment - I vote for Hervé's solution. I do not want to see extracting over and over. If upstream can provide a better solution in the longer term, well we can adopt that, but right now IMHO if we cannot fix the extraction over and over then we rip out colours from 3.5.0. Duplicating some code from HawtJNI is the lesser evil IMHO
          Hide
          michael-o Michael Osipov added a comment -

          Please also see MNGSITE-295.

          Show
          michael-o Michael Osipov added a comment - Please also see MNGSITE-295 .
          Hide
          michael-o Michael Osipov added a comment -

          So back to default, have extracted over and over again and leave it up to the user to solve this issue?

          Show
          michael-o Michael Osipov added a comment - So back to default, have extracted over and over again and leave it up to the user to solve this issue?
          Hide
          rfscholte Robert Scholte added a comment -

          ... but we can expect them to extract something from a JAR once.

          Well, I don't think we can expect that. What I expect is that that people simple download the latest Maven version, see colors, celebrate their upgrade and continu with the next build.
          If we did nothing, we could give a warning: please copy file X from /lib/jansi-x.jar to /lib/ext/, but if I would get such message, I'd say: why won't you Maven guys do that for me?

          I like the proposal from Hervé.

          Show
          rfscholte Robert Scholte added a comment - ... but we can expect them to extract something from a JAR once. Well, I don't think we can expect that. What I expect is that that people simple download the latest Maven version, see colors, celebrate their upgrade and continu with the next build. If we did nothing, we could give a warning: please copy file X from /lib/jansi-x.jar to /lib/ext/ , but if I would get such message, I'd say: why won't you Maven guys do that for me? I like the proposal from Hervé.
          Hide
          hboutemy Hervé Boutemy added a comment -

          we definitely don't live in the same world when we evaluate complexity and feasibility
          Then IMHO, the best thing is just to ignore the fact that there is an extraction on each run: other people using JAnsi don't care and everything works great for them.
          Let's remove this optimization that only gives us headaches

          Show
          hboutemy Hervé Boutemy added a comment - we definitely don't live in the same world when we evaluate complexity and feasibility Then IMHO, the best thing is just to ignore the fact that there is an extraction on each run: other people using JAnsi don't care and everything works great for them. Let's remove this optimization that only gives us headaches
          Hide
          michael-o Michael Osipov added a comment - - edited

          Just read the commit. I can only say: ouch. Not only we are duplicating the HawtJNI code, we are putting a lot of effort for just one library to load a native shared object. I don't expect people to compile native code, but we can expect them to extract something from a JAR once. I would rather have two things:

          1. Have Jansi remove the native libs from the default JAR and provide an attached artifact with them. Leave the task to the user adding the precompiled version to java.library.path.
          2. Improve HawtJNI to load from an exploded directly just like it would be a JAR.

          Consider that Jansi requires only one native operation: org.fusesource.jansi.internal.CLibrary.isatty.

          Show
          michael-o Michael Osipov added a comment - - edited Just read the commit. I can only say: ouch. Not only we are duplicating the HawtJNI code, we are putting a lot of effort for just one library to load a native shared object. I don't expect people to compile native code, but we can expect them to extract something from a JAR once. I would rather have two things: 1. Have Jansi remove the native libs from the default JAR and provide an attached artifact with them. Leave the task to the user adding the precompiled version to java.library.path . 2. Improve HawtJNI to load from an exploded directly just like it would be a JAR. Consider that Jansi requires only one native operation: org.fusesource.jansi.internal.CLibrary.isatty .
          Hide
          hboutemy Hervé Boutemy added a comment -

          Michael Osipov "I would people expect to put the library to /usr/local/lib once": perhaps you can tell that to FreeBSD advanced users, who are used to tweak everything. But you can't expect such work from normal users, this "won't scale" as you say

          I implemented the extraction of native libs in $maven.home/lib/jansi-native during distribution build (with maven-dependency-plugin + assembly configuration) then library.jansi.path calculation from Maven embedder's MavenCli in http://git-wip-us.apache.org/repos/asf/maven/commit/c36cf425
          Please review and confirm this is the way to go for now: I'll propose patches to HawtJNI to support such scenario and avoid us to copy/paste some platform calculation algorithm

          Show
          hboutemy Hervé Boutemy added a comment - Michael Osipov "I would people expect to put the library to /usr/local/lib once": perhaps you can tell that to FreeBSD advanced users, who are used to tweak everything. But you can't expect such work from normal users, this "won't scale" as you say I implemented the extraction of native libs in $maven.home/lib/jansi-native during distribution build (with maven-dependency-plugin + assembly configuration) then library.jansi.path calculation from Maven embedder's MavenCli in http://git-wip-us.apache.org/repos/asf/maven/commit/c36cf425 Please review and confirm this is the way to go for now: I'll propose patches to HawtJNI to support such scenario and avoid us to copy/paste some platform calculation algorithm
          Hide
          michael-o Michael Osipov added a comment -

          Hervé Boutemy, all the hard work is done by HawtJNI, see here.
          Robert Scholte, this won't work. See here. OS and arch are only evaluated if the library it is extracted from the JAR.

          Unless someone changes HawtJNI to our needs, this won't scale. I would ship a modified JAR without jansi-1.13.jar!/META-INF/native (or rename) to avoid the constant extraction, add the patch Hervé did and add documentation for users if they need istty support, they have to place libjansi.so/libjansi.dylib/jansi.dll to ${maven.home}/lib/ext.

          Personally, I would people expect to put the library to /usr/local/lib once. This is where a native lib belongs and modify library.jansi.path. This is what I will recommend for the FreeBSD port of Maven. Alternatively, the user can always set LD_LIBRARY_PATH in his mavenrc pointing to the directory where the native lib resides.

          There is a reason why I have created this ticket months ago.

          Show
          michael-o Michael Osipov added a comment - Hervé Boutemy , all the hard work is done by HawtJNI, see here . Robert Scholte , this won't work. See here . OS and arch are only evaluated if the library it is extracted from the JAR. Unless someone changes HawtJNI to our needs, this won't scale. I would ship a modified JAR without jansi-1.13.jar!/META-INF/native (or rename) to avoid the constant extraction, add the patch Hervé did and add documentation for users if they need istty support, they have to place libjansi.so / libjansi.dylib / jansi.dll to ${maven.home}/lib/ext . Personally, I would people expect to put the library to /usr/local/lib once. This is where a native lib belongs and modify library.jansi.path . This is what I will recommend for the FreeBSD port of Maven. Alternatively, the user can always set LD_LIBRARY_PATH in his mavenrc pointing to the directory where the native lib resides. There is a reason why I have created this ticket months ago.
          Hide
          rfscholte Robert Scholte added a comment -

          Step one would probably be: unpack jansi-1.13.jar!/META-INF/native to lib/arch, but it looks like such option is not yet supported by maven-assembly-plugin.

          Show
          rfscholte Robert Scholte added a comment - Step one would probably be: unpack jansi-1.13.jar!/META-INF/native to lib/arch , but it looks like such option is not yet supported by maven-assembly-plugin.
          Hide
          hboutemy Hervé Boutemy added a comment -

          I didn't look previously, but yes the libs come from META-INF/native/<arch>/libjansi.<type> in jansi-1.13.jar
          you're right, we should probably extract them from the jar in our distribution if we want to avoid temporary extracts: this idea about lib/ext and users to put files in it just won't work
          the only question I have now is: how to select the file at runtime?

          I'll have a look at jansi source code to see if there is a better integration trick than library.jansi.path that would find automatically the value of <arch>

          Show
          hboutemy Hervé Boutemy added a comment - I didn't look previously, but yes the libs come from META-INF/native/<arch>/libjansi.<type> in jansi-1.13.jar you're right, we should probably extract them from the jar in our distribution if we want to avoid temporary extracts: this idea about lib/ext and users to put files in it just won't work the only question I have now is: how to select the file at runtime? I'll have a look at jansi source code to see if there is a better integration trick than library.jansi.path that would find automatically the value of <arch>
          Hide
          rfscholte Robert Scholte added a comment -

          So the dll and all other native files are actually part of the jansi-1.13.jar. Just to verify, I've copied the jansi.dll to lib/ext and can confirm that I don't see any temp-files anymore.
          I'm wondering if this can be automated (and if we should do so), because I don't expect users to do this on their own.

          Show
          rfscholte Robert Scholte added a comment - So the dll and all other native files are actually part of the jansi-1.13.jar. Just to verify, I've copied the jansi.dll to lib/ext and can confirm that I don't see any temp-files anymore. I'm wondering if this can be automated (and if we should do so), because I don't expect users to do this on their own.
          Hide
          hboutemy Hervé Boutemy added a comment -

          do you mean you simply want to remove the dash? ok for me

          Show
          hboutemy Hervé Boutemy added a comment - do you mean you simply want to remove the dash? ok for me
          Hide
          michael-o Michael Osipov added a comment -

          Hervé, this won't scale for the shell script because it could be simply libjansi.so.

          Show
          michael-o Michael Osipov added a comment - Hervé, this won't scale for the shell script because it could be simply libjansi.so .
          Hide
          hboutemy Hervé Boutemy added a comment -

          reworked in http://git-wip-us.apache.org/repos/asf/maven/commit/5ee18b33
          should now work both on Windows and Linux
          please check and report

          Show
          hboutemy Hervé Boutemy added a comment - reworked in http://git-wip-us.apache.org/repos/asf/maven/commit/5ee18b33 should now work both on Windows and Linux please check and report
          Hide
          hboutemy Hervé Boutemy added a comment -

          notice https://github.com/fusesource/jansi/issues/67
          this native part seems to cause issues for some users...

          Show
          hboutemy Hervé Boutemy added a comment - notice https://github.com/fusesource/jansi/issues/67 this native part seems to cause issues for some users...
          Hide
          michael-o Michael Osipov added a comment - - edited

          OK, this is not build time, this is runtime. For the shell script, you need to make a difference at least between .so and .dylib. Looks good to me. Distributions can add the shared library to /usr/lib or /usr/local/lib and patch the shell script.

          Show
          michael-o Michael Osipov added a comment - - edited OK, this is not build time, this is runtime. For the shell script, you need to make a difference at least between .so and .dylib . Looks good to me. Distributions can add the shared library to /usr/lib or /usr/local/lib and patch the shell script.
          Hide
          rfscholte Robert Scholte added a comment -

          See e9e26f64 and 1a79bb3e for mvn.cmd. Need a similar solution for mvn

          Show
          rfscholte Robert Scholte added a comment - See e9e26f64 and 1a79bb3e for mvn.cmd . Need a similar solution for mvn
          Hide
          michael-o Michael Osipov added a comment -

          How are we supposed to do this at build time?

          Show
          michael-o Michael Osipov added a comment - How are we supposed to do this at build time?
          Hide
          rfscholte Robert Scholte added a comment -

          IMHO it should not be the distribution which get filled with temporary files. Instead I suggest to verify if lib/ext/libjansi.so and/or lib/ext/jansi.dll exist, and only in this case set the -Dlibrary.jansi.path

          Show
          rfscholte Robert Scholte added a comment - IMHO it should not be the distribution which get filled with temporary files. Instead I suggest to verify if lib/ext/libjansi.so and/or lib/ext/jansi.dll exist, and only in this case set the -Dlibrary.jansi.path
          Hide
          michael-o Michael Osipov added a comment -

          This always happens. Regardless of the location. Remove the property and see you /tmp spill over.

          Show
          michael-o Michael Osipov added a comment - This always happens. Regardless of the location. Remove the property and see you /tmp spill over.
          Hide
          rfscholte Robert Scholte added a comment -

          When using Apache Maven 3.5.0-alpha-1 with every Maven build, a new lib/ext/jansi-64-1-xxxxxxxxxxxxxxx.13 is generated.
          That's not the preferred behavior, so reopening.

          Show
          rfscholte Robert Scholte added a comment - When using Apache Maven 3.5.0-alpha-1 with every Maven build, a new lib/ext/jansi-64-1-xxxxxxxxxxxxxxx.13 is generated. That's not the preferred behavior, so reopening.
          Hide
          stephenc Stephen Connolly added a comment -

          Maven 3.4.0 has been dropped. See this thread for more details.

          This issue will need to be re-scheduled for a Maven release in the (hopefully near) future.

          Show
          stephenc Stephen Connolly added a comment - Maven 3.4.0 has been dropped. See this thread for more details. This issue will need to be re-scheduled for a Maven release in the (hopefully near) future.
          Hide
          michael-o Michael Osipov added a comment -

          Reassinged to you. Which do you prefer lib or lib/ext? I now even tend to lib/ext to leave lib untouched because it is under our control on contrast to lib.

          Show
          michael-o Michael Osipov added a comment - Reassinged to you. Which do you prefer lib or lib/ext ? I now even tend to lib/ext to leave lib untouched because it is under our control on contrast to lib .
          Hide
          hboutemy Hervé Boutemy added a comment -

          yes, sure

          Show
          hboutemy Hervé Boutemy added a comment - yes, sure
          Hide
          michael-o Michael Osipov added a comment -

          Hervé Boutemy, can I handle this over to you to be squashed with MNG-3507 during master cleanup? This feels right to do so. We can discuss wether this should be in ${maven.home}/lib or ${maven.home}/lib/ext.

          Show
          michael-o Michael Osipov added a comment - Hervé Boutemy , can I handle this over to you to be squashed with MNG-3507 during master cleanup? This feels right to do so. We can discuss wether this should be in ${maven.home}/lib or ${maven.home}/lib/ext .
          Hide
          hudson Hudson added a comment - - edited

          SUCCESS: Integrated in Jenkins build maven-3.x #1412 (See https://builds.apache.org/job/maven-3.x/1412/)
          MNG-6115 Add Jansi native library search path to our start scripts (michaelo: rev http://git-wip-us.apache.org/repos/asf/maven/3d4cd94450e167953c40028eaf1c71ce7a7cafc7)

          • (edit) apache-maven/src/bin/mvn
          • (edit) apache-maven/src/bin/mvn.cmd
          Show
          hudson Hudson added a comment - - edited SUCCESS: Integrated in Jenkins build maven-3.x #1412 (See https://builds.apache.org/job/maven-3.x/1412/ ) MNG-6115 Add Jansi native library search path to our start scripts (michaelo: rev http://git-wip-us.apache.org/repos/asf/maven/3d4cd94450e167953c40028eaf1c71ce7a7cafc7 ) (edit) apache-maven/src/bin/mvn (edit) apache-maven/src/bin/mvn.cmd
          Hide
          michael-o Michael Osipov added a comment -
          Show
          michael-o Michael Osipov added a comment - Fixed with 3d4cd94450e167953c40028eaf1c71ce7a7cafc7 .

            People

            • Assignee:
              hboutemy Hervé Boutemy
              Reporter:
              michael-o Michael Osipov
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development