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

cpp: add build-time flags to make compression dependencies optional #1013

Merged
merged 7 commits into from
Nov 14, 2023

Conversation

james-rms
Copy link
Collaborator

Public-Facing Changes

Adds two new build-time flags to optionally disable zstd and lz4 support. These are MCAP_COMPRESSION_NO_ZSTD and MCAP_COMPRESSION_NO_LZ4.

When reading MCAP files, if the reader encounters a known compression method but is compiled without support for it, it will yield Status::UnsupportedCompression.

Description

Based on #1008

A new test binary is added to CI, which exercises these build-time flags.

New defines: MCAP_COMPRESSION_NO_ZSTD, MCAP_COMPRESSION_NO_LZ4
@james-rms james-rms changed the title cpp: cpp: add build-time flags to make compression dependencies optional Nov 13, 2023
Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

👍

cpp/mcap/include/mcap/errors.hpp Outdated Show resolved Hide resolved
cpp/test/unit_tests.cpp Show resolved Hide resolved
@james-rms james-rms merged commit f87352f into main Nov 14, 2023
25 checks passed
@james-rms james-rms deleted the jrms/cpp-optional-compression-dependencies branch November 14, 2023 18:55
@asherikov
Copy link
Contributor

Well, I was lazy with my PR, but if you are not I have a few suggestions:

  • the defines should enable compression rather than disabling it;
  • the defines should be located in corresponsing headers, so that the end users would need to include the headers they need instead of playing with defines;
  • compression related stuff should be moved to mixin classes.

PS: oh, ok i guess too late.

pezy pushed a commit to pezy/mcap that referenced this pull request Jan 11, 2024
…oxglove#1013)

### Public-Facing Changes

Adds two new build-time flags to optionally disable `zstd` and `lz4`
support. These are `MCAP_COMPRESSION_NO_ZSTD` and
`MCAP_COMPRESSION_NO_LZ4`.

When reading MCAP files, if the reader encounters a known compression
method but is compiled without support for it, it will yield
`Status::UnsupportedCompression`.


### Description

Based on foxglove#1008

A new test binary is added to CI, which exercises these build-time
flags.

---------

Co-authored-by: Alexander Sherikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants