Include tinyformat as a subtree

Empact commented at 9:30 pm on August 1, 2018: member

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.

Empact renamed this:
~~Tinyformat subtree~~
Include tinyformat as a subtree
on Aug 1, 2018

in src/Makefile.am:443 in f488255aec outdated

437@@ -436,8 +438,8 @@ nodist_libbitcoin_util_a_SOURCES = $(srcdir)/obj/build.h
438 #
439 
440 # bitcoind binary #
441-bitcoind_SOURCES = bitcoind.cpp
442-bitcoind_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
443+bitcoind_SOURCES = bitcoind.cpp tinyformat/tinyformat.h

Empact commented at 9:34 pm on August 1, 2018:

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.

Empact commented at 9:36 pm on August 1, 2018: member

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.

MarcoFalke commented at 9:38 pm on August 1, 2018: member

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.

Empact commented at 9:41 pm on August 1, 2018: member

~~@MarcoFalke I dropped the undesired files in 993d88104d73c985b29d9356e3347d92f76b6a61, now it’s just the .h and readme.~~

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.

sipa commented at 10:01 pm on August 1, 2018: member

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.

DrahtBot commented at 10:03 pm on August 1, 2018: member

#13840 (Inline i64tostr and itostr by Empact)
#13787 (Test for #13426 by ken2812221)
#13723 (PSBT key path cleanups by sipa)
#13671 (Remove the boost/algorithm/string/case_conv.hpp dependency by 251Labs)
#13639 ([refactor] Fix the chainparamsbase -> util circular dependency by Empact)
#13525 (Report reason inputs are nonstandard from AreInputsStandard by Empact)
#13426 ([bugfix] Fix encoding issue for Windows by ken2812221)
#13392 (util: Make strprintf noexcept. Improve error message on format string error. by practicalswift)
#13311 (Don’t edit Chainparams after initialization by jtimon)
#13168 (Thread names in logs and deadlock debug tools (take 2) by jamesob)
#13088 (Log early messages with -printtoconsole by ajtowns)
#13062 (Make script interpreter independent from storage type CScript by sipa)
#12934 ([net] [validation] Call ProcessNewBlock() asynchronously by skeees)

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.

fanquake added the label Upstream on Aug 1, 2018

Squashed 'src/tinyformat/' content from commit 689695cf5

git-subtree-dir: src/tinyformat
git-subtree-split: 689695cf58700e6defe3741829564cd682d5ae57

6abd9bdbfd

Merge commit '6abd9bdbfd1ac646d239b34c7c9eb09e136a8176' as 'src/tinyformat' bc483a29df

Drop src/tinyformat.h in favor of the subtree src/tinyformat/

Introduce modifications to tinyformat via utilstrprintf.h

This includes changes in:
695041e4952ea40e0 - util: Update tinyformat
1b8fd35aadfad6a1e - Make tinyformat errors raise an exception instead of assert()ing
9b6d4c5cdc1ad7b12 - Move strprintf define to tinyformat.h
6e5fd003e04b81115 - Move `*Version()` functions to version.h/cpp
9eaa0afa6ec5d3dd0 - tinyformat: force USE_VARIADIC_TEMPLATES
b651270cd6bfdd6d7 - 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:
64fb0ac016c7fd01c - Declare single-argument (non-converting) constructors "explicit"
4d9b4256d89d1f7c6 - 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.

ffc28d8c62

Empact force-pushed on Aug 2, 2018

Empact commented at 2:15 am on August 2, 2018: member

This now includes the full, checked subtree, and I dropped a few other incidental commits in hopes of reducing the conflicts noted above.

Empact commented at 4:02 am on August 2, 2018: member

Reopening as a new PR so as to calculate new conflicts.

Empact closed this on Aug 2, 2018

MarcoFalke locked this on Sep 8, 2021

Include tinyformat as a subtree #13842