Let LLProcess consumer specify desired description for logging.

If caller runs (e.g.) a Python script, it's not very helpful to a human log
reader to keep seeing LLProcess instances logged as /pathname/to/python (pid).
If caller is aware, the code can at least use the script name as the desc --
or maybe even a hint as to the script's purpose.
If caller doesn't explicitly pass a desc, at least shorten to just the
basename of the executable.

3 files changed, 39 insertions, 9 deletions

diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index 9799ed1938..b4c6a647d7 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -50,6 +50,7 @@
 static const char* whichfile[] = { "stdin", "stdout", "stderr" };
 static std::string empty;
 static LLProcess::Status interpret_status(int status);
+static std::string getDesc(const LLProcess::Params& params);
 
 /**
  * Ref-counted "mainloop" listener. As long as there are still outstanding
@@ -404,7 +405,7 @@ LLProcessPtr LLProcess::create(const LLSDOrParams& params)
 			LLEventPumps::instance().obtain(params.postend)
 				.post(LLSDMap
 					  // no "id"
-					  ("desc", std::string(params.executable))
+					  ("desc", getDesc(params))
 					  ("state", LLProcess::UNSTARTED)
 					  // no "data"
 					  ("string", e.what())
@@ -561,8 +562,8 @@ LLProcess::LLProcess(const LLSDOrParams& params):
 	sProcessListener.addPoll(*this);
 	mStatus.mState = RUNNING;
 
-	mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcess.pid << ')');
-	LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcess.pid << ")" << LL_ENDL;
+	mDesc = STRINGIZE(getDesc(params) << " (" << mProcess.pid << ')');
+	LL_INFOS("LLProcess") << mDesc << ": launched " << params << LL_ENDL;
 
 	// Unless caller explicitly turned off autokill (child should persist),
 	// take steps to terminate the child. This is all suspenders-and-belt: in
@@ -604,6 +605,29 @@ LLProcess::LLProcess(const LLSDOrParams& params):
 	}
 }
 
+// Helper to obtain a description string, given a Params block
+static std::string getDesc(const LLProcess::Params& params)
+{
+	// If caller specified a description string, by all means use it.
+	std::string desc(params.desc);
+	if (! desc.empty())
+		return desc;
+
+	// Caller didn't say. Use the executable name -- but use just the filename
+	// part. On Mac, for instance, full pathnames get cumbersome.
+	// If there are Linden utility functions to manipulate pathnames, I
+	// haven't found them -- and for this usage, Boost.Filesystem seems kind
+	// of heavyweight.
+	std::string executable(params.executable);
+	std::string::size_type delim = executable.find_last_of("\\/");
+	// If executable contains no pathname delimiters, return the whole thing.
+	if (delim == std::string::npos)
+		return executable;
+
+	// Return just the part beyond the last delimiter.
+	return executable.substr(delim + 1);
+}
+
 LLProcess::~LLProcess()
 {
 	// Only in state RUNNING are we registered for callback. In UNSTARTED we
diff --git a/indra/llcommon/llprocess.h b/indra/llcommon/llprocess.h
index 96a3dce5b3..d005847e18 100644
--- a/indra/llcommon/llprocess.h
+++ b/indra/llcommon/llprocess.h
@@ -159,7 +159,8 @@ public:
 			cwd("cwd"),
 			autokill("autokill", true),
 			files("files"),
-			postend("postend")
+			postend("postend"),
+			desc("desc")
 		{}
 
 		/// pathname of executable
@@ -199,6 +200,13 @@ public:
 		 *   with code 0")
 		 */
 		Optional<std::string> postend;
+		/**
+		 * Description of child process for logging purposes. It need not be
+		 * unique; the logged description string will contain the PID as well.
+		 * If this is omitted, a description will be derived from the
+		 * executable name.
+		 */
+		Optional<std::string> desc;
 	};
 	typedef LLSDParamAdapter<Params> LLSDOrParams;
 
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 1a755c283c..fe599e7892 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -136,6 +136,7 @@ struct PythonProcessLauncher
         const char* PYTHON(getenv("PYTHON"));
         tut::ensure("Set $PYTHON to the Python interpreter", PYTHON);
 
+        mParams.desc = desc + " script";
         mParams.executable = PYTHON;
         mParams.args.add(mScript.getName());
     }
@@ -1244,17 +1245,14 @@ namespace tut
         std::string pumpname("postend");
         EventListener listener(LLEventPumps::instance().obtain(pumpname));
         LLProcess::Params params;
+        params.desc = "bad postend";
         params.postend = pumpname;
         LLProcessPtr child = LLProcess::create(params);
         ensure("shouldn't have launched", ! child);
         ensure_equals("number of postend events", listener.mHistory.size(), 1);
         LLSD postend(listener.mHistory.front());
         ensure("has id", ! postend.has("id"));
-        // Ha ha, in this case the implementation normally sets "desc" to
-        // params.executable. But as the nature of the problem is that
-        // params.executable is empty, expecting "desc" to be nonempty is a
-        // bit unreasonable!
-        //ensure("desc empty", ! postend["desc"].asString().empty());
+        ensure_equals("desc", postend["desc"].asString(), std::string(params.desc));
         ensure_equals("state", postend["state"].asInteger(), LLProcess::UNSTARTED);
         ensure("has data", ! postend.has("data"));
         std::string error(postend["string"]);