Merge lp:~cgregan/checkbox/hybrid-graphics-screenshot into lp:checkbox
- hybrid-graphics-screenshot
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Daniel Manrique |
Approved revision: | 3595 |
Merged at revision: | 3596 |
Proposed branch: | lp:~cgregan/checkbox/hybrid-graphics-screenshot |
Merge into: | lp:checkbox |
Diff against target: |
34 lines (+27/-0) 1 file modified
providers/plainbox-provider-checkbox/jobs/graphics.txt.in (+27/-0) |
To merge this branch: | bzr merge lp:~cgregan/checkbox/hybrid-graphics-screenshot |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel Manrique (community) | Approve | ||
Zygmunt Krynicki (community) | Needs Fixing | ||
Review via email: mp+250945@code.launchpad.net |
Commit message
Description of the change
Creation of 2 new jobs in Graphics:
graphics/
graphics/
Zygmunt Krynicki (zyga) wrote : | # |
I wonder if we could use templates for that job as well.
Zygmunt Krynicki (zyga) wrote : | # |
Looking at the output of that script, I think we can use templates for this job.
PATH=./
bus: pci
category: VIDEO
driver: i915
index: 1
path: /devices/
product: 3rd Gen Core processor Graphics Controller
product_id: 358
subproduct_id: 14711
subvendor_id: 6058
vendor: Intel Corporation
vendor_id: 32902
Zygmunt Krynicki (zyga) wrote : | # |
I see three units: the resource job that runs graphics_
unit: job
id: graphics_
plugin: resource
command: graphics_
unit: template
template-unit: job
template-resource: graphics_
plugin: shell
id: graphics/
_summary: Obtains a simple screen capture of {product}
estimated_duration: 1.0
requires: package.name == 'gnome-screenshot'
command: gnome-screenshot --file ${PLAINBOX_
_description: Obtains a simple screen capture of {product} using gnome-screenshot if present
unit: template
template-unit: job
template-resource: graphics_
plugin: attachment
id: graphics/
depends: graphics/
_summary: Attaches a simple screen capture of {product}
estimated_duration: 1.0
command: [ -f ${PLAINBOX_
_description: Attaches the simple screen capture file of {product}
Daniel Manrique (roadmr) wrote : | # |
woot, are template units now ready to be used? yay
- 3591. By Chris Gregan
-
Updated the new hybrid graphics jobs to use templates
Daniel Manrique (roadmr) wrote : | # |
This looks OK, however it fails:
$ plainbox run -i 2013.com.
ERROR plainbox.ctrl: Ignoring invalid instantiated unit Obtains a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
ERROR plainbox.ctrl: Ignoring invalid instantiated unit Attaches a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
Outcome: job passed
=======
☑ : graphics_
The reason is that the command mentions ${PLAINBOX_
I tested the theory by replacing the variable syntax to just $PLAINBOX_
I filed https:/
Zygmunt Krynicki (zyga) wrote : | # |
Yes, that is correct. In templates to type { use {{ and ditto for }.
Thanks for catching this!
On Wed, Feb 25, 2015 at 10:38 PM, Daniel Manrique
<email address hidden> wrote:
> Review: Needs Fixing
>
> This looks OK, however it fails:
>
> $ plainbox run -i 2013.com.
> ERROR plainbox.ctrl: Ignoring invalid instantiated unit Obtains a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
> ERROR plainbox.ctrl: Ignoring invalid instantiated unit Attaches a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
> Outcome: job passed
> =======
> ☑ : graphics_
>
>
> The reason is that the command mentions ${PLAINBOX_
>
> I tested the theory by replacing the variable syntax to just $PLAINBOX_
>
> I filed https:/
>
> --
> https:/
> You are reviewing the proposed merge of lp:~cgregan/checkbox/hybrid-graphics-screenshot into lp:checkbox.
- 3592. By Chris Gregan
-
updated $PLAINBOX_
SESSION_ SHARE - 3593. By Chris Gregan
-
Removing eroneous $ from depends and id lines
Daniel Manrique (roadmr) wrote : | # |
OK, two more things.
The id for the attachment job is still weird, as it's identical to the screenshot job. I suspect the screenshot needs a .png at the end of the id.
Also, this won't work yet due to bug 1425722 which I discovered while testing this. A fix for that is proposed:
https:/
I'll mark this merge as depending on that one, but don't forget to commit the id fix :)
Daniel Manrique (roadmr) wrote : | # |
oops, actually I can't add a prerequisite branch :( so let's just hold this until the bug fix is merged.
Daniel Manrique (roadmr) wrote : | # |
Hi Chris,
Zygmunt ended up doing a different fix, so the only change needed here is to quote the job name in the dependency, because {product} can contain spaces. See inline comment in the affected line.
Once that's done, we should be OK to merge.
- 3594. By Chris Gregan
-
updated the depends call
Chris Gregan (cgregan) wrote : | # |
Done.
Daniel Manrique (roadmr) wrote : | # |
Oh,sorry! one *final* change needed (I tested it locally and it's the only missing bit). see below...
Daniel Manrique (roadmr) wrote : | # |
Sorry, here's the required change.
Chris Gregan (cgregan) wrote : | # |
Ah..I wondered when I committed, but then skipped because I was not sure if
it applied to id and depends or just depends
On Thu, Feb 26, 2015 at 3:06 PM, Daniel Manrique <
<email address hidden>> wrote:
> Sorry, here's the required change.
>
> Diff comments:
>
> > === modified file 'providers/
> graphics.txt.in'
> > --- providers/
> 2015-02-03 11:05:57 +0000
> > +++ providers/
> 2015-02-26 19:08:18 +0000
> > @@ -550,3 +550,30 @@
> > _description: Attaches the simple screen capture file
> >
> >
> > +unit: job
> > +id: graphics_
> > +plugin: resource
> > +command: graphics_
> > +
> > +unit: template
> > +template-unit: job
> > +template-resource: graphics_
> > +plugin: shell
> > +id: graphics/
> > +_summary: Obtains a simple screen capture of {product}
> > +estimated_
> > +requires: package.name == 'gnome-screenshot'
> > +command: gnome-screenshot --file
> $PLAINBOX_
> > +_description: Obtains a simple screen capture of {product} using
> gnome-screenshot if present
> > +
> > +unit: template
> > +template-unit: job
> > +template-resource: graphics_
> > +plugin: attachment
> > +id: graphics/
>
> The ID is the same as the above job. CHange to:
>
> id: graphics/
>
> > +depends: "graphics/
> > +_summary: Attaches a simple screen capture of {product}
> > +estimated_
> > +command: [ -f $PLAINBOX_
> base64 $PLAINBOX_
> > +_description: Attaches the simple screen capture file of {product}
> > +
> >
>
>
> --
>
> https:/
> You are the owner of lp:~cgregan/checkbox/hybrid-graphics-screenshot.
>
--
Chris Gregan
Project Manager
Professional and Engineering Services
Canonical USA Inc.
<email address hidden>
cgregan[
W-781-761-9448
----
1024/8806032D
E70F 7391 6C78 9B9E 6461 1CC7 B168 E1E7 8806 032D
- 3595. By Chris Gregan
-
typo for id of attachment
Daniel Manrique (roadmr) wrote : | # |
Thanks, let's merge this!
Preview Diff
1 | === modified file 'providers/plainbox-provider-checkbox/jobs/graphics.txt.in' | |||
2 | --- providers/plainbox-provider-checkbox/jobs/graphics.txt.in 2015-02-03 11:05:57 +0000 | |||
3 | +++ providers/plainbox-provider-checkbox/jobs/graphics.txt.in 2015-02-26 20:21:18 +0000 | |||
4 | @@ -550,3 +550,30 @@ | |||
5 | 550 | _description: Attaches the simple screen capture file | 550 | _description: Attaches the simple screen capture file |
6 | 551 | 551 | ||
7 | 552 | 552 | ||
8 | 553 | unit: job | ||
9 | 554 | id: graphics_card_resource | ||
10 | 555 | plugin: resource | ||
11 | 556 | command: graphics_card_resource | ||
12 | 557 | |||
13 | 558 | unit: template | ||
14 | 559 | template-unit: job | ||
15 | 560 | template-resource: graphics_card_resource | ||
16 | 561 | plugin: shell | ||
17 | 562 | id: "graphics/{index}_screen-capture-internal_{product}" | ||
18 | 563 | _summary: Obtains a simple screen capture of {product} | ||
19 | 564 | estimated_duration: 1.0 | ||
20 | 565 | requires: package.name == 'gnome-screenshot' | ||
21 | 566 | command: gnome-screenshot --file $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png | ||
22 | 567 | _description: Obtains a simple screen capture of {product} using gnome-screenshot if present | ||
23 | 568 | |||
24 | 569 | unit: template | ||
25 | 570 | template-unit: job | ||
26 | 571 | template-resource: graphics_card_resource | ||
27 | 572 | plugin: attachment | ||
28 | 573 | id: graphics/{index}_screen-capture-internal_{product}.png | ||
29 | 574 | depends: "graphics/{index}_screen-capture-internal_{product}" | ||
30 | 575 | _summary: Attaches a simple screen capture of {product} | ||
31 | 576 | estimated_duration: 1.0 | ||
32 | 577 | command: [ -f $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png ] && base64 $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png | ||
33 | 578 | _description: Attaches the simple screen capture file of {product} | ||
34 | 579 |
Please don't change permissions of existing files.