Revert "Check whether FLUTTER_ROOT and FLUTTER_ROOT/bin are writable. (flutter#34291)" (flutter#34750)

jonahwilliams · web-flow · commit d4d10dac6122 · 2019-06-19T15:37:04.000-07:00
This reverts commit 4944950.
diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart
@@ -95,49 +95,8 @@ class Cache {
   final Directory _rootOverride;
   final List<CachedArtifact> _artifacts = <CachedArtifact>[];
 
-  // Check whether there is a writable bit in the usr permissions.
-  static bool _hasUserWritePermission(FileStat stat) {
-    // First grab the set of permissions for the usr group.
-    final int permissions = ((stat.mode & 0xFFF) >> 6) & 0x7;
-    // These values represent all of the octal permission bits that have
-    // readable and writable permission, though technically if we're missing
-    // readable we probably didn't make it this far.
-    return permissions == 6
-      || permissions == 7;
-  }
-
-  // Unfortunately the memory file system by default specifies a mode of `0`
-  // and is used by the majority of our tests. Default to false and only set
-  // to true when we know it is safe.
-  static bool checkPermissions = false;
-
   // Initialized by FlutterCommandRunner on startup.
-  static String get flutterRoot => _flutterRoot;
-  static String _flutterRoot;
-  static set flutterRoot(String value) {
-    if (value == null) {
-      _flutterRoot = null;
-      return;
-    }
-    if (checkPermissions) {
-      // Verify that we have writable permission in the flutter root. If not,
-      // we're liable to crash in unintuitive ways. This can happen if the user
-      // is using a homebrew or other unofficial channel, or otherwise installs
-      // Flutter into directory without permissions.
-      final FileStat binStat = fs.statSync(fs.path.join(value, 'bin'));
-      final FileStat rootStat = fs.statSync(value);
-      if (!_hasUserWritePermission(binStat) || !_hasUserWritePermission(rootStat)) {
-        throwToolExit(
-          'Warning: Flutter is missing permissions to write files '
-          'in its installation directory - "$value". '
-          'Please install Flutter from an official channel in a directory '
-          'where you have write permissions and that does not require '
-          'administrative or root access. For more information see '
-          'https://flutter.dev/docs/get-started/install');
-      }
-    }
-    _flutterRoot = value;
-  }
+  static String flutterRoot;
 
   // Whether to cache artifacts for all platforms. Defaults to only caching
   // artifacts for the current platform.
diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart
@@ -343,12 +343,6 @@ class FlutterCommandRunner extends CommandRunner<void> {
     // We must set Cache.flutterRoot early because other features use it (e.g.
     // enginePath's initializer uses it).
     final String flutterRoot = topLevelResults['flutter-root'] ?? defaultFlutterRoot;
-    bool checkPermissions = true;
-    assert(() {
-      checkPermissions = false;
-      return true;
-    }());
-    Cache.checkPermissions = checkPermissions;
     Cache.flutterRoot = fs.path.normalize(fs.path.absolute(flutterRoot));
 
     // Set up the tooling configuration.
@@ -483,6 +477,10 @@ class FlutterCommandRunner extends CommandRunner<void> {
     return EngineBuildPaths(targetEngine: engineBuildPath, hostEngine: engineHostBuildPath);
   }
 
+  static void initFlutterRoot() {
+    Cache.flutterRoot ??= defaultFlutterRoot;
+  }
+
   /// Get the root directories of the repo - the directories containing Dart packages.
   List<String> getRepoRoots() {
     final String root = fs.path.absolute(Cache.flutterRoot);
diff --git a/packages/flutter_tools/test/cache_test.dart b/packages/flutter_tools/test/cache_test.dart
@@ -6,8 +6,6 @@ import 'dart:async';
 
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
-import 'package:flutter_tools/src/base/common.dart';
-import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:mockito/mockito.dart';
 import 'package:platform/platform.dart';
 
@@ -42,7 +40,7 @@ void main() {
       await Cache.lock();
       Cache.checkLockAcquired();
     }, overrides: <Type, Generator>{
-      FileSystem: () => FakeFileSystem(),
+      FileSystem: () => MockFileSystem(),
     });
 
     testUsingContext('should not throw when FLUTTER_ALREADY_LOCKED is set', () async {
@@ -141,50 +139,12 @@ void main() {
     expect(flattenNameSubdirs(Uri.parse('http://docs.flutter.io/foo/bar')), 'docs.flutter.io/foo/bar');
     expect(flattenNameSubdirs(Uri.parse('https://www.flutter.dev')), 'www.flutter.dev');
   }, overrides: <Type, Generator>{
-    FileSystem: () => FakeFileSystem(),
-  });
-
-
-  group('Permissions test', () {
-    MockFileSystem mockFileSystem;
-    MockFileStat mockFileStat;
-
-    setUp(() {
-      mockFileSystem = MockFileSystem();
-      mockFileStat = MockFileStat();
-      when(mockFileSystem.path).thenReturn(fs.path);
-      Cache.checkPermissions = true;
-    });
-
-    tearDown(() {
-      Cache.checkPermissions = false;
-    });
-
-    testUsingContext('Throws error if missing usr write permissions in flutterRoot', () {
-      when(mockFileSystem.statSync(any)).thenReturn(mockFileStat);
-      when(mockFileStat.mode).thenReturn(344);
-
-      expect(() => Cache.flutterRoot = '', throwsA(isInstanceOf<ToolExit>()));
-    }, overrides: <Type, Generator>{
-      FileSystem: () => mockFileSystem,
-    }, initializeFlutterRoot: false);
-
-    testUsingContext('Doesnt error if we have usr write permissions in flutterRoot', () {
-      when(mockFileSystem.statSync(any)).thenReturn(mockFileStat);
-      when(mockFileStat.mode).thenReturn(493); // 0755 in decimal.
-
-      Cache.flutterRoot = '';
-    }, overrides: <Type, Generator>{
-      FileSystem: () => mockFileSystem,
-    }, initializeFlutterRoot: false);
-
+    FileSystem: () => MockFileSystem(),
   });
 }
 
-class MockFileSystem extends Mock implements FileSystem {}
-class MockFileStat extends Mock implements FileStat {}
-class FakeFileSystem extends ForwardingFileSystem {
-  FakeFileSystem() : super(MemoryFileSystem());
+class MockFileSystem extends ForwardingFileSystem {
+  MockFileSystem() : super(MemoryFileSystem());
 
   @override
   File file(dynamic path) {
diff --git a/packages/flutter_tools/test/commands/analyze_continuously_test.dart b/packages/flutter_tools/test/commands/analyze_continuously_test.dart
@@ -6,7 +6,6 @@ import 'dart:async';
 
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/os.dart';
-import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/dart/analysis.dart';
 import 'package:flutter_tools/src/dart/pub.dart';
 import 'package:flutter_tools/src/dart/sdk.dart';
@@ -20,7 +19,7 @@ void main() {
   Directory tempDir;
 
   setUp(() {
-    Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot;
+    FlutterCommandRunner.initFlutterRoot();
     tempDir = fs.systemTempDirectory.createTempSync('flutter_analysis_test.');
   });
 
