Skip to content
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

Move the Linux runner into a subdirectory #153812

Merged
merged 8 commits into from
Sep 25, 2024

Conversation

robert-ancell
Copy link
Contributor

Fixes #60213

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 21, 2024
@robert-ancell
Copy link
Contributor Author

I'm trying to work out what is failing in the plugin_test_linux - any hints as to how to run this locally would help!

@christopherfujino
Copy link
Member

The failure I'm seeing on Linux plugin_test_linux is:

[2024-08-20 21:55:02.546347] [STDOUT] stderr: [   +6 ms] CMake Error at flutter/generated_plugins.cmake:16 (target_link_libraries):
[2024-08-20 21:55:02.546404] [STDOUT] stderr: [        ]   Attempt to add link library "plugintest_plugin" to target
[2024-08-20 21:55:02.546422] [STDOUT] stderr: [        ]   "plugintest_example" which is not built in this directory.
[2024-08-20 21:55:02.546483] [STDOUT] stderr: [        ]   This is allowed only when policy CMP0079 is set to NEW.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8739022238067392529/+/u/run_plugin_test_linux/stdout

@robert-ancell
Copy link
Contributor Author

It should be good now with the CMake fixes for plugins. I think the Windows test failure is not related from reading the log.

The CMake code is the bit I am most unsure about, especially which parts go in the root CMakeLists.txt and which goes in runner/CMakeLists.txt. I tried to copy as much from the Windows implementation that I could.

@loic-sharma loic-sharma self-requested a review September 11, 2024 19:35
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me, but I'm a total CMake novice. @stuartmorgan will be able to give a much better review :)

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I completely lost track of this review somehow. Mostly looks good, but the version change and mismatch are odd.

What's the plan for updating docs? We have docs that refer to these file paths but if we just change all of them we'll have docs that are wrong for everyone with existing projects. Do we list both (forever, in each location)? But we can't auto-migrate, because they are user-editable files.

This is unfortunately a change that would have been much easier if I'd done it shortly after filing, instead of letting it stable support go out without fixing it :(

@stuartmorgan
Copy link
Contributor

test-exempt: code refactor with no semantic change

@robert-ancell
Copy link
Contributor Author

I propose we update the docs to the new location, with a footnote about it having been different in the past if required.

I'm not familiar with the auto-migration code but this should be possible to do, shouldn't it? If a developer has not modified the original files then we could move them without any issues.

@robert-ancell
Copy link
Contributor Author

For context - I was motivated to fix this due to the new FlApplication class - if we're going to change the template we might as well do this migration at the same time.

@robert-ancell
Copy link
Contributor Author

@stuartmorgan anything else required before landing this?

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM stamp from a Japanese personal seal

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, LGTM

Is there an issue tracking the docs audit/update that will need to happen when this reaches stable?

@robert-ancell robert-ancell merged commit 8925e1f into flutter:master Sep 25, 2024
130 checks passed
@robert-ancell robert-ancell deleted the linux-runner-dir branch September 25, 2024 23:52
@robert-ancell
Copy link
Contributor Author

The only reference to main.cc and my_application.cc on the website is in https://docs.flutter.dev/platform-integration/platform-channels. Do you think there's anywhere else this should be mentioned?

@stuartmorgan
Copy link
Contributor

I figured there would be references to some part of the runner code or build files in the general docs about building an app too, but if it's just that one page that's easy!

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2024
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2024
LouiseHsu pushed a commit to flutter/packages that referenced this pull request Sep 26, 2024
flutter/flutter@538e742...fa402c8

2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
6328a0597b68 to 9e6133e8d906 (1 revision) (flutter/flutter#155762)
2024-09-26 engine-flutter-autoroll@skia.org Roll Packages from
7da2374 to f38b780 (2 revisions) (flutter/flutter#155760)
2024-09-26 49699333+dependabot[bot]@users.noreply.github.com Bump
codecov/codecov-action from 4.4.1 to 4.5.0 (flutter/flutter#150229)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
08f935236e45 to 6328a0597b68 (1 revision) (flutter/flutter#155750)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
3a520a2a4399 to 08f935236e45 (1 revision) (flutter/flutter#155748)
2024-09-26 kustermann@google.com Reland "[flutter_tools] Cleanup of
native asset related code (removes around 50% of the native asset
related code) (#155430)" (flutter/flutter#155745)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
896208ee5828 to 3a520a2a4399 (1 revision) (flutter/flutter#155744)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
fc6c85292b57 to 896208ee5828 (2 revisions) (flutter/flutter#155743)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
3719454a879f to fc6c85292b57 (1 revision) (flutter/flutter#155738)
2024-09-26 captainsikandar47@gmail.com added ability to configure shadow
in banner (flutter/flutter#155296)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
d4850c1ae648 to 3719454a879f (1 revision) (flutter/flutter#155736)
2024-09-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from
d6d5fdba6ae1 to d4850c1ae648 (18 revisions) (flutter/flutter#155733)
2024-09-25 robert.ancell@canonical.com Move the Linux runner into a
subdirectory (flutter/flutter#153812)
2024-09-25 49699333+dependabot[bot]@users.noreply.github.com Bump
actions/checkout from 4.1.7 to 4.2.0 (flutter/flutter#155711)
2024-09-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"[flutter_tools] Cleanup of native asset related code (removes around
50% of the native asset related code) (#155430)"
(flutter/flutter#155713)
2024-09-25 kustermann@google.com [flutter_tools] Cleanup of native asset
related code (removes around 50% of the native asset related code)
(flutter/flutter#155430)
2024-09-25 mohellebiabdessalem@gmail.com reduce warnings inside
flutter.groovy file #2 (flutter/flutter#155628)
2024-09-25 43054281+camsim99@users.noreply.github.com [Android] Update
`SystemUiMode` and `setSystemChromeEnabledSystemUIMode ` docs to note
targeting Android 15+ change (flutter/flutter#153466)
2024-09-25 christopherfujino@gmail.com mark linux packages autoroller
bringup: true (flutter/flutter#155705)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC louisehsu@google.com,stuartmorgan@google.com on the revert to
ensure that a human
is aware of the problem.

To file a bug in Packages:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
sfshaza2 pushed a commit to flutter/website that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Linux desktop runner source to a runner/ subfolder
5 participants