Include tinyformat as a subtree #13842
Conversation
src/Makefile.am
Outdated
There was a problem hiding this comment.
I'm unclear on why this is required to be added to sources, but the build failed consistently when it was only provided via -I below, e.g.:
https://travis-ci.org/Empact/bitcoin/jobs/410971896
I'll look into this a bit more and report back.
|
Re-opened from #13834. This is a step toward creating an separate compilation context for bitcoin vs dependency code, which will allow for stricter internal checks during compilation. |
|
Seems like a lot of files to pull in just to make it a subtree, no? Imo tinyformat is really meant to be a single file just to be copied into the project. |
|
Mainly I just want tinyformat in a subdir, rather than mixed in with the project files, this achieves that and a bit more. Would be happy to scale back if preferred. |
|
What's the point of using a subtree if you're not actually going to include the exact tree? The advantage is being able to update in a convenient and verifiable way, but if you need to remove files first, that advantage disappears. |
Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
git-subtree-dir: src/tinyformat git-subtree-split: 689695cf58700e6defe3741829564cd682d5ae57
Introduce modifications to tinyformat via utilstrprintf.h This includes changes in: 695041e - util: Update tinyformat 1b8fd35 - Make tinyformat errors raise an exception instead of assert()ing 9b6d4c5 - Move strprintf define to tinyformat.h 6e5fd00 - Move `*Version()` functions to version.h/cpp 9eaa0af - tinyformat: force USE_VARIADIC_TEMPLATES b651270 - util: Throw tinyformat::format_error on formatting error This does not include changes below, and it protects against future unintended modification of upstream code via git-subtree-check: 64fb0ac - Declare single-argument (non-converting) constructors "explicit" 4d9b425 - Fix typos Note I excluded only the tinyformat cpp from the boost lint checks out of an abundance of caution - these files are not actually built into bitcoin so are not at risk of pulling in unwanted dependencies, whereas the header, and other dependencies might be.
Empact
force-pushed
the
tinyformat-subtree
branch
from
f488255 to
ffc28d8
Compare
|
This now includes the full, checked subtree, and I dropped a few other incidental commits in hopes of reducing the conflicts noted above. |
|
Reopening as a new PR so as to calculate new conflicts. |
5 participants
This adds the existing c42f/tinyformat@689695c of tinyformat as a subtree, and extracts the modifications that had been made to a utilstrprintf.h wrapper file which is included instead of tinyformat.h directly.
The benefits being that this should be easier to verifiably update and clearer what changes are being introduced, while making it easier to identify tinyformat.h as a dependency rather than a project file, and apply special treatment relative to that fact.