[docs] [refactor] Add help messages for datadir path mangling #12305

pull jamesob wants to merge 2 commits into bitcoin:master from jamesob:jamesob/conf-flag-path-help changing 5 files +25 −21
  1. jamesob commented at 10:09 PM on January 30, 2018: member

    Change -conf's help message to indicate that relative path values will be prefixed by the datadir path. This behavior probably merits clarification; it's kind of confusing when attempting to specify a configuration file in the current directory with -conf=bitcoin.conf, but instead loading the bitcoin.conf file in ~/.bitcoin datadir.

    Edit

    This PR has been modified to document all cases where relative path configurations are modified to be under datadir. A small refactoring has also been added which consolidates this normalization.

  2. laanwj commented at 10:13 PM on January 30, 2018: member

    Not just this one, all relative files and directories specified either on the command line or through RPC are supposed to be relative to the data directory.

  3. MarcoFalke commented at 10:27 PM on January 30, 2018: member

    OT: Wasn't walletdir temporarily allowed to be relative to the cwd?

  4. jamesob commented at 10:38 PM on January 30, 2018: member

    Sounds like we don't have any documentation indicating that all path arguments are relative to datadir; that information should live somewhere (preferably in a single place that isn't easy to miss). One option is to put it in a paragraph between "Usage" and "Options" in {bitcoind,bitcoin-cli,bitcoin-qt} -h. Any other suggestions?

  5. MarcoFalke commented at 11:22 PM on January 30, 2018: member

    Options that take paths or file names are:

    • -rpccookiefile=<loc>
    • -walletdir=<dir>
    • -wallet=<file>
    • -datadir=<dir>
    • -conf=<file>

    So, adding a general paragraph only makes sense if it holds true for all paths. Otherwise, I'd prefer if each command line help was clarified individually.

  6. fanquake added the label Docs on Jan 31, 2018
  7. jamesob force-pushed on Jan 31, 2018
  8. jamesob commented at 4:02 AM on January 31, 2018: member

    @MarcoFalke thanks for that list. I've looked at how each option on the list is parsed (as well as a few others):

    • -rpccookiefile=<loc>: Is put under datadir if relative.
    • -walletdir=<dir>: Isn't allowed to be relative.
    • -wallet=<file>: Moved under walletdir with fs::absolute(walletFile, GetWalletDir()).
    • -datadir=<dir>: Can be specified as relative but will warn. Obviously not put under itself.
    • -conf=<file>: Is put under datadir if relative.
    • -loadblock=<file>: Interpreted literally -- isn't modified if relative. Passed directly to fsbridge::fopen.
    • -debuglogfile=<file>: Is put under datadir if relative.
    • -pid=<file>: Is put under datadir if relative.

    So: each of these options is either not allowed to be relative or is put under datadir if relative with the exception of wallet and loadblock. Because we don't uniformly treat relative paths as relative to datadir, I've updated only the affected help messages.

    I've also added a refactoring commit which consolidates the "normalization" of relative paths under datadir.

  9. jamesob force-pushed on Jan 31, 2018
  10. jamesob renamed this:
    Clarify -conf help message to mention datadir path prefix
    [docs] [refactor] Add help messages for datadir path mangling
    on Jan 31, 2018
  11. jamesob force-pushed on Jan 31, 2018
  12. laanwj commented at 9:47 AM on January 31, 2018: member

    Thanks, utACK

  13. in src/util.cpp:931 in d76f43c65c outdated 
     926 | @@ -936,3 +927,10 @@ int64_t GetStartupTime()
 927 |  {
 928 |      return nStartupTime;
 929 |  }
 930 | +
 931 | +fs::path NormalizePathArg(const fs::path& pathArg, bool fNetSpecific)
    promag commented at 3:48 PM on January 31, 2018:

    I'd rename to GetAbsolutePath or more specific GetAbsoluteDataPath:

    fs::path GetAbsoluteDataPath(const fs::path& path, bool net_specific)

    I other contexts/languages, normalization can be interpreted as transforming from foo/../bar/ to bar.

    Nit, snake_case arguments.

  14. in src/wallet/init.cpp:10 in d76f43c65c outdated 
       6 | @@ -7,6 +7,7 @@
   7 |  
   8 |  #include <net.h>
   9 |  #include <util.h>
  10 | +#include <fs.h>
    promag commented at 3:48 PM on January 31, 2018:

    Nit, unrelated change, remove.

  15. promag commented at 3:56 PM on January 31, 2018: member

    Concept ACK.

  16. jamesob force-pushed on Jan 31, 2018
  17. jamesob commented at 4:15 PM on January 31, 2018: member

    Thanks for the feedback @promag. It's been incorporated and pushed.

  18. in src/util.cpp:934 in 23d295aa9e outdated 
     926 | @@ -936,3 +927,10 @@ int64_t GetStartupTime()
 927 |  {
 928 |      return nStartupTime;
 929 |  }
 930 | +
 931 | +fs::path GetAbsolutePathForArg(const fs::path& path, bool net_specific)
 932 | +{
 933 | +    if (!path.is_complete())
 934 | +        return GetDataDir(net_specific) / path;
    ryanofsky commented at 6:08 PM on January 31, 2018:

    Would be a little better to return: return fs::absolute(path, GetDataDir(net_specific)) to make it clear this returns an absolute path. You could also drop the is_complete check above if you did this.

    jamesob commented at 8:33 PM on January 31, 2018:

    Great, thanks.

  19. in src/util.cpp:933 in 23d295aa9e outdated 
     926 | @@ -936,3 +927,10 @@ int64_t GetStartupTime()
 927 |  {
 928 |      return nStartupTime;
 929 |  }
 930 | +
 931 | +fs::path GetAbsolutePathForArg(const fs::path& path, bool net_specific)
 932 | +{
 933 | +    if (!path.is_complete())
    ryanofsky commented at 6:10 PM on January 31, 2018:

    Could eliminate this check, but if you will keep it developer notes suggest adding braces or putting if body on same line as condition.

    jamesob commented at 8:37 PM on January 31, 2018:

    Taking your advice below, but the reminder about convention is appreciated.

  20. in src/util.h:202 in 23d295aa9e outdated 
     197 | + *
 198 | + * @param path The path to be conditionally prefixed with datadir.
 199 | + * @param net_specific Forwarded to GetDataDir().
 200 | + * @return The normalized path.
 201 | + */
 202 | +fs::path GetAbsolutePathForArg(const fs::path& path, bool net_specific = true);
    ryanofsky commented at 6:12 PM on January 31, 2018:

    "ForArg" in function name seems a little redundant. Just a suggestion, but I might call it something GetDataPath or GetDataDirPath.

    jamesob commented at 8:36 PM on January 31, 2018:

    My only reservation here is that the path won't necessarily be in datadir, so a name with Data or DataDir seems confusing. "Arg" here is meant to correspond to an argument to the binary, not the function, but maybe that isn't clear.

    jamesob commented at 8:38 PM on January 31, 2018:

    I'll try AbsPathForConfigVal.

  21. ryanofsky commented at 6:13 PM on January 31, 2018: member

    utACK 23d295aa9e16cb3591bbd8bb0f5ce026b861b7b6

  22. jamesob force-pushed on Jan 31, 2018
  23. jamesob commented at 8:41 PM on January 31, 2018: member

    Thanks for the good feedback, @ryanofsky. Incorporated and pushed.

  24. ajtowns commented at 10:14 AM on February 2, 2018: member

    Could update the commit title to match the renamed function :)

  25. jamesob force-pushed on Feb 2, 2018
  26. jamesob commented at 2:24 PM on February 2, 2018: member

    d'oh. Thanks, @ajtowns. Pushed.

  27. jnewbery commented at 8:07 PM on February 5, 2018: member

    Looks good.

    Minor note that -conf is relative to the base datadir, but -debuglogfile, -pid and -rpccookiefile are relative the net-specific subdirectories. It might be better to specify that in the help text.

  28. sipa commented at 9:10 PM on February 5, 2018: member

    utACK 95963c120c48ce750fdb2298f3eb70034e578e8d, but needs rebase.

  29. Clarify help messages for path args to mention datadir prefix
    Change `-conf`'s and others' help messages to indicate that relative path
values will be prefixed by the datadir path. This behavior is confusing when
attempting to specify a configuration file in the current directory with
`-conf=bitcoin.conf`, but loading the `bitcoin.conf` file in ~/.bitcoin
datadir.
    a1e13055c2
  30. Add AbsPathForConfigVal to consolidate datadir prefixing for path args
    Most commandline/config args are interpreted as relative to datadir if
not passed absolute. Consolidate the logic for this normalization.
    54604600c3
  31. jamesob force-pushed on Feb 5, 2018
  32. jamesob commented at 10:50 PM on February 5, 2018: member

    Rebased and incorporated @jnewbery's feedback.

  33. in src/util.h:200 in 54604600c3 
     195 | + * Most paths passed as configuration arguments are treated as relative to
 196 | + * the datadir if they are not absolute.
 197 | + *
 198 | + * @param path The path to be conditionally prefixed with datadir.
 199 | + * @param net_specific Forwarded to GetDataDir().
 200 | + * @return The normalized path.
    promag commented at 11:13 PM on February 5, 2018:

    The absolute path?

  34. jnewbery commented at 12:36 AM on February 6, 2018: member

    utACK 54604600c3de6cb18540c0911127173f68ad246c

  35. laanwj merged this on Feb 6, 2018
  36. laanwj closed this on Feb 6, 2018
  37. laanwj referenced this in commit f6cd41d93e on Feb 6, 2018
  38. MarcoFalke commented at 11:05 PM on February 6, 2018: member

    Post-merge utACK 54604600c3de6cb18540c0911127173f68ad246c

  39. Fabcien referenced this in commit b5de3d1079 on Aug 30, 2019
  40. UdjinM6 referenced this in commit 75c5a9c265 on Apr 29, 2020
  41. PastaPastaPasta referenced this in commit 7ec964e507 on Apr 30, 2020
  42. PastaPastaPasta referenced this in commit 8700f460e3 on May 9, 2020
  43. ckti referenced this in commit 80e238a9ec on Mar 28, 2021
  44. gades referenced this in commit 04c5186a08 on Jun 30, 2021
  45. DrahtBot locked this on Sep 8, 2021
Contributors
jamesoblaanwjMarcoFalkepromagryanofskyajtownsjnewberysipa
Labels
Docs
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me