zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From andschwa <...@git.apache.org>
Subject [GitHub] zookeeper pull request #313: ZOOKEEPER-2841: ZooKeeper public include files ...
Date Tue, 18 Jul 2017 17:52:38 GMT
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.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message