Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1679633002:
Android Incremental Install Always use the same device cache (Closed)
|
Created:
4 years, 10 months ago by agrieve Modified:
4 years, 10 months ago Reviewers:
jbudorick CC:
chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAndroid Incremental Install Always use the same device cache
Having two scripts that cache the same thing in two different spots is a bad recipe.
BUG=none
Committed: https://crrev.com/c0707840498b5d102b2df09d2042f5d93dfa0d13
Cr-Commit-Position: refs/heads/master@{#374415}
Patch Set 1 #
Total comments: 2
Depends on Patchset: Messages
Total messages: 18 (7 generated)
Description was changed from ========== Android Incremental Install Always use the same device cache Ran install.py (uses an on-device cache), then ran test_runner (uses an on-host cache) and ended up with the wrong info in my cache. BUG=none ========== to ========== Android Incremental Install Always use the same device cache Having two scripts that cache the same thing in two different spots is a bad recipe. BUG=none ==========
agrieve@chromium.org changed reviewers: + jbudorick@chromium.org
On 2016/02/08 14:56:33, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:jbudorick@chromium.org This actually bit me :(. I'm a bit torn between whether the cache should be kept on-device (should make things "just work" when sharing devices) vs on the local machine (might cause issues when sharing devices). My current thinking is that storing on the host is at least easier for people to clear the cache.
On 2016/02/08 14:58:12, agrieve wrote: > On 2016/02/08 14:56:33, agrieve wrote: > > mailto:agrieve@chromium.org changed reviewers: > > + mailto:jbudorick@chromium.org > > This actually bit me :(. I'm a bit torn between whether the cache should be kept > on-device (should make things "just work" when sharing devices) vs on the local > machine (might cause issues when sharing devices). > > My current thinking is that storing on the host is at least easier for people to > clear the cache. Yeah, that's probably true.
https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... File build/android/incremental_install/installer.py (right): https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... build/android/incremental_install/installer.py:81: enable_device_cache=False, use_concurrency=True, What's the point of flipping this default?
er, lgtm w/ question On 2016/02/08 16:32:46, jbudorick wrote: > https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... > File build/android/incremental_install/installer.py (right): > > https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... > build/android/incremental_install/installer.py:81: enable_device_cache=False, > use_concurrency=True, > What's the point of flipping this default?
https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... File build/android/incremental_install/installer.py (right): https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... build/android/incremental_install/installer.py:81: enable_device_cache=False, use_concurrency=True, On 2016/02/08 16:32:46, jbudorick wrote: > What's the point of flipping this default? test_runner.py uses this function to install incremental apks, and was not setting it to false explicitly. Since test_runner.py had enabled its own device cache, this was causing a second device cached to be loaded. We only ever want the outer-most script to read/write the cache I think, so I made this False by default.
On 2016/02/08 16:35:44, agrieve wrote: > https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... > File build/android/incremental_install/installer.py (right): > > https://codereview.chromium.org/1679633002/diff/1/build/android/incremental_i... > build/android/incremental_install/installer.py:81: enable_device_cache=False, > use_concurrency=True, > On 2016/02/08 16:32:46, jbudorick wrote: > > What's the point of flipping this default? > > test_runner.py uses this function to install incremental apks, and was not > setting it to false explicitly. Since test_runner.py had enabled its own device > cache, this was causing a second device cached to be loaded. > > We only ever want the outer-most script to read/write the cache I think, so I > made this False by default. Ah, ok. sgtm
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1679633002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1679633002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1679633002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1679633002/1
Message was sent while issue was closed.
Description was changed from ========== Android Incremental Install Always use the same device cache Having two scripts that cache the same thing in two different spots is a bad recipe. BUG=none ========== to ========== Android Incremental Install Always use the same device cache Having two scripts that cache the same thing in two different spots is a bad recipe. BUG=none ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Android Incremental Install Always use the same device cache Having two scripts that cache the same thing in two different spots is a bad recipe. BUG=none ========== to ========== Android Incremental Install Always use the same device cache Having two scripts that cache the same thing in two different spots is a bad recipe. BUG=none Committed: https://crrev.com/c0707840498b5d102b2df09d2042f5d93dfa0d13 Cr-Commit-Position: refs/heads/master@{#374415} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/c0707840498b5d102b2df09d2042f5d93dfa0d13 Cr-Commit-Position: refs/heads/master@{#374415} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
