fs: consistently use fsbridge:: for ifstream / ofstream #23857

pull fanquake wants to merge 3 commits into bitcoin:master from fanquake:always_use_fsbridge changing 9 files +11 −9

fanquake commented at 8:51 AM on December 24, 2021: member

Two of these changes are part of #20744, but are ok to do now, and reduce the diff that will eventually need to be reviewed in that PR. See commit messages for details.

fs: consistently use fsbridge for {i,o}fstream

Part of #20744, but this can be done now, and will simplify the diff.

2da5bc557d

fs: add missing <cassert> include

This is needed to prevent compilation failures once boost is removed,
however is still correct to include now, and reduces the diff in #20744.

<string> is extracted from the defines because it is used for Windows
and non-Windows code, i.e get_filesystem_error_message().

5668ad4339

fs: consistently use fsbridge for fopen() bcb36762ee
MarcoFalke commented at 8:54 AM on December 24, 2021: member

Please clarify if this changes behavior in any way or if this is a refactor.
fanquake added the label Refactoring on Dec 24, 2021

hebasto commented at 9:05 AM on December 24, 2021: member

https://cirrus-ci.com/task/6421734977961984:

bench/bench.cpp: In function ‘void {anonymous}::GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>&, const string&, const char*)’:
bench/bench.cpp:30:37: error: use of deleted function ‘fs::path::path(std::string)’
   30 |     fsbridge::ofstream fout{filename};
      |                                     ^
In file included from ./test/util/setup_common.h:9,
                 from bench/bench.cpp:7:
./fs.h:50:5: note: declared here
   50 |     path(std::string) = delete;
      |     ^~~~
make[2]: *** [Makefile:13538: bench/bench_bitcoin-bench.o] Error 1

DrahtBot commented at 6:34 PM on December 24, 2021: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #20744 (Use std::filesystem. Remove Boost Filesystem & System by fanquake)
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 commented at 11:52 AM on January 7, 2022: member

Closing as this is going in as part of #20744.
fanquake closed this on Jan 7, 2022
fanquake deleted the branch on Jan 8, 2022
MarcoFalke commented at 9:01 AM on January 26, 2022: member

Not sure why this was closed?

It looks like the CI failure hinted at a bug that needs fixup? In all other places where strings from the command line are converted to paths, PathFromString is used, but not here in bench.
fanquake commented at 9:06 AM on January 26, 2022: member

Not sure why this was closed?

It's part of #20744.
MarcoFalke commented at 9:17 AM on January 26, 2022: member

I think it might be worthwhile to fix the presumed bug in bench before 20744, in which case this should compile and can also be done before 20744?
fanquake commented at 2:05 PM on January 26, 2022: member

Ok. I had deleted the branch, but can open a new PR.
fanquake referenced this in commit d87a37a4ab on Jan 27, 2022
sidhujag referenced this in commit f9de29fcb9 on Jan 28, 2022
fanquake locked this on Jan 31, 2022

Contributors

Labels

Linked (view graph)

#20744 Use std::filesystem. Remove Boost Filesystem & System #24167 fs: consistently use fsbridge:: for ifstream / ofstream

fs: consistently use fsbridge:: for ifstream / ofstream #23857

Conflicts