Cargo
  1. Cargo
  2. CARGO-895

The default installDir for zipUrlInstaller (java.io.tmp) is inapproriate for some platforms (e.g. Mac OS)

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.6
    • Fix Version/s: 1.1.0
    • Component/s: Maven2
    • Labels:
      None
    • Environment:
      n/a
    • Complexity:
      Intermediate
    • Number of attachments :
      1

      Description

      When using the zipUrlInstaller, the default folder where the downloaded zip is stored as well as where the container is unpacked (installed) is java.io.tmp/something. This does not work with some containers (GlassFish 3.x, JOnAS 5.x, JBoss 5+ and probably others) and requires the installDir to be overriden, which is not good as defaults should just work our of a Maven perspective.

      Also, overriding the installDir to something like "installs/cargo", requires the zip to be downloadedfor every project where it is used (which it is not if the default java.io.tmp is used). Bad for performance.

      My suggestion is that the place where the zip is downloaded is separated from where the container is installed/unpacked. The place where the zip is stored could be viewed as a temporary place and could stay as java.io.tmp. However, the unpacked container should probably by default be placed in the target folder when used with Maven.
      This will also work well when we support some type of zipGavInstaller (or whatever we call it) where Cargo will retrieve the zip from a maven repo via GAV. In that case the zip will be in the local repo and shouldn't need to be copied anywhere. But the unpacked installation could be placed in the target folder.

      When it comes to the ant tasks I have no opinion. Suggestions appreciated. I would assume that the current issues with some platforms and the defaults also exist there. But I haven't verified.

      1. CARGO-895.patch
        41 kB
        Savas Ali Tokmen

        Issue Links

          Activity

          Hide
          Savas Ali Tokmen added a comment -

          Why wouldn't we simply install the ZIP/TGZ file to something like $

          {m2.repo}

          /org/codehaus/cargo/installs and then unzip it to $

          {project.build.directory}

          in the Maven2 plugin?

          Show
          Savas Ali Tokmen added a comment - Why wouldn't we simply install the ZIP/TGZ file to something like $ {m2.repo} /org/codehaus/cargo/installs and then unzip it to $ {project.build.directory} in the Maven2 plugin?
          Hide
          Anders Hammar added a comment -

          The local maven repo is not just some temporary storage area, it's something that Maven uses. We can't/shouldn't put any random file there.

          Show
          Anders Hammar added a comment - The local maven repo is not just some temporary storage area, it's something that Maven uses. We can't/shouldn't put any random file there.
          Hide
          Savas Ali Tokmen added a comment -

          ... and why not $HOME/.cargo/installs ?

          Show
          Savas Ali Tokmen added a comment - ... and why not $HOME/.cargo/installs ?
          Hide
          Anders Hammar added a comment -

          That would work, I guess. I was suggesting java.io.tmp just because that's the current default. I was thinking we shouldn't change too much.

          Show
          Anders Hammar added a comment - That would work, I guess. I was suggesting java.io.tmp just because that's the current default. I was thinking we shouldn't change too much.
          Hide
          Savas Ali Tokmen added a comment -

          So, current ideas:

          1) Put the ZIP/TGZ into $

          {java.io.tmpdir}

          , but extract to $

          {project.build.directory}

          . Good sides: resembles the existent for the installation directory
          . Bad sides: only solves for Maven
          2) Use $

          {user.home}

          /.cargo/containers
          . Good sides: solves on all platforms
          . Bad sides: if the user's profile is network-copied (for example, some Windows profile setups), this will make profile sizes and transfer times get longer

          And, refused ideas:

          1) Putting them into $

          {m2.repo}

          . Bad sides: is against Maven guidelines, only solves for Maven

          Show
          Savas Ali Tokmen added a comment - So, current ideas: 1) Put the ZIP/TGZ into $ {java.io.tmpdir} , but extract to $ {project.build.directory} . Good sides: resembles the existent for the installation directory . Bad sides: only solves for Maven 2) Use $ {user.home} /.cargo/containers . Good sides: solves on all platforms . Bad sides: if the user's profile is network-copied (for example, some Windows profile setups), this will make profile sizes and transfer times get longer And, refused ideas: 1) Putting them into $ {m2.repo} . Bad sides: is against Maven guidelines, only solves for Maven
          Hide
          Anders Hammar added a comment -

          I think we should start with separating where the zip files is stored and where the container installation is unpacked. The exact location for the zip file could be changed later on, but I suggest we use java.io.tmpdir for now as that's how it works in 1.0.x.

          This separation will greatly simplify the work of implementing CARGO-449 I think.

          Show
          Anders Hammar added a comment - I think we should start with separating where the zip files is stored and where the container installation is unpacked. The exact location for the zip file could be changed later on, but I suggest we use java.io.tmpdir for now as that's how it works in 1.0.x. This separation will greatly simplify the work of implementing CARGO-449 I think.
          Hide
          Savas Ali Tokmen added a comment -

          So you would suggest the installDir to be suppressed and replaced with downloadDir and extractDir, for example? And yes, it would greatly simplify CARGO-449

          Show
          Savas Ali Tokmen added a comment - So you would suggest the installDir to be suppressed and replaced with downloadDir and extractDir, for example? And yes, it would greatly simplify CARGO-449
          Hide
          Anders Hammar added a comment - - edited

          I was rather thinking that installDir could stay and be what it says, the installation directory i.e where the container is unpacked (with a default in target for the maven plugin and maybe the existing java.io.tmp for everything else like Java API and ant). And then we add a new downloadDir param.

          So, for CARGO-449 (which only makes sense for the maven plugin), the downloadDir would be set to the artifact location in the local repo by the plugin.

          I think we should try to change as little as possible from a user perspective and aim at being backwards compatible.

          Show
          Anders Hammar added a comment - - edited I was rather thinking that installDir could stay and be what it says, the installation directory i.e where the container is unpacked (with a default in target for the maven plugin and maybe the existing java.io.tmp for everything else like Java API and ant). And then we add a new downloadDir param. So, for CARGO-449 (which only makes sense for the maven plugin), the downloadDir would be set to the artifact location in the local repo by the plugin. I think we should try to change as little as possible from a user perspective and aim at being backwards compatible.
          Hide
          Savas Ali Tokmen added a comment -

          Well, it can stay the same as well. My perspective that since the semantics behind is different, the name should change as well -this way, people will have compile errors from the beginning and check the documentation to see what has changed.

          Show
          Savas Ali Tokmen added a comment - Well, it can stay the same as well. My perspective that since the semantics behind is different, the name should change as well -this way, people will have compile errors from the beginning and check the documentation to see what has changed.
          Show
          Savas Ali Tokmen added a comment - Proposed patch: Create two attributes, downloadDir and extractDir Deprecate installDir When installDir is set, show a warning and set both downloadDir and extractDir For documentation update: https://docs.codehaus.org/display/CARGO/Installer https://docs.codehaus.org/display/CARGO/Maven2+Plugin+Reference+Guide https://docs.codehaus.org/display/CARGO/Maven2+Plugin+Tips https://docs.codehaus.org/display/CARGO/Deploying+to+a+running+container https://docs.codehaus.org/display/CARGO/Functional+testing https://docs.codehaus.org/display/CARGO/Starting+and+stopping+a+container
          Hide
          Anders Hammar added a comment -

          So, when installDir is set, downloadDir is set to that value and extractDir is set to installDir/something?

          Regarding default values, they have to be different for Ant and Maven, at least when it comes to extractDir (which should below the build directory for Maven).

          Show
          Anders Hammar added a comment - So, when installDir is set, downloadDir is set to that value and extractDir is set to installDir/something? Regarding default values, they have to be different for Ant and Maven, at least when it comes to extractDir (which should below the build directory for Maven).
          Hide
          Savas Ali Tokmen added a comment -

          You're right... Here is the updated documentation:

          Proposed patch:

          • Create two attributes, downloadDir and extractDir
            • Default value for downloadDir = $ {java.io.tmpdir}/installs
              ** Default value for extractDir = ${java.io.tmpdir}

              /installs for the Java API and ANT tasks, $

              {project.build.directory}

              /cargo/installs for the Maven2 plugin

          • Deprecate installDir
          • When installDir is set, show a warning and set both downloadDir and extractDir

          For documentation update:

          Show
          Savas Ali Tokmen added a comment - You're right... Here is the updated documentation: Proposed patch: Create two attributes, downloadDir and extractDir Default value for downloadDir = $ {java.io.tmpdir}/installs ** Default value for extractDir = ${java.io.tmpdir} /installs for the Java API and ANT tasks, $ {project.build.directory} /cargo/installs for the Maven2 plugin Deprecate installDir When installDir is set, show a warning and set both downloadDir and extractDir For documentation update: https://docs.codehaus.org/display/CARGO/Installer https://docs.codehaus.org/display/CARGO/Maven2+Plugin+Reference+Guide https://docs.codehaus.org/display/CARGO/Maven2+Plugin+Tips https://docs.codehaus.org/display/CARGO/Deploying+to+a+running+container https://docs.codehaus.org/display/CARGO/Functional+testing https://docs.codehaus.org/display/CARGO/Starting+and+stopping+a+container
          Hide
          Savas Ali Tokmen added a comment -

          Documentation update OK.

          Show
          Savas Ali Tokmen added a comment - Documentation update OK.
          Hide
          Anders Hammar added a comment -

          Testing the current defaults for configuration home and container home, I get this result for a project with JBoss:

          target/cargo/configurations/jboss51x
          target/cargo/installs/jboss-5.1.0.GA

          The extracted installation path seems to contain the file name. Wouldn't it make more sense if we used the container id here as we do for the configuration? Or would there be problems if a different zip is specified but the same container id is kept?

          Show
          Anders Hammar added a comment - Testing the current defaults for configuration home and container home, I get this result for a project with JBoss: target/cargo/configurations/jboss51x target/cargo/installs/jboss-5.1.0.GA The extracted installation path seems to contain the file name. Wouldn't it make more sense if we used the container id here as we do for the configuration? Or would there be problems if a different zip is specified but the same container id is kept?
          Hide
          Savas Ali Tokmen added a comment -

          Well... We would have to complexify a bit: currently, the ZipUrlInstaller extracts to the <specified dir>/<zip name>; if we want to have the container name (and it's only possible on Maven2) we'd need another setter.

          Show
          Savas Ali Tokmen added a comment - Well... We would have to complexify a bit: currently, the ZipUrlInstaller extracts to the <specified dir>/<zip name>; if we want to have the container name (and it's only possible on Maven2) we'd need another setter.
          Hide
          Anders Hammar added a comment -

          Ok, then we keep it like this for now. We have lots of other issues to take care of.

          Show
          Anders Hammar added a comment - Ok, then we keep it like this for now. We have lots of other issues to take care of.

            People

            • Assignee:
              Savas Ali Tokmen
              Reporter:
              Anders Hammar
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: