Browse Source

refactor: make app logs dir creation opt-in (#17841)

Previously, we were creating the app logs folder at a predefined location during initial electron startup, which meant that it had to be manually removed and prevented clean app portability. This refactors that implementation such that it's now an opt-in feature and developers must call app.setAppLogsPath(path) with an optional custom path in order to set this directory.
Shelley Vohr 6 years ago
parent
commit
0749dc4cc1

+ 22 - 0
atom/browser/api/atom_api_app.cc

@@ -16,6 +16,7 @@
 #include "atom/browser/atom_paths.h"
 #include "atom/browser/login_handler.h"
 #include "atom/browser/relauncher.h"
+#include "atom/common/application_info.h"
 #include "atom/common/atom_command_line.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
@@ -830,6 +831,26 @@ void App::SetAppPath(const base::FilePath& app_path) {
   app_path_ = app_path;
 }
 
+#if !defined(OS_MACOSX)
+void App::SetAppLogsPath(mate::Arguments* args) {
+  base::FilePath custom_path;
+  if (args->GetNext(&custom_path)) {
+    if (!custom_path.IsAbsolute()) {
+      args->ThrowError("Path must be absolute");
+      return;
+    }
+    base::PathService::Override(DIR_APP_LOGS, custom_path);
+  } else {
+    base::FilePath path;
+    if (base::PathService::Get(DIR_USER_DATA, &path)) {
+      path = path.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
+      path = path.Append(base::FilePath::FromUTF8Unsafe("logs"));
+      base::PathService::Override(DIR_APP_LOGS, path);
+    }
+  }
+}
+#endif
+
 base::FilePath App::GetPath(mate::Arguments* args, const std::string& name) {
   bool succeed = false;
   base::FilePath path;
@@ -1377,6 +1398,7 @@ void App::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("getAppPath", &App::GetAppPath)
       .SetMethod("setPath", &App::SetPath)
       .SetMethod("getPath", &App::GetPath)
+      .SetMethod("setAppLogsPath", &App::SetAppLogsPath)
       .SetMethod("setDesktopName", &App::SetDesktopName)
       .SetMethod("getLocale", &App::GetLocale)
       .SetMethod("getLocaleCountryCode", &App::GetLocaleCountryCode)

+ 2 - 0
atom/browser/api/atom_api_app.h

@@ -175,6 +175,8 @@ class App : public AtomBrowserClient::Delegate,
   void ChildProcessLaunched(int process_type, base::ProcessHandle handle);
   void ChildProcessDisconnected(base::ProcessId pid);
 
+  void SetAppLogsPath(mate::Arguments* args);
+
   // Get/Set the pre-defined path in PathService.
   base::FilePath GetPath(mate::Arguments* args, const std::string& name);
   void SetPath(mate::Arguments* args,

+ 38 - 0
atom/browser/api/atom_api_app_mac.mm

@@ -0,0 +1,38 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/api/atom_api_app.h"
+#include "atom/browser/atom_paths.h"
+#include "atom/common/native_mate_converters/file_path_converter.h"
+#include "base/path_service.h"
+
+#import <Cocoa/Cocoa.h>
+
+namespace atom {
+
+namespace api {
+
+void App::SetAppLogsPath(mate::Arguments* args) {
+  base::FilePath custom_path;
+  if (args->GetNext(&custom_path)) {
+    if (!custom_path.IsAbsolute()) {
+      args->ThrowError("Path must be absolute");
+      return;
+    }
+    base::PathService::Override(DIR_APP_LOGS, custom_path);
+  } else {
+    NSString* bundle_name =
+        [[[NSBundle mainBundle] infoDictionary] objectForKey:@"CFBundleName"];
+    NSString* logs_path =
+        [NSString stringWithFormat:@"Library/Logs/%@", bundle_name];
+    NSString* library_path =
+        [NSHomeDirectory() stringByAppendingPathComponent:logs_path];
+    base::PathService::Override(DIR_APP_LOGS,
+                                base::FilePath([library_path UTF8String]));
+  }
+}
+
+}  // namespace atom
+
+}  // namespace api

+ 0 - 12
atom/browser/atom_browser_main_parts.cc

@@ -214,17 +214,6 @@ void AtomBrowserMainParts::InitializeFeatureList() {
   base::FeatureList::SetInstance(std::move(feature_list));
 }
 
-#if !defined(OS_MACOSX)
-void AtomBrowserMainParts::OverrideAppLogsPath() {
-  base::FilePath path;
-  if (base::PathService::Get(DIR_APP_DATA, &path)) {
-    path = path.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
-    path = path.Append(base::FilePath::FromUTF8Unsafe("logs"));
-    base::PathService::Override(DIR_APP_LOGS, path);
-  }
-}
-#endif
-
 // static
 AtomBrowserMainParts* AtomBrowserMainParts::self_ = nullptr;
 
@@ -284,7 +273,6 @@ void AtomBrowserMainParts::RegisterDestructionCallback(
 int AtomBrowserMainParts::PreEarlyInitialization() {
   InitializeFeatureList();
   field_trial_list_ = std::make_unique<base::FieldTrialList>(nullptr);
-  OverrideAppLogsPath();
 #if defined(USE_X11)
   views::LinuxUI::SetInstance(BuildGtkUi());
   OverrideLinuxAppDataPath();

+ 0 - 1
atom/browser/atom_browser_main_parts.h

@@ -88,7 +88,6 @@ class AtomBrowserMainParts : public content::BrowserMainParts {
 
  private:
   void InitializeFeatureList();
-  void OverrideAppLogsPath();
   void PreMainMessageLoopStartCommon();
 
 #if defined(OS_POSIX)

+ 0 - 13
atom/browser/atom_browser_main_parts_mac.mm

@@ -32,19 +32,6 @@ void AtomBrowserMainParts::FreeAppDelegate() {
   [NSApp setDelegate:nil];
 }
 
-void AtomBrowserMainParts::OverrideAppLogsPath() {
-  base::FilePath path;
-  NSString* bundleName =
-      [[[NSBundle mainBundle] infoDictionary] objectForKey:@"CFBundleName"];
-  NSString* logsPath =
-      [NSString stringWithFormat:@"Library/Logs/%@", bundleName];
-  NSString* libraryPath =
-      [NSHomeDirectory() stringByAppendingPathComponent:logsPath];
-
-  base::PathService::Override(DIR_APP_LOGS,
-                              base::FilePath([libraryPath UTF8String]));
-}
-
 // Replicates NSApplicationMain, but doesn't start a run loop.
 void AtomBrowserMainParts::InitializeMainNib() {
   auto infoDictionary = base::mac::OuterBundle().infoDictionary;

+ 13 - 5
docs/api/app.md

@@ -570,6 +570,14 @@ Hides all application windows without minimizing them.
 Shows application windows after they were hidden. Does not automatically focus
 them.
 
+### `app.setAppLogsPath(path)`
+
+* `path` String (optional) - A custom path for your logs. Must be absolute.
+
+Sets or creates a directory your app's logs which can then be manipulated with `app.getPath()` or `app.setPath(newPath)`.
+
+On _macOS_, this directory will be set by deafault to `/Library/Logs/YourAppName`, and on _Linux_ and _Windows_ it will be placed inside your `userData` directory.
+
 ### `app.getAppPath()`
 
 Returns `String` - The current application directory.
@@ -618,8 +626,8 @@ Fetches a path's associated icon.
 
 On _Windows_, there are 2 kinds of icons:
 
-- Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
-- Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
+* Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
+* Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
 
 On _Linux_ and _macOS_, icons depend on the application associated with file mime type.
 
@@ -640,8 +648,8 @@ Fetches a path's associated icon.
 
 On _Windows_, there a 2 kinds of icons:
 
-- Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
-- Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
+* Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
+* Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
 
 On _Linux_ and _macOS_, icons depend on the application associated with file mime type.
 
@@ -694,6 +702,7 @@ To set the locale, you'll want to use a command line switch at app startup, whic
 **Note:** On Windows, you have to call it after the `ready` events gets emitted.
 
 ### `app.getLocaleCountryCode()`
+
 Returns `string` - User operating system's locale two-letter [ISO 3166](https://www.iso.org/iso-3166-country-codes.html) country code. The value is taken from native OS APIs.
 
 **Note:** When unable to detect locale country code, it returns empty string.
@@ -753,7 +762,6 @@ Returns `Boolean` - Whether the call succeeded.
 This method checks if the current executable as the default handler for a
 protocol (aka URI scheme). If so, it will remove the app as the default handler.
 
-
 ### `app.isDefaultProtocolClient(protocol[, path, args])`
 
 * `protocol` String - The name of your protocol, without `://`.

+ 1 - 0
filenames.gni

@@ -127,6 +127,7 @@ filenames = {
     "atom/app/command_line_args.h",
     "atom/app/uv_task_runner.cc",
     "atom/app/uv_task_runner.h",
+    "atom/browser/api/atom_api_app_mac.mm",
     "atom/browser/api/atom_api_app.cc",
     "atom/browser/font_defaults.cc",
     "atom/browser/font_defaults.h",