Opt into CMake policy CMP0135 #125502
Opt into CMake policy CMP0135 #125502
Conversation
yaakovschectman
added
tool
yaakovschectman
requested review from
cbracken,
exaby73,
jonahwilliams,
jpnurmi,
loic-sharma and
stuartmorgan-g
There was a problem hiding this comment.
Should we migrate all the affected CMakeLists.txt in this repo? Otherwise flutter run will modify these files.
There was a problem hiding this comment.
Can you explain what modifications you expect flutter run to make to which files?
There was a problem hiding this comment.
Your change causes flutter run to migrate an app's ./windows/CMakeLists.txt file if it uses the old CMake policy. This repo has many apps that use the old CMake policy: https://github.com/search?q=repo%3Aflutter%2Fflutter+cmake_policy%28SET+CMP0063+NEW%29+path%3A**%2FCMakeLists.txt&type=code
There was a problem hiding this comment.
That link is not showing any results for me, but if you mean to check in the migration applied to existing/example apps in our repo, then good call
packages/flutter_tools/lib/src/migrations/cmake_policy_migration.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/lib/src/migrations/cmake_policy_migration.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I'm a little concerned that this needle isn't conservative enough. For example, imagine I accidentally remove the whitespace before the cmake_policy in an already migrated CMake file:
if(${CMAKE_VERSION} VERSION_LESS "3.24")
- cmake_policy(SET CMP0063 NEW)
+cmake_policy(SET CMP0063 NEW)
else()
cmake_policy(SET CMP0135 NEW)
endif()We should make the migration conservative to reduce the likelihood of us corrupting files. I would consider only applying the migration if we get a hit on this entire expected block:
# Explicitly opt in to modern CMake behaviors to avoid warnings with recent
# versions of CMake.
cmake_policy(SET CMP0063 NEW)
Notice the empty line at the end.
There was a problem hiding this comment.
The comments above the statement are not present in the CMakeList.txts of all first party packages. E.g. url_launcher_windows which I have been using to repro the problem does not contain those comments.
There was a problem hiding this comment.
It looks like the template changed substantially after cmake_policy(SET CMP0063 NEW) was introduced. Two potential options:
- We update this migration to recognize two patterns: the old template used to create first party packages and the current template with comments.
- We update this migration to recognize just the current template with comments.
I'd lean towards doing the first option if feasible.
There was a problem hiding this comment.
I think this is no longer relevant. As of flutter/packages#3828, we will not plan for the same kind of migration
There was a problem hiding this comment.
I'm confused. Why don't we need a migration if we modify the app template? (I'd understand why we wouldn't need a migration if we were only updating the plugin template)
There was a problem hiding this comment.
@stuartmorgan suggested updating the template going forward without needing a migration on the linked PR. Do you think you can explain the rationale here for clarity?
There was a problem hiding this comment.
The way I thought this would work when I recommended the migrator was that we could add the new policy setting at the top level, and it would fix everything, so we needed a migrator so that everyone seeing warnings would get fixes.
Since it turns out that this apparently doesn't work (per other comments here), fixing the current set of warnings has to be done in each plugin that is triggering them, and preventing this warning in new plugins is fixed by updating the plugin template.
Even though we can't fix the original issue in the app template, we should update the app template to make it better by setting a VERSION-based cmake_policy, since that is:
- recommended by CMake docs (I missed that when doing the initial policy setting),
- more comprehensive, so that new apps will have all the new policy behaviors, not just these two.
That's not a pressing concern though, so there's no need to migrate existing projects.
There was a problem hiding this comment.
Gotcha, thanks for the additional information!
packages/flutter_tools/lib/src/migrations/cmake_policy_migration.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
This logic looks wrong; why are you removing 63 for new versions? Doesn't that just trade one warning for another one?
Is there a reason not to use the simpler cmake-version-based variant that opts into all new behaviors, as I suggested in the issue?
There was a problem hiding this comment.
Am I understanding you correctly that you would want a statement like
cmake_policy(VERSION 2.4...3.25)
as per the cmake_policy docs?
If so, the above statement still produces the CMP0135 warning message.
There was a problem hiding this comment.
Well, this is odd. Not only does that not work in the top-level project, but setting it directly (via CMP0135) in the top level project also doesn't work. I have to set it in the plugin's CMakeLists.txt to eliminate the warning locally (VERSION also works in that context). I thought the setting would be inherited down the build hierarchy by default, and the new scopes would only allow locally overriding what was inherited.
So either I'm missing something about how CMake works here (which is certainly possible), or it's not actually possible to fix this except at the plugin level. (In which case we should add this to the plugin example template, since we use downloads there.)
yaakovschectman
force-pushed
the
cmp0135
branch
from
2d1dced to
0260978
Compare
flutter-dashboard
bot
added
d: api docs
Manual roll Flutter from 66fa4c5 to 828a040 (79 revisions) Manual roll requested by [email protected] flutter/flutter@66fa4c5...828a040 2023-05-01 [email protected] Roll Flutter Engine from 666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818) 2023-05-01 [email protected] Revert "Add migrator to upgrade gradle version when conflict with And… (flutter/flutter#125813) 2023-05-01 [email protected] Roll pub packages (flutter/flutter#125801) 2023-05-01 [email protected] [tools] fix `expect` calls in `FakeCommand` (flutter/flutter#125783) 2023-05-01 [email protected] Roll Packages from 7e3f5da to de6131d (41 revisions) (flutter/flutter#125811) 2023-05-01 [email protected] Introduce `TabBar.tabAlignment` (flutter/flutter#125036) 2023-05-01 [email protected] Roll Flutter Engine from b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805) 2023-05-01 [email protected] add support to customize Slider interacivity (flutter/flutter#121483) 2023-05-01 [email protected] Roll Flutter Engine from b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800) 2023-05-01 [email protected] Roll Flutter Engine from 605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795) 2023-05-01 [email protected] Roll Flutter Engine from bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793) 2023-05-01 [email protected] Roll Flutter Engine from 2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791) 2023-05-01 [email protected] Roll Flutter Engine from 30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789) 2023-05-01 [email protected] Roll Flutter Engine from d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787) 2023-05-01 [email protected] [tools] Apply Android Studio version detection logic to explicitly configured installation directory (`flutter config --android-studio-dir`) (flutter/flutter#125596) 2023-04-30 [email protected] Roll Flutter Engine from f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776) 2023-04-30 [email protected] Roll Flutter Engine from c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773) 2023-04-30 [email protected] Roll Flutter Engine from e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762) 2023-04-30 [email protected] Roll Flutter Engine from 1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758) 2023-04-30 [email protected] Roll Flutter Engine from 7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757) 2023-04-29 [email protected] Roll Flutter Engine from 8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750) 2023-04-29 [email protected] Roll Flutter Engine from 900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748) 2023-04-29 [email protected] Roll Flutter Engine from c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747) 2023-04-29 [email protected] Roll Flutter Engine from 0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746) 2023-04-29 [email protected] Roll Flutter Engine from 68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736) 2023-04-29 [email protected] Roll Flutter Engine from 0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735) 2023-04-29 [email protected] Fix crasher in DragableScrollableSheet when controller is animating and switching widgets (flutter/flutter#125721) 2023-04-29 [email protected] Roll Flutter Engine from 8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734) 2023-04-29 [email protected] Roll Flutter Engine from 788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731) 2023-04-29 [email protected] Roll Flutter Engine from 89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729) 2023-04-29 [email protected] Roll Flutter Engine from 3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725) 2023-04-29 [email protected] Roll Flutter Engine from 1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722) 2023-04-29 [email protected] fix package template create platform folders (flutter/flutter#125292) 2023-04-28 [email protected] Sliver Cross Axis Group (flutter/flutter#123862) 2023-04-28 [email protected] Roll Flutter Engine from 2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718) 2023-04-28 [email protected] Remove bringup from new_gallery_skia_ios__transition_perf (flutter/flutter#125715) 2023-04-28 [email protected] Roll Flutter Engine from 98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714) 2023-04-28 [email protected] Opt into CMake policy CMP0135 (flutter/flutter#125502) 2023-04-28 [email protected] Add a channel to query the engine keyboard state (flutter/flutter#122885) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125698) 2023-04-28 [email protected] `Checkbox.fillColor` should be applied to checkbox's background color when it is unchecked. (flutter/flutter#125643) 2023-04-28 [email protected] Add back one Skia test on iOS (flutter/flutter#125663) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125447) 2023-04-28 [email protected] Nit: grammar in documentation (flutter/flutter#125462) ...
) Manual roll Flutter from 66fa4c5 to 828a040 (79 revisions) Manual roll requested by [email protected] flutter/flutter@66fa4c5...828a040 2023-05-01 [email protected] Roll Flutter Engine from 666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818) 2023-05-01 [email protected] Revert "Add migrator to upgrade gradle version when conflict with And… (flutter/flutter#125813) 2023-05-01 [email protected] Roll pub packages (flutter/flutter#125801) 2023-05-01 [email protected] [tools] fix `expect` calls in `FakeCommand` (flutter/flutter#125783) 2023-05-01 [email protected] Roll Packages from 7e3f5da to de6131d (41 revisions) (flutter/flutter#125811) 2023-05-01 [email protected] Introduce `TabBar.tabAlignment` (flutter/flutter#125036) 2023-05-01 [email protected] Roll Flutter Engine from b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805) 2023-05-01 [email protected] add support to customize Slider interacivity (flutter/flutter#121483) 2023-05-01 [email protected] Roll Flutter Engine from b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800) 2023-05-01 [email protected] Roll Flutter Engine from 605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795) 2023-05-01 [email protected] Roll Flutter Engine from bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793) 2023-05-01 [email protected] Roll Flutter Engine from 2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791) 2023-05-01 [email protected] Roll Flutter Engine from 30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789) 2023-05-01 [email protected] Roll Flutter Engine from d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787) 2023-05-01 [email protected] [tools] Apply Android Studio version detection logic to explicitly configured installation directory (`flutter config --android-studio-dir`) (flutter/flutter#125596) 2023-04-30 [email protected] Roll Flutter Engine from f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776) 2023-04-30 [email protected] Roll Flutter Engine from c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773) 2023-04-30 [email protected] Roll Flutter Engine from e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762) 2023-04-30 [email protected] Roll Flutter Engine from 1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758) 2023-04-30 [email protected] Roll Flutter Engine from 7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757) 2023-04-29 [email protected] Roll Flutter Engine from 8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750) 2023-04-29 [email protected] Roll Flutter Engine from 900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748) 2023-04-29 [email protected] Roll Flutter Engine from c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747) 2023-04-29 [email protected] Roll Flutter Engine from 0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746) 2023-04-29 [email protected] Roll Flutter Engine from 68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736) 2023-04-29 [email protected] Roll Flutter Engine from 0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735) 2023-04-29 [email protected] Fix crasher in DragableScrollableSheet when controller is animating and switching widgets (flutter/flutter#125721) 2023-04-29 [email protected] Roll Flutter Engine from 8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734) 2023-04-29 [email protected] Roll Flutter Engine from 788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731) 2023-04-29 [email protected] Roll Flutter Engine from 89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729) 2023-04-29 [email protected] Roll Flutter Engine from 3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725) 2023-04-29 [email protected] Roll Flutter Engine from 1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722) 2023-04-29 [email protected] fix package template create platform folders (flutter/flutter#125292) 2023-04-28 [email protected] Sliver Cross Axis Group (flutter/flutter#123862) 2023-04-28 [email protected] Roll Flutter Engine from 2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718) 2023-04-28 [email protected] Remove bringup from new_gallery_skia_ios__transition_perf (flutter/flutter#125715) 2023-04-28 [email protected] Roll Flutter Engine from 98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714) 2023-04-28 [email protected] Opt into CMake policy CMP0135 (flutter/flutter#125502) 2023-04-28 [email protected] Add a channel to query the engine keyboard state (flutter/flutter#122885) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125698) 2023-04-28 [email protected] `Checkbox.fillColor` should be applied to checkbox's background color when it is unchecked. (flutter/flutter#125643) 2023-04-28 [email protected] Add back one Skia test on iOS (flutter/flutter#125663) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125447) 2023-04-28 [email protected] Nit: grammar in documentation (flutter/flutter#125462) ...
3 participants
Update the windows app template and migration to useCMP0135when cmake version is >= 3.24.Update app templates' and examples' CMakeLists.txt to use
cmake_policy(VERSION. flutter/packages#3828 should obviate the need for a migration.Addresses #116866
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.