[ https://issues.apache.org/jira/browse/ZOOKEEPER-2905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16183435#comment-16183435 ]
ASF GitHub Bot commented on ZOOKEEPER-2905: ------------------------------------------- Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/382 Thanks! > 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 > Fix For: 3.5.4, 3.6.0, 3.4.11 > > > 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)