[flutter_tools] Remove globals/mocks from GradleUtils #76020
Conversation
flutter-dashboard
bot
added
the
tool
| /// Gets the Gradle executable path and prepares the Gradle project. | ||
| /// This is the `gradlew` or `gradlew.bat` script in the `android/` directory. | ||
| String getExecutable(FlutterProject project) { | ||
| final Directory androidDir = project.android.hostAppGradleRoot; | ||
| gradleUtils.injectGradleWrapperIfNeeded(androidDir); |
There was a problem hiding this comment.
This line is sus. Basically, what this did is re-grabbed the global GradleUtils - the tests used this to test getExecutable against a mocked out injectgradleWrapperIfNeeded. I removed this functionality
jonahwilliams
changed the title
| } | ||
| // Don't log analytics for downstream Flutter commands. | ||
| // e.g. `flutter build bundle`. | ||
| environment['FLUTTER_SUPPRESS_ANALYTICS'] = 'true'; |
There was a problem hiding this comment.
We don't care about this anymore?
There was a problem hiding this comment.
No, we specifically don't want it actually - this is a holdover from each build requiring N flutter commands
| const int _kExecPermissionMask = 0x49; // a+x | ||
|
|
||
| /// Returns [true] if [executable] has all executable flag set. | ||
| bool _hasAllExecutableFlagSet(File executable) { | ||
| final FileStat stat = executable.statSync(); | ||
| assert(stat.type != FileSystemEntityType.notFound); | ||
| globals.printTrace('${executable.path} mode: ${stat.mode} ${stat.modeString()}.'); | ||
| return stat.mode & _kExecPermissionMask == _kExecPermissionMask; | ||
| } | ||
|
|
||
| /// Returns [true] if [executable] has any executable flag set. | ||
| bool _hasAnyExecutableFlagSet(File executable) { | ||
| final FileStat stat = executable.statSync(); | ||
| assert(stat.type != FileSystemEntityType.notFound); | ||
| globals.printTrace('${executable.path} mode: ${stat.mode} ${stat.modeString()}.'); | ||
| return stat.mode & _kExecPermissionMask != 0; | ||
| } | ||
|
|
||
| /// Gives execute permission to [executable] if it doesn't have it already. | ||
| void _giveExecutePermissionIfNeeded(File executable) { | ||
| if (!_hasAllExecutableFlagSet(executable)) { | ||
| globals.printTrace('Trying to give execute permission to ${executable.path}.'); | ||
| globals.os.makeExecutable(executable); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We also set the executable bit in create.dart, but that has a separate method for doing so. I can handle that cleanup in a separate PR
| @@ -65,34 +65,12 @@ class FileSystemUtils { | |||
| bool shouldCopyFile(File srcFile, File destFile), | |||
There was a problem hiding this comment.
make FileSystemUtils.copyDirectorySync delegate to copyDirectory, to avoid a large diff/breaking change
I don't mind reviewing large diffs to migrate existing FileSystemUtils.copyDirectorySync usages to just copyDirectory. What breaking changes are you concerned about (ono is this used in g3?)
There was a problem hiding this comment.
Done, just mostly dreading fixing the tests that mock it, but there weren't too many.
|
All usage of createDirectorySync removed |
|
Build is green - landing. |
|
|
||
| for (final FileSystemEntity entity in srcDir.listSync()) { | ||
| final String newPath = destDir.fileSystem.path.join(destDir.path, entity.basename); | ||
| if (entity is Link) { |
There was a problem hiding this comment.
Example of a link falling through to the exception: #35170
|
Part of #76264 |
|
I mean #71511 |
Refactor GradleUtils into a mostly global free class.
Remove tests that tested a mock of GradleUtils against itself.