Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Do not cache gclient sync #8098

Merged
merged 1 commit into from
Mar 9, 2019
Merged

Do not cache gclient sync #8098

merged 1 commit into from
Mar 9, 2019

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Mar 9, 2019

Fixes flutter/flutter#29013

This unblocks #8087

Unfortunately, caching depot_tools and the enigne checkout causes problems on Cirrus. depot_tools fails to update itself (which was temporarily resolved by not letting it update itself). If some DEPS change, the file system sometimes fails to write them (which we saw previously with buildtools simply erroring out, and now I'm seeing again when trying to update the SDK resources - it doesn't quite error out, but also fails to properly replace the assets).

This will make the builds slightly longer, but the gclient part still seems fast since it's still all running in Google cloud.

@dnfield dnfield merged commit 2134286 into flutter:master Mar 9, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 9, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Mar 9, 2019
flutter/engine@dc216bd...2134286

git log dc216bd..2134286 --no-merges --oneline
2134286 Do not cache gclient sync (flutter/engine#8098)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (mklim@google.com), and stop
the roller if necessary.
@liyuqian
Copy link
Contributor

LGTM. The time increase is tiny. Let's definitely trade that time to make the test more robust.

On the other hand, I'm more curious about the detailed error that it encountered if we do cache gclient sync and out directory. If such error could exist in our test environment with the GKE container, should it also be possible to happen in our own local Linux environment where things are cached all the time? Sure we can also clear the local directory but that's annoying. Should we report that error as a bug or issue? At least when someone encountered similar issues locally, they'll quickly know that a rm -rf could fix the problem.

@dnfield
Copy link
Contributor Author

dnfield commented Mar 10, 2019

@liyuqian when the error happened it would complain about trying to overwrite files in an overlay filesystem. In particular it was because cipd tries to do atomic overwrites and the overlay file system doesn't support that.

With the latest issue, it didn't even error on the write but then errored later when trying to use the files.

@liyuqian
Copy link
Contributor

I see. So basically cipd and overlayfs used by the container isn't compatible during overwrite, which means that our local linux dev environment shouldn't be affected unless someone is developing the engine inside a container?

@dnfield dnfield deleted the uncache_cirrus branch March 10, 2019 05:10
@dnfield
Copy link
Contributor Author

dnfield commented Mar 10, 2019

Right - this is a specific to container kind of thing. Even on LUCI this shouldn't be a problem AFAICT.

It might be possible to create a container with options that would allow overwriting the overlay fs this way, but then we start to lose the benefits of caching anyway for whatever we need to ovewrite.

RBogie pushed a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The GKE container can't be updated dynamically for Engine presubmits
4 participants