Make strata consumable: install rules, exported target, usage docs - #2
Merged
Conversation
strata had no install rules, no namespaced target and no package config, so the only way to use it from another project was to vendor the source tree and reverse-engineer the include paths. It also had zero tags and zero releases, which meant there was no version anyone could pin. - Export a `strata::strata` target that works identically through FetchContent and through find_package, so a consumer can switch between them without touching their code. - Give the public include directory BUILD_INTERFACE and INSTALL_INTERFACE generator expressions. The old plain source path is meaningless once installed, and would have made the exported target unusable. - Install strata_warnings alongside. It is a build-time-only interface target, but the export set still names it, so omitting it makes the consumer's find_package fail on a target it has never heard of. - Default tests, tools, fuzzers and the RocksDB benchmark OFF unless strata is the top-level project. A consumer pulling this in has no use for the crash harness, and building it would break their configure step on a missing RocksDB or a non-clang compiler. - Document both paths in the README, with a corrected durability note: the guarantee is governed by Options::fsync_policy (kAlways by default), not by a field on WriteOptions, which is empty. Verified by building two throwaway consumer projects outside the repo, one per path, each opening a DB, writing and reading back: both link and run. The FetchContent build produces libstrata.a and nothing else, confirming the option defaults hold. strata's own suite still builds its tools and passes 46/46.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
strata had no install rules, no namespaced target and no package config, so the only way to use it from another project was to vendor the source tree and reverse-engineer the include paths. It also had 0 tags and 0 releases, so there was no version anyone could pin.
What
strata::stratatarget that behaves identically throughFetchContentandfind_package, so a consumer can switch between them without touching their code.BUILD_INTERFACE/INSTALL_INTERFACEgenerator expressions. The old plain source path is meaningless once installed and would have made the exported target unusable.strata_warningsalongside. It is build-time-only, but the export set names it, so omitting it makes a consumer'sfind_packagefail on a target it has never heard of.A doc correction included here
The durability note I first wrote claimed
WriteOptions{.sync = true}. That is wrong:WriteOptionsis an empty struct. The guarantee is governed byOptions::fsync_policy, which defaults toFsyncPolicy::kAlways. Fixed before commit.Verification
Two throwaway consumer projects built outside the repo, one per path:
find_package(strata 0.1 REQUIRED)aftercmake --installread back: it linksFetchContent+strata::strataread back: it linkslibstrata.aonly, no tests or tools, so the option defaults holdINSTALL_GTEST OFFwas already set here