[Windows] Flow version information to the build output #106145
Conversation
loic-sharma
requested review from
cbracken,
christopherfujino,
gspencergoog and
stuartmorgan-g
flutter-dashboard
bot
added
d: api docs
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
There was a problem hiding this comment.
This file was re-generated using flutter create --platforms=windows .
loic-sharma
force-pushed
the
flow-version-to-exe
branch
from
b9e05b7 to
4056573
Compare
packages/flutter_tools/templates/app_shared/windows.tmpl/runner/Runner.rc.tmpl
Outdated
Show resolved
Hide resolved
loic-sharma
force-pushed
the
flow-version-to-exe
branch
from
33a9b2c to
851ae10
Compare
|
/cc @GaryQian as this is a candidate for the upcoming |
packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Regarding migration tool, is this file ever modified by the developer?
If this file is untouched by the developer, the migrate tool will handle it cleanly.
If it is user-modified, the tool will attempt to merge. Most small changes can be merged easily, but if there are conflicts, then they need to be resolved manually.
In general, the changes made in this PR don't seem to be in user-owned files, so migrate tool should be able to handle silently.
There was a problem hiding this comment.
Everything in runner/ is user-owned. Developers who have released updates to apps on Windows have almost certainly edited parts of this file that will conflict.
There was a problem hiding this comment.
Yep - this is all user-owned, and I'd expect potentially extensive additions to the rc file, but I suspect there are ways we could programmatically check that the migration of the version info is safe.
All of the following would need to be true:
- The patch applies cleanly
VERSION_AS_NUMBERandVERSION_AS_STRINGdon't exist outside the patched area and theVS_VERSION_INFOblock.- The
VS_VERSION_INFOblock is unchanged in any way, other than the expected templated strings.
If any of the above is not true, we couldn't apply the migration, and instead should emit a pointer to what the user can (optionally) do by hand.
@stuartmorgan are there cases you can think of where the above are true where this would break things?
There was a problem hiding this comment.
I don't think we can plausibly validate 2; it could be set in the project itself, or in an included file.
I think the sane options would be:
- Just do the migration if it doesn't conflict. For some people things will just start working (the updated PR won't conflict with nearly as many people, so it could even be most people), for some it'll be a no-op because they've replaced the use of the variables with hard-coded values, and for a very small number who plumbed this into their build system themselves it'll replace their workaround with something that works automatically (which seems pretty low-risk to me).
- Don't ever try to auto-migrate; instead just warn if the new code can't be found and suggest they update manually (with instructions).
There was a problem hiding this comment.
Here is the design doc on the upcoming flutter migrate command: flutter.dev/go/flutter-migrate
From my understanding, flutter migrate does option 2: flutter upgrade and flutter doctor will warn if your project needs to be updated, and the user will need to run flutter migrate to apply the migration.
There was a problem hiding this comment.
Ah that makes sense; at least this is a user-initiated action, so assuming it emits appropriate messages informing the user of what we're about to attempt and whether or not it was successful, it should be pretty unsurprising to the user. If they've got their app under source control (and hopefully they do), they can inspect the diffs themselves after the action is complete and elect not to commit if they don't like what they see.
loic-sharma
force-pushed
the
flow-version-to-exe
branch
from
a04da09 to
07a707a
Compare
loic-sharma
force-pushed
the
flow-version-to-exe
branch
from
fbd84b9 to
7700d91
Compare
christopherfujino
left a comment
christopherfujino
left a comment
There was a problem hiding this comment.
LGTM modulo stuart's nits
…hout --build-name
|
FYI I added one last commit that:
|
Previously developers had to edit their `Runner.rc` file to update their executable's version information. Now, version information will automatically be set from `flutter build`'s arguments or the `pubspec.yaml` file for new projects. Addresses flutter#73652
5 participants
Background
Currently developers have to edit their
Runner.rcfile to update their executable's version information. Now, version information will automatically be set fromflutter build's arguments or thepubspec.yamlfile.Addresses #73652
How it works
The version information flows to the build output using these steps:
Runner.rcfile uses these preprocessor definitions to apply the version informationUpgrading an existing project
Customers that already have a Windows Flutter project will need to update their projects for the version information to flow to the build output. This should be feasible using the upcoming
flutter migratecommand. Or, developers can update their projects manually:windows/runner/CMakeLists.txtandwindows/runner/Runner.rcfilesflutter create --platforms=windows .Customers that choose not to upgrade their project won't be affected by this change.
This PR also updates all Flutter Windows projects in the
flutterrepository.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.