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

[monorepo] split out build-only web engine into its own builder #57178

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Dec 13, 2024

@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 platform-web Code specifically for the web engine label Dec 13, 2024
@jtmcdole
Copy link
Contributor

This and some ET problems?

00:00 �[32m+0�[0m�[31m -1�[0m: test smoke test -- this test SHOULD FAIL �[1m�[31m[E]�[0m�[0m

  Expected: true
    Actual: <false>


  Expected: true
    Actual: <false>
Invalid argument (name): Expected to start with a valid platform name (i.e. linux/) or "ci/": "web_build/artifacts"
#0      mangleConfigName (package:engine_tool/src/build_utils.dart:57:3)

@eyebrowsoffire
Copy link
Contributor

I'm confused by this change. Does this mean we are going to actually build all the artifacts twice in every presubmit?

@yjbanov
Copy link
Contributor Author

yjbanov commented Dec 13, 2024

@eyebrowsoffire

I'm confused by this change. Does this mean we are going to actually build all the artifacts twice in every presubmit?

Unfortunately for now, yes. The good news is that it shouldn't be too much overhead. Currently, linux_web_engine performs the following work:

  • Build:
    • The engine build itself - 13 min (but due to drone overhead it currently costs 20 min).
    • Engine tests - 8 x 7 min = 56 min
  • Test:
    • Chrome: 2 x 25 min = 50 min
    • Safari: 12 min
    • Firefox: 12 min

So that's 20 + 56 + 50 + 12 + 12 = 150 min worth of work total.

The new linux_web_engine_build builder duplicates just the 13 min (we don't need drones, so there's no extra 7 min overhead), and it moves the artifact upload work to itself so there's no duplication there.

So overall, I do not expect more than 15% overhead total. However, it will remove massive amounts of overhead from the merge queue by reducing the amount of work from 150 min to something <20min.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[monorepo] need build-only builder for the web engine
3 participants