< troygiorshev>
fwiw too, I think this test should be moved to the "less than 2m" section
< troygiorshev>
Took 1:12 on my i7-8750H
< aj>
sipa: that seems to match the error in the travis log
< sipa>
troygiorshev: thanks, doing that
< sipa>
aj: yeah, i can reproduce after a rebase on master
< fanquake>
sipa: gotta keep up :p
< fanquake>
bitcoin/bitcoin is a fast moving repository
< sipa>
fanquake: i started this set of changes over a week ago, then when i was done rebased on master, and right then "someone" merges probably the only open PR in the whole repo that breaks the tests without causing a compilation failure :p
< bitcoin-git>
[bitcoin] JeremyRubin opened pull request #19677: WIP: Switch BlockMap to use an unordered_set under the hood (master...refactor-blockmap-blockset) https://github.com/bitcoin/bitcoin/pull/19677
< sipa>
sdaftuar: no no; there was an actual interaction between 19620 and 17977, and 17977's tests started failing after merging (as it didn't move taproot outputs out of WITNESS_UNKNOWN, those ended up being nonstandard to spend)
< sdaftuar>
ah!
< bitcoin-git>
[bitcoin] sdaftuar opened pull request #19680: 0.20: Add txids with non-standard inputs to reject filter (0.20...2020-08-reject-unknown-wit-0.20) https://github.com/bitcoin/bitcoin/pull/19680
< bitcoin-git>
[bitcoin] sdaftuar opened pull request #19681: 0.19: Add txids with non-standard inputs to reject filter (0.19...2020-08-reject-unknown-wit-0.19) https://github.com/bitcoin/bitcoin/pull/19681
< gwillen>
right, yeah. So I'm not sure if $use_cxx17 is referenced anywhere else, or if that change is enough to be the same as passing the flag to configure.
< gwillen>
I don't see any other uses, so off the top of my head I'd expect that to work
< Kiminuo>
Well, travis shows:
< Kiminuo>
./fs.h:14:10: fatal error: filesystem: No such file or directory
< gwillen>
looking at the travis failures it does look like some headers got rearranged in c++17 that you haven't accounted for, but I don't know if you were even looking at that yet (e.g. std::function got moved into #include <functional>, which is causing a ton of errors, but not the ones you're asking about)
< sipa>
Kiminuo: but your changes to use std::filesystem instead of boost::filesystem should only be active whenever you're building in C++17 mode
< sipa>
i think?
< sipa>
ah
< sipa>
sorry, just reading
< jeremyrubin>
sipa: they're building on mandatory c++17
< gwillen>
Kiminuo: if you still want to experiment, understanding it could be quite awhile before this is feasible in production, it looks like you could try including <experimental/filesystem> and linking with "-lstdc++fs"
< gwillen>
Kiminuo: no it seems you need 8.0
< Kiminuo>
gwillen, so that may be the issue, right?
< gwillen>
yeah I think that is indeed the issue
< jeremyrubin>
I think c++17 will be fine for even "as much as you have of 17" compilers with the current code
< jeremyrubin>
but we may not be able to use *all* c++17 features until then
< gwillen>
your local compiler is newer than the one that CI is using, so you have <filesystem> but CI does not
< gwillen>
I assume the compiler CI uses is the benchmark for "minimum supported to build bitcoin with"?
< Kiminuo>
gwillen, I don't think <experimental/filesystem> is way to go. It will make it more complex and it does not sound good to use something experimental in Bitcoin.
< gwillen>
I believe it is the same library, it just moved when it was finally standardized to a new name. But I agree it may not be suitable for inclusion into Bitcoin for real for now.
< gwillen>
especially since boost::filesystem is probably _also_ the same library under a different name, more or less
< Kiminuo>
std::filesystem is kind of renamed boost::filesystem with some modifications, but the details are sometimes not that easy to find (at least for me)
< gwillen>
yeah, that's usually how it goes with C++ library features being standardized I think -- they start in boost and work their way into the standard library
< sipa>
GCC 7 jusy predates the c++17 standard being finalized
< sipa>
so i guess that'z why it's only available under experimental - at the time it was written there was no way to know of that would eventually match the spec
< sipa>
GCC 7.5 is more than recent enough, but i guess these things are only updated in major releasesm
< sipa>
so perhaps the right thing to do is making std::filesystem based builds still configurable, and only enable them on GCC 8.1+ platforms and others that we know support it
< sipa>
so that the lack of full std::filesystem support doesn't hold up otherwise migrating to C++17
< sipa>
gwillen, Kiminuo: perhaps a discrepancy is due to libstdc++ and gcc not being the exact same thing and available in lockstep?
< sipa>
std::filesystem support is purely a libstdc++ side thing, i expect
< Kiminuo>
sipa, I don't know about that configuration. The APIs of both libraries is not in one-to-one correspondence. It can be done by a wrapper I guess.
< sipa>
well, that wrapper already exists
< sipa>
this is why we have the fs namespace
< Kiminuo>
True
< Kiminuo>
But you would need to add more functions
< sipa>
i expect that slight differences between boost::filesystem and std::filesystem make it not exactly a "using fs = boost::filesystem" vs "using fs = std::filesystem", but it shouldn't be too much more than that