diff --git a/packages/flutter_tools/lib/src/cmake.dart b/packages/flutter_tools/lib/src/cmake.dart index e57b9742ae..c611e98a26 100644 --- a/packages/flutter_tools/lib/src/cmake.dart +++ b/packages/flutter_tools/lib/src/cmake.dart @@ -40,12 +40,12 @@ file(TO_CMAKE_PATH "$escapedProjectDir" PROJECT_DIR) # Environment variables to pass to tool_backend.sh list(APPEND FLUTTER_TOOL_ENVIRONMENT - "FLUTTER_ROOT=\\"$escapedFlutterRoot\\"" - "PROJECT_DIR=\\"$escapedProjectDir\\"" + "FLUTTER_ROOT=$escapedFlutterRoot" + "PROJECT_DIR=$escapedProjectDir" '''); for (final String key in environment.keys) { final String value = _escapeBackslashes(environment[key]); - buffer.writeln(' "$key=\\"$value\\""'); + buffer.writeln(' "$key=$value"'); } buffer.writeln(')'); diff --git a/packages/flutter_tools/lib/src/linux/build_linux.dart b/packages/flutter_tools/lib/src/linux/build_linux.dart index 080d9393c5..3a20b674c7 100644 --- a/packages/flutter_tools/lib/src/linux/build_linux.dart +++ b/packages/flutter_tools/lib/src/linux/build_linux.dart @@ -7,12 +7,14 @@ import '../base/analyze_size.dart'; import '../base/common.dart'; import '../base/file_system.dart'; import '../base/logger.dart'; +import '../base/project_migrator.dart'; import '../base/utils.dart'; import '../build_info.dart'; import '../cache.dart'; import '../cmake.dart'; import '../convert.dart'; import '../globals.dart' as globals; +import '../migrations/cmake_custom_command_migration.dart'; import '../plugins.dart'; import '../project.dart'; @@ -35,6 +37,15 @@ Future buildLinux( 'to learn about adding Linux support to a project.'); } + final List migrators = [ + CmakeCustomCommandMigration(linuxProject, globals.logger), + ]; + + final ProjectMigration migration = ProjectMigration(migrators); + if (!migration.run()) { + throwToolExit('Unable to migrate project files'); + } + // Build the environment that needs to be set for the re-entrant flutter build // step. final Map environmentConfig = buildInfo.toEnvironmentConfig(); diff --git a/packages/flutter_tools/lib/src/migrations/cmake_custom_command_migration.dart b/packages/flutter_tools/lib/src/migrations/cmake_custom_command_migration.dart new file mode 100644 index 0000000000..1721e53f71 --- /dev/null +++ b/packages/flutter_tools/lib/src/migrations/cmake_custom_command_migration.dart @@ -0,0 +1,64 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import '../base/file_system.dart'; +import '../base/logger.dart'; +import '../base/project_migrator.dart'; +import '../project.dart'; + +// CMake's add_custom_command() should use VERBATIM to handle escaping of spaces +// and special characters correctly. +// See https://github.com/flutter/flutter/issues/67270. +class CmakeCustomCommandMigration extends ProjectMigrator { + CmakeCustomCommandMigration(CmakeBasedProject project, Logger logger) + : _cmakeFile = project.managedCmakeFile, + super(logger); + + final File _cmakeFile; + + @override + bool migrate() { + if (!_cmakeFile.existsSync()) { + logger.printTrace('CMake project not found, skipping add_custom_command() VERBATIM migration'); + return true; + } + + final String originalProjectContents = _cmakeFile.readAsStringSync(); + // Example: + // + // add_custom_command( + // OUTPUT ${FLUTTER_LIBRARY} ${FLUTTER_LIBRARY_HEADERS} + // ${CMAKE_CURRENT_BINARY_DIR}/_phony_ + // COMMAND ${CMAKE_COMMAND} -E env + // ${FLUTTER_TOOL_ENVIRONMENT} + // "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.sh" + // linux-x64 ${CMAKE_BUILD_TYPE} + // ) + + // Match the whole add_custom_command() and append VERBATIM unless it + // already exists. + final RegExp addCustomCommand = RegExp( + r'add_custom_command\(\s*(.*?)\s*\)', + multiLine: true, + dotAll: true, + ); + + String newProjectContents = originalProjectContents; + + final Iterable matches = addCustomCommand.allMatches(originalProjectContents); + + for (final RegExpMatch match in matches) { + final String addCustomCommandOriginal = match.group(1); + if (addCustomCommandOriginal?.contains('VERBATIM') == false) { + final String addCustomCommandReplacement = '$addCustomCommandOriginal\n VERBATIM'; + newProjectContents = newProjectContents.replaceAll(addCustomCommandOriginal, addCustomCommandReplacement); + } + } + if (originalProjectContents != newProjectContents) { + logger.printStatus('add_custom_command() missing VERBATIM, updating.'); + _cmakeFile.writeAsStringSync(newProjectContents.toString()); + } + return true; + } +} diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index 8c22cff21c..26df37dd98 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -364,6 +364,9 @@ abstract class CmakeBasedProject { /// The native project CMake specification. File get cmakeFile; + /// Contains definitions for the Flutter library and the tool. + File get managedCmakeFile; + /// Contains definitions for FLUTTER_ROOT, LOCAL_ENGINE, and more flags for /// the build. File get generatedCmakeConfigFile; @@ -1099,6 +1102,9 @@ class WindowsProject extends FlutterProjectPlatform implements CmakeBasedProject @override File get cmakeFile => _editableDirectory.childFile('CMakeLists.txt'); + @override + File get managedCmakeFile => managedDirectory.childFile('CMakeLists.txt'); + @override File get generatedCmakeConfigFile => ephemeralDirectory.childFile('generated_config.cmake'); @@ -1153,6 +1159,9 @@ class LinuxProject extends FlutterProjectPlatform implements CmakeBasedProject { @override File get cmakeFile => _editableDirectory.childFile('CMakeLists.txt'); + @override + File get managedCmakeFile => managedDirectory.childFile('CMakeLists.txt'); + @override File get generatedCmakeConfigFile => ephemeralDirectory.childFile('generated_config.cmake'); diff --git a/packages/flutter_tools/lib/src/windows/build_windows.dart b/packages/flutter_tools/lib/src/windows/build_windows.dart index da27a71df8..1c0639038d 100644 --- a/packages/flutter_tools/lib/src/windows/build_windows.dart +++ b/packages/flutter_tools/lib/src/windows/build_windows.dart @@ -7,12 +7,14 @@ import '../base/analyze_size.dart'; import '../base/common.dart'; import '../base/file_system.dart'; import '../base/logger.dart'; +import '../base/project_migrator.dart'; import '../base/utils.dart'; import '../build_info.dart'; import '../cache.dart'; import '../cmake.dart'; import '../convert.dart'; import '../globals.dart' as globals; +import '../migrations/cmake_custom_command_migration.dart'; import '../plugins.dart'; import '../project.dart'; import 'visual_studio.dart'; @@ -35,6 +37,15 @@ Future buildWindows(WindowsProject windowsProject, BuildInfo buildInfo, { 'to learn about adding Windows support to a project.'); } + final List migrators = [ + CmakeCustomCommandMigration(windowsProject, globals.logger), + ]; + + final ProjectMigration migration = ProjectMigration(migrators); + if (!migration.run()) { + throwToolExit('Unable to migrate project files'); + } + // Ensure that necessary ephemeral files are generated and up to date. _writeGeneratedFlutterConfig(windowsProject, buildInfo, target); createPluginSymlinks(windowsProject.parent); diff --git a/packages/flutter_tools/templates/app/linux.tmpl/flutter/CMakeLists.txt b/packages/flutter_tools/templates/app/linux.tmpl/flutter/CMakeLists.txt index 510701c7c1..a1da1b9e53 100644 --- a/packages/flutter_tools/templates/app/linux.tmpl/flutter/CMakeLists.txt +++ b/packages/flutter_tools/templates/app/linux.tmpl/flutter/CMakeLists.txt @@ -83,6 +83,7 @@ add_custom_command( ${FLUTTER_TOOL_ENVIRONMENT} "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.sh" linux-x64 ${CMAKE_BUILD_TYPE} + VERBATIM ) add_custom_target(flutter_assemble DEPENDS "${FLUTTER_LIBRARY}" diff --git a/packages/flutter_tools/templates/app/windows.tmpl/flutter/CMakeLists.txt b/packages/flutter_tools/templates/app/windows.tmpl/flutter/CMakeLists.txt index c7a8c7607d..744f08a938 100644 --- a/packages/flutter_tools/templates/app/windows.tmpl/flutter/CMakeLists.txt +++ b/packages/flutter_tools/templates/app/windows.tmpl/flutter/CMakeLists.txt @@ -91,6 +91,7 @@ add_custom_command( ${FLUTTER_TOOL_ENVIRONMENT} "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.bat" windows-x64 $ + VERBATIM ) add_custom_target(flutter_assemble DEPENDS "${FLUTTER_LIBRARY}" diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart index b524a736e9..1229ae7f08 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart @@ -364,17 +364,17 @@ clang: error: linker command failed with exit code 1 (use -v to see invocation) expect(configLines, containsAll([ 'file(TO_CMAKE_PATH "$_kTestFlutterRoot" FLUTTER_ROOT)', 'file(TO_CMAKE_PATH "${fileSystem.currentDirectory.path}" PROJECT_DIR)', - r' "DART_DEFINES=\"foo.bar%3D2,fizz.far%3D3\""', - r' "DART_OBFUSCATION=\"true\""', - r' "EXTRA_FRONT_END_OPTIONS=\"--enable-experiment%3Dnon-nullable\""', - r' "EXTRA_GEN_SNAPSHOT_OPTIONS=\"--enable-experiment%3Dnon-nullable\""', - r' "SPLIT_DEBUG_INFO=\"foo/\""', - r' "TRACK_WIDGET_CREATION=\"true\""', - r' "TREE_SHAKE_ICONS=\"true\""', - ' "FLUTTER_ROOT=\\"$_kTestFlutterRoot\\""', - ' "PROJECT_DIR=\\"${fileSystem.currentDirectory.path}\\""', - r' "FLUTTER_TARGET=\"lib/other.dart\""', - r' "BUNDLE_SKSL_PATH=\"foo/bar.sksl.json\""', + ' "DART_DEFINES=foo.bar%3D2,fizz.far%3D3"', + ' "DART_OBFUSCATION=true"', + ' "EXTRA_FRONT_END_OPTIONS=--enable-experiment%3Dnon-nullable"', + ' "EXTRA_GEN_SNAPSHOT_OPTIONS=--enable-experiment%3Dnon-nullable"', + ' "SPLIT_DEBUG_INFO=foo/"', + ' "TRACK_WIDGET_CREATION=true"', + ' "TREE_SHAKE_ICONS=true"', + ' "FLUTTER_ROOT=$_kTestFlutterRoot"', + ' "PROJECT_DIR=${fileSystem.currentDirectory.path}"', + ' "FLUTTER_TARGET=lib/other.dart"', + ' "BUNDLE_SKSL_PATH=foo/bar.sksl.json"', ])); }, overrides: { FileSystem: () => fileSystem, diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart index afd694c826..6be30f511c 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart @@ -315,17 +315,17 @@ C:\foo\windows\runner\main.cpp(17,1): error C2065: 'Baz': undeclared identifier expect(configLines, containsAll([ r'file(TO_CMAKE_PATH "C:\\flutter" FLUTTER_ROOT)', r'file(TO_CMAKE_PATH "C:\\" PROJECT_DIR)', - r' "DART_DEFINES=\"foo%3Da,bar%3Db\""', - r' "DART_OBFUSCATION=\"true\""', - r' "EXTRA_FRONT_END_OPTIONS=\"--enable-experiment%3Dnon-nullable\""', - r' "EXTRA_GEN_SNAPSHOT_OPTIONS=\"--enable-experiment%3Dnon-nullable\""', - r' "SPLIT_DEBUG_INFO=\"C:\\foo\\\""', - r' "TRACK_WIDGET_CREATION=\"true\""', - r' "TREE_SHAKE_ICONS=\"true\""', - r' "FLUTTER_ROOT=\"C:\\flutter\""', - r' "PROJECT_DIR=\"C:\\\""', - r' "FLUTTER_TARGET=\"lib\\other.dart\""', - r' "BUNDLE_SKSL_PATH=\"foo\\bar.sksl.json\""', + r' "DART_DEFINES=foo%3Da,bar%3Db"', + r' "DART_OBFUSCATION=true"', + r' "EXTRA_FRONT_END_OPTIONS=--enable-experiment%3Dnon-nullable"', + r' "EXTRA_GEN_SNAPSHOT_OPTIONS=--enable-experiment%3Dnon-nullable"', + r' "SPLIT_DEBUG_INFO=C:\\foo\\"', + r' "TRACK_WIDGET_CREATION=true"', + r' "TREE_SHAKE_ICONS=true"', + r' "FLUTTER_ROOT=C:\\flutter"', + r' "PROJECT_DIR=C:\\"', + r' "FLUTTER_TARGET=lib\\other.dart"', + r' "BUNDLE_SKSL_PATH=foo\\bar.sksl.json"', ])); }, overrides: { FileSystem: () => fileSystem, diff --git a/packages/flutter_tools/test/general.shard/migrations/cmake_project_migration_test.dart b/packages/flutter_tools/test/general.shard/migrations/cmake_project_migration_test.dart new file mode 100644 index 0000000000..14f5b93425 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/migrations/cmake_project_migration_test.dart @@ -0,0 +1,163 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/base/platform.dart'; +import 'package:flutter_tools/src/base/project_migrator.dart'; +import 'package:flutter_tools/src/base/terminal.dart'; +import 'package:flutter_tools/src/migrations/cmake_custom_command_migration.dart'; +import 'package:flutter_tools/src/project.dart'; +import 'package:meta/meta.dart'; +import 'package:mockito/mockito.dart'; + +import '../../src/common.dart'; + +void main () { + group('CMake project migration', () { + testWithoutContext('migrators succeed', () { + final FakeCmakeMigrator fakeCmakeMigrator = FakeCmakeMigrator(succeeds: true); + final ProjectMigration migration = ProjectMigration([fakeCmakeMigrator]); + expect(migration.run(), isTrue); + }); + + testWithoutContext('migrators fail', () { + final FakeCmakeMigrator fakeCmakeMigrator = FakeCmakeMigrator(succeeds: false); + final ProjectMigration migration = ProjectMigration([fakeCmakeMigrator]); + expect(migration.run(), isFalse); + }); + + group('migrate add_custom_command() to use VERBATIM', () { + MemoryFileSystem memoryFileSystem; + BufferLogger testLogger; + MockCmakeProject mockCmakeProject; + File managedCmakeFile; + + setUp(() { + memoryFileSystem = MemoryFileSystem.test(); + managedCmakeFile = memoryFileSystem.file('CMakeLists.txtx'); + + testLogger = BufferLogger( + terminal: AnsiTerminal( + stdio: null, + platform: const LocalPlatform(), + ), + outputPreferences: OutputPreferences.test(), + ); + + mockCmakeProject = MockCmakeProject(); + when(mockCmakeProject.managedCmakeFile).thenReturn(managedCmakeFile); + }); + + testWithoutContext('skipped if files are missing', () { + final CmakeCustomCommandMigration cmakeProjectMigration = CmakeCustomCommandMigration( + mockCmakeProject, + testLogger, + ); + expect(cmakeProjectMigration.migrate(), isTrue); + expect(managedCmakeFile.existsSync(), isFalse); + + expect(testLogger.traceText, contains('CMake project not found, skipping add_custom_command() VERBATIM migration')); + expect(testLogger.statusText, isEmpty); + }); + + testWithoutContext('skipped if nothing to migrate', () { + const String contents = 'Nothing to migrate'; + managedCmakeFile.writeAsStringSync(contents); + final DateTime projectLastModified = managedCmakeFile.lastModifiedSync(); + + final CmakeCustomCommandMigration cmakeProjectMigration = CmakeCustomCommandMigration( + mockCmakeProject, + testLogger, + ); + expect(cmakeProjectMigration.migrate(), isTrue); + + expect(managedCmakeFile.lastModifiedSync(), projectLastModified); + expect(managedCmakeFile.readAsStringSync(), contents); + + expect(testLogger.statusText, isEmpty); + }); + + testWithoutContext('skipped if already migrated', () { + const String contents = r''' +add_custom_command( + OUTPUT ${FLUTTER_LIBRARY} ${FLUTTER_LIBRARY_HEADERS} + ${CMAKE_CURRENT_BINARY_DIR}/_phony_ + COMMAND ${CMAKE_COMMAND} -E env + ${FLUTTER_TOOL_ENVIRONMENT} + "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.sh" + linux-x64 ${CMAKE_BUILD_TYPE} + VERBATIM +) +'''; + managedCmakeFile.writeAsStringSync(contents); + final DateTime projectLastModified = managedCmakeFile.lastModifiedSync(); + + final CmakeCustomCommandMigration cmakeProjectMigration = CmakeCustomCommandMigration( + mockCmakeProject, + testLogger, + ); + expect(cmakeProjectMigration.migrate(), isTrue); + + expect(managedCmakeFile.lastModifiedSync(), projectLastModified); + expect(managedCmakeFile.readAsStringSync(), contents); + + expect(testLogger.statusText, isEmpty); + }); + + testWithoutContext('is migrated to use VERBATIM', () { + managedCmakeFile.writeAsStringSync(r''' +add_custom_command( + OUTPUT ${FLUTTER_LIBRARY} ${FLUTTER_LIBRARY_HEADERS} + ${CMAKE_CURRENT_BINARY_DIR}/_phony_ + COMMAND ${CMAKE_COMMAND} -E env + ${FLUTTER_TOOL_ENVIRONMENT} + "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.sh" + linux-x64 ${CMAKE_BUILD_TYPE} +) +'''); + + final CmakeCustomCommandMigration cmakeProjectMigration = CmakeCustomCommandMigration( + mockCmakeProject, + testLogger, + ); + expect(cmakeProjectMigration.migrate(), isTrue); + + expect(managedCmakeFile.readAsStringSync(), r''' +add_custom_command( + OUTPUT ${FLUTTER_LIBRARY} ${FLUTTER_LIBRARY_HEADERS} + ${CMAKE_CURRENT_BINARY_DIR}/_phony_ + COMMAND ${CMAKE_COMMAND} -E env + ${FLUTTER_TOOL_ENVIRONMENT} + "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.sh" + linux-x64 ${CMAKE_BUILD_TYPE} + VERBATIM +) +'''); + + expect(testLogger.statusText, contains('add_custom_command() missing VERBATIM, updating.')); + }); + }); + }); +} + +class MockCmakeProject extends Mock implements CmakeBasedProject {} + +class FakeCmakeMigrator extends ProjectMigrator { + FakeCmakeMigrator({@required this.succeeds}) + : super(null); + + final bool succeeds; + + @override + bool migrate() { + return succeeds; + } + + @override + String migrateLine(String line) { + return line; + } +}