MI fix allowing multiple logging instances of lldb-mi to run simultaneously.

Summary:
Currently if two instances of lldb-mi are running with logging enabled using '--log' the log file conflicts. This produces the following error 
MI: Error: File Handler. Error Permission denied opening 'C:\Users\Ewan\LLVM\build\Debug\bin\lldb-mi-log.txt'

Fixed in this patch by renaming lldb-mi-log.txt based on the date, e.g. lldb-mi-log.txt-20150316163631.log, and moving the file into the temp directory by using the --log-dir option.

Regrading previous review comments the P_tmpdir macro is defined in Windows but always points to "\", which doesn't help much. Also when using the Windows API for GetTempPath() dynamic memory seems much more messy.

Patch from ewan@codeplay.com

Reviewers: abidh, EwanCrawford

Subscribers: zturner, lldb-commits, deepak2427

Differential Revision: http://reviews.llvm.org/D9054

llvm-svn: 235589

lldb/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py

@@ -127,6 +127,79 @@ class MiStartupOptionsTestCase(lldbmi_testcase.MiTestCaseBase):
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly = True)
+    
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    def test_lldbmi_log_option(self):
+        """Test that 'lldb-mi --log' creates a log file in the current directory."""
+    
+        logDirectory = "."
+        self.spawnLldbMi(args = "%s --log" % self.myexe)
 
+        # Test that lldb-mi is ready after startup
+        self.expect(self.child_prompt, exactly = True)
+
+        # Test that the executable is loaded when file was specified
+        self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
+        self.expect("\^done")
+
+        # Test that lldb-mi is ready when executable was loaded
+        self.expect(self.child_prompt, exactly = True)
+
+        # Run
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"exited-normally\"")
+
+        # Check log file is created
+        import glob,os
+        logFile = glob.glob(logDirectory + "/lldb-mi-*.log")
+
+        if not logFile:
+            self.fail("log file not found")
+
+        # Delete log
+        for f in logFile:
+            os.remove(f)
+
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    def test_lldbmi_log_directory_option(self):
+        """Test that 'lldb-mi --log --log-dir' creates a log file in the directory specified by --log-dir."""
+    
+        # Create log in temp directory
+        import tempfile
+        logDirectory = tempfile.gettempdir()
+
+        self.spawnLldbMi(args = "%s --log --log-dir=%s" % (self.myexe,logDirectory))
+
+        # Test that lldb-mi is ready after startup
+        self.expect(self.child_prompt, exactly = True)
+
+        # Test that the executable is loaded when file was specified
+        self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
+        self.expect("\^done")
+
+        # Test that lldb-mi is ready when executable was loaded
+        self.expect(self.child_prompt, exactly = True)
+
+        # Run
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"exited-normally\"")
+
+        # Check log file is created
+        import glob,os
+        logFile = glob.glob(logDirectory + "/lldb-mi-*.log")
+
+        if not logFile:
+            self.fail("log file not found")             
+   
+        # Delete log
+        for f in logFile:
+            os.remove(f)
+       
 if __name__ == '__main__':
     unittest2.main()

lldb/tools/lldb-mi/MICmnLogMediumFile.cpp

@@ -27,7 +27,9 @@
 //--
 CMICmnLogMediumFile::CMICmnLogMediumFile(void)
     : m_constThisMediumName(MIRSRC(IDS_MEDIUMFILE_NAME))
-    , m_constMediumFileName("lldb-mi-log.txt")
+    , m_constMediumFileNameFormat("lldb-mi-%s.log")
+    , m_strMediumFileName(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
+    , m_strMediumFileDirectory(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
     , m_fileNamePath(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
     , m_eVerbosityType(CMICmnLog::eLogVerbosity_Log)
     , m_strDate(CMIUtilDateTimeStd().GetDate())
@@ -72,7 +74,8 @@ CMICmnLogMediumFile::Instance(void)
 bool
 CMICmnLogMediumFile::Initialize(void)
 {
-    m_bInitialized = FileFormFileNamePath();
+    m_bInitialized = CMIUtilSystem().GetLogFilesPath(m_strMediumFileDirectory);
+    m_bInitialized &= FileFormFileNamePath();
 
     return m_bInitialized;
 }
@@ -213,21 +216,15 @@ CMICmnLogMediumFile::FileFormFileNamePath(void)
 
     m_fileNamePath = MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH);
 
-    CMIUtilString strPathName;
-    if (CMIUtilSystem().GetLogFilesPath(strPathName))
+    if (m_strMediumFileDirectory.compare(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH)) != 0)
     {
-        const CMIUtilString strPath = CMIUtilFileStd::StripOffFileName(strPathName);
+        CMIUtilDateTimeStd date;
+        m_strMediumFileName = CMIUtilString::Format(m_constMediumFileNameFormat.c_str(), date.GetDateTimeLogFilename().c_str());
 
-// ToDo: Review this LINUX log file quick fix so not hidden
-// AD:
-//      Linux was creating a log file here called '.\log.txt'.  The '.' on linux
-//      signifies that this file is 'hidden' and not normally visible.  A quick fix
-//      is to remove the path component all together.  Linux also normally uses '/'
-//      as directory separators, again leading to the problem of the hidden log.
 #if defined(_MSC_VER)
-        m_fileNamePath = CMIUtilString::Format("%s\\%s", strPath.c_str(), m_constMediumFileName.c_str());
+        m_fileNamePath = CMIUtilString::Format("%s\\%s", m_strMediumFileDirectory.c_str(), m_strMediumFileName.c_str());
 #else
-        m_fileNamePath = CMIUtilString::Format("%s", m_constMediumFileName.c_str());
+        m_fileNamePath = CMIUtilString::Format("%s/%s", m_strMediumFileDirectory.c_str(), m_strMediumFileName.c_str());
 #endif // defined ( _MSC_VER )
 
         return MIstatus::success;
@@ -261,7 +258,7 @@ CMICmnLogMediumFile::GetFileNamePath(void) const
 const CMIUtilString &
 CMICmnLogMediumFile::GetFileName(void) const
 {
-    return m_constMediumFileName;
+    return m_strMediumFileName;
 }
 
 //++ ------------------------------------------------------------------------------------
@@ -426,3 +423,19 @@ CMICmnLogMediumFile::GetLineReturn(void) const
 {
     return m_file.GetLineReturn();
 }
+
+//++ ------------------------------------------------------------------------------------
+// Details: Set the diretory to place the log file.
+// Type:    Method.
+// Args:    vPath   - (R) Path to log.
+// Return:  MIstatus::success - Functional succeeded.
+//          MIstatus::failure - Functional failed.
+// Throws:  None.
+//--
+bool
+CMICmnLogMediumFile::SetDirectory(const CMIUtilString &vPath)
+{
+    m_strMediumFileDirectory = vPath;
+
+    return FileFormFileNamePath();
+}

lldb/tools/lldb-mi/MICmnLogMediumFile.h

@@ -43,6 +43,7 @@ class CMICmnLogMediumFile : public CMICmnBase, public CMICmnLog::IMedium
     bool IsOk(void) const;
     bool IsFileExist(void) const;
     const CMIUtilString &GetLineReturn(void) const;
+    bool SetDirectory(const CMIUtilString &vPath);
 
     // Overridden:
   public:
@@ -71,8 +72,10 @@ class CMICmnLogMediumFile : public CMICmnBase, public CMICmnLog::IMedium
     // Attributes:
   private:
     const CMIUtilString m_constThisMediumName;
-    const CMIUtilString m_constMediumFileName;
+    const CMIUtilString m_constMediumFileNameFormat;
     //
+    CMIUtilString m_strMediumFileName;
+    CMIUtilString m_strMediumFileDirectory;
     CMIUtilString m_fileNamePath;
     MIuint m_eVerbosityType;
     CMIUtilString m_strDate;

lldb/tools/lldb-mi/MICmnResources.cpp

@@ -64,6 +64,7 @@ const CMICmnResources::SRsrcTextData CMICmnResources::ms_pResourceId2TextData[]
     {IDE_MI_APP_ARG_INTERPRETER, "--interpreter\n\t This option is kept for backward compatibility. This executable always run in MI mode"},
     {IDE_MI_APP_ARG_EXECUTEABLE, "--executable\n\tUse the MI Driver in MI mode for the debugging the specified executable." },
     {IDE_MI_APP_ARG_APP_LOG, "--log\n\tUse this argument to tell the MI Driver to update it's log\n\tfile '%s'."},
+    {IDE_MI_APP_ARG_APP_LOG_DIR, "--log-dir\n\tUse this argument to specify the directory the MI Driver\n\twill place the log file in, i.e --log-dir=/tmp." },
     {IDE_MI_APP_ARG_EXAMPLE, "Example MI command:\n\t3-info-gdb-mi-command gdb-set\n\t3^done,command={exists=\"true\"}"},
     {IDE_MI_APP_ARG_EXECUTABLE, "executable (NOT IMPLEMENTED)\n\tThe file path to the executable i.e. '\"C:\\My Dev\\foo.exe\"'."},
     {IDS_STDIN_ERR_INVALID_PROMPT, "Stdin. Invalid prompt description '%s'"},

lldb/tools/lldb-mi/MICmnResources.h

@@ -74,6 +74,7 @@ enum
     IDE_MI_APP_ARG_INTERPRETER,
     IDE_MI_APP_ARG_EXECUTEABLE,
     IDE_MI_APP_ARG_APP_LOG,
+    IDE_MI_APP_ARG_APP_LOG_DIR,
     IDE_MI_APP_ARG_EXAMPLE,
     IDE_MI_APP_ARG_EXECUTABLE,
 

lldb/tools/lldb-mi/MIDriverMgr.cpp

@@ -427,6 +427,7 @@ CMIDriverMgr::DriverGetTheDebugger(void)
 //              --versionLong
 //              --log
 //              --executable
+//              --log-dir
 //          The above arguments are not handled by any driver object except for --executable.
 //          The options --interpreter and --executable in code act very similar. The
 //          --executable is necessary to differentiate whither the MI Driver is being using
@@ -436,7 +437,8 @@ CMIDriverMgr::DriverGetTheDebugger(void)
 //          Driver is being called the command line and that the executable argument is indeed
 //          a specified executable an so actions commands to set up the executable for a
 //          debug session. Using --interpreter on the commnd line does not action additional
-//          commands to initialise a debug session and so be able to launch the process.
+//          commands to initialise a debug session and so be able to launch the process. The directory
+//          where the log file is created is specified using --log-dir.
 // Type:    Method.
 // Args:    argc        - (R)   An integer that contains the count of arguments that follow in
 //                              argv. The argc parameter is always greater than or equal to 1.
@@ -483,7 +485,9 @@ CMIDriverMgr::ParseArgs(const int argc, const char *argv[], bool &vwbExiting)
     bool bHaveArgVersion = false;
     bool bHaveArgVersionLong = false;
     bool bHaveArgLog = false;
+    bool bHaveArgLogDir = false;
     bool bHaveArgHelp = false;
+    CMIUtilString strLogDir;
 
     bHaveArgInterpret = true;
     if (bHaveArgs)
@@ -512,6 +516,11 @@ CMIDriverMgr::ParseArgs(const int argc, const char *argv[], bool &vwbExiting)
             {
                 bHaveArgLog = true;
             }
+            if (0 == strArg.compare(0,10,"--log-dir="))
+            {
+                strLogDir = strArg.substr(10,CMIUtilString::npos);
+                bHaveArgLogDir = true;
+            }
             if ((0 == strArg.compare("--help")) || (0 == strArg.compare("-h")))
             {
                 bHaveArgHelp = true;
@@ -524,6 +533,11 @@ CMIDriverMgr::ParseArgs(const int argc, const char *argv[], bool &vwbExiting)
         CMICmnLog::Instance().SetEnabled(true);
     }
 
+    if (bHaveArgLogDir)
+    {
+        bOk = bOk && CMICmnLogMediumFile::Instance().SetDirectory(strLogDir);
+    }
+
     // Todo: Remove this output when MI is finished. It is temporary to persuade Ecllipse plugin to work.
     //       Eclipse reads this literally and will not work unless it gets this exact version text.
     // Handle --version option (ignore the --interpreter option if present)
@@ -614,6 +628,7 @@ CMIDriverMgr::GetHelpOnCmdLineArgOptions(void) const
         MIRSRC(IDE_MI_APP_ARG_INTERPRETER),
         MIRSRC(IDE_MI_APP_ARG_EXECUTEABLE),
         CMIUtilString::Format(MIRSRC(IDE_MI_APP_ARG_APP_LOG), CMICmnLogMediumFile::Instance().GetFileName().c_str()),
+        MIRSRC(IDE_MI_APP_ARG_APP_LOG_DIR),
         MIRSRC(IDE_MI_APP_ARG_EXECUTABLE),
         MIRSRC(IDS_CMD_QUIT_HELP),
         MIRSRC(IDE_MI_APP_ARG_EXAMPLE)};

lldb/tools/lldb-mi/MIUtilDateTimeStd.cpp

@@ -71,3 +71,21 @@ CMIUtilDateTimeStd::GetTime(void)
 
     return strTime;
 }
+
+//++ ------------------------------------------------------------------------------------
+// Details: Retrieve system local current date and time in yyyy-MM-dd--HH-mm-ss format for log file names.
+// Type:    Method.
+// Args:    None.
+// Return:  CMIUtilString - Text description.
+// Throws:  None.
+//--
+CMIUtilString
+CMIUtilDateTimeStd::GetDateTimeLogFilename(void)
+{
+    std::time(&m_rawTime);
+    const std::tm *pTi = std::localtime(&m_rawTime);
+    const CMIUtilString strTime(CMIUtilString::Format("%d%02d%02d%02d%02d%02d", pTi->tm_year + 1900, pTi->tm_mon,
+                                                      pTi->tm_mday, pTi->tm_hour, pTi->tm_min, pTi->tm_sec));
+
+    return strTime;
+}

lldb/tools/lldb-mi/MIUtilDateTimeStd.h

@@ -30,6 +30,7 @@ class CMIUtilDateTimeStd
 
     CMIUtilString GetDate(void);
     CMIUtilString GetTime(void);
+    CMIUtilString GetDateTimeLogFilename(void);
 
     // Overrideable:
   public:

lldb/tools/lldb-mi/MIUtilSystemWindows.cpp

@@ -17,6 +17,7 @@
 // In-house headers:
 #include "MIUtilSystemWindows.h"
 #include "MICmnResources.h"
+#include "MIUtilFileStd.h"
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMIUtilSystemWindows constructor.
@@ -133,7 +134,8 @@ CMIUtilSystemWindows::GetExecutablesPath(CMIUtilString &vrwFileNamePath) const
 bool
 CMIUtilSystemWindows::GetLogFilesPath(CMIUtilString &vrwFileNamePath) const
 {
-    return GetExecutablesPath(vrwFileNamePath);
+    vrwFileNamePath = CMIUtilString(".");
+    return MIstatus::success;
 }
 
 #endif // #if defined( _MSC_VER )