Skip to content

CXX-2753 Refactor directory structure to allow for multiple ABI namespaces #1026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 5, 2023

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Sep 19, 2023

Description

Resolves CXX-2753 as part of CXX-1569. Verified by this patch.

This PR significantly changes the directory structure of CXX Driver sources files for bsoncxx and mongocxx in preparation for the addition of C++ sources and headers corresponding to additional ABI namespaces (not just v_noabi). Those will be introduced in subsequent PRs.

The only observable changes to contents of the install directory structure pre-PR vs. post-PR should be the legacy CMake package config files (to support multiple include directories) and the uninstall script (simply due to changes in ordering of generated commands). Any other changes should be considered bugs in this PR.

Both binary and source compatibility (should be 100%) is verified by this patch, which runs abi-complance-checker comparing the proposed PR changes relative to the latest state of the target branch (note: not against the latest release of the CXX Driver, as is the case for the C Driver abi-compliance-check task).

Directory Structure

In this PR description, "source directory" refers to files tracked by the Git repository (.cpp, .hpp, .in, etc.), "binary directory" refers to files that are generated by CMake during configuration and build, and "install directory" refers to files present in the install prefix after the CXX Driver is commanded to evaluate installation rules.

The pre-PR source directory structure for the bsoncxx and mongocxx libraries is as follows (only the minimal set of files necessary to explain the directory refactor are listed):

src
├── CMakeLists.txt
├── bsoncxx
│   ├── CMakeLists.txt
│   ├── cmake
│   │   ├── bsoncxx-config.cmake.in
│   │   └── ...
│   ├── config
│   │   ├── CMakeLists.txt
│   │   ├── config.hpp.in
│   │   ├── libbsoncxx.pc.in, libbsoncxx-static.pc.in
│   │   ├── private
│   │   │   ├── config.hh.in
│   │   │   └── prelude.hh, postlude.hh
│   │   └── prelude.hpp, postlude.hpp, ...
│   ├── private
│   │   └── libbson.hh, ...
│   ├── document
│   │   └── value.hpp, value.cpp, ...
│   ├── types.hpp, types.cpp, ...
│   ├── test
│   │   ├── CMakeLists.txt
│   │   └── ...
│   └── third_party
│       └── CMakeLists.txt
├── mongocxx
│   ├── CMakeLists.txt
│   ├── cmake
│   │   ├── mongocxx-config.cmake.in
│   │   └── ...
│   ├── config
│   │   ├── CMakeLists.txt
│   │   ├── config.hpp.in
│   │   ├── libmongocxx.pc.in, libmongocxx-static.pc.in
│   │   ├── private
│   │   │   ├── config.hh.in
│   │   │   └── prelude.hh, postlude.hh
│   │   └── prelude.hpp, postlude.hpp, ...
│   ├── private
│   │   └── libmongoc.hh, libmongoc.cpp, ...
│   ├── options
│   │   └── find.hpp, find.cpp, ...
│   ├── instance.hpp, instance.cpp, ...
│   └── test
│       ├── CMakeLists.txt
│       └── ...
└── third_party
    └── catch
        ├── include
        │   ├── catch.hpp
        │   └── helpers.hpp
        └── main.cpp

This contrasts with the install directory structure for the bsoncxx and mongocxx libraries, where all current header files are placed within a v_noabi subdirectory corresponding to the v_noabi ABI namespace:

<install-prefix>
├── include
│   ├── bsoncxx
│   │   └── v_noabi
│   │       └── bsoncxx
│   │           ├── array
│   │           │   └── value.hpp, ...
│   │           ├── config
│   │           │   ├── config.hpp
│   │           │   ├── prelude.hpp, postlude.hpp
│   │           │   └── ...
│   │           ├── types.hpp
│   │           └── ...
│   └── mongocxx
│       └── v_noabi
│           └── mongocxx
│               ├── config
│               │   ├── config.hpp
│               │   ├── prelude.hpp, postlude.hpp
│               │   └── ...
│               ├── options
│               │   └── find.hpp, ...
│               ├── instance.hpp
│               └── ...
├── lib
│   ├── cmake
│   │   └── ...
│   ├── pkgconfig
│   │   └── ...
│   ├── libbsoncxx.so._noabi
│   ├── libmongocxx.so._noabi
│   └── ...
└── share/mongo-cxx-driver
    └── ...

This PR proposes a source directory structure that better aligns with the install directory structure. The proposed source directory structure will also allow for the addition and maintenance of source files corresponding to additional ABI namespaces (i.e. v1) without conflicting with existing v_noabi namespace files.

The post-PR source directory structure is as follows:

src
├── CMakeLists.txt
├── bsoncxx
│   ├── CMakeLists.txt
│   ├── cmake
│   │   ├── bsoncxx-config.cmake.in
│   │   ├── libbsoncxx.pc.in, libbsoncxx-static.pc.in
│   │   └── ...
│   ├── include
│   │   └── bsoncxx
│   │       └── v_noabi
│   │           └── bsoncxx
│   │               ├── config
│   │               │   ├── prelude.hpp, postlude.hpp
│   │               │   └── ...
│   │               ├── document
│   │               │   └── value.hpp, ...
│   │               └── types.hpp, ...
│   ├── lib
│   │   └── bsoncxx
│   │       └── v_noabi
│   │           └── bsoncxx
│   │               ├── config
│   │               │   ├── CMakeLists.txt
│   │               │   ├── private
│   │               │   │   ├── config.hh.in
│   │               │   │   └── prelude.hh, postlude.hh
│   │               │   └── config.hpp.in, ...
│   │               ├── private
│   │               │   └── libbson.hh, ...
│   │               ├── document
│   │               │   └── value.cpp, ...
│   │               └── types.cpp, ...
│   ├── test
│   │   ├── CMakeLists.txt
│   │   └── ...
│   └── third_party
│       └── CMakeLists.txt
├── mongocxx
│   ├── CMakeLists.txt
│   ├── cmake
│   │   ├── mongocxx-config.cmake.in
│   │   └── libmongocxx.pc.in, libmongocxx-static.pc.in
│   │   └── ...
│   ├── include
│   │   └── mongocxx
│   │       └── v_noabi
│   │           └── mongocxx
│   │               ├── config
│   │               │   └── prelude.hpp, postlude.hpp, ...
│   │               ├── options
│   │               │   └── find.hpp, ...
│   │               └── instance.hpp, ...
│   ├── lib
│   │   └── mongocxx
│   │       └── v_noabi
│   │           └── mongocxx
│   │               ├── config
│   │               │   ├── CMakeLists.txt
│   │               │   ├── private
│   │               │   │   ├── config.hh.in
│   │               │   │   └── prelude.hh, postlude.hh
│   │               │   └── config.hpp.in, ...
│   │               ├── private
│   │               │   └── libmongoc.hh, libmongoc.cpp, ...
│   │               ├── options
│   │               │   └── find.cpp, ...
│   │               └── instance.cpp, ...
│   └── test
│       ├── CMakeLists.txt
│       └── ...
└── third_party
    └── catch
        ├── include
        │   ├── catch.hpp
        │   └── helpers.hpp
        └── main.cpp

In summary, this PR proposes bsoncxx and mongocxx follow this new directory structure:

<name>
├── cmake
│   └── packaging-files.in (generator input)
├── include/<name>
│   └── v_noabi
│       └── <name>
│           └── public-headers.hpp
├── lib/<name>
│   └── v_noabi
│       └── <name>
│           ├── public-headers.hpp.in (generator input)
│           ├── private-headers.hh.in (generator input)
│           ├── private-headers.hh
│           └── sources.cpp
└── test
    └── test-files.cpp

Notes:

  • Generator input files (.in) pertaining to packaging (CMake and pkgconfig) are located under cmake/ rather than lib/ as they do not directly contribute to the generated library files.
  • The include/ subdirectory contains public header files, and only public header files, such that the directory can be directly installed into the install prefix as-is. (Note: not all public headers, as some are generated.)
  • Generator input files (.in) under lib/ are located in subdirectories directly corresponding to their intended location in both the binary and install directories (e.g. lib/bsoncxx/v_noabi/bsoncxx/config/config.hpp.in -> <install-prefix>/bsoncxx/v_noabi/bsoncxx/config/config.hpp).
  • Test files that are only enabled by ENABLE_TESTS are located in a separate test/ subdirectory. (Note: test_util/client_helpers.cpp is a weird exception and will be dealt with properly later.)

Currently, no source or header files make explicit reference to the v_noabi subdirectory, as the include directories are set to point to include/bsoncxx/v_noabi/. However, to prepare for the addition of additional ABI namespace headers (e.g. under include/bsoncxx/v1/), all files under both include/ and lib/ are located under a <name> subdirectory to facilitate inclusion of both public and private header files using the same include directive prefix. In other words, this directory structure is prepared to support all of the following include directives (where <project-dir> refers to the src/<name> source directory containing the CMakeLists.txt file that calls project(<name>)):

  • #include <bsoncxx/config/config.hpp> (status quo: enabled by -I<project-dir>/include/bsoncxx/v_noabi).
  • #include <bsoncxx/private/libbson.hh> (status quo: enabled by -I<project-dir>/lib/bsoncxx/v_noabi).
  • #include <bsoncxx/v_noabi/bsoncxx/config.hpp> (future: enabled by -I<project-dir>/include, consistent with -I<install-prefix>/include).
  • #include <bsoncxx/v_noabi/bsoncxx/private/libbson.hh> (future: enabled by -I<project-dir>/lib).
  • #include <bsoncxx/v1/bsoncxx/config.hpp> (future: enabled by -I<project-dir>/include, consistent with -I<install-prefix>/include).
  • #include <bsoncxx/v1/bsoncxx/private/libbson.hh> (future: enabled by -I<project-dir>/lib).

For backwards-compatibility, -I<install-prefix>/bsoncxx/v_noabi is expected to remain the default and have higher priority than -I<install-prefix>. Users that want to opt into v1 ABI namespace headers (that are not already made available via v_noabi headers) would be able to do so via the bsoncxx/v1/ prefix, and users that want to explicitly opt into v_noabi ABI namespace headers (for clarity, forward-compatibility, etc.) would be able to do so via the bsoncxx/v_noabi/ prefix.

To prepare to support the above scenario, the legacy CMake package config files were updated to support more than one path in PACKAGE_INCLUDE_INSTALL_DIRS (pre-PR, it assumed exactly one path only). The modern CMake package config files and pkgconfig config files are already prepared to support multiple include directories.

Note: there are some files this PR currently places in the v_noabi subdirectory which may appear to deserve "promotion" to a multi-ABI-namespace file (i.e. files under config). Such files will be individually and incrementally promoted in subsequent PRs as needed.

Note: test files under test subdirectories have deliberately not been restructured to follow the v_noabi subdirectory pattern. This is to preserve the location of test executables in the binary directory (as currently expected by EVG scripts). The updating of test files to align with the ABI namespace structure is considered low-priority and less-motivated than updates to the library files themselves. As such, refactoring of test files will be deferred to when they are demanded by the addition of tests for new ABI namespace entities.

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I very much support the better alignment of the source and install directory structure.

Running ABI compliance check to verify compatibility is much appreciated.

@eramongodb eramongodb removed the request for review from kkloberdanz September 20, 2023 15:24
Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay. LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants