Correct description of blocksdir default value #16010

kristapsk commented at 9:46 PM on May 11, 2019: contributor

I recently moved all my Bitcoin datadir except blocksdir from a slow storage to the fast one. By previous description of blocksdir default I expected that blocksdir value should be "old_datadir/blocks", but it actually should be "old_datadir", as it expects / creates "blocks" subdirectory under it, so it's default is actually <datadir>, not <datadir>/blocks. See also GetDataDir() in src/util/system.cpp.

Correct description of blocksdir default value 1b6afadd5b

fanquake added the label Docs on May 11, 2019

DrahtBot commented at 12:15 AM on May 12, 2019: 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:

#15457 (Check std::system for -[alert|block|wallet]notify by Sjors)
#14364 (doc: Clarify -blocksdir usage by sangaman)
#12557 ([WIP] 64 bit iOS device support by Sjors)

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.

promag commented at 5:25 PM on May 12, 2019: member

From https://github.com/bitcoin/bitcoin/blob/e79bbb73e08e3f191e97d3b67a2fbb510c5545ff/src/util/system.cpp#L724-L735 looks like the default is correct?

kristapsk commented at 5:52 PM on May 12, 2019: contributor

I'm not saying default is not correct, I am saying it's current description in help is a lie.

promag commented at 7:39 PM on May 12, 2019: member

Oh right! ~~Didn't check history but is current behavior correct? I mean, is it normal to expect subdirectory blocks/ on the specified -blocksdir?~~

Edit: see #12828 (comment)

I think the help should say something like Specify root directory for blocks (*blk) and undo (*.rev) files (default: <datadir>).

in src/init.cpp:383 in 1b6afadd5b

 379 | @@ -380,7 +380,7 @@ void SetupServerArgs()
 380 |      gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS);
 381 |      gArgs.AddArg("-alertnotify=<cmd>", "Execute command when a relevant alert is received or we see a really long fork (%s in cmd is replaced by message)", false, OptionsCategory::OPTIONS);
 382 |      gArgs.AddArg("-assumevalid=<hex>", strprintf("If this block is in the chain assume that it and its ancestors are valid and potentially skip their script verification (0 to verify all, default: %s, testnet: %s)", defaultChainParams->GetConsensus().defaultAssumeValid.GetHex(), testnetChainParams->GetConsensus().defaultAssumeValid.GetHex()), false, OptionsCategory::OPTIONS);
 383 | -    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks directory (default: <datadir>/blocks)", false, OptionsCategory::OPTIONS);
 384 | +    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks directory (default: <datadir>)", false, OptionsCategory::OPTIONS);

MarcoFalke commented at 4:43 PM on May 13, 2019:

Should this put blocks into some quotes to clarify this is a string literal and not a English word (that e.g. can be translated). Thankfully we no longer translate those, but could still make it clear?

    gArgs.AddArg("-blocksdir=<dir>", "Specify 'blocks' directory (default: <datadir>)", false, OptionsCategory::OPTIONS);

MarcoFalke commented at 4:43 PM on May 13, 2019: member

ACK

DrahtBot commented at 5:13 PM on May 13, 2019: member

Needs rebase

DrahtBot added the label Needs rebase on May 13, 2019

kristapsk commented at 7:24 PM on May 13, 2019: contributor

#14364 got merged (noticed existence of such PR only after pushing this one), this isn't actual anymore.

kristapsk closed this on May 13, 2019

laanwj removed the label Needs rebase on Oct 24, 2019

DrahtBot locked this on Dec 16, 2021