search

Linaro / poplar-tools

4 stars 16 forks source link

poplar_recovery_builder.sh: Enable partial builds #11

Closed vchong closed 7 years ago

vchong commented 7 years ago

Signed-off-by: Victor Chong victor.chong@linaro.org

alexelder commented 7 years ago

This is great! I've had it on my to-do list for a long time, and I like how you did it. I scanned it and it all looks very good to me. My one comment is that maybe "rootfs" should be "filesystems" or "data" or something, because it's really saving everything that's data on the target, not just the root file system.

I'll leave it to Shawn to pull this when he's ready.

vchong commented 7 years ago

@alexelder Thanks! The weird thing about this is poplar_recovery_builder.sh rootfs.tar.gz {layout,loader,boot}, so I reversed the args position in https://github.com/Linaro/poplar-tools/pull/13 and make the rootfs tar file arg conditionally optional. Can you please see if that's better? In it I also used system (same as hikey*) instead of rootfs. If this is preferred then I can also change this one to use system instead.

vchong commented 7 years ago

@alexelder Also, boot partition dependency on loader is due to https://github.com/Linaro/poplar-tools/blob/latest/poplar_recovery_builder.sh#L584. What is this copy for?

alexelder commented 7 years ago

On 10/29/2017 10:43 AM, vchong wrote:

@alexelder https://github.com/alexelder Thanks! The weird thing about this is |poplar_recovery_builder.sh rootfs.tar.gz {layout,loader,boot}| looks weird, so I reversed the args position in #13 https://github.com/Linaro/poplar-tools/pull/13 and make the rootfs tar file arg conditionally optional. Can you please see if that's better? In it I also used |system| (same as hikey*) instead of |rootfs|. If this is preferred then I can also change this one to use |system| instead.

I suck at Gerrit and GitHub so I'm just going to respond by e-mail.

I agree that reordering the arguments is better--put the optional file system image argument at the end. And I think "system" to mean "whatever file systems get installed" (which covers the Android case better) is better than "rootfs".

                -Alex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Linaro/poplar-tools/pull/11#issuecomment-340271553, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-5Y5j3mdcXzkBQVaA9_IkF71EtaY-Jks5sxJ0QgaJpZM4QKPF0.

alexelder commented 7 years ago

On 10/29/2017 10:54 AM, vchong wrote:

@alexelder https://github.com/alexelder Also, boot partition dependency on loader is due to https://github.com/Linaro/poplar-tools/blob/latest/poplar_recovery_builder.sh#L584. What is this copy for?

The copy can go away. I put it there as a way to have the current "loader.bin" image that's sitting in partition 1 be resident in a file. But it doesn't really help anything--you can always get the real one from the partition anyway. So you can feel free to remove that copy and eliminate that dependency as far as I'm concerned.

You're not the first person who's asked about that...

                -Alex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Linaro/poplar-tools/pull/11#issuecomment-340272362, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-5YyVOLKTdhbxDi9ghV9eYeHBuYC_rks5sxJ-rgaJpZM4QKPF0.

vchong commented 7 years ago

Email response is fine. :) Thanks! I've removed the dependency on #13. Closing this as replaced by #13.