Explicitly search for bookkeeper-server jar #772
Conversation
|
from programming perspective, this change looks good. However, I didn't encounter this problem in current master: I tried two ways:
so I don't think this behavior has been broken since we added sources package. I might be a problem that command would produce different results when running on different shells. |
bookkeeper-server/bin/bookkeeper
Outdated
| @@ -71,18 +71,18 @@ else | |||
| fi | |||
|
|
|||
| # exclude tests jar | |||
| RELEASE_JAR=$(ls ${BK_HOME}/*bookkeeper-server-*.jar 2> /dev/null | grep -v tests | tail -1) | |||
| RELEASE_JAR=$(ls ${BK_HOME}/*bookkeeper-server-*.jar 2> /dev/null | grep -v tests | grep -v sources | tail -1) | |||
There was a problem hiding this comment.
if the problem comes from multiple package, or your shell doesn't emit the right result. You need to exclude following packages:
target/bookkeeper-server-4.6.0-SNAPSHOT-javadoc.jar
target/bookkeeper-server-4.6.0-SNAPSHOT-shaded.jar
target/bookkeeper-server-4.6.0-SNAPSHOT-sources.jar
However I would suggest digging into this issue a bit more and come up a command that can produce determistic results.
There was a problem hiding this comment.
Strange. Yes, we should make this more deterministic.
My shell(debian stable, bash) collates differently:
apidocs bookkeeper-server-4.7.0-SNAPSHOT-sources.jar classes maven-archiver
bookkeeper-server-4.7.0-SNAPSHOT.jar bookkeeper-server-4.7.0-SNAPSHOT-tests.jar generated-sources maven-shared-archive-resources
bookkeeper-server-4.7.0-SNAPSHOT-javadoc.jar checkstyle-checker.xml generated-test-sources maven-status
bookkeeper-server-4.7.0-SNAPSHOT-shaded.jar checkstyle-result.xml invoker test-classes
bookkeeper-server-4.7.0-SNAPSHOT-shaded-tests.jar checkstyle-suppressions.xml javadoc-bundle-options
There was a problem hiding this comment.
Maybe this is why I was seeing the shading problem also.
There was a problem hiding this comment.
the result returned by ls is unsorted. that means it will have different behaviors on different bash (mac vs linux).
a deterministic way is to ls ... | sort | tail -1
There was a problem hiding this comment.
I assumed ls would lexically sort by default, but I guess that would make ls very expensive on large directories. In any case, I think I'll either check that there's a version before the ".jar" or maybe directly use the shaded. This may fix #773 also.
bin/bookkeeper tries to figure out the location of the bookkeeper-server jar, so that a locally built jar can be run in place. However, this has been broken since we've been generating a sources jar. This patch excludes the sources jar from the search.
Rather than listing all jars, and excluding some exceptions, search for jars which match a specific pattern, namely: bookkeeper-server-<version>.jar Where <version> may include numbers, dots and optionally -SNAPSHOT.
ivankelly
force-pushed
the
fix-local-bookie
branch
from
a7a0bb1
to
4e4bc5e
Compare
ivankelly
changed the title
|
@merlimat I'll create another PR |
Rather than listing all jars, and excluding some exceptions, search for jars which match a specific pattern, namely: bookkeeper-server-[version].jar Where [version] may include numbers, dots and optionally -SNAPSHOT. Author: Ivan Kelly <ivank@apache.org> Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>, Matteo Merli <mmerli@apache.org> This closes #772 from ivankelly/fix-local-bookie (cherry picked from commit 90a82c2) Signed-off-by: Ivan Kelly <ivank@apache.org>
Rather than listing all jars, and excluding some exceptions, search for
jars which match a specific pattern, namely:
bookkeeper-server-[version].jar
Where [version] may include numbers, dots and optionally -SNAPSHOT.