1137059 - [gonk-L] flash.sh hangs because shell behavior of Android L changed

Status: VERIFIED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Hermes Cheng (inactive after July 27, 2015)]

* Description:
When I use flash.sh on Nexus 5 which is user build, it hangs on delete_single_variant_persist()" at "flash_fastboot()" and don't prompt any errors. The reason might be some behavior changes for shell of Android L.

If I run that step manually on Nexus-5-L, the result is as below.
 $ adb shell rm -r /persist/svoperapps
 override rwxr-x--- root:root for '/persist/svoperapps'?

* STR:
1. uncompress pvt build
2. run b2g-distro/flash.sh to a Nexus 5 with user build

* Expected result:
Restart adbd as root to avoid this kind of errors or prompt errors like 
"rm failed for /persist/svoperapps, Permission denied"

* Actual result:
nothing prompt at stdout and hangs...

* Env:
User build for Nexus 5 L

Comment 1

[Hermes Cheng (inactive after July 27, 2015)]

This problem does not exist on flame-kk user build when debugging via USB is enabled as "ADB".
Below command can also be manually executed on flame-kk without any problems.
 $ adb shell rm -r /persist/svoperapps

Comment 2

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Add a response to adb shell rm command.

Dave, I think we can solve this by add a response to shell command. Could you review this patch? Thanks!

Comment 3

[Kai-Zhen Li [:seinlin][:kli]]

Doesn't block 2.2 and affect any specific branch because flash.sh is from B2G repo and it gets master branch only.

Comment 4

[Dave Hylands [:dhylands]]

So - I'm a bit confused.

The shell where its failing is a user shell (indicated by the $ prompt rather than # prompt) and not a root shell, so the rm will fail anyways, even if you do answer y.

The reason you're getting the prompt is that you don't have write permissions in the directory.

The Android L version of rm (and the kitkat version) supports the -f flag, so I think it would be better to do:

rm -rf /persist/svoperapps

alternatively, you could also do:

if [ -w /persist/svoperapps ]; then rm -r /persist/svoperapps; fi

It seems to me that we need to running as root to do this properly, so why isn't this being tested directly?

Comment 5

[Kai-Zhen Li [:seinlin][:kli]]

From bug 982093, my understanding is /persist/svoperapps should be deleted.

When shell command needs a response, it will block flash.sh quietly and we won't know what is the error. If flash.sh can response the error message, we can know there is error. Then we can run as root and try to flash.sh again.

The patch try to delete /persist/svoperapps and expect to get an error message if there is permission issue.

If we use "if [ -w /persist/svoperapps ]; then rm -r /persist/svoperapps; fi", it will quietly ignore the delete process and we won't know it is permission issue and failed to delete /persist/svoperapps.

Comment 6

[Dave Hylands [:dhylands]]

Right but you could use:

if [ ! -w /persist/svoperapps ]; then echo "Some message"; exit 1; fi

that you look for rather than relying a particular behaviour from rm.

Comment 7

[Dave Hylands [:dhylands]]

Or just use rm -rf 

The problem with using echo y is that it only works for the 1 file/directory. Since you're going rm -r, there may be more than one.

So you either need to use yes (which doesn't seem to exist on the phone) or use rm -rf. The only issue with rm -rf is that it isn't supported on older versions.

So the if [ ! -w seems to be the most portable solution.

Comment 8

[Kai-Zhen Li [:seinlin][:kli]]

Dave, Thanks! PR is updated could you have a look?

Comment 9

[Kai-Zhen Li [:seinlin][:kli]]

BTW, I also try 'rm -rf' and I think it is also a good alternative. To see the error message we shouldn't redirect standout to /dev/null. such as:

run_adb shell 'rm -rf /persist/svoperapps'

Comment 10

[Dave Hylands [:dhylands]]

(In reply to Kai-Zhen Li [:kli][:seinlin] from comment #9)
> BTW, I also try 'rm -rf' and I think it is also a good alternative. To see
> the error message we shouldn't redirect standout to /dev/null. such as:
> 
> run_adb shell 'rm -rf /persist/svoperapps'

Yeah rm -rf is probably the preferred solution, as long is its supported on the oldest version of android that we want to flash.

It seems to be implemented in emulator-arm (which is the ICS version) so I think that this means its supported in all of the versions we care about.

Comment 11

[Dave Hylands [:dhylands]]

Comment on attachment 8569636 [details] [review]
Add a response to adb shell rm command.

This looks fine. You could combine the if test, echo and rm all in a single command, something like this:
> if ! run_adb shell 'if [ -w /persist/svoperapps ]; then echo writable; rm -r /persist/svoperapps; fi' | grep -qs 'writable'; then
>     echo 'delete_single_variant_persist: Permission denied'
> fi

I think using rm -rf would be better (since its supported in ICS and we I don't think we need to support older than ICS).

Comment 12

[Kai-Zhen Li [:seinlin][:kli]]

Thanks for review. PR is updated as comment 11 and merged.
Master: https://github.com/mozilla-b2g/B2G/commit/73a8a50fc96ec5f227bb944c921e3e4bbc2315ad

Comment 13

[Hermes Cheng (inactive after July 27, 2015)]

Verify this bug on nexus 5.

* env info
Build ID               20150305162505
Gaia Revision          eb86137e247224e86d17ed1a0a133b2a318dce3c
Gaia Date              2015-03-05 16:51:07
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e9d29a3b94e5
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150305.201134
Firmware Date          Thu Mar  5 20:11:51 EST 2015
Bootloader             HHZ12d

* flashing log
Archive:  pvt/mozilla-b2g37_v2_2-nexus-5-l/latest/nexus-5-l.zip
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/tools/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/tools/profiler/
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/tools/profiler/merge-profiles.py  
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/tools/rb/
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/gecko/tools/rb/fix_stack_using_bpsyms.py  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/flash.sh  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/sources.xml  
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/scripts/
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/scripts/profile-symbolicate.py  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/load-config.sh  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/profile.sh  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/.config  
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/
   creating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/ramdisk-recovery.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/ramdisk.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/boot.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/userdata.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/recovery.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/cache.img  
  inflating: /tmp/tmp.lQbaaXR86x/b2g-distro/out/target/product/hammerhead/system.img  
rm: /persist/svoperapps: Permission denied
< waiting for device >
...
FAILED (remote: Already Unlocked)
finished. total time: 0.001s
erasing 'cache'...
OKAY [  0.640s]
formatting 'cache' partition...
Creating filesystem with parameters:
    Size: 734003200
    Block size: 4096
    Blocks per group: 32768
    Inodes per group: 7472
    Inode size: 256
    Journal blocks: 2800
    Label: 
    Blocks: 179200
    Block groups: 6
    Reserved block group size: 47
Created filesystem with 11/44832 inodes and 5813/179200 blocks
sending 'cache' (13348 KB)...
writing 'cache'...
OKAY [  1.914s]
finished. total time: 2.554s
erasing 'userdata'...
OKAY [  8.451s]
formatting 'userdata' partition...
Creating filesystem with parameters:
    Size: 13725835264
    Block size: 4096
    Blocks per group: 32768
    Inodes per group: 8144
    Inode size: 256
    Journal blocks: 32768
    Label: 
    Blocks: 3351034
    Block groups: 103
    Reserved block group size: 823
Created filesystem with 11/838832 inodes and 93654/3351034 blocks
sending 'userdata' (137318 KB)...
writing 'userdata'...
OKAY [ 13.910s]
finished. total time: 22.361s
target reported max download size of 1073741824 bytes
erasing 'userdata'...
OKAY [  7.164s]
sending 'userdata' (137490 KB)...
OKAY [  4.551s]
writing 'userdata'...
OKAY [  9.197s]
finished. total time: 20.912s
target reported max download size of 1073741824 bytes
erasing 'cache'...
OKAY [  0.557s]
sending 'cache' (13348 KB)...
OKAY [  0.630s]
writing 'cache'...
OKAY [  1.077s]
finished. total time: 2.265s
target reported max download size of 1073741824 bytes
sending 'boot' (8786 KB)...
OKAY [  0.488s]
writing 'boot'...
OKAY [  0.746s]
finished. total time: 1.234s
target reported max download size of 1073741824 bytes
erasing 'system'...
OKAY [  0.830s]
sending 'system' (244028 KB)...
OKAY [  7.891s]
writing 'system'...
OKAY [ 17.679s]
finished. total time: 26.401s
rebooting...

finished. total time: 0.066s
Attempting to set the time on the device
time 1425619785 -> 1425619785.0