Skip to content

Commit

Permalink
removed git submodules to fix various tooling issues (closes #151)
Browse files Browse the repository at this point in the history
- removed submodules in `external/`
- vendored `Catch2` and `nlohmann/json directly`
- updated conformance test generator to look outside the repo for test sources
  • Loading branch information
marzer committed May 1, 2022
1 parent 1c09239 commit e47ef8c
Show file tree
Hide file tree
Showing 33 changed files with 41,572 additions and 312 deletions.
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,5 @@ meson.build text eol=lf encoding=UTF-8
*.psd binary
*.rc binary
*.xlsx binary

vendor/* linguist-vendored
6 changes: 2 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
- name: Install dependencies
run: |
apt-get -y update
apt-get -y install --no-install-recommends ${{ matrix.compiler }} lld meson pkgconf git ca-certificates locales-all python3-pip catch2 nlohmann-json3-dev
apt-get -y install --no-install-recommends ${{ matrix.compiler }} lld meson pkgconf git ca-certificates locales-all python3-pip
- uses: actions/checkout@v2

Expand Down Expand Up @@ -93,8 +93,6 @@ jobs:
pip3 install meson ninja
- uses: actions/checkout@v2
with:
submodules: true

- uses: ilammy/msvc-dev-cmd@v1

Expand Down Expand Up @@ -158,4 +156,4 @@ jobs:
# build
tipi . --dont-upgrade --verbose -t linux-cxx17
./build/linux-cxx17/bin/simple_parser ../example.toml
./build/linux-cxx17/bin/simple_parser ../example.toml
21 changes: 0 additions & 21 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,21 +0,0 @@
[submodule "external/toml-test"]
path = external/toml-test
url = https://github.com/BurntSushi/toml-test.git
shallow = true
[submodule "external/toml-spec-tests"]
path = external/toml-spec-tests
url = https://github.com/iarna/toml-spec-tests.git
shallow = true
[submodule "external/tloptional"]
path = external/tloptional
url = https://github.com/TartanLlama/optional.git
shallow = true
[submodule "external/json"]
path = external/json
url = https://github.com/nlohmann/json.git
shallow = true
[submodule "external/Catch2"]
path = external/Catch2
url = https://github.com/catchorg/Catch2.git
branch = v2.x
shallow = true
6 changes: 1 addition & 5 deletions .tipi/deps
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
{
"x": ["/external"],
"requires": {
"tipi-deps/Catch2@file://single_include": { "@": "v2.x" },
"nlohmann/json": { "@": "v3.10.5" }
}
"requires": {}
}
6 changes: 1 addition & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ If you wish to submit a PR, please be aware that:
<br>

## Building and running the tests
Testing is done using [Catch2], included in the respository as a submodule under `extern/Catch2`.
The first time you want to begin testing you'll need to ensure submodules have been fetched:
```bash
git submodule update --init --depth 1 external/Catch2 external/tloptional
```
Testing is done using [Catch2], vendored in the respository at `vendor/catch.hpp`.

### Testing on Windows with Visual Studio

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ FetchContent_MakeAvailable(tomlplusplus)
git submodule add --depth 1 https://github.com/marzer/tomlplusplus.git tomlplusplus
git config -f .gitmodules submodule.tomlplusplus.shallow true
```
> &#xFE0F; The toml++ repository has some submodules of its own, but **they are only used for testing**!
> You should not use the `--recursive` option for regular library consumption.
> &#xFE0F; The toml++ repository has some submodules of its own, but **they are only used for testing**!
> You should **not** use the `--recursive` option for regular library consumption.

### Other environments and package managers
Expand Down
10 changes: 5 additions & 5 deletions docs/pages/main_page.dox
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,8 @@ FetchContent_MakeAvailable(tomlplusplus)
git submodule add --depth 1 https://github.com/marzer/tomlplusplus.git tomlplusplus
git config -f .gitmodules submodule.tomlplusplus.shallow true
\ebash
\note The toml++ repository has some submodules of its own, but **they are only used for testing**! You should not use
the `--recursive` option for regular library consumption.
\attention The toml++ repository has some submodules of its own, but **they are only used for testing**!
You should **not** use the `--recursive` option for regular library consumption.



Expand All @@ -535,7 +535,7 @@ integration you'd like to see and have the technical know-how to make it happen,

\subsection mainpage-adding-lib-python Special mention: Python
There exists a python wrapper library built around toml++ called
\github{bobfang1992/pytomlpp, pytomlpp} which is, at the time of writing, the one of only two natively-compiled
\github{bobfang1992/pytomlpp, pytomlpp} which is, at the time of writing, one of only two natively-compiled
TOML libraries available for python, and thus one of the fastest options available:

\out
Expand All @@ -554,8 +554,8 @@ Install it using `pip`:
pip install pytomlpp
\ebash

Note that I'm not the owner of that project so if you wish to report a bug relating to the python
implementation, please do so at their repository, not on the main toml++ one.
Note that I'm not the owner of that project, so if you wish to report a bug relating to the python
implementation please do so at their repository, not on the main toml++ one.



Expand Down
1 change: 0 additions & 1 deletion external/Catch2
Submodule Catch2 deleted from d71b46
1 change: 0 additions & 1 deletion external/json
Submodule json deleted from fcc36f
1 change: 0 additions & 1 deletion external/tloptional
Submodule tloptional deleted from c28fcf
1 change: 0 additions & 1 deletion external/toml-spec-tests
Submodule toml-spec-tests deleted from 1880b1
1 change: 0 additions & 1 deletion external/toml-test
Submodule toml-test deleted from 10cdc9
2 changes: 1 addition & 1 deletion include/toml++/impl/node_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ TOML_NAMESPACE_START
private:
/// \cond
template <typename Func>
static constexpr bool visit_is_nothrow = noexcept(std::declval<viewed_type*>()->visit(std::declval<Func&&>()));
static constexpr bool visit_is_nothrow = noexcept(std::declval<viewed_type*>()->visit(std::declval<Func>()));
/// \endcond

public:
Expand Down
3 changes: 2 additions & 1 deletion include/toml++/impl/preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,10 @@
#endif

// dll/shared lib function exports (legacy - TOML_API was the old name for this setting)
#if !defined(TOML_EXPORTED_MEMBER_FUNCTION) \
#if !defined(TOML_EXPORTED_MEMBER_FUNCTION) \
&& !defined(TOML_EXPORTED_STATIC_FUNCTION) \
&& !defined(TOML_EXPORTED_FREE_FUNCTION) \
&& !defined(TOML_EXPORTED_CLASS) \
&& defined(TOML_API)
#define TOML_EXPORTED_MEMBER_FUNCTION TOML_API
#define TOML_EXPORTED_STATIC_FUNCTION TOML_API
Expand Down
1 change: 0 additions & 1 deletion meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ project(
version: '3.1.0',
meson_version: '>=0.54.0',
license: 'MIT',
subproject_dir: 'external',
default_options: [ # https://mesonbuild.com/Builtin-options.html
# core options
'buildtype=release',
Expand Down
4 changes: 0 additions & 4 deletions tests/impl_toml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@
#define TOML_IMPLEMENTATION
#endif

#if USE_TARTANLLAMA_OPTIONAL
#include "lib_tloptional.h"
#endif

#if USE_SINGLE_HEADER
#include "../toml.hpp"
#else
Expand Down
8 changes: 1 addition & 7 deletions tests/lib_catch2.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,7 @@
#pragma warning(disable : 5105)
#endif

#if __has_include(<Catch2/single_include/catch2/catch.hpp>)
#include <Catch2/single_include/catch2/catch.hpp>
#elif __has_include(<catch2/catch.hpp>)
#include <catch2/catch.hpp>
#else
#error Catch2 is missing! You probably need to fetch submodules ("git submodule update --init --depth 1 external/Catch2")
#endif
#include "../vendor/catch.hpp"

#ifdef __clang__
#pragma clang diagnostic pop
Expand Down
30 changes: 0 additions & 30 deletions tests/lib_tloptional.h

This file was deleted.

36 changes: 0 additions & 36 deletions tests/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,6 @@ test_deps = [tomlplusplus_dep]

fs = import('fs')

catch2_dep = dependency('catch2', required: false)
if catch2_dep.found()
test_deps += catch2_dep
elif fs.exists('..'/'external'/'Catch2'/'single_include')
catch2_dep = declare_dependency(include_directories: '..'/'external'/'Catch2'/'single_include')
test_deps += catch2_dep
else
error('Catch2 is missing! You probably need to fetch submodules ("git submodule update --init --depth 1 external/Catch2"')
endif

has_tl_optional = false
tl_optional_dep = dependency('tl-optional', required: false)
if tl_optional_dep.found()
test_deps += tl_optional_dep
has_tl_optional = true
elif fs.exists('..'/'external'/'tloptional'/'include')
tl_optional_dep = declare_dependency(include_directories: '..'/'external'/'tloptional'/'include')
test_deps += tl_optional_dep
has_tl_optional = true
else
warning('tl-optional not found, skipping its tests')
endif

This comment has been minimized.

Copy link
@marzer

marzer May 1, 2022

Author Owner

@Tachi107 I can't break meson's sandbox if I just vendor everything lmao

image

This comment has been minimized.

Copy link
@Tachi107

Tachi107 May 1, 2022

Contributor

You are not wrong, lol

This comment has been minimized.

Copy link
@Tachi107

Tachi107 May 1, 2022

Contributor

I think that this might cause issues with downstream packaging, but I'll figure that out

This comment has been minimized.

Copy link
@marzer

marzer May 1, 2022

Author Owner

Oh, really? What sort of issues?

This comment has been minimized.

Copy link
@Tachi107

Tachi107 May 1, 2022

Contributor

In Debian and Fedora, for example, you aren't allowed to vendor libraries into packages and you have to always use shared linking, probably even for tests. But again, don't worry, I'll come here bothering you once I'll have time to package tomlplusplus (i.e. not very soon)

This comment has been minimized.

Copy link
@marzer

marzer May 1, 2022

Author Owner

Even for tests too? Damn, that's pretty strict. O_O

This comment has been minimized.

Copy link
@Tachi107

Tachi107 May 1, 2022

Contributor

Yeah, but that's to make sure that the software shipped with the distro works with other software shipped with the distro. What if the vendored dependency is old and has a bug that make tests pass while the distro ship an updated (or even more outdated, as it's often the case lol) version that could cause issues when used by the users of that distro? I mean, that's not that common but to ensure that things always work as they should you have to be strict

This comment has been minimized.

Copy link
@marzer

marzer May 1, 2022

Author Owner

Ah, yeah that makes sense. The main motivation for vendoring them was that I'm sick of dealing with some of the weird quirks of git submodules, but still want it to 'just work' everywhere.

I'll add a meson config option to not use the vendored libs and instead use the system ones, that way you could just set that option and not ship the vendor/ dir for those builds. That sound good?

This comment has been minimized.

Copy link
@Tachi107

Tachi107 May 1, 2022

Contributor

It would be great :)

#######################################################################################################################
# fast math check
#######################################################################################################################
Expand Down Expand Up @@ -153,14 +130,6 @@ foreach cpp20 : cpp20_modes
and not get_option('compile_library')
)

# use tl::optional to test that the TOML_OPTIONAL_TYPE config option works as advertised
tl_optional = (
test_counter % 2 == 1
and has_tl_optional
and not get_option('compile_library')
and exceptions # requires exceptions in its API
)

# use asan in the tests
address_sanitizer = (
get_option('asan_tests')
Expand Down Expand Up @@ -241,11 +210,6 @@ foreach cpp20 : cpp20_modes
test_args += '-DUSE_SINGLE_HEADER=1'
endif

if tl_optional
test_args += '-DUSE_TARTANLLAMA_OPTIONAL=1'
test_name = test_name + '_tlopt'
endif

test_executables += [[
test_name,
executable(
Expand Down
8 changes: 0 additions & 8 deletions tests/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@
#define LEAK_TESTS 0
#endif

// use tl::optional?
#if defined(USE_TARTANLLAMA_OPTIONAL) && USE_TARTANLLAMA_OPTIONAL
#define TOML_OPTIONAL_TYPE tl::optional
#else
#undef USE_TARTANLLAMA_OPTIONAL
#define USE_TARTANLLAMA_OPTIONAL 0
#endif

// catch2 config
#define CATCH_CONFIG_CPP11_TO_STRING
#define CATCH_CONFIG_CPP17_OPTIONAL
Expand Down
4 changes: 0 additions & 4 deletions tests/tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
#pragma once
#include "settings.h"

#if USE_TARTANLLAMA_OPTIONAL
#include "lib_tloptional.h"
#endif

#if USE_SINGLE_HEADER
#include "../toml.hpp"
#else
Expand Down
Loading

0 comments on commit e47ef8c

Please sign in to comment.