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

Stale data on bots is interfering with the GN step #147651

Closed
gaaclarke opened this issue May 1, 2024 · 9 comments
Closed

Stale data on bots is interfering with the GN step #147651

gaaclarke opened this issue May 1, 2024 · 9 comments
Labels
P1 High-priority issues at the top of the work list team-infra Owned by Infrastructure team

Comments

@gaaclarke
Copy link
Member

The dart roll is broken. It appears that old data is still around at build time which interferes with the GN step. The step called clobber build output is printing out the following warning which may be related: WARNING: Failed to find /Volumes/Work/s/w/ir/cache/builder/src/out during rmtree. Ignoring.

My theory is that since the clobber is failing, we have stale data around which is causing the GN problems.

example

Here we end up with 2 versions of icu, one where icu currently exists and one where it used to exist:

https://github.com/flutter/engine/runs/24466203369

Using prebuilt Dart SDK binary. If you are editing Dart sources and wish to compile the Dart SDK, set `--no-prebuilt-dart-sdk`.
Generating GN files in: out/ci/ios_debug_sim_clang_tidy
ERROR at //third_party/icu/config.gni:32:42: Duplicate build argument declaration.
    icu_copy_icudata_to_root_build_dir = true
                                         ^---
Here you're declaring an argument that was already declared elsewhere.
You can only declare each argument once in the entire build so there is one
canonical place for documentation and the default value. Either move this
argument to the build config file (for visibility everywhere) or to a .gni file
that you "import" from the files where you need it (preferred).
See //flutter/third_party/icu/config.gni:32:42: Previous declaration.
    icu_copy_icudata_to_root_build_dir = true
                                         ^---
See also "gn help buildargs" for more on how build arguments work.
See //third_party/icu/BUILD.gn:7:1: whence it was imported.
import("config.gni")
^------------------
See //flutter/third_party/harfbuzz/BUILD.gn:94:5: which caused the file to be included.
    "//third_party/icu:icuuc",
    ^------------------------
@gaaclarke gaaclarke added the team-infra Owned by Infrastructure team label May 1, 2024
@gaaclarke
Copy link
Member Author

Here is a successful run from yesterday, it doesn't print out that warning at the clobber step: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20Engine%20Drone/856718/overview

That seems to indicate that is a valid warning that should be investigated.

@gaaclarke gaaclarke added the P0 Critical issues such as a build break or regression label May 1, 2024
@gaaclarke
Copy link
Member Author

I'm dropping this to p1 since it seems that when you play bot roulette you may get one not in a bad state.

@gaaclarke gaaclarke added P1 High-priority issues at the top of the work list and removed P0 Critical issues such as a build break or regression labels May 1, 2024
@zanderso
Copy link
Member

zanderso commented May 1, 2024

https://flutter-review.googlesource.com/c/recipes/+/57400 <- forces the cache to be remounted, which should avoid the inconsistency until the cache is back in a good state.

@christopherfujino
Copy link
Member

Once the caches are back in a good state, we should revert https://flutter-review.googlesource.com/c/recipes/+/57400

@gaaclarke
Copy link
Member Author

I don't understand the code, but would it be preferable not to revert that CL? It seems like slower but more correct builds would be preferable. An optimization that requires manual massaging doesn't seem like a good tradeoff these days.

@christopherfujino
Copy link
Member

I don't understand the code, but would it be preferable not to revert that CL? It seems like slower but more correct builds would be preferable. An optimization that requires manual massaging doesn't seem like a good tradeoff these days.

That's a fair question. My sense is this doesn't happen that often, so not sure it's worth increased build time, but I have no strong opinion either way.

@zanderso
Copy link
Member

zanderso commented May 1, 2024

If there are no obvious negative side-effects (like builds timing out and turning the tree red), we can leave it landed for the rest of the week, and I'll monitor the build cycle time chart to look for large changes to the average build times.

@zanderso
Copy link
Member

The builds are slightly longer, but are finishing within existing timeouts, so I'll close this.

Copy link

github-actions bot commented Jun 5, 2024

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 High-priority issues at the top of the work list team-infra Owned by Infrastructure team
Projects
None yet
Development

No branches or pull requests

3 participants