Skip to content
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

Update lib and apply PR for YAML Merge Key #2

Merged
merged 90 commits into from
May 10, 2024

Conversation

azat
Copy link

@azat azat commented Apr 16, 2024

theamarin and others added 30 commits July 4, 2021 22:30
Removed the variable name in the defaulted function to make GCC happy.
* Export YAML::detail::node::m_amount

The internal header node/detail/node.h is included by public headers;
YAML::detail::node is implemented in the header itself, and thus it gets
inlined... except for its static m_amount class member, which is
instantiated in the library only. Right now all the symbols of yaml-cpp
are exported (nothing is hidden), so the linker will find node::m_amount
in the yaml-cpp library.

As solution/workaround, explicitly export YAML::detail::node::m_amount.

* CMake: use GenerateExportHeader

Make use of the GenerateExportHeader CMake module to generate the dll.h
header with export macros.

While the produced dll.h is different, the result should be the same,
i.e. nothing changes for yaml-cpp or its users.

* CMake: hide all the symbols by default

Hide all the symbols that are not explicitly exported with YAML_CPP_API.
This way the ABI will be way smaller, and only actually exposing the
public classes/functions.
GNUInstallDirs provided may be absolute paths, in which case appending
to the install prefix is not correct. We can instead use the provided
CMAKE_INSTALL_FULL_* variables, which are precomputed absolute paths.

https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html
…beder#1064)

Partial revert of "Revert "Revert "Hide most of non-public symbols by default (jbeder#984)" (jbeder#1038)" (jbeder#1045)"

This reverts commit 0733aeb.
This replaces the old Travis CI badge that didn't mean anything.
Add copying of shared library to the output directory with a test binary.

[binary] removed using of non unsigned char as argument at
'std::isspace' function that was provokes undefined behavior.

[.github/workflows/build.yml] enabled run of test at the
'windows-latest' environment.
Also refactor the build action to use a matrix.
Complies with YAML Standard [5.4](https://yaml.org/spec/1.2.2/#54-line-break-characters) [25] instead of matching `\r` only in combination with `\n`.
After configuring the file `yaml-cpp-config.cmake.in`, the result ends up with
empty variables.  (see also the discussion in jbeder#774).

Rework this file and the call to `configure_package_config_file` according the
cmake documentation
(https://cmake.org/cmake/help/v3.22/module/CMakePackageConfigHelpers.html?highlight=configure_package_config#command:configure_package_config_file)
to overcome this issue and allow a simple `find_package` after install.

As there was some discussion about the place where to install the
`yaml-cpp-config.cmake` file, e.g. jbeder#1055, factor out the install location into
an extra variable to make it easier changing this location in the future.

Also untabify CMakeLists.txt in some places to align with the other code parts in this file.
…endencies also. (jbeder#1039)

The option `YAML_CPP_BUILD_TESTS` currently enables or disables building of tests; but unconditionally the CMake file includes CTest; this PR makes that conditional on the option.

Also, there is no option for enabling formatting, but it does check whether it can find the `clang-format` executable; this PR adds an option (default to true) that skips even looking for the executable if disabled.
Changes YAML_CPP_INSTALL from a cmake_dependent_option to an option.

Fixes jbeder#756, jbeder#847, and jbeder#1011.
LocutusOfBorg and others added 18 commits October 23, 2023 07:20
Support for YAML Merge keys ( <<: [*dict1, *dict2] ) is added. The merge
key is a specific scalar with value << (and tag !!merge) that implies
that during node construction, the map (or sequence of maps) are merged
into the current map. The priority rules are that each key from maps
within the value associated with  << are added iff the key is not yet
present in the current map (and first map gets higher priority). Test
cases have been added accordingly.
Protect from regressions due to use of undefined or
implementation-specific behavior when using `std::` containers and smart
pointers.

This only has effect on platforms with the GNU standard C++ library.

Refer to https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode.html.
Bumps the github-actions group with 1 update: [actions/checkout](https://github.com/actions/checkout).

- [Release notes](https://github.com/actions/checkout/releases)
- [Commits](actions/checkout@v2...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Apr 16, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 55 committers have signed the CLA.

✅ azat
❌ theamarin
❌ mjvankampen
❌ jbeder
❌ snowp
❌ JBPennington
❌ jasonbeach
❌ pinotree
❌ Ngiong
❌ janisozaur
❌ r-burns
❌ marcospb19
❌ PhilipDeegan
❌ TheVice
❌ dota17
❌ christian-rauch
❌ Sam4uk
❌ theOtherMichael
❌ acxz
❌ Skywol
❌ SpaceIm
❌ felix2010
❌ xiaozhuai
❌ hyperxor
❌ vehre-x41
❌ jwindgassen
❌ BMBurstein
❌ stephematician
❌ sfalmo
❌ tgurr
❌ darktohka
❌ zenno-leaky-bucket
❌ hkarel
❌ Ryanf55
❌ craigscott-crascit
❌ tchaikov
❌ jelin-sh
❌ LocutusOfBorg
❌ pauljurczak
❌ stonier
❌ diogoteles08
❌ 0xFireWolf
❌ rdzehtsiar
❌ PJayB
❌ zaucy
❌ humbertodias
❌ FtZPetruska
❌ rico-chet
❌ nlescoua
❌ Alejandro-FA
❌ Trompettesib
❌ Levi-Armstrong
❌ Megamouse
❌ MatthijsBurgh
❌ threeal
You have signed the CLA already but the status is still pending? Let us recheck it.

@azat azat force-pushed the update-lib-and-merge-operator branch 2 times, most recently from 365663a to e9e5057 Compare May 3, 2024 11:57
azat added 3 commits May 3, 2024 14:36
…the node)

The problem is that const_map_to.get(*j->first) compares only the
shared_ptr's, while we need to compare the key itself.

v2: remove const iterator
v3: fix use-after-free due to const reference
Consider the following YAML:

    trait1: &t1
      foo: 1

    trait2: &t2
      foo: 2

    merged:
      <<: *t1
      <<: *t2

yq reports:

    $ yq .merged.foo < /tmp/yaml
    2

while the order that yaml-cpp returns is different, since it will
firstly handle 1, and will not replace it with 2:

    $ util/parse < /tmp/yaml
    trait1:
      ? &1 foo
      : &2 1
    trait2:
      foo: 2
    merged:
      *1 : *2

(Don't mix up "*2" with "2", it is trait1)
* merge-operator:
  Fix order for merging iterator
  Fix merge operator support (that can be visible by iterating through the node)
  Fix merge-key handling in case the dictionary contains a sub-dictionary
  Adding support for handling YAML Merge Key (jbeder#41)
@azat azat force-pushed the update-lib-and-merge-operator branch from e9e5057 to 2ce752f Compare May 3, 2024 12:38
@vitlibar vitlibar merged commit f91e938 into ClickHouse:master May 10, 2024
azat added a commit to azat/ClickHouse that referenced this pull request May 10, 2024
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.