Turn TryCreateDirectory() into TryCreateDirectories() #9895
Conversation
|
The blocks directory is now created here: https://github.com/benma/bitcoin/blob/de85040fc552178815a71cacb00d16faff994c9a/src/dbwrapper.cpp#L52 and is created the first time it is needed with the initialization of CBlockTreeDB in init.cpp. |
src/util.cpp
Outdated
There was a problem hiding this comment.
Is this utility function needed at all? I think the definition of fs::create_directories is already exactly what we want here: http://www.boost.org/doc/libs/1_61_0/libs/filesystem/doc/reference.html#create_directories
Returns: true if a new directory was created, otherwise false.
Throws: As specified in Error reporting.
There was a problem hiding this comment.
The function is apparently needed, the comment explains why:
/**
* Ignores exceptions thrown by Boost's create_directory if the requested directory exists.
* Specifically handles case where path p exists, but it wasn't possible for the user to
* write to the parent directory.
*/While I can't recreate this condition, digging up the issue that introduced this talks about some edge cases where it does happen (Truecrypt, ...?): #432
So we should probably leave it.
There was a problem hiding this comment.
Ugh. I think not being able to rely on the postcondition is incredibly ugly, and should simply be patched in boost upstream. But okay...
|
This needs a rebase. |
|
Rebased. |
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK e94c40c770a5de6c95a8c53cfb1717a35c008d83, nice change.
|
@laanwj bump :) |
|
utACK e94c40c770a5de6c95a8c53cfb1717a35c008d83 |
|
utACK e94c40c |
src/util.cpp
Outdated
There was a problem hiding this comment.
Since you're touching this line anyways, you could also remove the reference to Boost, which isn't used here anymore (also, the comment at the end of the function could be updated)
There was a problem hiding this comment.
But it is using Boost.
namespace fs = boost::filesystem;.
I fixed the comment at the end of the function.
There was a problem hiding this comment.
hm, ok. I was under the impression that this was changed in benma@bac5c9c, but I guess I misread
Use case: TryCreateDirectory(GetDataDir() / "blocks" / "index") would fail if the blocks directory was not explicitly created before. The line that did so was in a weird location and could be removed as a result.
1d1ea9f Turn TryCreateDirectory() into TryCreateDirectories() (Marko Bencun) Tree-SHA512: 49a524167bcf66e351a964c88d09cb3bcee12769a32da83410e3ba649fa4bcdbf0478d41e4d09bb55adb9b3f122e742271db6feb30bbafe2a7973542b5f10f79
…es() 1d1ea9f Turn TryCreateDirectory() into TryCreateDirectories() (Marko Bencun) Tree-SHA512: 49a524167bcf66e351a964c88d09cb3bcee12769a32da83410e3ba649fa4bcdbf0478d41e4d09bb55adb9b3f122e742271db6feb30bbafe2a7973542b5f10f79
… Allow to specify blocks storage directory b0e0d55 doc: add '-blocksdir' option to release-notes (furszy) 50e66c2 Turn TryCreateDirectory() into TryCreateDirectories() (Marko Bencun) 035d3c2 util: Simplify path argument for CBlockTreeDB ctor (Hennadii Stepanov) a571d24 Fail if either disk space check fails (Ben Woosley) e567003 Improve blocksdir functional test. (Hennadii Stepanov) ff0ae45 Make blockdir always net specific (Hennadii Stepanov) 13a4119 doc: Clarify -blocksdir usage (Daniel McNally) a4ff899 -blocksdir: keep blockindex leveldb database in datadir (Jonas Schnelli) 3054076 QA: Add -blocksdir test (Jonas Schnelli) 39b8127 Allow to optional specify the directory for the blocks storage (Jonas Schnelli) a821b0b BugFix: Move TestingSetup::ECCVerifyHandle member to BasicTestingSetup. (furszy) 5844452 Remove unneeded GetTempPath, only use temp directory in unit tests (furszy) 7fc893b [Test] BasicTestingSetup constructor receiving the network param. (furszy) Pull request description: Grouped two different, zero risk, modifications here to not have to create two/three small and straightforward PRs: #### 1) Unit Tests: * Fixing basic unit test setup issue, was not initializing the libsecp256k1 context. Thus why we have been forced to use the extended unit testing setup for almost every test case (which includes CConnman, scheduler, validation interface, etc.. that are not needed so often). * Refactor: `BasicTestingSetup` receiving the network in the constructor. #### 2) Back ported the ability to specify a custom directory for the blocks storage. > Since the actual block files taking up more and more space, it may be desirable to have them stored in a different location then the data directory (use case: SSD for chainstate, etc., HD for blocks). This PR adds a -blocksdir option that allows one to keep the blockfiles external from the data directory (instead of creating symlinks). * bitcoin#9895. * bitcoin#12653. * bitcoin#14364. * bitcoin#14409. * bitcoin#15124. * bitcoin#17059. ACKs for top commit: Fuzzbawls: re-ACK b0e0d55 random-zebra: utACK b0e0d55 and merging... Tree-SHA512: 289e5b826c8fc23a5933a5b30e6c043f82bad827b10f3093cbc5869a9e278d7ad0a325dd19b5ad3c97f23a3dee69f5b0bee3869e486eeed5e8d8342a39919cf5
8 participants
Use case: TryCreateDirectory(GetDataDir() / "blocks" / "index") would
fail if the blocks directory was not explicitly created before.
The line that did so was in a weird location and could be removed as a
result.