You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by mfenes <gi...@git.apache.org> on 2018/02/20 16:27:08 UTC
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
GitHub user mfenes opened a pull request:
https://github.com/apache/zookeeper/pull/469
ZOOKEEPER-2983: Print the classpath when running compile and test ant targets
ZOOKEEPER-2983: Print the classpath when running compile and test ant targets
I've added 2 new ant targets to print the compile and test classpath in a formatted way.
Printing the classpath helps to verify that we have only the intended classes, jars on the classpath, e.g. clover.jar is included only when running coverage tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mfenes/zookeeper ZOOKEEPER-2983
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/469.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #469
----
commit 44b1f07cd129bc1f77fcb27bb509d9890d10207f
Author: Mark Fenes <mf...@...>
Date: 2018-02-20T14:12:52Z
ZOOKEEPER-2983: Print the classpath when running compile and test ant targets
----
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r169435681
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
+ <pathconvert pathsep="${line.separator}| |-- " property="echo.compile.classpath" refid="java.classpath"/>
--- End diff --
I found a "one-liner" for this here: http://ant.apache.org/manual/using.html#pathshortcut
For example:
```xml
<echo>${toString:java.classpath}</echo>
```
What do you think?
---
[GitHub] zookeeper issue #469: ZOOKEEPER-2983: Print the classpath when running compi...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes commented on the issue:
https://github.com/apache/zookeeper/pull/469
@phunt @anmolnar Should I close this PR then?
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r179420191
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
--- End diff --
@afine ok, I've replaced "_" with "-" in the new ant target names.
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes closed the pull request at:
https://github.com/apache/zookeeper/pull/469
---
[GitHub] zookeeper issue #469: ZOOKEEPER-2983: Print the classpath when running compi...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/469
@phunt This approach is a bit prettier and might help diagnose classpath-related problems on build servers, but I can live without it. Given that we're migrating to Maven, this patch wouldn't make any difference now.
---
[GitHub] zookeeper issue #469: ZOOKEEPER-2983: Print the classpath when running compi...
Posted by phunt <gi...@git.apache.org>.
Github user phunt commented on the issue:
https://github.com/apache/zookeeper/pull/469
Yes, please close it out. Thanks @mfenes , et. al.
---
[GitHub] zookeeper issue #469: ZOOKEEPER-2983: Print the classpath when running compi...
Posted by phunt <gi...@git.apache.org>.
Github user phunt commented on the issue:
https://github.com/apache/zookeeper/pull/469
@mfenes @anmolnar @afine - ant already has a -d and a -v option, why do we need to add additional? Is it really necessary that all the builds include this information? Why is -d/-v not sufficient?
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r169435199
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
--- End diff --
nit: I think most of our other ant targets use "-" instead of "_" between words.
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r170379824
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
--- End diff --
let's replace them for the sake of consistency
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r170379991
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
+ <pathconvert pathsep="${line.separator}| |-- " property="echo.compile.classpath" refid="java.classpath"/>
--- End diff --
fine with me
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r169695831
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
--- End diff --
True, my intention with the "_" was to indicate that these targets are purely utility targets i.e. it does not make too much sense to run them individually, unlike clean, compile, test-core-java etc. targets. If you don't like this idea, I can replace the "_"-s with "-"-s if you wish.
---
[GitHub] zookeeper issue #469: ZOOKEEPER-2983: Print the classpath when running compi...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/469
@phunt @afine I believe @mfenes finished all the requested changes on this patch. Shall we merge it?
---
[GitHub] zookeeper pull request #469: ZOOKEEPER-2983: Print the classpath when runnin...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/469#discussion_r169691663
--- Diff: build.xml ---
@@ -1861,4 +1861,18 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<delete dir="${build.dir.eclipse}" />
</target>
+ <target name="print_compile_classpath">
+ <pathconvert pathsep="${line.separator}| |-- " property="echo.compile.classpath" refid="java.classpath"/>
--- End diff --
The difference between the two is that the one I used prints the paths in a pretty printed way, while the "one-liner" prints all paths continuously which is hard to read.
---