Closed Bug 1047350 Opened 10 years ago Closed 10 years ago

Cannot apply FOTA update to KitKat device

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set
normal

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S2 (15aug)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: casper.vandonderen, Assigned: gerard-majax)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/34.0.1847.116 Chrome/34.0.1847.116 Safari/537.36

Steps to reproduce:

- Build an image for a KitKat-based device, for instance Dolphin and flash the device.
- Update the sources and rebuild.
- Run './build.sh gecko-update-fota-full'
- Upload the image to the update channel and download on device.
- Start to install the update.


Actual results:

The device reboots into FOTA mode after downloading the update but errors in the early steps with "format() expects 5 args, got 4".


Expected results:

The FOTA update should have applied succesfully.
[Blocking Requested - why for this release]:

There has been a change in the AOSP source code for the FOTA updater in that the FormatFn after ICS went from having 4 arguments to 5 (there is a new mount_point var). This can be seen in the difference between FormatFn in  https://android.googlesource.com/platform/bootable/recovery/+/android-4.0.3_r1/updater/install.c and https://android.googlesource.com/platform/bootable/recovery/+/android-4.4.2_r1/updater/install.c

The updater-script that gets generated in update.zip/META-INF/com/google/android/updater-script from gecko-update-fota-full creates lines like format("ubifs", "UBI", "system", 0); which misses the new 5th mount_point argument. This updater-script seems to be generated by https://github.com/mozilla-b2g/B2G/blob/master/tools/update-tools/update_tools.py#L982

For devices running older Android/gonk than Jelly Bean this should stay the same, but for the newer devices the extra argument is mandatory.

This bug should have made it impossible for for instance Flame and Dolphin to receive full FOTA updates.
blocking-b2g: --- → 1.4?
Flags: needinfo?(marshall)
Flags: needinfo?(dhylands)
Status: UNCONFIRMED → NEW
Ever confirmed: true
CC'ing :gerard-majax who did most of the work there. I think this boils down to detecting at build time which base Android version we're using in the build system and then changing the updater-script output accordingly. We could add an option to build-flash-fota.py to switch between the two versions and then detect in the build system which base Android version we're using when building the gecko-update-fota-full target.
(In reply to Casper van Donderen (Telenor) from comment #1)
> [Blocking Requested - why for this release]:
> 
> There has been a change in the AOSP source code for the FOTA updater in that
> the FormatFn after ICS went from having 4 arguments to 5 (there is a new
> mount_point var). This can be seen in the difference between FormatFn in 
> https://android.googlesource.com/platform/bootable/recovery/+/android-4.0.
> 3_r1/updater/install.c and
> https://android.googlesource.com/platform/bootable/recovery/+/android-4.4.
> 2_r1/updater/install.c
> 
> The updater-script that gets generated in
> update.zip/META-INF/com/google/android/updater-script from
> gecko-update-fota-full creates lines like format("ubifs", "UBI", "system",
> 0); which misses the new 5th mount_point argument. This updater-script seems
> to be generated by
> https://github.com/mozilla-b2g/B2G/blob/master/tools/update-tools/
> update_tools.py#L982
> 
> For devices running older Android/gonk than Jelly Bean this should stay the
> same, but for the newer devices the extra argument is mandatory.
> 
> This bug should have made it impossible for for instance Flame and Dolphin
> to receive full FOTA updates.

That's even trickier, reading AOSP KK FormatFn function: it seems the mount_point extra parameter is only really needed for an Ext4 filesystem.
Assignee: nobody → lissyx+mozillians
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S1 (1aug)
Please find attached a patch that should do the job: we detect the API level based on the PLATFORM_SDK_VERSION environment variable, and if we have lower than 19 (Kitkat) then we issue the legacy format() edify statement, otherwise we add the mount point.
Attachment #8466202 - Flags: review?(gsvelto)
Attachment #8466202 - Flags: feedback?(casper.vandonderen)
Flags: needinfo?(marshall)
Flags: needinfo?(dhylands)
Comment on attachment 8466202 [details] [review]
Issuing a proper Edify format() statement depending on API level

Step back. Looks like PLATFORM_SDK_VERSION is not exposed as I expected.
Attachment #8466202 - Flags: review?(gsvelto)
Attachment #8466202 - Flags: feedback?(casper.vandonderen)
Attachment #8466225 - Flags: review?(gsvelto)
I just tested it with exporting PLATFORM_SDK_VERSION manually.

Then when applying the update I get a bit further, the next failure is related to symlinks. There is an assert on https://github.com/lissyx/B2G/blob/b8c51efaa701c0232233c8c6e9873cb873dc0d5f/tools/update-tools/update_tools.py#L869 which tries to use /syste/bin/touch, but /system/bin/touch will be created as a symlink to 'toolbox' later in the build process, so it doesn't exist yet.
Flags: needinfo?(lissyx+mozillians)
Comment on attachment 8466202 [details] [review]
Issuing a proper Edify format() statement depending on API level

That should be ready. It seems that FormatFn signature changed indeed with API level 16 and not 19.
Attachment #8466202 - Flags: review?(gsvelto)
Flags: needinfo?(lissyx+mozillians)
According to my findinds, when we build such FOTA we are using ./tools/update-tools/bin/gonk/update-binary. This code implements FormatFn, and thus it should not be impacted by the fact that you are building for a Kitkat device itself.
Flags: needinfo?(casper.vandonderen)
Depends on: 1008239
I believe you are relying on the feature introduced in bug 1008239.
That's now okay on my Flame is I issue:
> $ TARGET_UPDATE_BINARY=out/target/product/flame/system/bin/updater ./build.sh out/target/product/flame/fota/full/update.zip

So the culprit was the old default update-binary. We either need to add TARGET_UPDATE_BINARY for the Flame or maybe cleanup the old prebuilt update-binary that we have in the system.
Flags: needinfo?(gsvelto)
Yeah, we are doing this for dolphin where a new update-binary is being passed in through the make files. So that's where the FormatFn mismatch occurs then.
Blocks: dolphin
Flags: needinfo?(casper.vandonderen)
(In reply to Casper van Donderen (Telenor) from comment #7)
> I just tested it with exporting PLATFORM_SDK_VERSION manually.
> 
> Then when applying the update I get a bit further, the next failure is
> related to symlinks. There is an assert on
> https://github.com/lissyx/B2G/blob/b8c51efaa701c0232233c8c6e9873cb873dc0d5f/
> tools/update-tools/update_tools.py#L869 which tries to use /syste/bin/touch,
> but /system/bin/touch will be created as a symlink to 'toolbox' later in the
> build process, so it doesn't exist yet.

Yes, and even after formatting it does not make a lot of sense of testing /system/bin/ since it does not exists :).

That should be fixed now.
When run './build.sh gecko-update-fota-full', update-binary can be specified by
TARGET_UPDATE_BINARY=__path_to_update-binary__

If TARGET_UPDATE_BINARY is defined, --update-bin will be passed to the build script.

You can also see here for detail.
https://github.com/mozilla-b2g/gonk-misc/blob/master/Android.mk#L430TARGET_UPDATE_BINARY
Blocks: 1048212
We are quite a lot further along now. It does seem like the build for Dolphin wants to package a symlink to a file in /data, which is not part of the FOTA package. Should that be filed as a separate bug?
Comment on attachment 8466225 [details] [review]
Exposing SDK version to FOTA builder

LGTM and thanks for having quoted the B2G_FOTA_ENV_PATH uses.
Attachment #8466225 - Flags: review?(gsvelto) → review+
(In reply to Casper van Donderen (Telenor) from comment #15)
> We are quite a lot further along now. It does seem like the build for
> Dolphin wants to package a symlink to a file in /data, which is not part of
> the FOTA package. Should that be filed as a separate bug?

Well as far as I can tell from our conversation on IRC, it's something different. Let's fix this bug for now, and investigate further in another bug.
Flags: needinfo?(gsvelto)
Comment on attachment 8466202 [details] [review]
Issuing a proper Edify format() statement depending on API level

LGTM with the added comment I mentioned on GitHub.
Attachment #8466202 - Flags: review?(gsvelto) → review+
Flags: needinfo?(lianxiang.zhou)
Alexandre -

Can your patch apply directly to 1.4 or can you do one for 1.4? Since we'll also have a device on 1.4 with kk
blocking-b2g: 1.4? → 1.4+
(In reply to Wayne Chang [:wchang] from comment #20)
> Alexandre -
> 
> Can your patch apply directly to 1.4 or can you do one for 1.4? Since we'll
> also have a device on 1.4 with kk

I don't think it should be a problem. Casper (Telenor) seems to have been able to test it without any issue.
Flags: needinfo?(casper.vandonderen)
We still need attachment 8466225 [details] [review] uplifted to v1.4. It merges cleanly. Will be needed for 2.0 as well.
Flags: needinfo?(casper.vandonderen)
Comment on attachment 8466225 [details] [review]
Exposing SDK version to FOTA builder

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: unable to build fota
[Testing completed]: tested on 1.4 (by telenor) and master
[Risk to taking this patch] (and alternatives if risky):
[String changes made]: none
Attachment #8466225 - Flags: approval-gaia-v2.0?(bbajaj)
Attachment #8466225 - Flags: approval-gaia-v1.4?(bbajaj)
Depends on: 1048819
(In reply to Alexandre LISSY :gerard-majax from comment #23)
> Comment on attachment 8466225 [details] [review]
> Exposing SDK version to FOTA builder
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #):
> [User impact] if declined: unable to build fota
> [Testing completed]: tested on 1.4 (by telenor) and master
> [Risk to taking this patch] (and alternatives if risky):
> [String changes made]: none

I forgot to add that this is needed by partner as of comment 20 and comment 22.
Comment on attachment 8466225 [details] [review]
Exposing SDK version to FOTA builder

1.4+ = auto-approval (you still need the v2.0 approval, though)
Attachment #8466225 - Flags: approval-gaia-v1.4?(bbajaj)
Flags: needinfo?(lianxiang.zhou)
Attachment #8466225 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Blocks: 1051191
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: