Skip to content

Conversation

@b-luk
Copy link
Contributor

@b-luk b-luk commented Oct 9, 2025

  • Move "gclient bootstrap" out of engine/README.md into a section in the "Setting up the Engine development environment" doc.
  • In the "gclient bootstrap" section, instruct Googlers to follow the RBE setup docs.
  • Update the "Additional Steps for Web Engine" to reflect the fact that the user's gclient file is copied from a template that contains a commented-out web engine entry.
  • Link to the "Engine Tool" docs instead of to "Compiling the engine" for the next steps.
  • In the RBE doc:
    • Update the gclient instructions to reflect that the gclient file may be copied from rbe.gclient.
    • Move the "gcloud" section into the "Getting started" parent section. The gcloud setup is required before any of the next steps.
    • Instead of suggested the "gcloud auth application-default login" command in case of an error, directly instruct the user to run this command. It's required for things to work.
    • Add a "Too many open files" section to the Troubleshooting parent section.

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

- Move "gclient bootstrap" out of engine/README.md into a section in
the "Setting up the Engine development environment" doc.
- In the "gclient bootstrap" section, instruct Googlers to follow the
RBE setup docs.
- Update the "Additional Steps for Web Engine" to reflect the fact that
the user's gclient file is copied from a template that contains a
commented-out web engine entry.
- Link to the "Engine Tool" docs instead of to "Compiling the engine"
for the next steps.
- In the RBE doc:
  - Update the gclient instructions to reflect that the gclient file
  may be copied from rbe.gclient.
  - Move the "gcloud" section into the "Getting started" parent section.
  The gcloud setup is required before any of the next steps.
  - Instead of suggested the "gcloud auth application-default login"
  command in case of an error, directly instruct the user to run this
  command. It's required for things to work.
  - Add a "Too many open files" section to the Troubleshooting
  parent section.
@b-luk b-luk requested a review from gaaclarke October 9, 2025 00:07
@github-actions github-actions bot added engine flutter/engine related. See also e: labels. team-engine Owned by Engine team d: docs/ flutter/flutter/docs, for contributors labels Oct 9, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request does a great job of centralizing and clarifying the engine setup documentation. The changes make the instructions easier to follow, especially for new contributors. I have a couple of minor suggestions to further improve the documentation's clarity and long-term maintainability.


### Add `et` to `PATH`

The "Engine Tool" called `et` is useful when working with the engine. It is located in the [`flutter/engine/src/flutter/bin`](https://github.com/flutter/flutter/tree/0c3359df8c8342c8907316488b1404a216f215b6/engine/src/flutter/bin) directory. Add this to your `$PATH` in your `.rc` file: e.g. on UNIX, using `export PATH=/path/to/flutter/engine/src/flutter/bin:$PATH`.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The link to the et tool's directory contains a hardcoded commit hash. This can become outdated and point to a non-existent path if the repository history is changed. It's better to link to the main branch to ensure the link always points to the latest version of the directory.

Suggested change
The "Engine Tool" called `et` is useful when working with the engine. It is located in the [`flutter/engine/src/flutter/bin`](https://github.com/flutter/flutter/tree/0c3359df8c8342c8907316488b1404a216f215b6/engine/src/flutter/bin) directory. Add this to your `$PATH` in your `.rc` file: e.g. on UNIX, using `export PATH=/path/to/flutter/engine/src/flutter/bin:$PATH`.
The "Engine Tool" called `et` is useful when working with the engine. It is located in the [`flutter/engine/src/flutter/bin`](https://github.com/flutter/flutter/tree/main/engine/src/flutter/bin) directory. Add this to your `$PATH` in your `.rc` file: e.g. on UNIX, using `export PATH=/path/to/flutter/engine/src/flutter/bin:$PATH`.

Comment on lines +162 to +163
Increase the maximum number of open files on your machine with the instructions
[here](go/building-chrome-mac#configure-your-mac-for-remote-execution).
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This go/ link is internal to Google and will appear as a broken link to the public on GitHub. While this document is targeted at Googlers, it's still in a public repository. For better accessibility and to avoid confusion, consider either inlining the instructions if they are simple, or replacing the go/ link with a publicly accessible one if available.

For example, you could replace the link with the actual instructions:

Increase the maximum number of open files on your machine. For example, you can add `ulimit -n 10240` to your shell's startup script (e.g., `~/.zshenv` or `~/.bash_profile`) and start a new shell session.
Suggested change
Increase the maximum number of open files on your machine with the instructions
[here](go/building-chrome-mac#configure-your-mac-for-remote-execution).
Increase the maximum number of open files on your machine. For example, you can add `ulimit -n 10240` to your shell's startup script (e.g., `~/.zshenv` or `~/.bash_profile`) and start a new shell session.

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Comment on lines +63 to +65
In the engine repo, all RBE builds must be initiated through the `et` tool whose
entrypoint is the script `//flutter/bin/et`. This is so that the local RBE
proxy is correctly initialized and shut down around invocations of `ninja`.
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that's true? I think ninja directly will use RBE too, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This paragraph is unchanged by this commit. It's just highlighted in this diff because I moved some lines around in the file.

It likely does have some incorrect information though:

  1. There is no more engine repo, since it's been moved inside the flutter repo.
  2. As far as I can tell, the et tool is located in //engine/src/flutter/bin, not //flutter/bin/et.

For your question about whether et is required for RBE builds, or whether using ninja directly uses RBE: I don't know enough about how things work to have an informed take on this. Can you suggest a more correct way to reword this?

Copy link
Member

Choose a reason for hiding this comment

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

no action required: I don't think that what is written is true. If it already was there and this is just moved I'm fine with just assuming it's true. It's not worth further investigation.

@b-luk b-luk requested a review from gaaclarke October 9, 2025 16:34
@b-luk b-luk added this pull request to the merge queue Oct 9, 2025
Merged via the queue into flutter:master with commit 597f07b Oct 10, 2025
184 checks passed
@b-luk b-luk deleted the enginedocs branch October 10, 2025 01:09
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 15, 2025
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
- Move "gclient bootstrap" out of engine/README.md into a section in the
"Setting up the Engine development environment" doc.
- In the "gclient bootstrap" section, instruct Googlers to follow the
RBE setup docs.
- Update the "Additional Steps for Web Engine" to reflect the fact that
the user's gclient file is copied from a template that contains a
commented-out web engine entry.
- Link to the "Engine Tool" docs instead of to "Compiling the engine"
for the next steps.
- In the RBE doc:
- Update the gclient instructions to reflect that the gclient file may
be copied from rbe.gclient.
- Move the "gcloud" section into the "Getting started" parent section.
The gcloud setup is required before any of the next steps.
- Instead of suggested the "gcloud auth application-default login"
command in case of an error, directly instruct the user to run this
command. It's required for things to work.
- Add a "Too many open files" section to the Troubleshooting parent
section.

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

*Replace this paragraph with a description of what this PR is changing
or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one
issue. An issue is not required if the PR fixes something trivial like a
typo.*

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

d: docs/ flutter/flutter/docs, for contributors engine flutter/engine related. See also e: labels. team-engine Owned by Engine team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants