[Logging] Only log “Using config file PATH_TO_bitcoin.conf” message on startup if conf file exists #14057

pull leishman wants to merge 1 commits into bitcoin:master from leishman:leishman--only-show-conf-message-if-file-exists changing 1 files +13 −1
  1. leishman commented at 10:37 pm on August 24, 2018: contributor

    Currently we log a message indicating that a bitcoin.conf file is being used even if one does not exist. This PR changes the logic to:

    If config file does not exist and no -conf flag passed, log: Config file: FILE_PATH (not found, skipping). Where FILE_PATH is the default or the path passed in with the -conf flag.

    If config file does not exist and -conf flag passed with incorrect path, log warning: Warning: The specified config file FILE_PATH does not exist

    If config file exists, log: Config file: FILE_PATH

    Note: This is a (modified) subset of changes introduced in #13761 which creates a default example config file. I think it makes sense to extract this small bit out into a separate PR.

  2. fanquake added the label Utils/log/libs on Aug 24, 2018
  3. DrahtBot commented at 11:03 pm on August 24, 2018: member
    • #13761 (Docs: Create default bitcoin.conf file on startup by leishman)
    • #13746 (-masterdatadir for datadir bootstrapping by kallewoof)

    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.

  4. practicalswift commented at 9:26 pm on August 25, 2018: contributor

    Concept ACK

    Perhaps we should get rid of the else-case log message (“Not using config file”) to increase signal to noise in our logging?

  5. leishman commented at 5:13 am on August 26, 2018: contributor
    @practicalswift I’m ok with removing the else statement, but the reason I added this is to improve the user experience. Currently a user has no reliable feedback regarding where bitcoind is getting config values. If an invalid conf file path is passed in, bitcoind doesn’t tell the user this file doesn’t exist and says it’s using the file. If we remove the else statement, the user will have no idea if they pass an incorrect path to -conf.
  6. Sjors commented at 11:20 am on August 26, 2018: member

    tACK ac7771d4388ca82a4e36fc058cb328b2860ed7c9 on macOS (testing Windows might a good idea). @practicalswift maybe we could hide the “Not using config file” log message if -conf is not explicitly set (!gArgs.IsArgSet("-conf"))?

    A future update could abort if -conf is explicitly set by the user to a non-existent path, but for now at least the log message is more useful.

  7. practicalswift commented at 11:37 am on August 26, 2018: contributor

    @leishman Good point! I think @Sjors suggestion is a good compromise – to show the “Not using config file” only if -conf is set.

    utACK ac7771d4388ca82a4e36fc058cb328b2860ed7c9 assuming @Sjors suggestion is implemented :-)

  8. leishman commented at 9:41 pm on August 26, 2018: contributor
    @practicalswift @Sjors the reason i didn’t add that extra condition was for the following scenario: A user creates a bitcoin.conf file in the wrong directory and they can’t figure out why bitcoind is not using their settings. At least with this change they will be able to see where bitcoind is looking by default for a configuration file and they will know that bitcoind does not see what it’s looking for.
  9. Sjors commented at 10:56 am on August 28, 2018: member
    @leishman that makes sense. Perhaps if -conf is not set, the error could say “No file exists at the default location %s”, but maybe that’s overkill.
  10. leishman commented at 10:18 pm on August 29, 2018: contributor
    @Sjors I don’t think it hurts, as this is a known source of confusion for new users. I’ve updated with your suggestion and tested on Mac. Anyone have a windows machine they can test on?
  11. in src/init.cpp:1255 in 6a15b4eadc outdated 
    1247@@ -1248,8 +1248,12 @@ bool AppInitMain()
1248     fs::path config_file_path = GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
1249     if (fs::exists(config_file_path)) {
1250         LogPrintf("Using config file %s\n", config_file_path.string());
1251+    } else if (gArgs.IsArgSet("-conf")) {
1252+        // Warn if no conf file exists at path provided by user
1253+        LogPrintf("Warning: Not using config file. No file exists at %s\n", config_file_path.string());
1254     } else {
1255-        LogPrintf("Not using config file. No file exists at %s\n", config_file_path.string());
1256+        // Not categorizing as "Warning" because it's the defualt behavior
    practicalswift commented at 7:08 am on August 30, 2018:
    Typo: “defualt”
  12. in src/init.cpp:1253 in 6a15b4eadc outdated 
    1247@@ -1248,8 +1248,12 @@ bool AppInitMain()
1248     fs::path config_file_path = GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
1249     if (fs::exists(config_file_path)) {
1250         LogPrintf("Using config file %s\n", config_file_path.string());
1251+    } else if (gArgs.IsArgSet("-conf")) {
1252+        // Warn if no conf file exists at path provided by user
1253+        LogPrintf("Warning: Not using config file. No file exists at %s\n", config_file_path.string());
    practicalswift commented at 7:09 am on August 30, 2018:

    Suggested warning message:

    Warning: The specified config file %s does not exist

  13. in src/init.cpp:1250 in 6a15b4eadc outdated 
    1247@@ -1248,8 +1248,12 @@ bool AppInitMain()
1248     fs::path config_file_path = GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
1249     if (fs::exists(config_file_path)) {
1250         LogPrintf("Using config file %s\n", config_file_path.string());
    practicalswift commented at 7:48 am on August 30, 2018:

    Suggested message:

    Config file: %s

  14. in src/init.cpp:1256 in 6a15b4eadc outdated 
    1252+        // Warn if no conf file exists at path provided by user
1253+        LogPrintf("Warning: Not using config file. No file exists at %s\n", config_file_path.string());
1254     } else {
1255-        LogPrintf("Not using config file. No file exists at %s\n", config_file_path.string());
1256+        // Not categorizing as "Warning" because it's the defualt behavior
1257+        LogPrintf("Not using config file. No file exists at default location %s\n", config_file_path.string());
    practicalswift commented at 7:48 am on August 30, 2018:

    Suggested message:

    Config file: %s (not found, skipping)

  15. leishman commented at 4:57 pm on August 30, 2018: contributor
    @practicalswift thanks for the suggestions and for catching the typo. I’ve updated accordingly.
  16. Only log "Using PATH_TO_bitcoin.conf" message on startup if conf file exists. 
    Currently we log a message indicating that a bitcoin.conf file is being used
even if one does not exists. This commit changes the logic to only display
this message if a config file exists and logs a separate message
if no config file exists. Additionally, a warning is now logged if the file
path passed in the -conf flag does not exist.
    946107a68f
  17. in src/init.cpp:1253 in 0b48ce02f1 outdated 
    1249+    fs::path config_file_path = GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
1250+    if (fs::exists(config_file_path)) {
1251+        LogPrintf("Config file: %s\n", config_file_path.string());
1252+    } else if (gArgs.IsArgSet("-conf")) {
1253+        // Warn if no conf file exists at path provided by user
1254+        LogPrintf("Warning: The specified config file %s does not exist\n", config_file_path.string());
    MarcoFalke commented at 5:22 pm on August 30, 2018:
    Shouldn’t this be an InitWarning?
    leishman commented at 5:29 pm on August 30, 2018:
    Ah good call. Fixed.
  18. practicalswift commented at 7:38 pm on August 30, 2018: contributor
    utACK 8b5a693db76b70baf5964df3609b5218cc3711a8
  19. leishman force-pushed on Aug 30, 2018
  20. leishman commented at 8:56 pm on August 30, 2018: contributor
    @practicalswift @Sjors rebased to clean up commit history
  21. practicalswift commented at 9:03 pm on August 30, 2018: contributor
    utACK 946107a68ffce8c586f9f1657fd7d67d075c321e
  22. Sjors commented at 7:56 am on September 2, 2018: member

    tACK 946107a

    Note that InitWarning causes QT to pop up a warning if you give it an invalid config path (which makes sense):

  23. MarcoFalke commented at 7:21 pm on September 5, 2018: member
    utACK 946107a68ffce8c586f9f1657fd7d67d075c321e
  24. leishman commented at 9:33 pm on September 9, 2018: contributor
    @laanwj is this good to merge or is there anything else you’d like to see tested?
  25. laanwj commented at 1:38 pm on September 10, 2018: member
    utACK 946107a68ffce8c586f9f1657fd7d67d075c321e
  26. laanwj merged this on Sep 10, 2018
  27. laanwj closed this on Sep 10, 2018
  28. ken2812221 referenced this in commit fbfa2e46ff on Sep 10, 2018
  29. deadalnix referenced this in commit 7735559672 on Apr 22, 2020
  30. ftrader referenced this in commit 1cfb750b18 on Aug 17, 2020
  31. PastaPastaPasta referenced this in commit f9e48bdde0 on Jun 27, 2021
  32. PastaPastaPasta referenced this in commit 776d44fac0 on Jun 28, 2021
  33. PastaPastaPasta referenced this in commit 4ea255593d on Jun 29, 2021
  34. PastaPastaPasta referenced this in commit b654ea2332 on Jul 1, 2021
  35. Munkybooty referenced this in commit 47fe958a7d on Jul 1, 2021
  36. DrahtBot locked this on Sep 8, 2021


leishman DrahtBot practicalswift Sjors MarcoFalke laanwj

Labels
Utils/log/libs

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: 2024-11-17 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me