Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2769543003:
Add DEPS roll script for SwiftShader. (Closed)
DescriptionAdd DEPS roll script for SwiftShader.
Based directly off of roll_angle.py.
BUG=swiftshader:36
R=dpranke@chromium.org
Review-Url: https://codereview.chromium.org/2769543003
Cr-Commit-Position: refs/heads/master@{#461325}
Committed: https://chromium.googlesource.com/chromium/src/+/42820a412447065d7db96f10d7db2f3b61de2410
Patch Set 1 #
Total comments: 2
Patch Set 2 : Made script executable on Linux. #Messages
Total messages: 32 (16 generated)
Description was changed from ========== Add DEPS roll script for SwiftShader. Based directly off of roll_angle.py. BUG=swiftshader:36 R=dpranke@chromium.org ========== to ========== Add DEPS roll script for SwiftShader. Based directly off of roll_angle.py. BUG=swiftshader:36 R=dpranke@chromium.org ==========
capn@chromium.org changed reviewers: + sugoi@google.com
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
sugoi@chromium.org changed reviewers: + sugoi@chromium.org
https://codereview.chromium.org/2769543003/diff/1/tools/roll_swiftshader.py File tools/roll_swiftshader.py (right): https://codereview.chromium.org/2769543003/diff/1/tools/roll_swiftshader.py#n... tools/roll_swiftshader.py:30: "buildernames": ["android_optional_gpu_tests_rel"] Why Android?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/2769543003/diff/1/tools/roll_swiftshader.py File tools/roll_swiftshader.py (right): https://codereview.chromium.org/2769543003/diff/1/tools/roll_swiftshader.py#n... tools/roll_swiftshader.py:30: "buildernames": ["android_optional_gpu_tests_rel"] On 2017/03/22 15:51:28, sugoi1 wrote: > Why Android? ANGLE's roll script had it since crbug.com/626498. It seems prudent to run some Android tests for SwiftShader as well. Note that DEPS rolls aren't that frequent, so I think it's justified even if we're not touching anything Android related yet.
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
capn@chromium.org changed reviewers: + thakis@chromium.org - sugoi@chromium.org, sugoi@google.com
PTAL
See discussion at https://codereview.chromium.org/1289873005/ -- tl;dr: have you looked at skia's autoroller?
On 2017/03/22 19:22:03, Nico wrote: > See discussion at https://codereview.chromium.org/1289873005/ -- tl;dr: have you > looked at skia's autoroller? I had not. Thanks for the pointer. "roll-dep src/third_party/swiftshader" seems to do the essential part, but it lacks some features the above script takes care of: - It automatically creates a branch. - It adds additional tryjobs. - It adds BUG numbers to the commit message. - It can update the revision number in a readme. - It adds a link to the revision range to the commit message (with roll-deps I see the link being shown in bash, but it's not actually in the commit message). - It allows adding a TBR. - It can automatically submit to the CQ. It seems like some of these features could be generalized and factored out to depot_tool's roll-dep.py. But I have no experience with Python so that could take some time, while we have an immediate need for this script. I could work with the ANGLE team to move the common features over to roll-dep, after this has landed.
I've had a closer look at an example of a Skia DEPS roll (https://chromium-review.googlesource.com/458104) and at https://cs.chromium.org/chromium/infra/recipes-py/recipe_engine/autoroll.py . I don't think we want SwiftShader to automatically roll on a regular basis. At least not yet. We need to at the very least add WebGL tests to the CQ for that. For now the above script suits our needs better, and as I mentioned before we're willing to refactor it over time to move the duplicate bits to roll-dep.
PTAL
lgtm. It might be nice to consolidate deps-rolling-scripts at some point, but I don't think that point needs to be now.
Fine with me, but +agable who had concerns about this. On Mar 31, 2017 6:28 PM, <dpranke@chromium.org> wrote: > lgtm. > > It might be nice to consolidate deps-rolling-scripts at some point, but I > don't > think that point needs to be now. > > https://codereview.chromium.org/2769543003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
agable@chromium.org changed reviewers: + agable@chromium.org
Sigh. Yeah, I'm not super happy about adding 400 more lines of direct copy-paste. There's already 800 extraneous lines in this directory (not to mention the nearly-identical roll-v8 and roll-skia scripts which happen to exist in their own repositories instead of in this directory, but are still nearly identical). But I don't have the cycles to be the person who unifies them. Please file a bug CCing the owners of the various copies of this script so that maybe someone will go ahead and do it, and then feel free to submit this.
On 2017/04/01 00:29:11, agable wrote: > Sigh. Yeah, I'm not super happy about adding 400 more lines of direct > copy-paste. There's already 800 extraneous lines in this directory (not to > mention the nearly-identical roll-v8 and roll-skia scripts which happen to exist > in their own repositories instead of in this directory, but are still nearly > identical). > > But I don't have the cycles to be the person who unifies them. Please file a bug > CCing the owners of the various copies of this script so that maybe someone will > go ahead and do it, and then feel free to submit this. Done: Issue 707512
The CQ bit was checked by capn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491059536361260,
"parent_rev": "bff9177a06145165275e20245a3b7ecf9c4156e2", "commit_rev":
"42820a412447065d7db96f10d7db2f3b61de2410"}
Message was sent while issue was closed.
Description was changed from ========== Add DEPS roll script for SwiftShader. Based directly off of roll_angle.py. BUG=swiftshader:36 R=dpranke@chromium.org ========== to ========== Add DEPS roll script for SwiftShader. Based directly off of roll_angle.py. BUG=swiftshader:36 R=dpranke@chromium.org Review-Url: https://codereview.chromium.org/2769543003 Cr-Commit-Position: refs/heads/master@{#461325} Committed: https://chromium.googlesource.com/chromium/src/+/42820a412447065d7db96f10d7db... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/42820a412447065d7db96f10d7db... |
