Simplify skiatest framework.

tools/TempDir.cpp

Index: tools/TempDir.cpp
diff --git a/tools/TempDir.cpp b/tools/TempDir.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..d8fbe679579a67d49e02768edfc1f12e8c0866c8
--- /dev/null
+++ b/tools/TempDir.cpp
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <stdlib.h>
+
+#include "TempDir.h"
+
+#include "SkCommandLineFlags.h"
+#include "SkOSFile.h"
+
+DEFINE_string2(tmpDir, t, NULL, "Temp directory to use.");

[mtklein, 2015/01/18 22:57:55]

It doesn't look like anyone sets this flag.

So, at the very least, let's delete the flag.  I'd also strongly consider
deleting all this code and all the tests that appear to use the results of
GetTmpDir() today.

[hal.canary, 2015/01/20 16:01:52]

Done.

+
+static bool nonempty_string(const char s[]) { return s && s[0]; }
+
+// Logic borrowed from
+// https://docs.python.org/2/library/tempfile.html#tempfile.tempdir

[mtklein, 2015/01/18 22:57:55]

Let's not do this.  This is way more complicated than it needs to be, which at
the very mildest is going to make someone confused over why we wrote this code.

[hal.canary, 2015/01/20 16:01:52]

Done.

+SkString sk_tools::GetTmpDir() {
+    const char* tmpDir = FLAGS_tmpDir.isEmpty() ? NULL : FLAGS_tmpDir[0];
+    if (nonempty_string(tmpDir)) {

[mtklein, 2015/01/18 22:57:55]

This seems like a weird way to write

if (!FLAGS_tmpDir.isEmpty()) {
  return SkString(FLAGS_tmpDir[0]);
}

[hal.canary, 2015/01/20 16:01:52]

Acknowledged.

+        return SkString(tmpDir);
+    }
+
+    const char* const kVariables[] = {"TMPDIR", "TEMP", "TMP"};

[mtklein, 2015/01/18 22:57:55]

Why are we getting so complicated with features no one will ever use?

#if SK_BUILD_FOR_WIN
  return "C:\\TEMP";  (Or whatever the normal Windows temp dir is.)
#else
  return "/tmp";
#endif

[hal.canary, 2015/01/20 16:01:52]

Acknowledged.

+    for (size_t i = 0; i < SK_ARRAY_COUNT(kVariables); ++i) {
+        tmpDir = getenv(kVariables[i]);
+        if (nonempty_string(tmpDir)) {
+            return SkString(tmpDir);
+        }
+    }
+
+#ifdef SK_BUILD_FOR_WIN
+    const char* const kDirectories[] = {
+            "C:\\TEMP", "C:\\TMP", "\\TEMP", "\\TMP"};
+#else
+    const char* const kDirectories[] = {"/tmp", "/var/tmp", "/usr/tmp"};
+#endif
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kDirectories); ++i) {
+        if (sk_isdir(kDirectories[i])) {
+            return SkString(kDirectories[i]);
+        }
+    }
+    return SkString(".");

[mtklein, 2015/01/18 22:57:55]

This seems like a bad idea.  I'd rather return no temporary directory than say
the cwd is a temporary directory.  If we write to ., no one will ever clean that
up.

[hal.canary, 2015/01/20 16:01:52]

Acknowledged.

+}