Merge lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools
- citrain-on-rtm
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Oliver Grawert |
Approved revision: | 331 |
Merged at revision: | 321 |
Proposed branch: | lp:~robru/phablet-tools/citrain-on-rtm |
Merge into: | lp:phablet-tools |
Diff against target: |
48 lines (+16/-7) 1 file modified
citrain (+16/-7) |
To merge this branch: | bzr merge lp:~robru/phablet-tools/citrain-on-rtm |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot | continuous-integration | Approve | |
Brendan Donegan (community) | Approve | ||
Robert Bruce Park (community) | Approve | ||
Oliver Grawert | Approve | ||
Mathieu Trudel-Lapierre | Pending | ||
Review via email: mp+235069@code.launchpad.net |
Commit message
Support RTM in citrain tool.
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:314
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Oliver Grawert (ogra) wrote : | # |
hmm, this perl command prints nothing on a devel-proposed install ...
phablet@
current build number: 244
device name: mako
channel: ubuntu-
alias: ubuntu-
last update: 2014-09-17 09:24:18
version version: 244
version ubuntu: 20140917
version device: 20140917
version custom: mako-0.5
phablet@
phablet@
how about instead something like:
DISTRO=""
if system-image-cli -i | grep ^channel: |grep -q rtm; then
DISTRO=
fi
phablet-config writable-image --ppa $PPA/${
(also note that this phablet-config command will fail if you dont hand over the sudo password with the -r or --remotepassword options, you should add a citrain option to hand this through to the phablet-config command)
Oliver Grawert (ogra) wrote : | # |
whoops, i just noticed you execute this remotely, in this case:
if [ -n "$(adb shell "system-image-cli -i | grep ^channel: |grep rtm")" ]; then
...
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:315
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:316
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Robert Bruce Park (robru) wrote : | # |
This works for me on utopic images, and well, doesn't work in RTM but it Does The Right Thing and RTM should soon be fixed:
- 317. By Robert Bruce Park
-
Special case for RTM.
- 318. By Robert Bruce Park
-
Why doesn't this shit work?
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:317
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 319. By Robert Bruce Park
-
It works!!!!
Robert Bruce Park (robru) wrote : | # |
Ok, this is really working for real for me on RTM images, please approve.
- 320. By Robert Bruce Park
-
Don't hard-code utopic.
- 321. By Robert Bruce Park
-
Apparently no universe.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:319
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:321
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Brendan Donegan (brendan-donegan) wrote : | # |
First of all, thanks for this - it will make our lives a lot easier. Just a couple of queries:
1. If cjwatsons change to software-properties is landing soon, why the hack to write the .list file?
2. The logic to add a PPA was originally in phablet-config writable-image, why are we duplicating it here?
3. Won't adding the PPA without the key break other tools? You should probably fit recv-key for the ci-train-
Robert Bruce Park (robru) wrote : | # |
1. Cyphermox suggested that even once add-apt-repository gets fixed in rtm, it would attempt to add distro series as "utopic" even though that's not valid. So the only way to get the distro and series correct is to write the full deb line manually.
You can pass the full deb line to add-apt-repository, but if you do, it adds the line to /etc/apt/
So I realized, since I'm reduced to writing my own deb line anyway, i might as well just make my own sources.list and reference it myself.
2. Because phablet-config just wraps add-apt-repository which I'm not convinced can be fixed for rtm, due to what cyphermox said about it.
3. I dunno, will it break other tools? Patches welcome.
Brendan Donegan (brendan-donegan) wrote : | # |
> 1. Cyphermox suggested that even once add-apt-repository gets fixed in rtm, it
> would attempt to add distro series as "utopic" even though that's not valid.
> So the only way to get the distro and series correct is to write the full deb
> line manually.
I keep hearing mixed messages on that - cjwatson told me earlier today he thought it was possible and just had to confer with mvo over the exact right way to do it. Keep an eye on https:/
>
> You can pass the full deb line to add-apt-repository, but if you do, it adds
> the line to /etc/apt/
> the logic of "set sources.list to /dev/null to isolate the silo contents" hack
> below.
>
> So I realized, since I'm reduced to writing my own deb line anyway, i might as
> well just make my own sources.list and reference it myself.
>
> 2. Because phablet-config just wraps add-apt-repository which I'm not
> convinced can be fixed for rtm, due to what cyphermox said about it.
>
> 3. I dunno, will it break other tools? Patches welcome.
Yes, apt-get install and dist-upgrade will fail unless they have --force-yes specified, which many tools in phablet-tools don't. I could submit a patch but it's as simple as adding:
apt-key adv --keyserver keyserver.
prior to running apt-get update
- 322. By Robert Bruce Park
-
Add apt-get for Brendan.
- 323. By Robert Bruce Park
-
Slightly less sloppy set -x output.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:322
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 324. By Robert Bruce Park
-
More fixes from Brendan.
- 325. By Robert Bruce Park
-
Well that didn't work.
Brendan Donegan (brendan-donegan) wrote : | # |
You can't echo into /etc/apt/
- 326. By Robert Bruce Park
-
So this is just totally broken.
- 327. By Robert Bruce Park
-
So that's a thing.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:324
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Sergio Schvezov (sergiusens) wrote : | # |
On Thu, Sep 18, 2014 at 8:36 PM, Brendan Donegan <
<email address hidden>> wrote:
> Review: Needs Fixing
>
> You can't echo into /etc/apt/
> even if you use sudo. I'm not sure why - maybe some restriction on creating
> files. I had to create it in /tmp/ and then mv it.
>
You need to "echo blah | sudo tee file" it
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:327
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 328. By Robert Bruce Park
-
Switch back to add-apt-repository now that it landed.
Robert Bruce Park (robru) wrote : | # |
Ok guys, this is ready for testing again, please test both utopic and rtm.
- 329. By Robert Bruce Park
-
Drop unused variable.
Brendan Donegan (brendan-donegan) wrote : | # |
I'll test this - although it seems like it should work. Just two things inline.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:329
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 330. By Robert Bruce Park
-
--yes
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:330
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Robert Bruce Park (robru) wrote : | # |
> Why not pass --ppa=$
Well, I didn't check, but I was just assuming that phablet-config didn't support the $PPA/$DISTRO/$SILO syntax for PPAs, so I was working around that.
Even if phablet-config really does work on RTM, it's just calling add-apt-repository anyway. What benefit do we get from going through a middleman?
> I don't think --force-yes is necessary any more and might even be a little unsafe since we should be dealing with a PPA whose key we have received already.
Yeah I can fix that tomorrow, EOD now!
Brendan Donegan (brendan-donegan) wrote : | # |
On Thu, Sep 25, 2014 at 2:22 AM, ♫ Robert Bruce Park ♫ <
<email address hidden>> wrote:
> > Why not pass --ppa=$
> instead of calling that seperately?
>
> Well, I didn't check, but I was just assuming that phablet-config didn't
> support the $PPA/$DISTRO/$SILO syntax for PPAs, so I was working around
> that.
>
> Even if phablet-config really does work on RTM, it's just calling
> add-apt-repository anyway. What benefit do we get from going through a
> middleman?
>
phablet-config calls add-apt-repository so basically you're just avoiding
code duplication where possible - it's sound practice
>
> > I don't think --force-yes is necessary any more and might even be a
> little unsafe since we should be dealing with a PPA whose key we have
> received already.
>
> Yeah I can fix that tomorrow, EOD now!
> --
>
> https:/
> You are reviewing the proposed merge of
> lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools.
>
Brendan Donegan (brendan-donegan) wrote : | # |
Having said that, assuming this works I'm not going to block too hard on
that discrepancy
On Thu, Sep 25, 2014 at 9:47 AM, Brendan Donegan <
<email address hidden>> wrote:
> On Thu, Sep 25, 2014 at 2:22 AM, ♫ Robert Bruce Park ♫ <
> <email address hidden>> wrote:
>
>> > Why not pass --ppa=$
>> instead of calling that seperately?
>>
>> Well, I didn't check, but I was just assuming that phablet-config didn't
>> support the $PPA/$DISTRO/$SILO syntax for PPAs, so I was working around
>> that.
>>
>> Even if phablet-config really does work on RTM, it's just calling
>> add-apt-repository anyway. What benefit do we get from going through a
>> middleman?
>>
> phablet-config calls add-apt-repository so basically you're just avoiding
> code duplication where possible - it's sound practice
>
>>
>> > I don't think --force-yes is necessary any more and might even be a
>> little unsafe since we should be dealing with a PPA whose key we have
>> received already.
>>
>> Yeah I can fix that tomorrow, EOD now!
>> --
>>
>> https:/
>> You are reviewing the proposed merge of
>> lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools.
>>
>
>
- 331. By Robert Bruce Park
-
Fixes requested by brendand.
Robert Bruce Park (robru) wrote : | # |
Ok, addressed all of Brendan's concerns.
> assuming this works
Yeah that's the trick, please actually test it.
Robert Bruce Park (robru) wrote : | # |
Ok, I finally found some time to actually test this, and found it to be working in both Utopic and RTM, so I'm happy to finally merge this as long as everybody else is satisfied with the code!
Brendan Donegan (brendan-donegan) wrote : | # |
LGTM +1
(N.B. I only tested on RTM)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:331
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
1 | === modified file 'citrain' | |||
2 | --- citrain 2014-08-20 01:30:24 +0000 | |||
3 | +++ citrain 2014-09-25 16:10:27 +0000 | |||
4 | @@ -16,7 +16,7 @@ | |||
5 | 16 | 16 | ||
6 | 17 | usage () { | 17 | usage () { |
7 | 18 | cat <<EOF | 18 | cat <<EOF |
9 | 19 | usage: $0 COMMAND SILO-NUMBER | 19 | usage: $0 COMMAND SILO-NUMBER [DEVICE-PASSWORD] |
10 | 20 | 20 | ||
11 | 21 | COMMANDS: | 21 | COMMANDS: |
12 | 22 | host-install Deprecated. Please use host-upgrade instead. | 22 | host-install Deprecated. Please use host-upgrade instead. |
13 | @@ -43,8 +43,6 @@ | |||
14 | 43 | 43 | ||
15 | 44 | # Defaults | 44 | # Defaults |
16 | 45 | PPA="ppa:ci-train-ppa-service" | 45 | PPA="ppa:ci-train-ppa-service" |
17 | 46 | HTTP="http://ppa.launchpad.net/ci-train-ppa-service" | ||
18 | 47 | SOURCES="ubuntu/dists/devel/main/source/Sources" | ||
19 | 48 | 46 | ||
20 | 49 | # Read the first positional argument. | 47 | # Read the first positional argument. |
21 | 50 | COMMAND="$1" | 48 | COMMAND="$1" |
22 | @@ -75,12 +73,23 @@ | |||
23 | 75 | 73 | ||
24 | 76 | device-upgrade) | 74 | device-upgrade) |
25 | 77 | check_devices | 75 | check_devices |
27 | 78 | echo "These PPAs are enabled on the device:" | 76 | # Read the third positional argument. |
28 | 77 | PASSWORD="$1" | ||
29 | 78 | [ $# -gt 0 ] && shift || usage | ||
30 | 79 | |||
31 | 79 | set -x | 80 | set -x |
32 | 80 | adb shell egrep ^deb /etc/apt/sources.list.d/\*.list | 81 | adb shell egrep ^deb /etc/apt/sources.list.d/\*.list |
36 | 81 | phablet-config writable-image --ppa $PPA/$SILO | 82 | if adb shell system-image-cli -i | grep -q ubuntu-rtm; then |
37 | 82 | adb shell sudo apt-get -o Dir::Etc::SourceList=/dev/null update | 83 | DISTRO="ubuntu-rtm" |
38 | 83 | adb shell sudo apt-get dist-upgrade --yes | 84 | else |
39 | 85 | DISTRO="ubuntu" | ||
40 | 86 | fi | ||
41 | 87 | phablet-config writable-image -r $PASSWORD --ppa $PPA/$DISTRO/$SILO | ||
42 | 88 | adb shell "echo -e '#\x21/bin/sh\necho $PASSWORD' >/tmp/askpass.sh" | ||
43 | 89 | adb shell chmod +x /tmp/askpass.sh | ||
44 | 90 | adb shell SUDO_ASKPASS=/tmp/askpass.sh sudo -A apt-get -o Dir::Etc::SourceList=/dev/null update | ||
45 | 91 | adb shell SUDO_ASKPASS=/tmp/askpass.sh sudo -A apt-get dist-upgrade --yes | ||
46 | 92 | adb shell rm -f /tmp/askpass.sh | ||
47 | 84 | adb reboot | 93 | adb reboot |
48 | 85 | ;; | 94 | ;; |
49 | 86 | 95 |
PASSED: Continuous integration, rev:313 jenkins. qa.ubuntu. com/job/ phablet- tools-ci/ 394/ jenkins. qa.ubuntu. com/job/ phablet- tools-utopic- amd64-ci/ 75 jenkins. qa.ubuntu. com/job/ phablet- tools-utopic- armhf-ci/ 75 jenkins. qa.ubuntu. com/job/ phablet- tools-utopic- i386-ci/ 75
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/phablet- tools-ci/ 394/rebuild
http://