-
Notifications
You must be signed in to change notification settings - Fork 26.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Windows] Add version info migration #123414
Merged
auto-submit
merged 6 commits into
flutter:master
from
loic-sharma:windows_version_migration
Mar 29, 2023
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
77dbffe
[Windows] Add version info migration
loic-sharma 32b10a3
Support both LF and CRLF
loic-sharma 76ba09f
Format
loic-sharma a1b0818
Switch to multi-line strings
loic-sharma 8ff4146
Feedback
loic-sharma 659486b
Potentially address feedback
loic-sharma File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
147 changes: 147 additions & 0 deletions
147
packages/flutter_tools/lib/src/windows/migrations/version_migration.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
// 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/project_migrator.dart'; | ||
import '../../cmake_project.dart'; | ||
|
||
const String _cmakeFileBefore = r''' | ||
# Apply the standard set of build settings. This can be removed for applications | ||
# that need different build settings. | ||
apply_standard_settings(${BINARY_NAME}) | ||
|
||
# Disable Windows macros that collide with C++ standard library functions. | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "NOMINMAX") | ||
'''; | ||
const String _cmakeFileAfter = r''' | ||
# Apply the standard set of build settings. This can be removed for applications | ||
# that need different build settings. | ||
apply_standard_settings(${BINARY_NAME}) | ||
|
||
# Add preprocessor definitions for the build version. | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "FLUTTER_VERSION=\"${FLUTTER_VERSION}\"") | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "FLUTTER_VERSION_MAJOR=${FLUTTER_VERSION_MAJOR}") | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "FLUTTER_VERSION_MINOR=${FLUTTER_VERSION_MINOR}") | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "FLUTTER_VERSION_PATCH=${FLUTTER_VERSION_PATCH}") | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "FLUTTER_VERSION_BUILD=${FLUTTER_VERSION_BUILD}") | ||
|
||
# Disable Windows macros that collide with C++ standard library functions. | ||
target_compile_definitions(${BINARY_NAME} PRIVATE "NOMINMAX") | ||
'''; | ||
|
||
const String _resourceFileBefore = ''' | ||
#ifdef FLUTTER_BUILD_NUMBER | ||
#define VERSION_AS_NUMBER FLUTTER_BUILD_NUMBER | ||
#else | ||
#define VERSION_AS_NUMBER 1,0,0 | ||
#endif | ||
|
||
#ifdef FLUTTER_BUILD_NAME | ||
#define VERSION_AS_STRING #FLUTTER_BUILD_NAME | ||
#else | ||
#define VERSION_AS_STRING "1.0.0" | ||
#endif | ||
'''; | ||
const String _resourceFileAfter = ''' | ||
#if defined(FLUTTER_VERSION_MAJOR) && defined(FLUTTER_VERSION_MINOR) && defined(FLUTTER_VERSION_PATCH) && defined(FLUTTER_VERSION_BUILD) | ||
#define VERSION_AS_NUMBER FLUTTER_VERSION_MAJOR,FLUTTER_VERSION_MINOR,FLUTTER_VERSION_PATCH,FLUTTER_VERSION_BUILD | ||
#else | ||
#define VERSION_AS_NUMBER 1,0,0,0 | ||
#endif | ||
|
||
#if defined(FLUTTER_VERSION) | ||
#define VERSION_AS_STRING FLUTTER_VERSION | ||
#else | ||
#define VERSION_AS_STRING "1.0.0" | ||
#endif | ||
'''; | ||
|
||
/// Migrates Windows apps to set Flutter's version information. | ||
/// | ||
/// See https://github.com/flutter/flutter/issues/73652. | ||
class VersionMigration extends ProjectMigrator { | ||
VersionMigration(WindowsProject project, super.logger) | ||
: _cmakeFile = project.runnerCmakeFile, _resourceFile = project.runnerResourceFile; | ||
|
||
final File _cmakeFile; | ||
final File _resourceFile; | ||
|
||
@override | ||
void migrate() { | ||
// Skip this migration if the affected files do not exist. This indicates | ||
// the app has done non-trivial changes to its runner and this migration | ||
// might not work as expected if applied. | ||
if (!_cmakeFile.existsSync()) { | ||
logger.printTrace(''' | ||
windows/runner/CMakeLists.txt file not found, skipping version migration. | ||
|
||
This indicates non-trivial changes have been made to the Windows runner in the | ||
"windows" folder. If needed, you can reset the Windows runner by deleting the | ||
"windows" folder and then using the "flutter create --platforms=windows ." command. | ||
'''); | ||
return; | ||
} | ||
|
||
if (!_resourceFile.existsSync()) { | ||
logger.printTrace(''' | ||
windows/runner/Runner.rc file not found, skipping version migration. | ||
|
||
This indicates non-trivial changes have been made to the Windows runner in the | ||
"windows" folder. If needed, you can reset the Windows runner by deleting the | ||
"windows" folder and then using the "flutter create --platforms=windows ." command. | ||
'''); | ||
return; | ||
} | ||
|
||
// Migrate the windows/runner/CMakeLists.txt file. | ||
final String originalCmakeContents = _cmakeFile.readAsStringSync(); | ||
final String newCmakeContents = _replaceFirst( | ||
originalCmakeContents, | ||
_cmakeFileBefore, | ||
_cmakeFileAfter, | ||
); | ||
if (originalCmakeContents != newCmakeContents) { | ||
logger.printStatus('windows/runner/CMakeLists.txt does not define version information, updating.'); | ||
_cmakeFile.writeAsStringSync(newCmakeContents); | ||
} | ||
|
||
// Migrate the windows/runner/Runner.rc file. | ||
final String originalResourceFileContents = _resourceFile.readAsStringSync(); | ||
final String newResourceFileContents = _replaceFirst( | ||
originalResourceFileContents, | ||
_resourceFileBefore, | ||
_resourceFileAfter, | ||
); | ||
if (originalResourceFileContents != newResourceFileContents) { | ||
logger.printStatus( | ||
'windows/runner/Runner.rc does not define use Flutter version information, updating.', | ||
); | ||
_resourceFile.writeAsStringSync(newResourceFileContents); | ||
} | ||
} | ||
} | ||
|
||
/// Creates a new string with the first occurrence of [before] replaced by | ||
/// [after]. | ||
/// | ||
/// If the [originalContents] uses CRLF line endings, the [before] and [after] | ||
/// will be converted to CRLF line endings before the replacement is made. | ||
/// This is necessary for users that have git autocrlf enabled. | ||
/// | ||
/// Example: | ||
/// ```dart | ||
/// 'a\n'.replaceFirst('a\n', 'b\n'); // 'b\n' | ||
/// 'a\r\n'.replaceFirst('a\n', 'b\n'); // 'b\r\n' | ||
/// ``` | ||
String _replaceFirst(String originalContents, String before, String after) { | ||
final String result = originalContents.replaceFirst(before, after); | ||
if (result != originalContents) { | ||
return result; | ||
} | ||
|
||
final String beforeCrlf = before.replaceAll('\n', '\r\n'); | ||
final String afterCrlf = after.replaceAll('\n', '\r\n'); | ||
|
||
return originalContents.replaceFirst(beforeCrlf, afterCrlf); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to clarify, we only want to do any migrations if BOTH of these files exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I'd like to make this migration conservative and only run on projects that are clearly safe. If one of these files is missing, it indicates the app has done non-trivial changes to its runner.
That said, this migration should be safe even if one file is missing: each file's changes are backwards compatible and would work even if the other file's migration wasn't applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a docs page we can point people to when this fails, so they can do it by hand if desired? If so, in addition, log something like "For more information see: URL".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some more information to the trace to make it more actionable. This failure scenario seems unlikely (it wouldn't be unreasonable for
flutter build windows
to break if the runner's CMakeLists.txt file was missing), so I don't think it warrants a dedicated docs page.