You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by jpeach <gi...@git.apache.org> on 2018/07/10 16:57:42 UTC
[GitHub] zookeeper pull request #565: ZOOKEEPER-3067: Optionally disable client envir...
GitHub user jpeach opened a pull request:
https://github.com/apache/zookeeper/pull/565
ZOOKEEPER-3067: Optionally disable client environment logging.
Although logging the client environment at initialization can be
helpful for filing bug reports, it tends not to be that helpful
for internal applications. It also introduces a dependency on UID
lookups (via getlogin) that application otherwise may not desire.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jpeach/zookeeper ZOOKEEPER-3067
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/565.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 #565
----
commit 28abb101236e2247dbef7d6b8c4a45874e2c536d
Author: James Peach <jp...@...>
Date: 2018-06-20T17:11:29Z
ZOOKEEPER-3067: Optionally disable client environment logging.
Although logging the client environment at initialization can be
helpful for filing bug reports, it tends not to be that helpful
for internal applications. It also introduces a dependency on UID
lookups (via getlogin) that application otherwise may not desire.
----
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/zookeeper/pull/565
Unrelated test failure https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1936/testReport/org.apache.zookeeper.server.quorum/QuorumPeerMainTest/testFailedTxnAsPartOfQuorumLoss/
---
[GitHub] zookeeper pull request #565: ZOOKEEPER-3067: Optionally disable client envir...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/565
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/zookeeper/pull/565
Added a test. I needed the following patch to get the tests to build on Fedora 28:
```
diff --git a/src/c/Makefile.am b/src/c/Makefile.am
index a81e3da2..0230419b 100644
--- a/src/c/Makefile.am
+++ b/src/c/Makefile.am
@@ -120,14 +120,14 @@ check_PROGRAMS = zktest-st
TESTS_ENVIRONMENT = ZKROOT=${srcdir}/../.. \
CLASSPATH=$$CLASSPATH:$$CLOVER_HOME/lib/clover.jar
nodist_zktest_st_SOURCES = $(TEST_SOURCES)
-zktest_st_LDADD = libzkst.la libhashtable.la $(CPPUNIT_LIBS)
+zktest_st_LDADD = libzkst.la libhashtable.la $(CPPUNIT_LIBS) -ldl
zktest_st_CXXFLAGS = -DUSE_STATIC_LIB $(CPPUNIT_CFLAGS) $(USEIPV6) $(SOLARIS_CPPFLAGS)
zktest_st_LDFLAGS = -shared $(SYMBOL_WRAPPERS) $(SOLARIS_LIB_LDFLAGS)
if WANT_SYNCAPI
check_PROGRAMS += zktest-mt
nodist_zktest_mt_SOURCES = $(TEST_SOURCES) tests/PthreadMocks.cc
- zktest_mt_LDADD = libzkmt.la libhashtable.la -lpthread $(CPPUNIT_LIBS)
+ zktest_mt_LDADD = libzkmt.la libhashtable.la -lpthread $(CPPUNIT_LIBS) -ldl
zktest_mt_CXXFLAGS = -DUSE_STATIC_LIB -DTHREADED $(CPPUNIT_CFLAGS) $(USEIPV6)
if SOLARIS
SHELL_SYMBOL_WRAPPERS_MT = cat ${srcdir}/tests/wrappers-mt.opt
diff --git a/src/c/configure.ac b/src/c/configure.ac
index 9811618d..e905aa4d 100644
--- a/src/c/configure.ac
+++ b/src/c/configure.ac
@@ -34,7 +34,7 @@ if test "$with_cppunit" = "no" ; then
CPPUNIT_INCLUDE=
CPPUNIT_LIBS=
else
- AM_PATH_CPPUNIT(1.10.2)
+ PKG_CHECK_MODULES([CPPUNIT], [cppunit], [HAVE_CPPUNIT=yes])
fi
if test "$CALLER" = "ANT" ; then
@@ -52,6 +52,7 @@ AM_PROG_CC_C_O
AC_PROG_CXX
AC_PROG_INSTALL
AC_PROG_LN_S
+AM_PROG_AR
# AC_DISABLE_SHARED
AC_PROG_LIBTOOL
```
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/zookeeper/pull/565
Looks like CI failed on `org.apache.zookeeper.test.FLETest.testTripleElection`. That seems like it should be unrelated to this change.
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/zookeeper/pull/565
Is this ready to merge?
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by breed <gi...@git.apache.org>.
Github user breed commented on the issue:
https://github.com/apache/zookeeper/pull/565
+1 let's make this happen
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/565
Committed to master branch.
Thanks @jpeach !
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/zookeeper/pull/565
AFAICT the test failure is from findbugs, which shouldn't have been affected in this commit. I couldn't find what it was happy about in the test output.
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/565
The code looks good to me, the default behavior is the same as before.
---
[GitHub] zookeeper issue #565: ZOOKEEPER-3067: Optionally disable client environment ...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/565
@jpeach Issue was on master, but it's already fixed.
Please trigger another build by amending your latest commit.
---