Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-2841

ZooKeeper public include files leak porting changes

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: c client
    • Labels:
    • Environment:

      Windows 10 with Visual Studio 2017

      Description

      The fundamental problem is that the port of the C client to Windows is now close to six years old, with very few updates. This port leaks a lot of changes that should be internal to ZooKeeper, and many of those changes are simply no longer relevant. The correct thing to do is attempt to refactor the Windows port for new versions of ZooKeeper, removing dead/unneeded porting code, and moving dangerous porting code to C files instead of public headers.

      Two primary examples of this problem are ZOOKEEPER-2491 and MESOS-7541.

      The first issue stems from this ancient porting code:

      #define snprintf _snprintf
      

      in winconfig.h. Newer versions of Windows C libraries define snprintf as a function, and so it cannot be redefined.

      The second issue comes from this undocumented change:

      #undef AF_INET6
      

      again in winconfig.h which breaks any library that uses IPv6 and winsock2.h.

      Furthermore, the inclusion of the following defines and headers causes terrible problems for consuming libraries, as they leak into ZooKeeper's public headers:

      #define _CRT_SECURE_NO_WARNINGS
      #define WIN32_LEAN_AND_MEAN
      #include <Windows.h>
      #include <Winsock2.h>
      #include <winstdint.h>
      #include <process.h>
      #include <ws2tcpip.h>
      

      Depending on the order that a project includes or compiles files, this may or may not cause WIN32_LEAN_AND_MEAN to become unexpectedly defined, and windows.h to be unexpectedly included. This problem is exacberated by the fact that the winsock2.h and windows.h headers are order-dependent (if you read up on this, you'll see that defining WIN32_LEAN_AND_MEAN was meant to work-around this).

      Going forward, porting changes should live next to where they are used, preferably in source files, not header files, so they remain contained.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user andschwa opened a pull request:

          https://github.com/apache/zookeeper/pull/306

          ZOOKEEPER-2841: ZooKeeper public include files leak porting changes

          This PR includes the patches ZOOKEEPER-2756(https://issues.apache.org/jira/browse/ZOOKEEPER-2756) and ZOOKEEPER-2841(https://issues.apache.org/jira/browse/ZOOKEEPER-2841), and can supplant PR #255.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/andschwa/zookeeper ZOOKEEPER-2841

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zookeeper/pull/306.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 #306


          commit 187ce8acc1707a0dd20752b624a3fa5648706f97
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-04-10T23:12:40Z

          ZOOKEEPER-2756: Add CMake build system for better cross-platform support

          This notably lacks Solaris and libtool support.

          Almost everything else from Autotools has been ported,
          including header/function/library checks, and all targets
          (zookeeper, hashtable, cli, load_gen, and tests).

          Both Linux and Windows are supported.

          The primary work involved (other than the writing of `CMakeLists.txt`)
          was transitioning the hand-written `winconfig.h` to an
          auto-generated `config.h` file, and guarding code with `#ifdef
          HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after
          the Autotools config file so that the feature guards share the same
          names.

          While `load_gen.c` looks at first glance as if it were ported to Windows,
          it never actually was, so the erroneous `#include "win32port.h"` was
          removed, and the target is not built on Windows.

          There are existent warnings which this patch did not attempt to fix.

          A guard was placed around `#define snprintf` in order to enable
          compiling with modern versions of MSVC.

          Fixed DLL_EXPORT and USE_STATIC_LIB redefinition.

          As this commit deprecates (and breaks) the previously existing Visual
          Studio solutions and projects, they've been removed.

          Building tests on Windows is still not supported.

          commit 22a378ae2a7c30657326b6a602b62116ab7c050a
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-07-07T22:35:37Z

          ZOOKEEPER-2841: ZooKeeper public include files leak porting changes

          This patch refactors the Windows port of the C client. Notably, it moves
          as much porting code as possible out of the publicly included
          `winconfig.h` header and into the relevant source files. This also
          removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes
          problems for upstream libraries by removing `#undef AF_INET6` and
          `#include <windows.h>`.

          Future work should resolve the `ACL` / `ZKACL` conflict, and potentially
          remove `include/winconfig.h` altogether.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user andschwa opened a pull request: https://github.com/apache/zookeeper/pull/306 ZOOKEEPER-2841 : ZooKeeper public include files leak porting changes This PR includes the patches ZOOKEEPER-2756 ( https://issues.apache.org/jira/browse/ZOOKEEPER-2756 ) and ZOOKEEPER-2841 ( https://issues.apache.org/jira/browse/ZOOKEEPER-2841 ), and can supplant PR #255. You can merge this pull request into a Git repository by running: $ git pull https://github.com/andschwa/zookeeper ZOOKEEPER-2841 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/306.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 #306 commit 187ce8acc1707a0dd20752b624a3fa5648706f97 Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-04-10T23:12:40Z ZOOKEEPER-2756 : Add CMake build system for better cross-platform support This notably lacks Solaris and libtool support. Almost everything else from Autotools has been ported, including header/function/library checks, and all targets (zookeeper, hashtable, cli, load_gen, and tests). Both Linux and Windows are supported. The primary work involved (other than the writing of `CMakeLists.txt`) was transitioning the hand-written `winconfig.h` to an auto-generated `config.h` file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after the Autotools config file so that the feature guards share the same names. While `load_gen.c` looks at first glance as if it were ported to Windows, it never actually was, so the erroneous `#include "win32port.h"` was removed, and the target is not built on Windows. There are existent warnings which this patch did not attempt to fix. A guard was placed around `#define snprintf` in order to enable compiling with modern versions of MSVC. Fixed DLL_EXPORT and USE_STATIC_LIB redefinition. As this commit deprecates (and breaks) the previously existing Visual Studio solutions and projects, they've been removed. Building tests on Windows is still not supported. commit 22a378ae2a7c30657326b6a602b62116ab7c050a Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-07-07T22:35:37Z ZOOKEEPER-2841 : ZooKeeper public include files leak porting changes This patch refactors the Windows port of the C client. Notably, it moves as much porting code as possible out of the publicly included `winconfig.h` header and into the relevant source files. This also removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes problems for upstream libraries by removing `#undef AF_INET6` and `#include <windows.h>`. Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove `include/winconfig.h` altogether.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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/870//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/870//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/870//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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/870//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/870//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/870//console This message is automatically generated.
          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 Is this [failing test] ( https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/870/testReport/org.apache.zookeeper.server.quorum/ReconfigDuringLeaderSyncTest/testDuringLeaderSync/ ) my fault? It's... odd.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm I'm not sure how you guys like to do dependent patches. ZOOKEEPER-2756 and ZOOKEEPER-2841 should both be taken to resolve problems using the C client in other projects (e.g. Mesos 😉), and the latter depends on the former. I've confirmed that 2756 (first commit) allows us to build with Visual Studio 2017 (and on Linux, because CMake!), and that 2841 (second commit) resolves MESOS-7541(https://issues.apache.org/jira/browse/MESOS-7541), and should eliminate a lot of problems that projects which include ZooKeeper headers can encounter on Windows. That said, the two patches are clearly two very separate pieces of work.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm I'm not sure how you guys like to do dependent patches. ZOOKEEPER-2756 and ZOOKEEPER-2841 should both be taken to resolve problems using the C client in other projects (e.g. Mesos 😉), and the latter depends on the former. I've confirmed that 2756 (first commit) allows us to build with Visual Studio 2017 (and on Linux, because CMake!), and that 2841 (second commit) resolves MESOS-7541 ( https://issues.apache.org/jira/browse/MESOS-7541 ), and should eliminate a lot of problems that projects which include ZooKeeper headers can encounter on Windows. That said, the two patches are clearly two very separate pieces of work.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          >> Is this failing test my fault? It's... odd.

          No it's not your fault. This is a known flaky test, and I will have a patch for this soon.

          >> I'm not sure how you guys like to do dependent patches.

          It's OK to include ZOOKEEPER-2841 in ZOOKEEPER-2756; otherwise it might take years to get the improvement in because the lack of love on the C client. While we are at this, let's get both in.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 >> Is this failing test my fault? It's... odd. No it's not your fault. This is a known flaky test, and I will have a patch for this soon. >> I'm not sure how you guys like to do dependent patches. It's OK to include ZOOKEEPER-2841 in ZOOKEEPER-2756 ; otherwise it might take years to get the improvement in because the lack of love on the C client. While we are at this, let's get both in.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126290847

          — Diff: src/c/CMakeLists.txt —
          @@ -0,0 +1,220 @@
          +# Licensed to the Apache Software Foundation (ASF) under one
          +# or more contributor license agreements. See the NOTICE file
          +# distributed with this work for additional information
          +# regarding copyright ownership. The ASF licenses this file
          +# to you under the Apache License, Version 2.0 (the
          +# "License"); you may not use this file except in compliance
          +# with the License. You may obtain a copy of the License at
          +#
          +# http://www.apache.org/licenses/LICENSE-2.0
          +#
          +# Unless required by applicable law or agreed to in writing, software
          +# distributed under the License is distributed on an "AS IS" BASIS,
          +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          +# See the License for the specific language governing permissions and
          +# limitations under the License.
          +
          +cmake_minimum_required(VERSION 3.6)
          +
          +project(zookeeper VERSION 3.5.2)
          — End diff –

          The version here should be 3.5.3. I think you got 3.5.2 because when you start working on this, 3.5.3 was not released.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126290847 — Diff: src/c/CMakeLists.txt — @@ -0,0 +1,220 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +cmake_minimum_required(VERSION 3.6) + +project(zookeeper VERSION 3.5.2) — End diff – The version here should be 3.5.3. I think you got 3.5.2 because when you start working on this, 3.5.3 was not released.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm Awesome! The patches seem to be working well, and I think we don't need to worry about maintaining compatibility with, say, Visual Studio 2008, in new versions of the ZooKeeper client. Old versions of the client are still available for those in need, and it's more important that current versions of VS/MSVC are supported.

          Thanks for your help in this!

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm Awesome! The patches seem to be working well, and I think we don't need to worry about maintaining compatibility with, say, Visual Studio 2008, in new versions of the ZooKeeper client. Old versions of the client are still available for those in need, and it's more important that current versions of VS/MSVC are supported. Thanks for your help in this!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126290881

          — Diff: src/c/include/winconfig.h —
          @@ -1,196 +1,15 @@
          -/* Define to 1 if you have the <arpa/inet.h> header file. */
          -#undef HAVE_ARPA_INET_H
          -
          -/* Define to 1 if you have the <dlfcn.h> header file. */
          -#undef HAVE_DLFCN_H
          -
          -/* Define to 1 if you have the <fcntl.h> header file. */
          -#undef HAVE_FCNTL_H
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1
          -
          -/* Define to 1 if you have the `getcwd' function. */
          -#undef HAVE_GETCWD
          -
          -/* Define to 1 if you have the `gethostbyname' function. */
          -#undef HAVE_GETHOSTBYNAME
          -
          -/* Define to 1 if you have the `gethostname' function. */
          -#define HAVE_GETHOSTNAME 1
          -
          -/* Define to 1 if you have the `getlogin' function. */
          -#undef HAVE_GETLOGIN
          -
          -/* Define to 1 if you have the `getpwuid_r' function. */
          -#undef HAVE_GETPWUID_R
          -
          -/* Define to 1 if you have the `gettimeofday' function. */
          -#undef HAVE_GETTIMEOFDAY
          -
          -/* Define to 1 if you have the `getuid' function. */
          -#undef HAVE_GETUID
          -
          -/* Define to 1 if you have the <inttypes.h> header file. */
          -#undef HAVE_INTTYPES_H
          -
          -/* Define to 1 if you have the `memmove' function. */
          -#undef HAVE_MEMMOVE
          -
          -/* Define to 1 if you have the <memory.h> header file. */
          -#undef HAVE_MEMORY_H
          -
          -/* Define to 1 if you have the `memset' function. */
          -#undef HAVE_MEMSET
          -
          -/* Define to 1 if you have the <netdb.h> header file. */
          -#undef HAVE_NETDB_H
          -
          -/* Define to 1 if you have the <netinet/in.h> header file. */
          -#undef HAVE_NETINET_IN_H
          -
          -/* Define to 1 if you have the `poll' function. */
          -#undef HAVE_POLL
          -
          -/* Define to 1 if you have the `socket' function. */
          -#undef HAVE_SOCKET
          -
          -/* Define to 1 if you have the <stdint.h> header file. */
          -#undef HAVE_STDINT_H
          -
          -/* Define to 1 if you have the <stdlib.h> header file. */
          -#define HAVE_STDLIB_H 1
          -
          -/* Define to 1 if you have the `strchr' function. */
          -#define HAVE_STRCHR 1
          -
          -/* Define to 1 if you have the `strdup' function. */
          -#define HAVE_STRDUP 1
          -
          -/* Define to 1 if you have the `strerror' function. */
          -#define HAVE_STRERROR 1
          -
          -/* Define to 1 if you have the <strings.h> header file. */
          -#undef HAVE_STRINGS_H
          -
          -/* Define to 1 if you have the <string.h> header file. */
          -#undef HAVE_STRING_H
          -
          -/* Define to 1 if you have the `strtol' function. */
          -#undef HAVE_STRTOL
          -
          -/* Define to 1 if you have the <sys/socket.h> header file. */
          -#undef HAVE_SYS_SOCKET_H
          -
          -/* Define to 1 if you have the <sys/stat.h> header file. */
          -#undef HAVE_SYS_STAT_H
          -
          -/* Define to 1 if you have the <sys/time.h> header file. */
          -#undef HAVE_SYS_TIME_H
          -
          -/* Define to 1 if you have the <sys/types.h> header file. */
          -#undef HAVE_SYS_TYPES_H
          -
          -/* Define to 1 if you have the <sys/utsname.h> header file. */
          -#undef HAVE_SYS_UTSNAME_H
          -
          -/* Define to 1 if you have the <unistd.h> header file. */
          -#undef HAVE_UNISTD_H
          -
          -/* Define to the sub-directory in which libtool stores uninstalled libraries.

          • */
            -#define LT_OBJDIR
            -
            -/* Define to 1 if your C compiler doesn't accept -c and -o together. */
            -/* #undef NO_MINUS_C_MINUS_O */
            -
            -/* Name of package */
            -#define PACKAGE "c-client-src"
            -
            -/* Define to the address where bug reports for this package should be sent. */
            -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org"
            -
            -/* Define to the full name of this package. */
            -#define PACKAGE_NAME "zookeeper C client"
            -
            -/* Define to the full name and version of this package. */
            -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32"
            -
            -/* Define to the one symbol short name of this package. */
            -#define PACKAGE_TARNAME "c-client-src"
            -
            -/* Define to the home page for this package. */
            -#define PACKAGE_URL ""
            -
            -/* Define to the version of this package. */
            -#define PACKAGE_VERSION "3.5.0"
              • End diff –

          This 3.5.0 indicates the base version of this file you were using when start the patch is pretty old. Could you rebase the pull request on latest master?

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126290881 — Diff: src/c/include/winconfig.h — @@ -1,196 +1,15 @@ -/* Define to 1 if you have the <arpa/inet.h> header file. */ -#undef HAVE_ARPA_INET_H - -/* Define to 1 if you have the <dlfcn.h> header file. */ -#undef HAVE_DLFCN_H - -/* Define to 1 if you have the <fcntl.h> header file. */ -#undef HAVE_FCNTL_H - -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1 - -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1 - -/* Define to 1 if you have the `getcwd' function. */ -#undef HAVE_GETCWD - -/* Define to 1 if you have the `gethostbyname' function. */ -#undef HAVE_GETHOSTBYNAME - -/* Define to 1 if you have the `gethostname' function. */ -#define HAVE_GETHOSTNAME 1 - -/* Define to 1 if you have the `getlogin' function. */ -#undef HAVE_GETLOGIN - -/* Define to 1 if you have the `getpwuid_r' function. */ -#undef HAVE_GETPWUID_R - -/* Define to 1 if you have the `gettimeofday' function. */ -#undef HAVE_GETTIMEOFDAY - -/* Define to 1 if you have the `getuid' function. */ -#undef HAVE_GETUID - -/* Define to 1 if you have the <inttypes.h> header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `memmove' function. */ -#undef HAVE_MEMMOVE - -/* Define to 1 if you have the <memory.h> header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the `memset' function. */ -#undef HAVE_MEMSET - -/* Define to 1 if you have the <netdb.h> header file. */ -#undef HAVE_NETDB_H - -/* Define to 1 if you have the <netinet/in.h> header file. */ -#undef HAVE_NETINET_IN_H - -/* Define to 1 if you have the `poll' function. */ -#undef HAVE_POLL - -/* Define to 1 if you have the `socket' function. */ -#undef HAVE_SOCKET - -/* Define to 1 if you have the <stdint.h> header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the <stdlib.h> header file. */ -#define HAVE_STDLIB_H 1 - -/* Define to 1 if you have the `strchr' function. */ -#define HAVE_STRCHR 1 - -/* Define to 1 if you have the `strdup' function. */ -#define HAVE_STRDUP 1 - -/* Define to 1 if you have the `strerror' function. */ -#define HAVE_STRERROR 1 - -/* Define to 1 if you have the <strings.h> header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the <string.h> header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the `strtol' function. */ -#undef HAVE_STRTOL - -/* Define to 1 if you have the <sys/socket.h> header file. */ -#undef HAVE_SYS_SOCKET_H - -/* Define to 1 if you have the <sys/stat.h> header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the <sys/time.h> header file. */ -#undef HAVE_SYS_TIME_H - -/* Define to 1 if you have the <sys/types.h> header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the <sys/utsname.h> header file. */ -#undef HAVE_SYS_UTSNAME_H - -/* Define to 1 if you have the <unistd.h> header file. */ -#undef HAVE_UNISTD_H - -/* Define to the sub-directory in which libtool stores uninstalled libraries. */ -#define LT_OBJDIR - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -/* #undef NO_MINUS_C_MINUS_O */ - -/* Name of package */ -#define PACKAGE "c-client-src" - -/* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org" - -/* Define to the full name of this package. */ -#define PACKAGE_NAME "zookeeper C client" - -/* Define to the full name and version of this package. */ -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32" - -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "c-client-src" - -/* Define to the home page for this package. */ -#define PACKAGE_URL "" - -/* Define to the version of this package. */ -#define PACKAGE_VERSION "3.5.0" End diff – This 3.5.0 indicates the base version of this file you were using when start the patch is pretty old. Could you rebase the pull request on latest master?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126291131

          — Diff: src/c/include/winconfig.h —
          @@ -1,196 +1,15 @@
          -/* Define to 1 if you have the <arpa/inet.h> header file. */
          -#undef HAVE_ARPA_INET_H
          -
          -/* Define to 1 if you have the <dlfcn.h> header file. */
          -#undef HAVE_DLFCN_H
          -
          -/* Define to 1 if you have the <fcntl.h> header file. */
          -#undef HAVE_FCNTL_H
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1
          -
          -/* Define to 1 if you have the `getcwd' function. */
          -#undef HAVE_GETCWD
          -
          -/* Define to 1 if you have the `gethostbyname' function. */
          -#undef HAVE_GETHOSTBYNAME
          -
          -/* Define to 1 if you have the `gethostname' function. */
          -#define HAVE_GETHOSTNAME 1
          -
          -/* Define to 1 if you have the `getlogin' function. */
          -#undef HAVE_GETLOGIN
          -
          -/* Define to 1 if you have the `getpwuid_r' function. */
          -#undef HAVE_GETPWUID_R
          -
          -/* Define to 1 if you have the `gettimeofday' function. */
          -#undef HAVE_GETTIMEOFDAY
          -
          -/* Define to 1 if you have the `getuid' function. */
          -#undef HAVE_GETUID
          -
          -/* Define to 1 if you have the <inttypes.h> header file. */
          -#undef HAVE_INTTYPES_H
          -
          -/* Define to 1 if you have the `memmove' function. */
          -#undef HAVE_MEMMOVE
          -
          -/* Define to 1 if you have the <memory.h> header file. */
          -#undef HAVE_MEMORY_H
          -
          -/* Define to 1 if you have the `memset' function. */
          -#undef HAVE_MEMSET
          -
          -/* Define to 1 if you have the <netdb.h> header file. */
          -#undef HAVE_NETDB_H
          -
          -/* Define to 1 if you have the <netinet/in.h> header file. */
          -#undef HAVE_NETINET_IN_H
          -
          -/* Define to 1 if you have the `poll' function. */
          -#undef HAVE_POLL
          -
          -/* Define to 1 if you have the `socket' function. */
          -#undef HAVE_SOCKET
          -
          -/* Define to 1 if you have the <stdint.h> header file. */
          -#undef HAVE_STDINT_H
          -
          -/* Define to 1 if you have the <stdlib.h> header file. */
          -#define HAVE_STDLIB_H 1
          -
          -/* Define to 1 if you have the `strchr' function. */
          -#define HAVE_STRCHR 1
          -
          -/* Define to 1 if you have the `strdup' function. */
          -#define HAVE_STRDUP 1
          -
          -/* Define to 1 if you have the `strerror' function. */
          -#define HAVE_STRERROR 1
          -
          -/* Define to 1 if you have the <strings.h> header file. */
          -#undef HAVE_STRINGS_H
          -
          -/* Define to 1 if you have the <string.h> header file. */
          -#undef HAVE_STRING_H
          -
          -/* Define to 1 if you have the `strtol' function. */
          -#undef HAVE_STRTOL
          -
          -/* Define to 1 if you have the <sys/socket.h> header file. */
          -#undef HAVE_SYS_SOCKET_H
          -
          -/* Define to 1 if you have the <sys/stat.h> header file. */
          -#undef HAVE_SYS_STAT_H
          -
          -/* Define to 1 if you have the <sys/time.h> header file. */
          -#undef HAVE_SYS_TIME_H
          -
          -/* Define to 1 if you have the <sys/types.h> header file. */
          -#undef HAVE_SYS_TYPES_H
          -
          -/* Define to 1 if you have the <sys/utsname.h> header file. */
          -#undef HAVE_SYS_UTSNAME_H
          -
          -/* Define to 1 if you have the <unistd.h> header file. */
          -#undef HAVE_UNISTD_H
          -
          -/* Define to the sub-directory in which libtool stores uninstalled libraries.

          • */
            -#define LT_OBJDIR
            -
            -/* Define to 1 if your C compiler doesn't accept -c and -o together. */
            -/* #undef NO_MINUS_C_MINUS_O */
            -
            -/* Name of package */
            -#define PACKAGE "c-client-src"
            -
            -/* Define to the address where bug reports for this package should be sent. */
            -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org"
            -
            -/* Define to the full name of this package. */
            -#define PACKAGE_NAME "zookeeper C client"
            -
            -/* Define to the full name and version of this package. */
            -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32"
            -
            -/* Define to the one symbol short name of this package. */
            -#define PACKAGE_TARNAME "c-client-src"
            -
            -/* Define to the home page for this package. */
            -#define PACKAGE_URL ""
            -
            -/* Define to the version of this package. */
            -#define PACKAGE_VERSION "3.5.0"
              • End diff –

          It should be freshly rebased as of yesterday, but yeah I didn't catch the version update.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126291131 — Diff: src/c/include/winconfig.h — @@ -1,196 +1,15 @@ -/* Define to 1 if you have the <arpa/inet.h> header file. */ -#undef HAVE_ARPA_INET_H - -/* Define to 1 if you have the <dlfcn.h> header file. */ -#undef HAVE_DLFCN_H - -/* Define to 1 if you have the <fcntl.h> header file. */ -#undef HAVE_FCNTL_H - -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1 - -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1 - -/* Define to 1 if you have the `getcwd' function. */ -#undef HAVE_GETCWD - -/* Define to 1 if you have the `gethostbyname' function. */ -#undef HAVE_GETHOSTBYNAME - -/* Define to 1 if you have the `gethostname' function. */ -#define HAVE_GETHOSTNAME 1 - -/* Define to 1 if you have the `getlogin' function. */ -#undef HAVE_GETLOGIN - -/* Define to 1 if you have the `getpwuid_r' function. */ -#undef HAVE_GETPWUID_R - -/* Define to 1 if you have the `gettimeofday' function. */ -#undef HAVE_GETTIMEOFDAY - -/* Define to 1 if you have the `getuid' function. */ -#undef HAVE_GETUID - -/* Define to 1 if you have the <inttypes.h> header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `memmove' function. */ -#undef HAVE_MEMMOVE - -/* Define to 1 if you have the <memory.h> header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the `memset' function. */ -#undef HAVE_MEMSET - -/* Define to 1 if you have the <netdb.h> header file. */ -#undef HAVE_NETDB_H - -/* Define to 1 if you have the <netinet/in.h> header file. */ -#undef HAVE_NETINET_IN_H - -/* Define to 1 if you have the `poll' function. */ -#undef HAVE_POLL - -/* Define to 1 if you have the `socket' function. */ -#undef HAVE_SOCKET - -/* Define to 1 if you have the <stdint.h> header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the <stdlib.h> header file. */ -#define HAVE_STDLIB_H 1 - -/* Define to 1 if you have the `strchr' function. */ -#define HAVE_STRCHR 1 - -/* Define to 1 if you have the `strdup' function. */ -#define HAVE_STRDUP 1 - -/* Define to 1 if you have the `strerror' function. */ -#define HAVE_STRERROR 1 - -/* Define to 1 if you have the <strings.h> header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the <string.h> header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the `strtol' function. */ -#undef HAVE_STRTOL - -/* Define to 1 if you have the <sys/socket.h> header file. */ -#undef HAVE_SYS_SOCKET_H - -/* Define to 1 if you have the <sys/stat.h> header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the <sys/time.h> header file. */ -#undef HAVE_SYS_TIME_H - -/* Define to 1 if you have the <sys/types.h> header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the <sys/utsname.h> header file. */ -#undef HAVE_SYS_UTSNAME_H - -/* Define to 1 if you have the <unistd.h> header file. */ -#undef HAVE_UNISTD_H - -/* Define to the sub-directory in which libtool stores uninstalled libraries. */ -#define LT_OBJDIR - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -/* #undef NO_MINUS_C_MINUS_O */ - -/* Name of package */ -#define PACKAGE "c-client-src" - -/* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org" - -/* Define to the full name of this package. */ -#define PACKAGE_NAME "zookeeper C client" - -/* Define to the full name and version of this package. */ -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32" - -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "c-client-src" - -/* Define to the home page for this package. */ -#define PACKAGE_URL "" - -/* Define to the version of this package. */ -#define PACKAGE_VERSION "3.5.0" End diff – It should be freshly rebased as of yesterday, but yeah I didn't catch the version update.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Should we put the CMake files in a dedicated folder called "windows" or something like that? Otherwise we will have the CMakeList file, and the existing makefile and config files (which are not touched by this pull request) coexisting under the C client folder. This might create confusions for user because makefile can be generated from the CMakeList file, so the question of which makefile is the source of truth and so on...

          Ultimately we might drop existing makefiles in favor of the CMake so build files across every platform are generated by a single source of truth (cmakelist), but that's more scope of work (the ant build file has to be modified and the system requirements of ZK has to be updated to make cmake a dependency for source installations etc to avoid break existing workflows), so for now I think it's good to scope cmake so it only impact windows C client builds.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 Should we put the CMake files in a dedicated folder called "windows" or something like that? Otherwise we will have the CMakeList file, and the existing makefile and config files (which are not touched by this pull request) coexisting under the C client folder. This might create confusions for user because makefile can be generated from the CMakeList file, so the question of which makefile is the source of truth and so on... Ultimately we might drop existing makefiles in favor of the CMake so build files across every platform are generated by a single source of truth (cmakelist), but that's more scope of work (the ant build file has to be modified and the system requirements of ZK has to be updated to make cmake a dependency for source installations etc to avoid break existing workflows), so for now I think it's good to scope cmake so it only impact windows C client builds.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126291135

          — Diff: src/c/CMakeLists.txt —
          @@ -0,0 +1,220 @@
          +# Licensed to the Apache Software Foundation (ASF) under one
          +# or more contributor license agreements. See the NOTICE file
          +# distributed with this work for additional information
          +# regarding copyright ownership. The ASF licenses this file
          +# to you under the Apache License, Version 2.0 (the
          +# "License"); you may not use this file except in compliance
          +# with the License. You may obtain a copy of the License at
          +#
          +# http://www.apache.org/licenses/LICENSE-2.0
          +#
          +# Unless required by applicable law or agreed to in writing, software
          +# distributed under the License is distributed on an "AS IS" BASIS,
          +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          +# See the License for the specific language governing permissions and
          +# limitations under the License.
          +
          +cmake_minimum_required(VERSION 3.6)
          +
          +project(zookeeper VERSION 3.5.2)
          — End diff –

          I can fix it up in a few minutes here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126291135 — Diff: src/c/CMakeLists.txt — @@ -0,0 +1,220 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +cmake_minimum_required(VERSION 3.6) + +project(zookeeper VERSION 3.5.2) — End diff – I can fix it up in a few minutes here.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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/871//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/871//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/871//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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/871//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/871//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/871//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126292294

          — Diff: src/c/include/winconfig.h —
          @@ -1,196 +1,15 @@
          -/* Define to 1 if you have the <arpa/inet.h> header file. */
          -#undef HAVE_ARPA_INET_H
          -
          -/* Define to 1 if you have the <dlfcn.h> header file. */
          -#undef HAVE_DLFCN_H
          -
          -/* Define to 1 if you have the <fcntl.h> header file. */
          -#undef HAVE_FCNTL_H
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1
          -
          -/* Define to 1 if you have the `getcwd' function. */
          -#undef HAVE_GETCWD
          -
          -/* Define to 1 if you have the `gethostbyname' function. */
          -#undef HAVE_GETHOSTBYNAME
          -
          -/* Define to 1 if you have the `gethostname' function. */
          -#define HAVE_GETHOSTNAME 1
          -
          -/* Define to 1 if you have the `getlogin' function. */
          -#undef HAVE_GETLOGIN
          -
          -/* Define to 1 if you have the `getpwuid_r' function. */
          -#undef HAVE_GETPWUID_R
          -
          -/* Define to 1 if you have the `gettimeofday' function. */
          -#undef HAVE_GETTIMEOFDAY
          -
          -/* Define to 1 if you have the `getuid' function. */
          -#undef HAVE_GETUID
          -
          -/* Define to 1 if you have the <inttypes.h> header file. */
          -#undef HAVE_INTTYPES_H
          -
          -/* Define to 1 if you have the `memmove' function. */
          -#undef HAVE_MEMMOVE
          -
          -/* Define to 1 if you have the <memory.h> header file. */
          -#undef HAVE_MEMORY_H
          -
          -/* Define to 1 if you have the `memset' function. */
          -#undef HAVE_MEMSET
          -
          -/* Define to 1 if you have the <netdb.h> header file. */
          -#undef HAVE_NETDB_H
          -
          -/* Define to 1 if you have the <netinet/in.h> header file. */
          -#undef HAVE_NETINET_IN_H
          -
          -/* Define to 1 if you have the `poll' function. */
          -#undef HAVE_POLL
          -
          -/* Define to 1 if you have the `socket' function. */
          -#undef HAVE_SOCKET
          -
          -/* Define to 1 if you have the <stdint.h> header file. */
          -#undef HAVE_STDINT_H
          -
          -/* Define to 1 if you have the <stdlib.h> header file. */
          -#define HAVE_STDLIB_H 1
          -
          -/* Define to 1 if you have the `strchr' function. */
          -#define HAVE_STRCHR 1
          -
          -/* Define to 1 if you have the `strdup' function. */
          -#define HAVE_STRDUP 1
          -
          -/* Define to 1 if you have the `strerror' function. */
          -#define HAVE_STRERROR 1
          -
          -/* Define to 1 if you have the <strings.h> header file. */
          -#undef HAVE_STRINGS_H
          -
          -/* Define to 1 if you have the <string.h> header file. */
          -#undef HAVE_STRING_H
          -
          -/* Define to 1 if you have the `strtol' function. */
          -#undef HAVE_STRTOL
          -
          -/* Define to 1 if you have the <sys/socket.h> header file. */
          -#undef HAVE_SYS_SOCKET_H
          -
          -/* Define to 1 if you have the <sys/stat.h> header file. */
          -#undef HAVE_SYS_STAT_H
          -
          -/* Define to 1 if you have the <sys/time.h> header file. */
          -#undef HAVE_SYS_TIME_H
          -
          -/* Define to 1 if you have the <sys/types.h> header file. */
          -#undef HAVE_SYS_TYPES_H
          -
          -/* Define to 1 if you have the <sys/utsname.h> header file. */
          -#undef HAVE_SYS_UTSNAME_H
          -
          -/* Define to 1 if you have the <unistd.h> header file. */
          -#undef HAVE_UNISTD_H
          -
          -/* Define to the sub-directory in which libtool stores uninstalled libraries.

          • */
            -#define LT_OBJDIR
            -
            -/* Define to 1 if your C compiler doesn't accept -c and -o together. */
            -/* #undef NO_MINUS_C_MINUS_O */
            -
            -/* Name of package */
            -#define PACKAGE "c-client-src"
            -
            -/* Define to the address where bug reports for this package should be sent. */
            -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org"
            -
            -/* Define to the full name of this package. */
            -#define PACKAGE_NAME "zookeeper C client"
            -
            -/* Define to the full name and version of this package. */
            -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32"
            -
            -/* Define to the one symbol short name of this package. */
            -#define PACKAGE_TARNAME "c-client-src"
            -
            -/* Define to the home page for this package. */
            -#define PACKAGE_URL ""
            -
            -/* Define to the version of this package. */
            -#define PACKAGE_VERSION "3.5.0"
              • End diff –

          (Actually, this is old because `winconfig.h` is unmaintained. I just checked the tip of `master`, it's still 3.5.0 in this file. My changes make the set based off the single version variable in the CMake build file instead.)

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126292294 — Diff: src/c/include/winconfig.h — @@ -1,196 +1,15 @@ -/* Define to 1 if you have the <arpa/inet.h> header file. */ -#undef HAVE_ARPA_INET_H - -/* Define to 1 if you have the <dlfcn.h> header file. */ -#undef HAVE_DLFCN_H - -/* Define to 1 if you have the <fcntl.h> header file. */ -#undef HAVE_FCNTL_H - -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1 - -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1 - -/* Define to 1 if you have the `getcwd' function. */ -#undef HAVE_GETCWD - -/* Define to 1 if you have the `gethostbyname' function. */ -#undef HAVE_GETHOSTBYNAME - -/* Define to 1 if you have the `gethostname' function. */ -#define HAVE_GETHOSTNAME 1 - -/* Define to 1 if you have the `getlogin' function. */ -#undef HAVE_GETLOGIN - -/* Define to 1 if you have the `getpwuid_r' function. */ -#undef HAVE_GETPWUID_R - -/* Define to 1 if you have the `gettimeofday' function. */ -#undef HAVE_GETTIMEOFDAY - -/* Define to 1 if you have the `getuid' function. */ -#undef HAVE_GETUID - -/* Define to 1 if you have the <inttypes.h> header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `memmove' function. */ -#undef HAVE_MEMMOVE - -/* Define to 1 if you have the <memory.h> header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the `memset' function. */ -#undef HAVE_MEMSET - -/* Define to 1 if you have the <netdb.h> header file. */ -#undef HAVE_NETDB_H - -/* Define to 1 if you have the <netinet/in.h> header file. */ -#undef HAVE_NETINET_IN_H - -/* Define to 1 if you have the `poll' function. */ -#undef HAVE_POLL - -/* Define to 1 if you have the `socket' function. */ -#undef HAVE_SOCKET - -/* Define to 1 if you have the <stdint.h> header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the <stdlib.h> header file. */ -#define HAVE_STDLIB_H 1 - -/* Define to 1 if you have the `strchr' function. */ -#define HAVE_STRCHR 1 - -/* Define to 1 if you have the `strdup' function. */ -#define HAVE_STRDUP 1 - -/* Define to 1 if you have the `strerror' function. */ -#define HAVE_STRERROR 1 - -/* Define to 1 if you have the <strings.h> header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the <string.h> header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the `strtol' function. */ -#undef HAVE_STRTOL - -/* Define to 1 if you have the <sys/socket.h> header file. */ -#undef HAVE_SYS_SOCKET_H - -/* Define to 1 if you have the <sys/stat.h> header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the <sys/time.h> header file. */ -#undef HAVE_SYS_TIME_H - -/* Define to 1 if you have the <sys/types.h> header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the <sys/utsname.h> header file. */ -#undef HAVE_SYS_UTSNAME_H - -/* Define to 1 if you have the <unistd.h> header file. */ -#undef HAVE_UNISTD_H - -/* Define to the sub-directory in which libtool stores uninstalled libraries. */ -#define LT_OBJDIR - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -/* #undef NO_MINUS_C_MINUS_O */ - -/* Name of package */ -#define PACKAGE "c-client-src" - -/* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org" - -/* Define to the full name of this package. */ -#define PACKAGE_NAME "zookeeper C client" - -/* Define to the full name and version of this package. */ -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32" - -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "c-client-src" - -/* Define to the home page for this package. */ -#define PACKAGE_URL "" - -/* Define to the version of this package. */ -#define PACKAGE_VERSION "3.5.0" End diff – (Actually, this is old because `winconfig.h` is unmaintained. I just checked the tip of `master`, it's still 3.5.0 in this file. My changes make the set based off the single version variable in the CMake build file instead.)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm the PR is rebased, retested, and has some extra notes in it now for some weird CMake things.

          I would not put the CMake files in a separate folder. There are couple of reasons for this:

          • It is a very strong convention with CMake that the top level `CMakeLists.txt` file exists at the top level of the source directory (thus making variables like `CMAKE_SOURCE_DIR` make sense). This is also necessary for the macro `ExternalProject_Add` to be used with older (pre 3.8) versions of CMake, when it didn't have the `SOURCE_SUBDIR` option. (This macro is often used by other projects, e.g. Mesos, to download and build a dependency.)
          • The CMake system can also be used on Linux platforms (in fact, it's even setup to build and run the tests with `ctest`). If `cmake` has not been invoked, or if `autoconf`/`configure` have not been invoked, they will not interfere with each other (so the source files themselves can co-exist). It's only the generated files, and only on systems that use `make`, that may become confusing. But this confusion is easily cleared up by asking: which configure system did you invoke? So I don't think it'll be much of a problem. (We have both Autotools and CMake concurrently in Mesos, with the eventual plan of deprecating the former with the latter. Developers still using Autotools have been just fine ignoring CMake.)
          • CMake can easily build out-of-tree. I've tested this, as I usually build with `mkdir build; cd build; cmake ..; cmake --build .`. So you can use the Autotools and CMake systems concurrently, if you're, say, testing both systems.

          > Ultimately we might drop existing makefiles in favor of the CMake

          That would be fantastic :smile:

          The only annoying thing is that, until that day, there are now two systems to maintain. I'd posit that this is better than the previous situation where there was the Linux system, and then at least, what, three? Windows systems being (not) maintained. If (when) CMake does get broken on Linux because a change was made to Autotools and not replicated to CMake, it won't be the end of the world. You'll have one working system, and someone (like me) will end up fixing the other system. Not the greatest situation, but not the worst.

          So all that said, leaving CMake available for both operating systems I think is the right thing to do. The scope of impact is still only on Windows, as we're deprecating (deleting) the previous build files, and we're adding a choice for Linux developers. (When ZooKeeper 3.5.3 is released, I'll replace Mesos's if/else code to build ZK on Linux and Windows with a single piece of code, using CMake. It'll be awesome.)

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm the PR is rebased, retested, and has some extra notes in it now for some weird CMake things. I would not put the CMake files in a separate folder. There are couple of reasons for this: It is a very strong convention with CMake that the top level `CMakeLists.txt` file exists at the top level of the source directory (thus making variables like `CMAKE_SOURCE_DIR` make sense). This is also necessary for the macro `ExternalProject_Add` to be used with older (pre 3.8) versions of CMake, when it didn't have the `SOURCE_SUBDIR` option. (This macro is often used by other projects, e.g. Mesos, to download and build a dependency.) The CMake system can also be used on Linux platforms (in fact, it's even setup to build and run the tests with `ctest`). If `cmake` has not been invoked, or if `autoconf`/`configure` have not been invoked, they will not interfere with each other (so the source files themselves can co-exist). It's only the generated files, and only on systems that use `make`, that may become confusing. But this confusion is easily cleared up by asking: which configure system did you invoke? So I don't think it'll be much of a problem. (We have both Autotools and CMake concurrently in Mesos, with the eventual plan of deprecating the former with the latter. Developers still using Autotools have been just fine ignoring CMake.) CMake can easily build out-of-tree. I've tested this, as I usually build with `mkdir build; cd build; cmake ..; cmake --build .`. So you can use the Autotools and CMake systems concurrently, if you're, say, testing both systems. > Ultimately we might drop existing makefiles in favor of the CMake That would be fantastic :smile: The only annoying thing is that, until that day, there are now two systems to maintain. I'd posit that this is better than the previous situation where there was the Linux system, and then at least, what, three? Windows systems being (not) maintained. If (when) CMake does get broken on Linux because a change was made to Autotools and not replicated to CMake, it won't be the end of the world. You'll have one working system, and someone (like me) will end up fixing the other system. Not the greatest situation, but not the worst. So all that said, leaving CMake available for both operating systems I think is the right thing to do. The scope of impact is still only on Windows, as we're deprecating (deleting) the previous build files, and we're adding a choice for Linux developers. (When ZooKeeper 3.5.3 is released, I'll replace Mesos's if/else code to build ZK on Linux and Windows with a single piece of code, using CMake. It'll be awesome.)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Just to give you an idea of what we currently have to do in order to use the ZooKeeper C Client in Mesos, we have [this](https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/cmake/Mesos3rdpartyConfigure.cmake#L51) and [this](https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/zookeeper-3.5.2-alpha.patch) and [this](https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/CMakeLists.txt#L394), all to work around the current state of the client build system.

          My goal is that, with these changes, almost everything can be replaced with just this [one piece of code](https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/CMakeLists.txt#L424).

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 Just to give you an idea of what we currently have to do in order to use the ZooKeeper C Client in Mesos, we have [this] ( https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/cmake/Mesos3rdpartyConfigure.cmake#L51 ) and [this] ( https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/zookeeper-3.5.2-alpha.patch ) and [this] ( https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/CMakeLists.txt#L394 ), all to work around the current state of the client build system. My goal is that, with these changes, almost everything can be replaced with just this [one piece of code] ( https://github.com/apache/mesos/blob/b4f8d1f588841b0a7395c24630963be5620db7cb/3rdparty/CMakeLists.txt#L424 ).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126292628

          — Diff: src/c/include/winconfig.h —
          @@ -1,196 +1,15 @@
          -/* Define to 1 if you have the <arpa/inet.h> header file. */
          -#undef HAVE_ARPA_INET_H
          -
          -/* Define to 1 if you have the <dlfcn.h> header file. */
          -#undef HAVE_DLFCN_H
          -
          -/* Define to 1 if you have the <fcntl.h> header file. */
          -#undef HAVE_FCNTL_H
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1
          -
          -/* Define to 1 if you have the `getcwd' function. */
          -#undef HAVE_GETCWD
          -
          -/* Define to 1 if you have the `gethostbyname' function. */
          -#undef HAVE_GETHOSTBYNAME
          -
          -/* Define to 1 if you have the `gethostname' function. */
          -#define HAVE_GETHOSTNAME 1
          -
          -/* Define to 1 if you have the `getlogin' function. */
          -#undef HAVE_GETLOGIN
          -
          -/* Define to 1 if you have the `getpwuid_r' function. */
          -#undef HAVE_GETPWUID_R
          -
          -/* Define to 1 if you have the `gettimeofday' function. */
          -#undef HAVE_GETTIMEOFDAY
          -
          -/* Define to 1 if you have the `getuid' function. */
          -#undef HAVE_GETUID
          -
          -/* Define to 1 if you have the <inttypes.h> header file. */
          -#undef HAVE_INTTYPES_H
          -
          -/* Define to 1 if you have the `memmove' function. */
          -#undef HAVE_MEMMOVE
          -
          -/* Define to 1 if you have the <memory.h> header file. */
          -#undef HAVE_MEMORY_H
          -
          -/* Define to 1 if you have the `memset' function. */
          -#undef HAVE_MEMSET
          -
          -/* Define to 1 if you have the <netdb.h> header file. */
          -#undef HAVE_NETDB_H
          -
          -/* Define to 1 if you have the <netinet/in.h> header file. */
          -#undef HAVE_NETINET_IN_H
          -
          -/* Define to 1 if you have the `poll' function. */
          -#undef HAVE_POLL
          -
          -/* Define to 1 if you have the `socket' function. */
          -#undef HAVE_SOCKET
          -
          -/* Define to 1 if you have the <stdint.h> header file. */
          -#undef HAVE_STDINT_H
          -
          -/* Define to 1 if you have the <stdlib.h> header file. */
          -#define HAVE_STDLIB_H 1
          -
          -/* Define to 1 if you have the `strchr' function. */
          -#define HAVE_STRCHR 1
          -
          -/* Define to 1 if you have the `strdup' function. */
          -#define HAVE_STRDUP 1
          -
          -/* Define to 1 if you have the `strerror' function. */
          -#define HAVE_STRERROR 1
          -
          -/* Define to 1 if you have the <strings.h> header file. */
          -#undef HAVE_STRINGS_H
          -
          -/* Define to 1 if you have the <string.h> header file. */
          -#undef HAVE_STRING_H
          -
          -/* Define to 1 if you have the `strtol' function. */
          -#undef HAVE_STRTOL
          -
          -/* Define to 1 if you have the <sys/socket.h> header file. */
          -#undef HAVE_SYS_SOCKET_H
          -
          -/* Define to 1 if you have the <sys/stat.h> header file. */
          -#undef HAVE_SYS_STAT_H
          -
          -/* Define to 1 if you have the <sys/time.h> header file. */
          -#undef HAVE_SYS_TIME_H
          -
          -/* Define to 1 if you have the <sys/types.h> header file. */
          -#undef HAVE_SYS_TYPES_H
          -
          -/* Define to 1 if you have the <sys/utsname.h> header file. */
          -#undef HAVE_SYS_UTSNAME_H
          -
          -/* Define to 1 if you have the <unistd.h> header file. */
          -#undef HAVE_UNISTD_H
          -
          -/* Define to the sub-directory in which libtool stores uninstalled libraries.

          • */
            -#define LT_OBJDIR
            -
            -/* Define to 1 if your C compiler doesn't accept -c and -o together. */
            -/* #undef NO_MINUS_C_MINUS_O */
            -
            -/* Name of package */
            -#define PACKAGE "c-client-src"
            -
            -/* Define to the address where bug reports for this package should be sent. */
            -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org"
            -
            -/* Define to the full name of this package. */
            -#define PACKAGE_NAME "zookeeper C client"
            -
            -/* Define to the full name and version of this package. */
            -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32"
            -
            -/* Define to the one symbol short name of this package. */
            -#define PACKAGE_TARNAME "c-client-src"
            -
            -/* Define to the home page for this package. */
            -#define PACKAGE_URL ""
            -
            -/* Define to the version of this package. */
            -#define PACKAGE_VERSION "3.5.0"
              • End diff –

          Yes, your observation is correct. We only bump versions encoded in this file of master branch when we do a major release (e.g. from 3.4.x to 3.5.0). The same file in branch-3.5 contains more up to date version number for previous release (3.5.3 for 3.5, and 3.4.10 for 3.4). So this issue is not a problem.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126292628 — Diff: src/c/include/winconfig.h — @@ -1,196 +1,15 @@ -/* Define to 1 if you have the <arpa/inet.h> header file. */ -#undef HAVE_ARPA_INET_H - -/* Define to 1 if you have the <dlfcn.h> header file. */ -#undef HAVE_DLFCN_H - -/* Define to 1 if you have the <fcntl.h> header file. */ -#undef HAVE_FCNTL_H - -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1 - -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1 - -/* Define to 1 if you have the `getcwd' function. */ -#undef HAVE_GETCWD - -/* Define to 1 if you have the `gethostbyname' function. */ -#undef HAVE_GETHOSTBYNAME - -/* Define to 1 if you have the `gethostname' function. */ -#define HAVE_GETHOSTNAME 1 - -/* Define to 1 if you have the `getlogin' function. */ -#undef HAVE_GETLOGIN - -/* Define to 1 if you have the `getpwuid_r' function. */ -#undef HAVE_GETPWUID_R - -/* Define to 1 if you have the `gettimeofday' function. */ -#undef HAVE_GETTIMEOFDAY - -/* Define to 1 if you have the `getuid' function. */ -#undef HAVE_GETUID - -/* Define to 1 if you have the <inttypes.h> header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `memmove' function. */ -#undef HAVE_MEMMOVE - -/* Define to 1 if you have the <memory.h> header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the `memset' function. */ -#undef HAVE_MEMSET - -/* Define to 1 if you have the <netdb.h> header file. */ -#undef HAVE_NETDB_H - -/* Define to 1 if you have the <netinet/in.h> header file. */ -#undef HAVE_NETINET_IN_H - -/* Define to 1 if you have the `poll' function. */ -#undef HAVE_POLL - -/* Define to 1 if you have the `socket' function. */ -#undef HAVE_SOCKET - -/* Define to 1 if you have the <stdint.h> header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the <stdlib.h> header file. */ -#define HAVE_STDLIB_H 1 - -/* Define to 1 if you have the `strchr' function. */ -#define HAVE_STRCHR 1 - -/* Define to 1 if you have the `strdup' function. */ -#define HAVE_STRDUP 1 - -/* Define to 1 if you have the `strerror' function. */ -#define HAVE_STRERROR 1 - -/* Define to 1 if you have the <strings.h> header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the <string.h> header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the `strtol' function. */ -#undef HAVE_STRTOL - -/* Define to 1 if you have the <sys/socket.h> header file. */ -#undef HAVE_SYS_SOCKET_H - -/* Define to 1 if you have the <sys/stat.h> header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the <sys/time.h> header file. */ -#undef HAVE_SYS_TIME_H - -/* Define to 1 if you have the <sys/types.h> header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the <sys/utsname.h> header file. */ -#undef HAVE_SYS_UTSNAME_H - -/* Define to 1 if you have the <unistd.h> header file. */ -#undef HAVE_UNISTD_H - -/* Define to the sub-directory in which libtool stores uninstalled libraries. */ -#define LT_OBJDIR - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -/* #undef NO_MINUS_C_MINUS_O */ - -/* Name of package */ -#define PACKAGE "c-client-src" - -/* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org" - -/* Define to the full name of this package. */ -#define PACKAGE_NAME "zookeeper C client" - -/* Define to the full name and version of this package. */ -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32" - -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "c-client-src" - -/* Define to the home page for this package. */ -#define PACKAGE_URL "" - -/* Define to the version of this package. */ -#define PACKAGE_VERSION "3.5.0" End diff – Yes, your observation is correct. We only bump versions encoded in this file of master branch when we do a major release (e.g. from 3.4.x to 3.5.0). The same file in branch-3.5 contains more up to date version number for previous release (3.5.3 for 3.5, and 3.4.10 for 3.4). So this issue is not a problem.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @andschwa Thanks for clarification regarding the coexisting of CMake and makefile. I agree and let's keep the current folder structure, but I think some documentations would be good to help clarify the case. How about document the build toolchain for windows in the README file under the same folder?

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 @andschwa Thanks for clarification regarding the coexisting of CMake and makefile. I agree and let's keep the current folder structure, but I think some documentations would be good to help clarify the case. How about document the build toolchain for windows in the README file under the same folder?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/306#discussion_r126294494

          — Diff: src/c/include/winconfig.h —
          @@ -1,196 +1,15 @@
          -/* Define to 1 if you have the <arpa/inet.h> header file. */
          -#undef HAVE_ARPA_INET_H
          -
          -/* Define to 1 if you have the <dlfcn.h> header file. */
          -#undef HAVE_DLFCN_H
          -
          -/* Define to 1 if you have the <fcntl.h> header file. */
          -#undef HAVE_FCNTL_H
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1
          -
          -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */
          -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1
          -
          -/* Define to 1 if you have the `getcwd' function. */
          -#undef HAVE_GETCWD
          -
          -/* Define to 1 if you have the `gethostbyname' function. */
          -#undef HAVE_GETHOSTBYNAME
          -
          -/* Define to 1 if you have the `gethostname' function. */
          -#define HAVE_GETHOSTNAME 1
          -
          -/* Define to 1 if you have the `getlogin' function. */
          -#undef HAVE_GETLOGIN
          -
          -/* Define to 1 if you have the `getpwuid_r' function. */
          -#undef HAVE_GETPWUID_R
          -
          -/* Define to 1 if you have the `gettimeofday' function. */
          -#undef HAVE_GETTIMEOFDAY
          -
          -/* Define to 1 if you have the `getuid' function. */
          -#undef HAVE_GETUID
          -
          -/* Define to 1 if you have the <inttypes.h> header file. */
          -#undef HAVE_INTTYPES_H
          -
          -/* Define to 1 if you have the `memmove' function. */
          -#undef HAVE_MEMMOVE
          -
          -/* Define to 1 if you have the <memory.h> header file. */
          -#undef HAVE_MEMORY_H
          -
          -/* Define to 1 if you have the `memset' function. */
          -#undef HAVE_MEMSET
          -
          -/* Define to 1 if you have the <netdb.h> header file. */
          -#undef HAVE_NETDB_H
          -
          -/* Define to 1 if you have the <netinet/in.h> header file. */
          -#undef HAVE_NETINET_IN_H
          -
          -/* Define to 1 if you have the `poll' function. */
          -#undef HAVE_POLL
          -
          -/* Define to 1 if you have the `socket' function. */
          -#undef HAVE_SOCKET
          -
          -/* Define to 1 if you have the <stdint.h> header file. */
          -#undef HAVE_STDINT_H
          -
          -/* Define to 1 if you have the <stdlib.h> header file. */
          -#define HAVE_STDLIB_H 1
          -
          -/* Define to 1 if you have the `strchr' function. */
          -#define HAVE_STRCHR 1
          -
          -/* Define to 1 if you have the `strdup' function. */
          -#define HAVE_STRDUP 1
          -
          -/* Define to 1 if you have the `strerror' function. */
          -#define HAVE_STRERROR 1
          -
          -/* Define to 1 if you have the <strings.h> header file. */
          -#undef HAVE_STRINGS_H
          -
          -/* Define to 1 if you have the <string.h> header file. */
          -#undef HAVE_STRING_H
          -
          -/* Define to 1 if you have the `strtol' function. */
          -#undef HAVE_STRTOL
          -
          -/* Define to 1 if you have the <sys/socket.h> header file. */
          -#undef HAVE_SYS_SOCKET_H
          -
          -/* Define to 1 if you have the <sys/stat.h> header file. */
          -#undef HAVE_SYS_STAT_H
          -
          -/* Define to 1 if you have the <sys/time.h> header file. */
          -#undef HAVE_SYS_TIME_H
          -
          -/* Define to 1 if you have the <sys/types.h> header file. */
          -#undef HAVE_SYS_TYPES_H
          -
          -/* Define to 1 if you have the <sys/utsname.h> header file. */
          -#undef HAVE_SYS_UTSNAME_H
          -
          -/* Define to 1 if you have the <unistd.h> header file. */
          -#undef HAVE_UNISTD_H
          -
          -/* Define to the sub-directory in which libtool stores uninstalled libraries.

          • */
            -#define LT_OBJDIR
            -
            -/* Define to 1 if your C compiler doesn't accept -c and -o together. */
            -/* #undef NO_MINUS_C_MINUS_O */
            -
            -/* Name of package */
            -#define PACKAGE "c-client-src"
            -
            -/* Define to the address where bug reports for this package should be sent. */
            -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org"
            -
            -/* Define to the full name of this package. */
            -#define PACKAGE_NAME "zookeeper C client"
            -
            -/* Define to the full name and version of this package. */
            -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32"
            -
            -/* Define to the one symbol short name of this package. */
            -#define PACKAGE_TARNAME "c-client-src"
            -
            -/* Define to the home page for this package. */
            -#define PACKAGE_URL ""
            -
            -/* Define to the version of this package. */
            -#define PACKAGE_VERSION "3.5.0"
              • End diff –

          Ah, that makes more sense!

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/306#discussion_r126294494 — Diff: src/c/include/winconfig.h — @@ -1,196 +1,15 @@ -/* Define to 1 if you have the <arpa/inet.h> header file. */ -#undef HAVE_ARPA_INET_H - -/* Define to 1 if you have the <dlfcn.h> header file. */ -#undef HAVE_DLFCN_H - -/* Define to 1 if you have the <fcntl.h> header file. */ -#undef HAVE_FCNTL_H - -/* Define to 1 if you have the file `generated/zookeeper.jute.c'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_C 1 - -/* Define to 1 if you have the file `generated/zookeeper.jute.h'. */ -#define HAVE_GENERATED_ZOOKEEPER_JUTE_H 1 - -/* Define to 1 if you have the `getcwd' function. */ -#undef HAVE_GETCWD - -/* Define to 1 if you have the `gethostbyname' function. */ -#undef HAVE_GETHOSTBYNAME - -/* Define to 1 if you have the `gethostname' function. */ -#define HAVE_GETHOSTNAME 1 - -/* Define to 1 if you have the `getlogin' function. */ -#undef HAVE_GETLOGIN - -/* Define to 1 if you have the `getpwuid_r' function. */ -#undef HAVE_GETPWUID_R - -/* Define to 1 if you have the `gettimeofday' function. */ -#undef HAVE_GETTIMEOFDAY - -/* Define to 1 if you have the `getuid' function. */ -#undef HAVE_GETUID - -/* Define to 1 if you have the <inttypes.h> header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `memmove' function. */ -#undef HAVE_MEMMOVE - -/* Define to 1 if you have the <memory.h> header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the `memset' function. */ -#undef HAVE_MEMSET - -/* Define to 1 if you have the <netdb.h> header file. */ -#undef HAVE_NETDB_H - -/* Define to 1 if you have the <netinet/in.h> header file. */ -#undef HAVE_NETINET_IN_H - -/* Define to 1 if you have the `poll' function. */ -#undef HAVE_POLL - -/* Define to 1 if you have the `socket' function. */ -#undef HAVE_SOCKET - -/* Define to 1 if you have the <stdint.h> header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the <stdlib.h> header file. */ -#define HAVE_STDLIB_H 1 - -/* Define to 1 if you have the `strchr' function. */ -#define HAVE_STRCHR 1 - -/* Define to 1 if you have the `strdup' function. */ -#define HAVE_STRDUP 1 - -/* Define to 1 if you have the `strerror' function. */ -#define HAVE_STRERROR 1 - -/* Define to 1 if you have the <strings.h> header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the <string.h> header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the `strtol' function. */ -#undef HAVE_STRTOL - -/* Define to 1 if you have the <sys/socket.h> header file. */ -#undef HAVE_SYS_SOCKET_H - -/* Define to 1 if you have the <sys/stat.h> header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the <sys/time.h> header file. */ -#undef HAVE_SYS_TIME_H - -/* Define to 1 if you have the <sys/types.h> header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the <sys/utsname.h> header file. */ -#undef HAVE_SYS_UTSNAME_H - -/* Define to 1 if you have the <unistd.h> header file. */ -#undef HAVE_UNISTD_H - -/* Define to the sub-directory in which libtool stores uninstalled libraries. */ -#define LT_OBJDIR - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -/* #undef NO_MINUS_C_MINUS_O */ - -/* Name of package */ -#define PACKAGE "c-client-src" - -/* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "user@zookeeper.apache.org" - -/* Define to the full name of this package. */ -#define PACKAGE_NAME "zookeeper C client" - -/* Define to the full name and version of this package. */ -#define PACKAGE_STRING "zookeeper C client 3.5.0 win32" - -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "c-client-src" - -/* Define to the home page for this package. */ -#define PACKAGE_URL "" - -/* Define to the version of this package. */ -#define PACKAGE_VERSION "3.5.0" End diff – Ah, that makes more sense!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm ah, documentation for it would indeed be good! Can do. Anything else you need for this? If not, I'll get the readme fixed up Monday morning and ping you afterward.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm ah, documentation for it would indeed be good! Can do. Anything else you need for this? If not, I'll get the readme fixed up Monday morning and ping you afterward.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm I ended up just adding it [straight to the src/c/README](https://github.com/apache/zookeeper/pull/306/files#diff-a722fe5ba032cb8da6c78d0e929f4ac5R74), let me know if you think it should have more/less etc.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm I ended up just adding it [straight to the src/c/README] ( https://github.com/apache/zookeeper/pull/306/files#diff-a722fe5ba032cb8da6c78d0e929f4ac5R74 ), let me know if you think it should have more/less etc.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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/874//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/874//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/874//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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/874//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/874//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/874//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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 passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/875//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Hey, yay, Jenkins passed!

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 Hey, yay, Jenkins passed!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @andschwa
          Patch looks good to me. The readme update looks good too.

          I also verified Linux and Mac c client builds with the patch. Unfortunately I don't have a windows box to test. Can you please describe what kinds of test / integration test you did on windows?

          There are two remaining issues:

          • Can you please update the pull request description by adding a brief description on what this patch is for and how it did it. The pull request description will be part of commit message, and it's good to have a informative commit message.
          • This pull request is targeting master, which is not going to be released soon. branch-3.5 and branch-3.4 are branches for next releases, are you going to send separate pull requests to those branches, or you are fine just with this merged into master?
          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 @andschwa Patch looks good to me. The readme update looks good too. I also verified Linux and Mac c client builds with the patch. Unfortunately I don't have a windows box to test. Can you please describe what kinds of test / integration test you did on windows? There are two remaining issues: Can you please update the pull request description by adding a brief description on what this patch is for and how it did it. The pull request description will be part of commit message, and it's good to have a informative commit message. This pull request is targeting master, which is not going to be released soon. branch-3.5 and branch-3.4 are branches for next releases, are you going to send separate pull requests to those branches, or you are fine just with this merged into master?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          > Can you please describe what kinds of test / integration test you did on windows?

          Unfortunately, we don't have the unit tests building on Windows (under any tools). So I ensured I could reliably build the project with CMake and get the expected outputs. Moreover, I have a [zk-test branch](https://github.com/andschwa/mesos/tree/zk-test) in Mesos where I updated our system to apply these patches to the 3.5.2 release, and build with them (verifying that the second patch resolved MESOS-7541(https://issues.apache.org/jira/browse/MESOS-7541)). I verified I could link to the libraries as expected, and ran some tests (though I'd like to give a full `mesos-test` pass of our entire suite).

          Furthermore, we have been building and using the first patch (the addition of the CMake build, but not the re-port changes) since [April 25](https://github.com/apache/mesos/commit/6e64ffaca365ed1e28256d7cb87bf9e1af626a75) with no problems; so I'd say the CMake system is thoroughly tested for Windows (at least in its previous form, but only some cosmetics, e.g. TODOs/comments/default option for building unit tests, were changed when I updated this PR).

          > Can you please update the pull request description by adding a brief description on what this patch is for and how it did it. The pull request description will be part of commit message, and it's good to have a informative commit message.

          Wouldn't you rather use the two commit messages, a58a5a95aef9cafc267a1b4a2bdb37f9e9e26363 and c550a3e3babcaa3b3891280ac2d61b89fd294d06 as-is (those should be two links to the commits with their respective messages, but the ASF bot might not copy them since they're automatic from GitHub)? These are the patches as I'd commit them (as in, I wouldn't squash them into one, since they're resolving two bugs).

          > This pull request is targeting master, which is not going to be released soon. branch-3.5 and branch-3.4 are branches for next releases, are you going to send separate pull requests to those branches, or you are fine just with this merged into master?

          I would be happy to provide backports of these two patches, especially for branch-3.5, so that I can remove the manual patch step in Mesos's build process as soon as possible . But first, let's figure out the patch messages. I could copy them into the PR description, but then I'm afraid it's going to be squashed as one patch instead of the two logical patches. I could replace this PR with two separate PRs, one for each patch, and then repeat that for the two branches; but I'm not sure you want that either.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 > Can you please describe what kinds of test / integration test you did on windows? Unfortunately, we don't have the unit tests building on Windows (under any tools). So I ensured I could reliably build the project with CMake and get the expected outputs. Moreover, I have a [zk-test branch] ( https://github.com/andschwa/mesos/tree/zk-test ) in Mesos where I updated our system to apply these patches to the 3.5.2 release, and build with them (verifying that the second patch resolved MESOS-7541 ( https://issues.apache.org/jira/browse/MESOS-7541 )). I verified I could link to the libraries as expected, and ran some tests (though I'd like to give a full `mesos-test` pass of our entire suite). Furthermore, we have been building and using the first patch (the addition of the CMake build, but not the re-port changes) since [April 25] ( https://github.com/apache/mesos/commit/6e64ffaca365ed1e28256d7cb87bf9e1af626a75 ) with no problems; so I'd say the CMake system is thoroughly tested for Windows (at least in its previous form, but only some cosmetics, e.g. TODOs/comments/default option for building unit tests, were changed when I updated this PR). > Can you please update the pull request description by adding a brief description on what this patch is for and how it did it. The pull request description will be part of commit message, and it's good to have a informative commit message. Wouldn't you rather use the two commit messages, a58a5a95aef9cafc267a1b4a2bdb37f9e9e26363 and c550a3e3babcaa3b3891280ac2d61b89fd294d06 as-is (those should be two links to the commits with their respective messages, but the ASF bot might not copy them since they're automatic from GitHub)? These are the patches as I'd commit them (as in, I wouldn't squash them into one, since they're resolving two bugs). > This pull request is targeting master, which is not going to be released soon. branch-3.5 and branch-3.4 are branches for next releases, are you going to send separate pull requests to those branches, or you are fine just with this merged into master? I would be happy to provide backports of these two patches, especially for branch-3.5, so that I can remove the manual patch step in Mesos's build process as soon as possible . But first, let's figure out the patch messages. I could copy them into the PR description, but then I'm afraid it's going to be squashed as one patch instead of the two logical patches. I could replace this PR with two separate PRs, one for each patch, and then repeat that for the two branches; but I'm not sure you want that either.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          If we want two commits, then we need two pull requests. Two (or more ) commits appertain to a single pull request will be squashed to a single commit at commit time by our commit script. I think it's fine to have this pull request resolve both issue with a single commit.

          I can massage the commit message at commit time to include the messages from both commit, or you could update the pull request description with both message, I am ok with either way.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 If we want two commits, then we need two pull requests. Two (or more ) commits appertain to a single pull request will be squashed to a single commit at commit time by our commit script. I think it's fine to have this pull request resolve both issue with a single commit. I can massage the commit message at commit time to include the messages from both commit, or you could update the pull request description with both message, I am ok with either way.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm ah, I see! That is no problem. I have taken the two messages, improved them a bit as one message, and updated the description. It will be easier to backport it this way.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm ah, I see! That is no problem. I have taken the two messages, improved them a bit as one message, and updated the description. It will be easier to backport it this way.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm I'm going to let you take care of the title, maybe it should be:

          > ZOOKEEPER-2756 and ZOOKEEPER-2841: Improve cross-platform support with CMake and refactor

          But I don't know.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm I'm going to let you take care of the title, maybe it should be: > ZOOKEEPER-2756 and ZOOKEEPER-2841 : Improve cross-platform support with CMake and refactor But I don't know.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Thanks for update. LGTM. Will merge by end of this week. Give a few more days in case someone has additional feedback for this.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 Thanks for update. LGTM. Will merge by end of this week. Give a few more days in case someone has additional feedback for this.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @hanm I'm likewise giving this some more testing too. I'm integrating the patch into Mesos on the Linux side (where we were building with Autotools), and porting our ZooKeeper-explicit unit tests to Windows (because apparently they weren't). We do have tests that I've run that indirectly tested the ZooKeeper client, but I want certain tests. Expect a small update by the end of the week.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 @hanm I'm likewise giving this some more testing too. I'm integrating the patch into Mesos on the Linux side (where we were building with Autotools), and porting our ZooKeeper-explicit unit tests to Windows (because apparently they weren't). We do have tests that I've run that indirectly tested the ZooKeeper client, but I want certain tests. Expect a small update by the end of the week.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          This [here](https://github.com/apache/zookeeper/pull/306/files#diff-1ea258ac6b9efc24b67d2c2a704cfe5fR145) is all I changed; it ensures that the `config.h` file is always put into the source `include` directory, which is what upstream projects include. This was needed because `zookeeper.h` includes the file, so this makes sure it exists next to it (instead of in the binary directory, which works for the ZK build, but if it's out-of-tree, is misplaced for upstream consumption).

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 This [here] ( https://github.com/apache/zookeeper/pull/306/files#diff-1ea258ac6b9efc24b67d2c2a704cfe5fR145 ) is all I changed; it ensures that the `config.h` file is always put into the source `include` directory, which is what upstream projects include. This was needed because `zookeeper.h` includes the file, so this makes sure it exists next to it (instead of in the binary directory, which works for the ZK build, but if it's out-of-tree, is misplaced for upstream consumption).
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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/882//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/882//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/882//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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/882//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/882//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/882//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          looks good. test failures are irrelevant (we should really get the flaky tests fixed soon.).

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 looks good. test failures are irrelevant (we should really get the flaky tests fixed soon.).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          I managed to get the Mesos unit tests for ZooKeeper ported to our CMake system, which much more thoroughly exercises the C client. I've integrated this patch and the CMake system with the Mesos build on Linux, and all our tests passed:

          ```
          [----------] 7 tests from ZooKeeperTest
          [ RUN ] ZooKeeperTest.Auth
          [ OK ] ZooKeeperTest.Auth (6923 ms)
          [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation
          [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (46 ms)
          [ RUN ] ZooKeeperTest.Create
          [ OK ] ZooKeeperTest.Create (6770 ms)
          [ RUN ] ZooKeeperTest.LeaderDetector
          [ OK ] ZooKeeperTest.LeaderDetector (57 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling
          2017-07-14 15:23:18,510:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:43434] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-14 15:23:18,510:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          2017-07-14 15:23:18,514:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          2017-07-14 15:23:18,514:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (50 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling
          [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (54 ms)
          [ RUN ] ZooKeeperTest.LeaderContender
          2017-07-14 15:23:18,630:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-14 15:23:18,632:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:38085] zk retcode=-112, errno=116(Stale file handle): sessionId=0x10000305fd80000 has expired.
          2017-07-14 15:23:18,655:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-14 15:23:18,657:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:38085] zk retcode=-112, errno=116(Stale file handle): sessionId=0x10000305fd80001 has expired.
          2017-07-14 15:23:18,676:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-14 15:23:18,677:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          2017-07-14 15:23:18,688:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          2017-07-14 15:23:18,688:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          [ OK ] ZooKeeperTest.LeaderContender (304 ms)
          [----------] 7 tests from ZooKeeperTest (14204 ms total)

          [----------] Global test environment tear-down
          [==========] 7 tests from 1 test case ran. (14376 ms total)
          [ PASSED ] 7 tests.
          ```

          And then I brought these changes over to Windows too, and while it's currently building with some a lot of irrelevant-to-this-patch hacks, they pass:

          ```
          [----------] 7 tests from ZooKeeperTest
          [ RUN ] ZooKeeperTest.Auth
          [ OK ] ZooKeeperTest.Auth (7101 ms)
          [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation
          [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (737 ms)
          [ RUN ] ZooKeeperTest.Create
          [ OK ] ZooKeeperTest.Create (6752 ms)
          [ RUN ] ZooKeeperTest.LeaderDetector
          [ OK ] ZooKeeperTest.LeaderDetector (107 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling
          2017-07-17 12:52:08,546:15936(0x3334):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54312] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (68 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling
          [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (72 ms)
          [ RUN ] ZooKeeperTest.LeaderContender
          2017-07-17 12:52:08,709:15936(0x47c8):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-17 12:52:08,714:15936(0x47c8):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:54336] zk retcode=-112, errno=19(No such device): sessionId=0x1001b9dddba0000 has expired.
          2017-07-17 12:52:08,749:15936(0x39b8):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-17 12:52:08,754:15936(0x39b8):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:54336] zk retcode=-112, errno=19(No such device): sessionId=0x1001b9dddba0001 has expired.
          2017-07-17 12:52:08,788:15936(0xb94):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          [ OK ] ZooKeeperTest.LeaderContender (701 ms)
          [----------] 7 tests from ZooKeeperTest (15548 ms total)

          [----------] Global test environment tear-down
          [==========] 7 tests from 1 test case ran. (16724 ms total)
          [ PASSED ] 7 tests.
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 I managed to get the Mesos unit tests for ZooKeeper ported to our CMake system, which much more thoroughly exercises the C client. I've integrated this patch and the CMake system with the Mesos build on Linux, and all our tests passed: ``` [----------] 7 tests from ZooKeeperTest [ RUN ] ZooKeeperTest.Auth [ OK ] ZooKeeperTest.Auth (6923 ms) [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (46 ms) [ RUN ] ZooKeeperTest.Create [ OK ] ZooKeeperTest.Create (6770 ms) [ RUN ] ZooKeeperTest.LeaderDetector [ OK ] ZooKeeperTest.LeaderDetector (57 ms) [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling 2017-07-14 15:23:18,510:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:43434] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-14 15:23:18,510:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client 2017-07-14 15:23:18,514:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client 2017-07-14 15:23:18,514:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:43434] zk retcode=-4, errno=111(Connection refused): server refused to accept the client [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (50 ms) [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (54 ms) [ RUN ] ZooKeeperTest.LeaderContender 2017-07-14 15:23:18,630:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-14 15:23:18,632:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:38085] zk retcode=-112, errno=116(Stale file handle): sessionId=0x10000305fd80000 has expired. 2017-07-14 15:23:18,655:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-14 15:23:18,657:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:38085] zk retcode=-112, errno=116(Stale file handle): sessionId=0x10000305fd80001 has expired. 2017-07-14 15:23:18,676:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:38085] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-14 15:23:18,677:29970(0x7f01dffff700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client 2017-07-14 15:23:18,688:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client 2017-07-14 15:23:18,688:29970(0x7f01e4d0a700):ZOO_ERROR@handle_socket_error_msg@2384: Socket [127.0.0.1:38085] zk retcode=-4, errno=111(Connection refused): server refused to accept the client [ OK ] ZooKeeperTest.LeaderContender (304 ms) [----------] 7 tests from ZooKeeperTest (14204 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test case ran. (14376 ms total) [ PASSED ] 7 tests. ``` And then I brought these changes over to Windows too, and while it's currently building with some a lot of irrelevant-to-this-patch hacks, they pass: ``` [----------] 7 tests from ZooKeeperTest [ RUN ] ZooKeeperTest.Auth [ OK ] ZooKeeperTest.Auth (7101 ms) [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (737 ms) [ RUN ] ZooKeeperTest.Create [ OK ] ZooKeeperTest.Create (6752 ms) [ RUN ] ZooKeeperTest.LeaderDetector [ OK ] ZooKeeperTest.LeaderDetector (107 ms) [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling 2017-07-17 12:52:08,546:15936(0x3334):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54312] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (68 ms) [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (72 ms) [ RUN ] ZooKeeperTest.LeaderContender 2017-07-17 12:52:08,709:15936(0x47c8):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-17 12:52:08,714:15936(0x47c8):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:54336] zk retcode=-112, errno=19(No such device): sessionId=0x1001b9dddba0000 has expired. 2017-07-17 12:52:08,749:15936(0x39b8):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-17 12:52:08,754:15936(0x39b8):ZOO_ERROR@handle_socket_error_msg@2428: Socket [127.0.0.1:54336] zk retcode=-112, errno=19(No such device): sessionId=0x1001b9dddba0001 has expired. 2017-07-17 12:52:08,788:15936(0xb94):ZOO_ERROR@handle_socket_error_msg@2409: Socket [127.0.0.1:54336] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response [ OK ] ZooKeeperTest.LeaderContender (701 ms) [----------] 7 tests from ZooKeeperTest (15548 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test case ran. (16724 ms total) [ PASSED ] 7 tests. ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user andschwa opened a pull request:

          https://github.com/apache/zookeeper/pull/311

          ZOOKEEPER-2841: ZooKeeper public include files leak porting change

          This patch primarily adds a cross-platform CMake build system. This is in
          addition to the Autotools system on Linux, until it can be deprecated, and this
          replaces the existing committed Visual Studio Solutions for Windows.

          As this commit deprecates (and breaks) the previously existing Visual Studio
          solutions and projects, they've been removed. Building on Windows now requires
          CMake, but this enables the support of any code-supported Visual Studio version.
          Support for Visual Studio 2017 was explicitly added by this patch, and support
          for future versions, barring software bugs, should be available automatically
          through CMake.

          The notable features lacking in comparison to Autotools are Solaris support,
          shared library builds, whitelisted symbol exports, libtool integration, and
          doxygen document generation. Almost everything else from Autotools has been
          ported, including header/function/library checks, and all targets (zookeeper,
          hashtable, cli, load_gen, and tests).

          The primary work involved for CMake (other than the writing of `CMakeLists.txt`)
          was transitioning the hand-written `winconfig.h` to an auto-generated `config.h`
          file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in`
          template was modeled after the Autotools config file so that the feature guards
          share the same names.

          This patch also refactors the Windows port of the C client. Notably, it moves as
          much porting code as possible out of the publicly included `winconfig.h` header
          and into the relevant source files, or the private `winport.h` header.

          While `load_gen.c` looks at first glance as if it were ported to Windows, it
          never actually was, so the erroneous `#include "win32port.h"` was removed, and
          the target is not built on Windows.

          The `include/winstdint.h` header was removed as it has been replaced by
          `<stdint.h>`. This might break very old versions of Visual Studio; but in those
          cases, previous versions of the client are available.

          A bug for upstream libraries including the ZooKeeper headers was fixed by
          removing `#undef AF_INET6` entirely, and removing `#include <windows.h>`, as it
          "pulls in the world" and should not be included publicly.

          A guard was placed around `#define snprintf` in order to enable compiling with
          modern versions of MSVC, including Visual Studio 2015.

          A problem with `DLL_EXPORT` and `USE_STATIC_LIB` being redefined was fixed.

          There are existent warnings which this patch did not attempt to fix.

          Building tests on Windows is not supported, but was not previously supported.
          The tests need to be ported.

          Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove
          `include/winconfig.h` altogether.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/andschwa/zookeeper cmake-backport-3.5

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zookeeper/pull/311.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 #311


          commit 081833e9899afd4937075038f78bb4faff446ef4
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-04-10T23:12:40Z

          ZOOKEEPER-2756: Add CMake build system for better cross-platform support

          This notably lacks Solaris and libtool support.

          Almost everything else from Autotools has been ported,
          including header/function/library checks, and all targets
          (zookeeper, hashtable, cli, load_gen, and tests).

          Both Linux and Windows are supported.

          The primary work involved (other than the writing of `CMakeLists.txt`)
          was transitioning the hand-written `winconfig.h` to an
          auto-generated `config.h` file, and guarding code with `#ifdef
          HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after
          the Autotools config file so that the feature guards share the same
          names.

          While `load_gen.c` looks at first glance as if it were ported to Windows,
          it never actually was, so the erroneous `#include "win32port.h"` was
          removed, and the target is not built on Windows.

          There are existent warnings which this patch did not attempt to fix.

          A guard was placed around `#define snprintf` in order to enable
          compiling with modern versions of MSVC.

          Fixed DLL_EXPORT and USE_STATIC_LIB redefinition.

          As this commit deprecates (and breaks) the previously existing Visual
          Studio solutions and projects, they've been removed.

          Building tests on Windows is still not supported.

          commit 49f9558dea6504e5f082edd0d4d5191503c54235
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-07-07T22:35:37Z

          ZOOKEEPER-2841: ZooKeeper public include files leak porting changes

          This patch refactors the Windows port of the C client. Notably, it moves
          as much porting code as possible out of the publicly included
          `winconfig.h` header and into the relevant source files. This also
          removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes
          problems for upstream libraries by removing `#undef AF_INET6` and
          `#include <windows.h>`.

          Future work should resolve the `ACL` / `ZKACL` conflict, and potentially
          remove `include/winconfig.h` altogether.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user andschwa opened a pull request: https://github.com/apache/zookeeper/pull/311 ZOOKEEPER-2841 : ZooKeeper public include files leak porting change This patch primarily adds a cross-platform CMake build system. This is in addition to the Autotools system on Linux, until it can be deprecated, and this replaces the existing committed Visual Studio Solutions for Windows. As this commit deprecates (and breaks) the previously existing Visual Studio solutions and projects, they've been removed. Building on Windows now requires CMake, but this enables the support of any code-supported Visual Studio version. Support for Visual Studio 2017 was explicitly added by this patch, and support for future versions, barring software bugs, should be available automatically through CMake. The notable features lacking in comparison to Autotools are Solaris support, shared library builds, whitelisted symbol exports, libtool integration, and doxygen document generation. Almost everything else from Autotools has been ported, including header/function/library checks, and all targets (zookeeper, hashtable, cli, load_gen, and tests). The primary work involved for CMake (other than the writing of `CMakeLists.txt`) was transitioning the hand-written `winconfig.h` to an auto-generated `config.h` file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after the Autotools config file so that the feature guards share the same names. This patch also refactors the Windows port of the C client. Notably, it moves as much porting code as possible out of the publicly included `winconfig.h` header and into the relevant source files, or the private `winport.h` header. While `load_gen.c` looks at first glance as if it were ported to Windows, it never actually was, so the erroneous `#include "win32port.h"` was removed, and the target is not built on Windows. The `include/winstdint.h` header was removed as it has been replaced by `<stdint.h>`. This might break very old versions of Visual Studio; but in those cases, previous versions of the client are available. A bug for upstream libraries including the ZooKeeper headers was fixed by removing `#undef AF_INET6` entirely, and removing `#include <windows.h>`, as it "pulls in the world" and should not be included publicly. A guard was placed around `#define snprintf` in order to enable compiling with modern versions of MSVC, including Visual Studio 2015. A problem with `DLL_EXPORT` and `USE_STATIC_LIB` being redefined was fixed. There are existent warnings which this patch did not attempt to fix. Building tests on Windows is not supported, but was not previously supported. The tests need to be ported. Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove `include/winconfig.h` altogether. You can merge this pull request into a Git repository by running: $ git pull https://github.com/andschwa/zookeeper cmake-backport-3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/311.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 #311 commit 081833e9899afd4937075038f78bb4faff446ef4 Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-04-10T23:12:40Z ZOOKEEPER-2756 : Add CMake build system for better cross-platform support This notably lacks Solaris and libtool support. Almost everything else from Autotools has been ported, including header/function/library checks, and all targets (zookeeper, hashtable, cli, load_gen, and tests). Both Linux and Windows are supported. The primary work involved (other than the writing of `CMakeLists.txt`) was transitioning the hand-written `winconfig.h` to an auto-generated `config.h` file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after the Autotools config file so that the feature guards share the same names. While `load_gen.c` looks at first glance as if it were ported to Windows, it never actually was, so the erroneous `#include "win32port.h"` was removed, and the target is not built on Windows. There are existent warnings which this patch did not attempt to fix. A guard was placed around `#define snprintf` in order to enable compiling with modern versions of MSVC. Fixed DLL_EXPORT and USE_STATIC_LIB redefinition. As this commit deprecates (and breaks) the previously existing Visual Studio solutions and projects, they've been removed. Building tests on Windows is still not supported. commit 49f9558dea6504e5f082edd0d4d5191503c54235 Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-07-07T22:35:37Z ZOOKEEPER-2841 : ZooKeeper public include files leak porting changes This patch refactors the Windows port of the C client. Notably, it moves as much porting code as possible out of the publicly included `winconfig.h` header and into the relevant source files. This also removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes problems for upstream libraries by removing `#undef AF_INET6` and `#include <windows.h>`. Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove `include/winconfig.h` altogether.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/311

          @hanm this backports #306 to `branch-3.5`. I rebuilt and retested, all looks good. Since original branch came from `master` the version was already what I believe to be correct at `3.5.3`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/311 @hanm this backports #306 to `branch-3.5`. I rebuilt and retested, all looks good. Since original branch came from `master` the version was already what I believe to be correct at `3.5.3`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/311

          Hm... it might not backport so easily to `branch-3.4`. Let me know how badly it's wanted.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/311 Hm... it might not backport so easily to `branch-3.4`. Let me know how badly it's wanted.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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 passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/884//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Thanks for the extra efforts on integration testing, @andschwa. I'll commit this tomorrow.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 Thanks for the extra efforts on integration testing, @andschwa. I'll commit this tomorrow.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/311

          @andschwa branch-3.4 is the stable branch that we actually ship - I assume your use case (Mesos on windows?) will integrate with that version as well.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/311 @andschwa branch-3.4 is the stable branch that we actually ship - I assume your use case (Mesos on windows?) will integrate with that version as well.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/311

          @hanm oh.. good to know! Okay I'll give it another whack, thanks

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/311 @hanm oh.. good to know! Okay I'll give it another whack, thanks
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/zookeeper/pull/306

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/306
          Hide
          hanm Michael Han added a comment -

          Committed to master. https://github.com/apache/zookeeper/commit/f60928787a908f358a64763f802a6d0371ad4404. Will resolve JIRA after committing ported patch to branch-3.4 and 3.5.

          Show
          hanm Michael Han added a comment - Committed to master. https://github.com/apache/zookeeper/commit/f60928787a908f358a64763f802a6d0371ad4404 . Will resolve JIRA after committing ported patch to branch-3.4 and 3.5.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          @andschwa This patch just landed on master. Thanks for your contribution. I'll review the branch-3.5 port and commit it later this week if it looks good.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/306 @andschwa This patch just landed on master. Thanks for your contribution. I'll review the branch-3.5 port and commit it later this week if it looks good.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user andschwa opened a pull request:

          https://github.com/apache/zookeeper/pull/313

          ZOOKEEPER-2841: ZooKeeper public include files leak porting changes

          This patch primarily adds a cross-platform CMake build system. This is in
          addition to the Autotools system on Linux, until it can be deprecated, and this
          replaces the existing committed Visual Studio Solutions for Windows.

          As this commit deprecates (and breaks) the previously existing Visual Studio
          solutions and projects, they've been removed. Building on Windows now requires
          CMake, but this enables the support of any code-supported Visual Studio version.
          Support for Visual Studio 2017 was explicitly added by this patch, and support
          for future versions, barring software bugs, should be available automatically
          through CMake.

          The notable features lacking in comparison to Autotools are Solaris support,
          shared library builds, whitelisted symbol exports, libtool integration, and
          doxygen document generation. Almost everything else from Autotools has been
          ported, including header/function/library checks, and all targets (zookeeper,
          hashtable, cli, load_gen, and tests).

          The primary work involved for CMake (other than the writing of `CMakeLists.txt`)
          was transitioning the hand-written `winconfig.h` to an auto-generated `config.h`
          file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in`
          template was modeled after the Autotools config file so that the feature guards
          share the same names.

          This patch also refactors the Windows port of the C client. Notably, it moves as
          much porting code as possible out of the publicly included `winconfig.h` header
          and into the relevant source files, or the private `winport.h` header.

          While `load_gen.c` looks at first glance as if it were ported to Windows, it
          never actually was, so the erroneous `#include "win32port.h"` was removed, and
          the target is not built on Windows.

          The `include/winstdint.h` header was removed as it has been replaced by
          `<stdint.h>`. This might break very old versions of Visual Studio; but in those
          cases, previous versions of the client are available.

          A bug for upstream libraries including the ZooKeeper headers was fixed by
          removing `#undef AF_INET6` entirely, and removing `#include <windows.h>`, as it
          "pulls in the world" and should not be included publicly.

          A guard was placed around `#define snprintf` in order to enable compiling with
          modern versions of MSVC, including Visual Studio 2015.

          A problem with `DLL_EXPORT` and `USE_STATIC_LIB` being redefined was fixed.

          There are existent warnings which this patch did not attempt to fix.

          Building tests on Windows is not supported, but was not previously supported.
          The tests need to be ported.

          Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove
          `include/winconfig.h` altogether.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/andschwa/zookeeper cmake-backport-3.4

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zookeeper/pull/313.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 #313


          commit 8b903e3089a575bbead03275c7d1e8591245cca0
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-04-10T23:12:40Z

          ZOOKEEPER-2756: Add CMake build system for better cross-platform support

          This notably lacks Solaris and libtool support.

          Almost everything else from Autotools has been ported,
          including header/function/library checks, and all targets
          (zookeeper, hashtable, cli, load_gen, and tests).

          Both Linux and Windows are supported.

          The primary work involved (other than the writing of `CMakeLists.txt`)
          was transitioning the hand-written `winconfig.h` to an
          auto-generated `config.h` file, and guarding code with `#ifdef
          HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after
          the Autotools config file so that the feature guards share the same
          names.

          While `load_gen.c` looks at first glance as if it were ported to Windows,
          it never actually was, so the erroneous `#include "win32port.h"` was
          removed, and the target is not built on Windows.

          There are existent warnings which this patch did not attempt to fix.

          A guard was placed around `#define snprintf` in order to enable
          compiling with modern versions of MSVC.

          Fixed DLL_EXPORT and USE_STATIC_LIB redefinition.

          As this commit deprecates (and breaks) the previously existing Visual
          Studio solutions and projects, they've been removed.

          Building tests on Windows is still not supported.

          commit bf53c535bcda4a92728140770482919055573ba0
          Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
          Date: 2017-07-07T22:35:37Z

          ZOOKEEPER-2841: ZooKeeper public include files leak porting changes

          This patch refactors the Windows port of the C client. Notably, it moves
          as much porting code as possible out of the publicly included
          `winconfig.h` header and into the relevant source files. This also
          removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes
          problems for upstream libraries by removing `#undef AF_INET6` and
          `#include <windows.h>`.

          Future work should resolve the `ACL` / `ZKACL` conflict, and potentially
          remove `include/winconfig.h` altogether.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user andschwa opened a pull request: https://github.com/apache/zookeeper/pull/313 ZOOKEEPER-2841 : ZooKeeper public include files leak porting changes This patch primarily adds a cross-platform CMake build system. This is in addition to the Autotools system on Linux, until it can be deprecated, and this replaces the existing committed Visual Studio Solutions for Windows. As this commit deprecates (and breaks) the previously existing Visual Studio solutions and projects, they've been removed. Building on Windows now requires CMake, but this enables the support of any code-supported Visual Studio version. Support for Visual Studio 2017 was explicitly added by this patch, and support for future versions, barring software bugs, should be available automatically through CMake. The notable features lacking in comparison to Autotools are Solaris support, shared library builds, whitelisted symbol exports, libtool integration, and doxygen document generation. Almost everything else from Autotools has been ported, including header/function/library checks, and all targets (zookeeper, hashtable, cli, load_gen, and tests). The primary work involved for CMake (other than the writing of `CMakeLists.txt`) was transitioning the hand-written `winconfig.h` to an auto-generated `config.h` file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after the Autotools config file so that the feature guards share the same names. This patch also refactors the Windows port of the C client. Notably, it moves as much porting code as possible out of the publicly included `winconfig.h` header and into the relevant source files, or the private `winport.h` header. While `load_gen.c` looks at first glance as if it were ported to Windows, it never actually was, so the erroneous `#include "win32port.h"` was removed, and the target is not built on Windows. The `include/winstdint.h` header was removed as it has been replaced by `<stdint.h>`. This might break very old versions of Visual Studio; but in those cases, previous versions of the client are available. A bug for upstream libraries including the ZooKeeper headers was fixed by removing `#undef AF_INET6` entirely, and removing `#include <windows.h>`, as it "pulls in the world" and should not be included publicly. A guard was placed around `#define snprintf` in order to enable compiling with modern versions of MSVC, including Visual Studio 2015. A problem with `DLL_EXPORT` and `USE_STATIC_LIB` being redefined was fixed. There are existent warnings which this patch did not attempt to fix. Building tests on Windows is not supported, but was not previously supported. The tests need to be ported. Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove `include/winconfig.h` altogether. You can merge this pull request into a Git repository by running: $ git pull https://github.com/andschwa/zookeeper cmake-backport-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/313.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 #313 commit 8b903e3089a575bbead03275c7d1e8591245cca0 Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-04-10T23:12:40Z ZOOKEEPER-2756 : Add CMake build system for better cross-platform support This notably lacks Solaris and libtool support. Almost everything else from Autotools has been ported, including header/function/library checks, and all targets (zookeeper, hashtable, cli, load_gen, and tests). Both Linux and Windows are supported. The primary work involved (other than the writing of `CMakeLists.txt`) was transitioning the hand-written `winconfig.h` to an auto-generated `config.h` file, and guarding code with `#ifdef HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after the Autotools config file so that the feature guards share the same names. While `load_gen.c` looks at first glance as if it were ported to Windows, it never actually was, so the erroneous `#include "win32port.h"` was removed, and the target is not built on Windows. There are existent warnings which this patch did not attempt to fix. A guard was placed around `#define snprintf` in order to enable compiling with modern versions of MSVC. Fixed DLL_EXPORT and USE_STATIC_LIB redefinition. As this commit deprecates (and breaks) the previously existing Visual Studio solutions and projects, they've been removed. Building tests on Windows is still not supported. commit bf53c535bcda4a92728140770482919055573ba0 Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com> Date: 2017-07-07T22:35:37Z ZOOKEEPER-2841 : ZooKeeper public include files leak porting changes This patch refactors the Windows port of the C client. Notably, it moves as much porting code as possible out of the publicly included `winconfig.h` header and into the relevant source files. This also removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes problems for upstream libraries by removing `#undef AF_INET6` and `#include <windows.h>`. Future work should resolve the `ACL` / `ZKACL` conflict, and potentially remove `include/winconfig.h` altogether.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/313

          @hanm this backports #306 to `branch-3.4`. I think I caught everything, but do give it a review. The build changed a bit between 3.4 and 3.5, so I had to remove some files from `CMakeLists.txt`, and I set the version to `3.4.10`. Then there were some fixes I had to do a bit differently (moved the `#define random` into `zookeeper.c` since `addrvec.c` wasn't created yet, reapply the changes I made to `zookeeper_interest` by hand, and one trivial merge conflict in `recordio.h`).

          I build and ran the tests on Linux, all passing save:

          ```
          1: Zookeeper_simpleSystem::testAsyncWatcherAutoResetterminate called after throwing an instance of 'CppUnit::Exception'
          1: what(): equality assertion failed
          1: - Expected: -101
          1: - Actual : -4
          ```

          which is odd. Is it a flaky test, or very machine dependent?

          I built on Windows, and it all compiled and linked successfully. If you want, I can integration test it. I probably should anyway so I can move Mesos on Windows back to down to ZK 3.4 instead of 3.5.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/313 @hanm this backports #306 to `branch-3.4`. I think I caught everything, but do give it a review. The build changed a bit between 3.4 and 3.5, so I had to remove some files from `CMakeLists.txt`, and I set the version to `3.4.10`. Then there were some fixes I had to do a bit differently (moved the `#define random` into `zookeeper.c` since `addrvec.c` wasn't created yet, reapply the changes I made to `zookeeper_interest` by hand, and one trivial merge conflict in `recordio.h`). I build and ran the tests on Linux, all passing save: ``` 1: Zookeeper_simpleSystem::testAsyncWatcherAutoResetterminate called after throwing an instance of 'CppUnit::Exception' 1: what(): equality assertion failed 1: - Expected: -101 1: - Actual : -4 ``` which is odd. Is it a flaky test, or very machine dependent? I built on Windows, and it all compiled and linked successfully. If you want, I can integration test it. I probably should anyway so I can move Mesos on Windows back to down to ZK 3.4 instead of 3.5.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/306

          Thanks for all your help @hanm! I added a PR for the `branch-3.4` port too.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/306 Thanks for all your help @hanm! I added a PR for the `branch-3.4` port too.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified 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 passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified 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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/886//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/313

          @andschwa Thanks for update. I will review this later this week in detail. Regarding the "flaky" test, AFAIK, this one is not particularly flaky (and in general, there are a lot less flaky tests in C client tests comparing to Java tests). The pre-commit build is green though. Did you observe fairly consistent failures for this `Zookeeper_simpleSystem::testAsyncWatcherAutoResetterminate`? One thing we can try is to run this test with and without your patch and see if there are any difference. Re integration tests - if you have time yeah please go ahead and try it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/313 @andschwa Thanks for update. I will review this later this week in detail. Regarding the "flaky" test, AFAIK, this one is not particularly flaky (and in general, there are a lot less flaky tests in C client tests comparing to Java tests). The pre-commit build is green though. Did you observe fairly consistent failures for this `Zookeeper_simpleSystem::testAsyncWatcherAutoResetterminate`? One thing we can try is to run this test with and without your patch and see if there are any difference. Re integration tests - if you have time yeah please go ahead and try it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/313

          @hanm While the Windows version of Mesos had been using ZooKeeper 3.5.2, the Linux version was using ZooKeeper 3.4.8. I took this patch and cherry-picked back to 3.4.8, and changed the Windows version of Mesos to use 3.4.8 with this patch. However, I also needed 97e598b6c (ZOOKEEPER-1643) in order to build, which is not in `branch-3.4`. Would you be willing to backport ZOOKEEPER-1643 too? If so, then we can switch to the next 3.4 release without any patches at all.

          As for `Zookeeper_simpleSystem::testAsyncWatcherAutoReset`, I observed it failing consistently even on `branch-3.4` built with Autotools (i.e. none of my changes at all). I'm inferring that it's a machine issue, and since it didn't fail on Jenkins, I don't think we need to worry about it.

          Furthermore, with this patch and ZOOKEEPER-1643 (currently in this branch), I was able to integration test with Mesos on Windows and Linux successfully.

          CentOS 7:
          ```
          [==========] Running 7 tests from 1 test case.
          [----------] Global test environment set-up.
          [----------] 7 tests from ZooKeeperTest
          [ RUN ] ZooKeeperTest.Auth
          [ OK ] ZooKeeperTest.Auth (6898 ms)
          [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation
          [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (46 ms)
          [ RUN ] ZooKeeperTest.Create
          [ OK ] ZooKeeperTest.Create (6728 ms)
          [ RUN ] ZooKeeperTest.LeaderDetector
          [ OK ] ZooKeeperTest.LeaderDetector (70 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling
          2017-07-19 12:13:30,626:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:45292] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-19 12:13:30,642:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1757: Socket [127.0.0.1:45292] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (51 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling
          [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (74 ms)
          [ RUN ] ZooKeeperTest.LeaderContender
          2017-07-19 12:13:30,759:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-19 12:13:34,096:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:44849] zk retcode=-112, errno=116(Stale file handle): sessionId=0x15d5c44fa610000 has expired.
          2017-07-19 12:13:34,119:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-19 12:13:34,121:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:44849] zk retcode=-112, errno=116(Stale file handle): sessionId=0x15d5c44fa610001 has expired.
          2017-07-19 12:13:34,142:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response
          2017-07-19 12:13:34,153:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1757: Socket [127.0.0.1:44849] zk retcode=-4, errno=111(Connection refused): server refused to accept the client
          [ OK ] ZooKeeperTest.LeaderContender (6787 ms)
          [----------] 7 tests from ZooKeeperTest (20654 ms total)

          [----------] Global test environment tear-down
          [==========] 7 tests from 1 test case ran. (20795 ms total)
          [ PASSED ] 7 tests.
          ```

          Windows 10:
          ```
          [==========] Running 7 tests from 1 test case.
          [----------] Global test environment set-up.
          [----------] 7 tests from ZooKeeperTest
          [ RUN ] ZooKeeperTest.Auth
          [ OK ] ZooKeeperTest.Auth (7641 ms)
          [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation
          [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (71 ms)
          [ RUN ] ZooKeeperTest.Create
          [ OK ] ZooKeeperTest.Create (7018 ms)
          [ RUN ] ZooKeeperTest.LeaderDetector
          [ OK ] ZooKeeperTest.LeaderDetector (93 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling
          2017-07-19 12:13:55,269:18232(0x53d8):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57541] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-19 12:13:55,271:18232(0x53d8):ZOO_ERROR@handle_socket_error_msg@1519: Socket [127.0.0.1:57541] zk retcode=-4, errno=140(Unknown error): failed to send a handshake packet: Unknown error
          [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (76 ms)
          [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling
          [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (66 ms)
          [ RUN ] ZooKeeperTest.LeaderContender
          2017-07-19 12:13:55,470:18232(0xaf4):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-19 12:13:55,473:18232(0xaf4):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:57557] zk retcode=-112, errno=19(No such device): sessionId=0x15d5c455ad70000 has expired.
          2017-07-19 12:13:55,519:18232(0x4dfc):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-19 12:13:55,523:18232(0x4dfc):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:57557] zk retcode=-112, errno=19(No such device): sessionId=0x15d5c455ad70001 has expired.
          2017-07-19 12:13:55,562:18232(0xc0c):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response
          2017-07-19 12:13:55,564:18232(0xc0c):ZOO_ERROR@handle_socket_error_msg@1519: Socket [127.0.0.1:57557] zk retcode=-4, errno=140(Unknown error): failed to send a handshake packet: Unknown error
          [ OK ] ZooKeeperTest.LeaderContender (714 ms)
          [----------] 7 tests from ZooKeeperTest (15727 ms total)

          [----------] Global test environment tear-down
          [==========] 7 tests from 1 test case ran. (16802 ms total)
          [ PASSED ] 7 tests.
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/313 @hanm While the Windows version of Mesos had been using ZooKeeper 3.5.2, the Linux version was using ZooKeeper 3.4.8. I took this patch and cherry-picked back to 3.4.8, and changed the Windows version of Mesos to use 3.4.8 with this patch. However, I also needed 97e598b6c ( ZOOKEEPER-1643 ) in order to build, which is not in `branch-3.4`. Would you be willing to backport ZOOKEEPER-1643 too? If so, then we can switch to the next 3.4 release without any patches at all. As for `Zookeeper_simpleSystem::testAsyncWatcherAutoReset`, I observed it failing consistently even on `branch-3.4` built with Autotools (i.e. none of my changes at all). I'm inferring that it's a machine issue, and since it didn't fail on Jenkins, I don't think we need to worry about it. Furthermore, with this patch and ZOOKEEPER-1643 (currently in this branch), I was able to integration test with Mesos on Windows and Linux successfully. CentOS 7: ``` [==========] Running 7 tests from 1 test case. [----------] Global test environment set-up. [----------] 7 tests from ZooKeeperTest [ RUN ] ZooKeeperTest.Auth [ OK ] ZooKeeperTest.Auth (6898 ms) [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (46 ms) [ RUN ] ZooKeeperTest.Create [ OK ] ZooKeeperTest.Create (6728 ms) [ RUN ] ZooKeeperTest.LeaderDetector [ OK ] ZooKeeperTest.LeaderDetector (70 ms) [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling 2017-07-19 12:13:30,626:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:45292] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-19 12:13:30,642:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1757: Socket [127.0.0.1:45292] zk retcode=-4, errno=111(Connection refused): server refused to accept the client [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (51 ms) [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (74 ms) [ RUN ] ZooKeeperTest.LeaderContender 2017-07-19 12:13:30,759:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-19 12:13:34,096:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:44849] zk retcode=-112, errno=116(Stale file handle): sessionId=0x15d5c44fa610000 has expired. 2017-07-19 12:13:34,119:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-19 12:13:34,121:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:44849] zk retcode=-112, errno=116(Stale file handle): sessionId=0x15d5c44fa610001 has expired. 2017-07-19 12:13:34,142:200313(0x7fd96cfff700):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:44849] zk retcode=-4, errno=112(Host is down): failed while receiving a server response 2017-07-19 12:13:34,153:200313(0x7fd99cb08700):ZOO_ERROR@handle_socket_error_msg@1757: Socket [127.0.0.1:44849] zk retcode=-4, errno=111(Connection refused): server refused to accept the client [ OK ] ZooKeeperTest.LeaderContender (6787 ms) [----------] 7 tests from ZooKeeperTest (20654 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test case ran. (20795 ms total) [ PASSED ] 7 tests. ``` Windows 10: ``` [==========] Running 7 tests from 1 test case. [----------] Global test environment set-up. [----------] 7 tests from ZooKeeperTest [ RUN ] ZooKeeperTest.Auth [ OK ] ZooKeeperTest.Auth (7641 ms) [ RUN ] ZooKeeperTest.SessionTimeoutNegotiation [ OK ] ZooKeeperTest.SessionTimeoutNegotiation (71 ms) [ RUN ] ZooKeeperTest.Create [ OK ] ZooKeeperTest.Create (7018 ms) [ RUN ] ZooKeeperTest.LeaderDetector [ OK ] ZooKeeperTest.LeaderDetector (93 ms) [ RUN ] ZooKeeperTest.LeaderDetectorTimeoutHandling 2017-07-19 12:13:55,269:18232(0x53d8):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57541] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-19 12:13:55,271:18232(0x53d8):ZOO_ERROR@handle_socket_error_msg@1519: Socket [127.0.0.1:57541] zk retcode=-4, errno=140(Unknown error): failed to send a handshake packet: Unknown error [ OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (76 ms) [ RUN ] ZooKeeperTest.LeaderDetectorCancellationHandling [ OK ] ZooKeeperTest.LeaderDetectorCancellationHandling (66 ms) [ RUN ] ZooKeeperTest.LeaderContender 2017-07-19 12:13:55,470:18232(0xaf4):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-19 12:13:55,473:18232(0xaf4):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:57557] zk retcode=-112, errno=19(No such device): sessionId=0x15d5c455ad70000 has expired. 2017-07-19 12:13:55,519:18232(0x4dfc):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-19 12:13:55,523:18232(0x4dfc):ZOO_ERROR@handle_socket_error_msg@1799: Socket [127.0.0.1:57557] zk retcode=-112, errno=19(No such device): sessionId=0x15d5c455ad70001 has expired. 2017-07-19 12:13:55,562:18232(0xc0c):ZOO_ERROR@handle_socket_error_msg@1781: Socket [127.0.0.1:57557] zk retcode=-4, errno=32(Broken pipe): failed while receiving a server response 2017-07-19 12:13:55,564:18232(0xc0c):ZOO_ERROR@handle_socket_error_msg@1519: Socket [127.0.0.1:57557] zk retcode=-4, errno=140(Unknown error): failed to send a handshake packet: Unknown error [ OK ] ZooKeeperTest.LeaderContender (714 ms) [----------] 7 tests from ZooKeeperTest (15727 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test case ran. (16802 ms total) [ PASSED ] 7 tests. ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/311

          `branch-3.5` has ZOOKEEPER-1643, so this patch should just be good to go. It was integration tested.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/311 `branch-3.5` has ZOOKEEPER-1643 , so this patch should just be good to go. It was integration tested.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified 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 failed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified 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 failed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/892//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified 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/893//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/893//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/893//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified 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/893//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/893//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/893//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user andschwa commented on the issue:

          https://github.com/apache/zookeeper/pull/313

          [This tree](https://github.com/andschwa/zookeeper/tree/cmake-backport-3.4.8) is the exact patch I tested on Windows with the 3.4.8 tarball.

          Show
          githubbot ASF GitHub Bot added a comment - Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/313 [This tree] ( https://github.com/andschwa/zookeeper/tree/cmake-backport-3.4.8 ) is the exact patch I tested on Windows with the 3.4.8 tarball.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified 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 passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified 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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/891//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on the issue:

          https://github.com/apache/zookeeper/pull/313

          >> Would you be willing to backport ZOOKEEPER-1643 too?

          Sounds good to me. I don't see why this change should not be part of 3.4 release.

          >> I observed it failing consistently even on branch-3.4 built with Autotools

          Hmm, that is interesting. I did not remember this test was ever on radar. I'll try run it on my env and see what i got.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/313 >> Would you be willing to backport ZOOKEEPER-1643 too? Sounds good to me. I don't see why this change should not be part of 3.4 release. >> I observed it failing consistently even on branch-3.4 built with Autotools Hmm, that is interesting. I did not remember this test was ever on radar. I'll try run it on my env and see what i got.

            People

            • Assignee:
              andschwa Andrew Schwartzmeyer
              Reporter:
              andschwa Andrew Schwartzmeyer
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development