-datadir or -datadir="" option implies default datadir #16416

1. 
   hebasto commented at 5:38 pm on July 18, 2019: member

   This PR introduces a new behavior for -datadir (w/o a value) or -datadir="": it means “use the default one”. It allows to unset a datadir option specified in the config file by passing -datadir or -datadir="" as a command line option.

   Credits: This PR is inspired by ryanofsky’s idea:

   If somebody has a datadir option specified in the config file, but wants to unset it on the command line by passing -nodatadir or -datadir="", it seems like this should be allowed.

   The more general approach, however, has been described by ryanofsky here.

2. 
   hebasto commented at 5:38 pm on July 18, 2019: member

   ping @ryanofsky
3. in src/util/system.cpp:744 in 76bccefa17 outdated

   740@@ -741,7 +741,9 @@ const fs::path &GetDataDir(bool fNetSpecific)
   741     // this function
   742     if (!path.empty()) return path;
   743 
   744-    if (gArgs.IsArgSet("-datadir")) {
   745+    tfm::format(std::cerr, "HEBASTO: -datadir negation is %s\n", gArgs.IsArgNegated("-datadir"));
   

   ---

   ---

   

   MarcoFalke commented at 6:31 pm on July 18, 2019:

   :eyes:

   ---

   

   hebasto commented at 6:38 pm on July 18, 2019:

   I beg your pardon.
4. hebasto force-pushed on Jul 18, 2019
5. 
   ryanofsky commented at 6:42 pm on July 18, 2019: member

   This seems ok. I think it could make sense for -nodatadir and -datadir= on the command line to override any datadir string in the configuration file and override any datadir in qt settings, and use default platform datadir as if no value was ever specified.

   Or alternately, it could make sense for -nodatadir to print an error, because it’s similar enough to -nowallet, -nowalletdir, -nopid, -norpccookiefile options where you might want to be able to disable bitcoin from creating files & directories, but different from those options because bitcoin can’t function without a datadir.

   In either case, I think the PR description here should be updated to be more self contained. I think an ideal PR description would first say what the change in behavior is, then give reasons for the change, and only then go into deeper history and assigning credit for ideas, to be comprehensible as possible without digging into previous discussion.

   Also a change like this should have release notes.

6. DrahtBot added the label Utils/log/libs on Jul 18, 2019
7. 
   DrahtBot commented at 7:32 pm on July 18, 2019: member

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

   Conflicts

   No conflicts as of last run.

8. 
   hebasto commented at 5:32 am on July 19, 2019: member

   @ryanofsky

   In either case, I think the PR description here should be updated to be more self contained. I think an ideal PR description would first say what the change in behavior is, then give reasons for the change, and only then go into deeper history and assigning credit for ideas, to be comprehensible as possible without digging into previous discussion.

   The PR description has been updated.

9. 
   promag commented at 8:57 am on July 23, 2019: member

   I would rather see just -datadir or -datadir="" as a way to set the default and override config.

   -nodatadir doesn’t make any sense to me.

10. 
    promag commented at 9:00 am on July 23, 2019: member

    Each arg could have a flag whether it can be negated or not. For instance ArgsManager::AddArg(..., bool allow_negated = true).
11. 
    hebasto commented at 1:00 pm on July 23, 2019: member

    @promag

    I would rather see just -datadir or -datadir="" as a way to set the default and override config. -nodatadir doesn’t make any sense to me.

    You and @ryanofsky have convinced me. Going to implement your suggestion tonight.

    Each arg could have a flag whether it can be negated or not. For instance ArgsManager::AddArg(..., bool allow_negated = true).

    Could you look into #16097?

12. Add a way to set default -datadir

    -datadir or -datadir="" in command line will set the default datadir 
    (overriding config file options).

    3a1ea8e6e7
13. hebasto force-pushed on Jul 23, 2019
14. hebasto renamed this:
    Negated -datadir option implies default datadir
    -datadir or -datadir="" option implies default datadir
    on Jul 23, 2019
15. 
    hebasto commented at 1:42 pm on July 23, 2019: member

    @ryanofsky and @promag Thank you for your reviews.

    I would rather see just -datadir or -datadir="" as a way to set the default and override config.

    Done. The OP has been updated.

    … it could make sense for -nodatadir to print an error…

    I’d rather use a more general solution like #16097.

16. 
    hebasto commented at 4:34 pm on July 24, 2019: member

    Closed in favor of #15864.

    See: https://github.com/bitcoin/bitcoin/pull/15864/commits/740d41ce9f7fdf209366e930bd0fdcc6b1bc6b79#r306906349

17. hebasto closed this on Jul 24, 2019

    ---

18. hebasto deleted the branch on Jul 26, 2019
19. DrahtBot locked this on Dec 16, 2021

Contributors
hebasto MarcoFalke ryanofsky DrahtBot promag

Labels
Utils/log/libs