Merge #11781: Add `-debuglogfile` option

5a7c09a test: Add tests for `-debuglogfile` with subdirs (Anthony Towns)
4158734 doc: Update release notes for `-debuglogfile` (Wladimir J. van der Laan)
2323242 test: Add test for `-debuglogfile` (Wladimir J. van der Laan)
cf5f432 Add `-debuglogfile` option (Wladimir J. van der Laan)

Pull request description:

  This patch adds an option to configure the name and/or directory of the debug log file.

  The user can specify either a relative path, in which case the path is relative to the (network specific) data directory. They can also specify an absolute path to put the log anywhere else in the file system.

  Alternative to #11741 that gets rid of the concept of a "log directory" by specifying the path for the specific kind of log, the debug log. Which happens to be the only kind of log we have at this point*, but a hypothetical new kind of log (say, an audit log) would get a new option. This has more flexibility than specifying a directory which has to contain all of them.

  \* excluding `db.log` which is internally generated by the wallet database library, but that one moves along with `-walletdir`.

Tree-SHA512: 4434d0e598dc23504e5c9e67fdbaef56db4f0fd490f9f54fd503e69d4dda9b5b69c539e1794ed841e72161b7b1dc3374d2f1193dd431b057566750e56fd8f24b

6 files changed, 98 insertions, 14 deletions

diff --git a/doc/release-notes.md b/doc/release-notes.md
index 2c63b1f88e..43009b0813 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -90,6 +90,9 @@ Low-level RPC changes
   * `getmininginfo`
 - The wallet RPC `getreceivedbyaddress` will return an error if called with an address not in the wallet.
 
+Changed command-line options
+-----------------------------
+- `-debuglogfile=<file>` can be used to specify an alternative debug logging file.
 
 Credits
 =======
diff --git a/src/init.cpp b/src/init.cpp
index 9356862999..871a585267 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -342,6 +342,7 @@ std::string HelpMessage(HelpMessageMode mode)
     if (showDebug)
         strUsage += HelpMessageOpt("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER));
     strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file on startup"));
+    strUsage += HelpMessageOpt("-debuglogfile=<file>", strprintf(_("Specify location of debug log file: this can be an absolute path or a path relative to the data directory (default: %s)"), DEFAULT_DEBUGLOGFILE));
     strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
     strUsage += HelpMessageOpt("-maxmempool=<n>", strprintf(_("Keep the transaction memory pool below <n> megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE));
     strUsage += HelpMessageOpt("-mempoolexpiry=<n>", strprintf(_("Do not keep transactions in the mempool longer than <n> hours (default: %u)"), DEFAULT_MEMPOOL_EXPIRY));
@@ -1209,8 +1210,11 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
         ShrinkDebugFile();
     }
 
-    if (fPrintToDebugLog)
-        OpenDebugLog();
+    if (fPrintToDebugLog) {
+        if (!OpenDebugLog()) {
+            return InitError(strprintf("Could not open debug log file %s", GetDebugLogPath().string()));
+        }
+    }
 
     if (!fLogTimestamps)
         LogPrintf("Startup time: %s\n", DateTimeStrFormat("%Y-%m-%d %H:%M:%S", GetTime()));
diff --git a/src/util.cpp b/src/util.cpp
index b2023b8322..6631c236f1 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -89,6 +89,7 @@ const int64_t nStartupTime = GetTime();
 
 const char * const BITCOIN_CONF_FILENAME = "bitcoin.conf";
 const char * const BITCOIN_PID_FILENAME = "bitcoind.pid";
+const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
 ArgsManager gArgs;
 bool fPrintToConsole = false;
@@ -189,26 +190,40 @@ static void DebugPrintInit()
     vMsgsBeforeOpenLog = new std::list<std::string>;
 }
 
-void OpenDebugLog()
+fs::path GetDebugLogPath()
+{
+    fs::path logfile(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
+    if (logfile.is_absolute()) {
+        return logfile;
+    } else {
+        return GetDataDir() / logfile;
+    }
+}
+
+bool OpenDebugLog()
 {
     boost::call_once(&DebugPrintInit, debugPrintInitFlag);
     boost::mutex::scoped_lock scoped_lock(*mutexDebugLog);
 
     assert(fileout == nullptr);
     assert(vMsgsBeforeOpenLog);
-    fs::path pathDebug = GetDataDir() / "debug.log";
+    fs::path pathDebug = GetDebugLogPath();
+
     fileout = fsbridge::fopen(pathDebug, "a");
-    if (fileout) {
-        setbuf(fileout, nullptr); // unbuffered
-        // dump buffered messages from before we opened the log
-        while (!vMsgsBeforeOpenLog->empty()) {
-            FileWriteStr(vMsgsBeforeOpenLog->front(), fileout);
-            vMsgsBeforeOpenLog->pop_front();
-        }
+    if (!fileout) {
+        return false;
+    }
+
+    setbuf(fileout, nullptr); // unbuffered
+    // dump buffered messages from before we opened the log
+    while (!vMsgsBeforeOpenLog->empty()) {
+        FileWriteStr(vMsgsBeforeOpenLog->front(), fileout);
+        vMsgsBeforeOpenLog->pop_front();
     }
 
     delete vMsgsBeforeOpenLog;
     vMsgsBeforeOpenLog = nullptr;
+    return true;
 }
 
 struct CLogCategoryDesc
@@ -355,7 +370,7 @@ int LogPrintStr(const std::string &str)
             // reopen the log file, if requested
             if (fReopenDebugLog) {
                 fReopenDebugLog = false;
-                fs::path pathDebug = GetDataDir() / "debug.log";
+                fs::path pathDebug = GetDebugLogPath();
                 if (fsbridge::freopen(pathDebug,"a",fileout) != nullptr)
                     setbuf(fileout, nullptr); // unbuffered
             }
@@ -774,7 +789,7 @@ void ShrinkDebugFile()
     // Amount of debug.log to save at end when shrinking (must fit in memory)
     constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000;
     // Scroll debug.log if it's getting too big
-    fs::path pathLog = GetDataDir() / "debug.log";
+    fs::path pathLog = GetDebugLogPath();
     FILE* file = fsbridge::fopen(pathLog, "r");
     // If debug.log file is more than 10% bigger the RECENT_DEBUG_HISTORY_SIZE
     // trim it down by saving only the last RECENT_DEBUG_HISTORY_SIZE bytes
diff --git a/src/util.h b/src/util.h
index 08de43d29f..6687b865d2 100644
--- a/src/util.h
+++ b/src/util.h
@@ -36,6 +36,7 @@ int64_t GetStartupTime();
 static const bool DEFAULT_LOGTIMEMICROS = false;
 static const bool DEFAULT_LOGIPS        = false;
 static const bool DEFAULT_LOGTIMESTAMPS = true;
+extern const char * const DEFAULT_DEBUGLOGFILE;
 
 /** Signals for translation. */
 class CTranslationInterface
@@ -180,7 +181,8 @@ void CreatePidFile(const fs::path &path, pid_t pid);
 #ifdef WIN32
 fs::path GetSpecialFolderPath(int nFolder, bool fCreate = true);
 #endif
-void OpenDebugLog();
+fs::path GetDebugLogPath();
+bool OpenDebugLog();
 void ShrinkDebugFile();
 void runCommand(const std::string& strCommand);
 
diff --git a/test/functional/feature_logging.py b/test/functional/feature_logging.py
new file mode 100755
index 0000000000..da4e7b0398
--- /dev/null
+++ b/test/functional/feature_logging.py
@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test debug logging."""
+
+import os
+
+from test_framework.test_framework import BitcoinTestFramework
+
+class LoggingTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def run_test(self):
+        # test default log file name
+        assert os.path.isfile(os.path.join(self.nodes[0].datadir, "regtest", "debug.log"))
+
+        # test alternative log file name in datadir
+        self.restart_node(0, ["-debuglogfile=foo.log"])
+        assert os.path.isfile(os.path.join(self.nodes[0].datadir, "regtest", "foo.log"))
+
+        # test alternative log file name outside datadir
+        tempname = os.path.join(self.options.tmpdir, "foo.log")
+        self.restart_node(0, ["-debuglogfile=%s" % tempname])
+        assert os.path.isfile(tempname)
+
+        # check that invalid log (relative) will cause error
+        invdir = os.path.join(self.nodes[0].datadir, "regtest", "foo")
+        invalidname = os.path.join("foo", "foo.log")
+        self.stop_node(0)
+        self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % (invalidname)],
+                                                "Error: Could not open debug log file")
+        assert not os.path.isfile(os.path.join(invdir, "foo.log"))
+
+        # check that invalid log (relative) works after path exists
+        self.stop_node(0)
+        os.mkdir(invdir)
+        self.start_node(0, ["-debuglogfile=%s" % (invalidname)])
+        assert os.path.isfile(os.path.join(invdir, "foo.log"))
+
+        # check that invalid log (absolute) will cause error
+        self.stop_node(0)
+        invdir = os.path.join(self.options.tmpdir, "foo")
+        invalidname = os.path.join(invdir, "foo.log")
+        self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % invalidname],
+                                               "Error: Could not open debug log file")
+        assert not os.path.isfile(os.path.join(invdir, "foo.log"))
+
+        # check that invalid log (absolute) works after path exists
+        self.stop_node(0)
+        os.mkdir(invdir)
+        self.start_node(0, ["-debuglogfile=%s" % (invalidname)])
+        assert os.path.isfile(os.path.join(invdir, "foo.log"))
+
+
+if __name__ == '__main__':
+    LoggingTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 8c5654a85d..e38146a79a 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -127,6 +127,7 @@ BASE_SCRIPTS= [
     'p2p-fingerprint.py',
     'uacomment.py',
     'p2p-acceptblock.py',
+    'feature_logging.py',
 ]
 
 EXTENDED_SCRIPTS = [