Apache Whirr (retired)
  1. Apache Whirr (retired)
  2. WHIRR-156

Cli script doesn't launch post-modularization

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.3.0
    • Component/s: cli
    • Labels:
      None

      Description

      bin/whirr is looking for the cli jar in the wrong location: the following change works, but would not be forwards compatible:

      -java -jar $bin/../whirr-cli-*.jar "$@"
      +java -jar $bin/../cli/target/whirr-cli-*-incubating-SNAPSHOT.jar "$@"

      Note that the naive forwards compatible approach grabs the source jar instead.

      1. WHIRR-156.patch
        0.5 kB
        Tom White
      2. WHIRR-156.patch
        0.5 kB
        Tom White
      3. WHIRR-156.patch
        0.5 kB
        Tom White

        Activity

        Hide
        Tom White added a comment -

        This is set up to work for the binary tarball, since the CLI JAR is copied to the top-level in that case. However, it would be good to make the script work when launched from a svn/git working tree. Could we make the script look in each location in turn?

        Show
        Tom White added a comment - This is set up to work for the binary tarball, since the CLI JAR is copied to the top-level in that case. However, it would be good to make the script work when launched from a svn/git working tree. Could we make the script look in each location in turn?
        Hide
        Tom White added a comment -

        Here's a patch to make the script look for the CLI JAR in both places.

        Show
        Tom White added a comment - Here's a patch to make the script look for the CLI JAR in both places.
        Hide
        Andrei Savu added a comment -

        How about using test and not ls? I haven't tested the updated patch.

        Index: bin/whirr
        ===================================================================
        --- bin/whirr	(revision 1049657)
        +++ bin/whirr	(working copy)
        @@ -18,4 +18,14 @@
         bin=`dirname "$0"`
         bin=`cd "$bin"; pwd`
         
        -java -jar $bin/../whirr-cli-*.jar "$@"
        +release_jar=$bin/../whirr-cli-*.jar
        +snapshot_jar=$bin/../cli/target/whirr-cli-*-SNAPSHOT.jar
        +
        +if [ -f $release_jar ]; then
        +  java -jar $release_jar "$@"
        +elif [ -f $snapshot_jar ]; then
        +  java -jar $snapshot_jar "$@"
        +else
        +  echo "No CLI JAR found."
        +  exit 1
        +fi
        
        Show
        Andrei Savu added a comment - How about using test and not ls? I haven't tested the updated patch. Index: bin/whirr =================================================================== --- bin/whirr (revision 1049657) +++ bin/whirr (working copy) @@ -18,4 +18,14 @@ bin=`dirname "$0" ` bin=`cd "$bin" ; pwd` -java -jar $bin/../whirr-cli-*.jar "$@" +release_jar=$bin/../whirr-cli-*.jar +snapshot_jar=$bin/../cli/target/whirr-cli-*-SNAPSHOT.jar + + if [ -f $release_jar ]; then + java -jar $release_jar "$@" +elif [ -f $snapshot_jar ]; then + java -jar $snapshot_jar "$@" + else + echo "No CLI JAR found." + exit 1 +fi
        Hide
        Tom White added a comment -

        There's no real difference in this case, since the glob patterns match 0 or 1 files. The ls approach is preferable when there are multiple glob matches. Your code is slightly shorter, so perhaps we can use that?

        You can test it by running "bin/whirr" from both a source directory, and from an unpacked tarball built with "mvn package assembly:assembly".

        Show
        Tom White added a comment - There's no real difference in this case, since the glob patterns match 0 or 1 files. The ls approach is preferable when there are multiple glob matches. Your code is slightly shorter, so perhaps we can use that? You can test it by running "bin/whirr" from both a source directory, and from an unpacked tarball built with "mvn package assembly:assembly".
        Hide
        Andrei Savu added a comment -

        It's your choice. I was just making a suggestion ( I suck at writing bash scripts ). I have tested both approaches and they work as expected.

        Show
        Andrei Savu added a comment - It's your choice. I was just making a suggestion ( I suck at writing bash scripts ). I have tested both approaches and they work as expected.
        Hide
        Tom White added a comment -

        Here's the simpler version, which I plan commit unless there's any objection.

        Show
        Tom White added a comment - Here's the simpler version, which I plan commit unless there's any objection.
        Hide
        Tom White added a comment -

        I've just committed this.

        Show
        Tom White added a comment - I've just committed this.

          People

          • Assignee:
            Tom White
            Reporter:
            Stu Hood
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development