Merge lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools

[Robert Bruce Park]

Support RTM in citrain tool.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:313
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/394/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/75
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/75
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/75

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/394/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:314
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/395/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/76
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/76
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/76

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/395/rebuild

review: Approve (continuous-integration)

[Oliver Grawert (ogra)]

hmm, this perl command prints nothing on a devel-proposed install ...

phablet@ubuntu-phablet:~$ system-image-cli -i
current build number: 244
device name: mako
channel: ubuntu-touch/devel-proposed
alias: ubuntu-touch/utopic-proposed
last update: 2014-09-17 09:24:18
version version: 244
version ubuntu: 20140917
version device: 20140917
version custom: mako-0.5
phablet@ubuntu-phablet:~$ system-image-cli -i | perl -lne 'm|^channel.*/(.*)/.*$| && print "$1"'
phablet@ubuntu-phablet:~$

how about instead something like:

DISTRO=""
if system-image-cli -i | grep ^channel: |grep -q rtm; then
    DISTRO="ubuntu-rtm/"
fi

phablet-config writable-image --ppa $PPA/${DISTRO}${SILO}

(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)]

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)]

PASSED: Continuous integration, rev:315
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/397/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/78
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/78
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/78

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/397/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:316
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/398/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/79
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/79
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/79

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/398/rebuild

review: Approve (continuous-integration)

[Oliver Grawert (ogra)]

looks good

review: Approve

[Robert Bruce Park (robru)]

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:

http://paste.ubuntu.com/8373889/

review: Approve

[lp:~robru/phablet-tools/citrain-on-rtm]

317. By Robert Bruce Park on 2014-09-18

Special case for RTM.

318. By Robert Bruce Park on 2014-09-18

Why doesn't this shit work?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:317
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/399/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/80
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/80
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/80

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/399/rebuild

review: Approve (continuous-integration)

[lp:~robru/phablet-tools/citrain-on-rtm]

319. By Robert Bruce Park on 2014-09-18

It works!!!!

[Robert Bruce Park (robru)]

Ok, this is really working for real for me on RTM images, please approve.

review: Approve

[lp:~robru/phablet-tools/citrain-on-rtm]

320. By Robert Bruce Park on 2014-09-18

Don't hard-code utopic.

321. By Robert Bruce Park on 2014-09-18

Apparently no universe.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:319
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/400/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/81
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/81
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/81

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/400/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:321
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/401/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/82
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/82
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/82

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/401/rebuild

review: Approve (continuous-integration)

[Brendan Donegan (brendan-donegan)]

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-ppa-service in here if you continue with adding the .list file instead of using add-apt-repository

review: Needs Information

[Robert Bruce Park (robru)]

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/sources.list rather than to sources.list.d, which breaks 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.

[Brendan Donegan (brendan-donegan)]

> 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://launchpad.net/~ci-train-ppa-service/+archive/ubuntu-rtm/landing-017 to see what happens with it.
>
> You can pass the full deb line to add-apt-repository, but if you do, it adds
> the line to /etc/apt/sources.list rather than to sources.list.d, which breaks
> 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.ubuntu.com --recv-keys ECF1204C # <- the ci-train-ppa-service key

prior to running apt-get update

[lp:~robru/phablet-tools/citrain-on-rtm]

322. By Robert Bruce Park on 2014-09-18

Add apt-get for Brendan.

323. By Robert Bruce Park on 2014-09-18

Slightly less sloppy set -x output.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:322
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/404/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/85
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/85
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/85

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/404/rebuild

review: Approve (continuous-integration)

[lp:~robru/phablet-tools/citrain-on-rtm]

324. By Robert Bruce Park on 2014-09-18

More fixes from Brendan.

325. By Robert Bruce Park on 2014-09-18

Well that didn't work.

[Brendan Donegan (brendan-donegan)]

You can't echo into /etc/apt/sources.list.d/citrain.list using adb shell, 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.

review: Needs Fixing

[lp:~robru/phablet-tools/citrain-on-rtm]

326. By Robert Bruce Park on 2014-09-18

So this is just totally broken.

327. By Robert Bruce Park on 2014-09-18

So that's a thing.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:324
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/405/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/86
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/86
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/86

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/405/rebuild

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

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/sources.list.d/citrain.list using adb shell,
> 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)]

PASSED: Continuous integration, rev:327
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/406/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/87
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/87
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/87

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/406/rebuild

review: Approve (continuous-integration)

[lp:~robru/phablet-tools/citrain-on-rtm]

328. By Robert Bruce Park on 2014-09-24

Switch back to add-apt-repository now that it landed.

[Robert Bruce Park (robru)]

Ok guys, this is ready for testing again, please test both utopic and rtm.

review: Approve

[lp:~robru/phablet-tools/citrain-on-rtm]

329. By Robert Bruce Park on 2014-09-24

Drop unused variable.

[Brendan Donegan (brendan-donegan)]

I'll test this - although it seems like it should work. Just two things inline.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:329
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/409/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/90

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/409/rebuild

review: Approve (continuous-integration)

[lp:~robru/phablet-tools/citrain-on-rtm]

330. By Robert Bruce Park on 2014-09-25

--yes

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:330
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/410/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/91

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/410/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

> Why not pass --ppa=$PPA/$DISTRO/$SILO to phablet-config writable-image 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?

> 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)]

On Thu, Sep 25, 2014 at 2:22 AM, ♫ Robert Bruce Park ♫ <
<email address hidden>> wrote:

> > Why not pass --ppa=$PPA/$DISTRO/$SILO to phablet-config writable-image
> 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://code.launchpad.net/~robru/phablet-tools/citrain-on-rtm/+merge/235069
> You are reviewing the proposed merge of
> lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools.
>

[Brendan Donegan (brendan-donegan)]

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=$PPA/$DISTRO/$SILO to phablet-config writable-image
>> 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://code.launchpad.net/~robru/phablet-tools/citrain-on-rtm/+merge/235069
>> You are reviewing the proposed merge of
>> lp:~robru/phablet-tools/citrain-on-rtm into lp:phablet-tools.
>>
>
>

[lp:~robru/phablet-tools/citrain-on-rtm]

331. By Robert Bruce Park on 2014-09-25

Fixes requested by brendand.

[Robert Bruce Park (robru)]

Ok, addressed all of Brendan's concerns.

> assuming this works

Yeah that's the trick, please actually test it.

review: Approve

[Robert Bruce Park (robru)]

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!

review: Approve

[Brendan Donegan (brendan-donegan)]

LGTM +1

(N.B. I only tested on RTM)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:331
http://jenkins.qa.ubuntu.com/job/phablet-tools-ci/411/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-amd64-ci/92
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-armhf-ci/92
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-tools-utopic-i386-ci/92

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-tools-ci/411/rebuild

review: Approve (continuous-integration)

=== modified file 'citrain'
--- citrain	2014-08-20 01:30:24 +0000
+++ citrain	2014-09-25 16:10:27 +0000
@@ -16,7 +16,7 @@
 
 usage () {
 cat <<EOF
-usage: $0 COMMAND SILO-NUMBER
+usage: $0 COMMAND SILO-NUMBER [DEVICE-PASSWORD]
 
 COMMANDS:
     host-install        Deprecated. Please use host-upgrade instead.
@@ -43,8 +43,6 @@
 
 # Defaults
 PPA="ppa:ci-train-ppa-service"
-HTTP="http://ppa.launchpad.net/ci-train-ppa-service"
-SOURCES="ubuntu/dists/devel/main/source/Sources"
 
 # Read the first positional argument.
 COMMAND="$1"
@@ -75,12 +73,23 @@
 
     device-upgrade)
         check_devices
-        echo "These PPAs are enabled on the device:"
+        # Read the third positional argument.
+        PASSWORD="$1"
+        [ $# -gt 0 ] && shift || usage
+
         set -x
         adb shell egrep ^deb /etc/apt/sources.list.d/\*.list
-        phablet-config writable-image --ppa $PPA/$SILO
-        adb shell sudo apt-get -o Dir::Etc::SourceList=/dev/null update
-        adb shell sudo apt-get dist-upgrade --yes
+        if adb shell system-image-cli -i | grep -q ubuntu-rtm; then
+            DISTRO="ubuntu-rtm"
+        else
+            DISTRO="ubuntu"
+        fi
+        phablet-config writable-image -r $PASSWORD --ppa $PPA/$DISTRO/$SILO
+        adb shell "echo -e '#\x21/bin/sh\necho $PASSWORD' >/tmp/askpass.sh"
+        adb shell chmod +x /tmp/askpass.sh
+        adb shell SUDO_ASKPASS=/tmp/askpass.sh sudo -A apt-get -o Dir::Etc::SourceList=/dev/null update
+        adb shell SUDO_ASKPASS=/tmp/askpass.sh sudo -A apt-get dist-upgrade --yes
+        adb shell rm -f /tmp/askpass.sh
         adb reboot
         ;;