[ https://issues.apache.org/jira/browse/ZOOKEEPER-2905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16179949#comment-16179949 ]
Hadoop QA commented on ZOOKEEPER-2905: -------------------------------------- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1043//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1043//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1043//console This message is automatically generated. > Don't include `config.h` in `zookeeper.h` > ----------------------------------------- > > Key: ZOOKEEPER-2905 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2905 > Project: ZooKeeper > Issue Type: Bug > Environment: Linux-ish environments. > Reporter: Andrew Schwartzmeyer > Assignee: Andrew Schwartzmeyer > > In ZOOKEEPER-2841 I fixed the inclusion of project-specific porting changes > that were included in the public headers, which then broke upstream projects > (in my case, Mesos). > Unfortunately, I inadvertently created the exact same problem for Linux (or > really any system that uses Autotools), and it wasn't evident until the build > was coupled with another project with the same problem. More specifically, > when including ZooKeeper (with my changes) in Mesos, and including Google's > Glog in Mesos, and building both with Autotools (which we also support), both > packages define the pre-processor macro {{PACKAGE_VERSION}}, and so so > publicly. This is defined in {{config.h}} by Autotools, and is not a problem > _unless included publicly_. > When refactoring, I saw two includes in {{zookeeper.h}} that instead of being > guarded by e.g. {{#ifdef HAVE_SYS_SOCKET_H}} were guarded by {{#ifndef > WIN32}}. Without realizing that I would create the exact same problem I was > elsewhere fixing, I erroneously added {{#include "config.h"}} and guarded the > includes "properly." But there is _very good reasons_ not to do this > (explained above). > The patch to fix this is simple: > {noformat} > diff --git a/src/c/include/zookeeper.h b/src/c/include/zookeeper.h > index d20e70af4..b0bb09e3f 100644 > --- a/src/c/include/zookeeper.h > +++ b/src/c/include/zookeeper.h > @@ -21,13 +21,9 @@ > #include <stdlib.h> > -#include "config.h" > - > -#ifdef HAVE_SYS_SOCKET_H > +/* we must not include config.h as a public header */ > +#ifndef WIN32 > #include <sys/socket.h> > -#endif > - > -#ifdef HAVE_SYS_TIME_H > #include <sys/time.h> > #endif > diff --git a/src/c/src/zookeeper.c b/src/c/src/zookeeper.c > index 220c57dc4..9b837f227 100644 > --- a/src/c/src/zookeeper.c > +++ b/src/c/src/zookeeper.c > @@ -24,6 +24,7 @@ > #define USE_IPV6 > #endif > +#include "config.h" > #include <zookeeper.h> > #include <zookeeper.jute.h> > #include <proto.h> > {noformat} > I am opening pull requests in a few minutes to have this applied to branch > 3.4 and 3.5. > I'm sorry! -- This message was sent by Atlassian JIRA (v6.4.14#64029)