Merge lp://staging/~achiang/ubuntu-seeds/ubuntu-touch.utopic into lp://staging/~ubuntu-core-dev/ubuntu-seeds/ubuntu-touch.utopic

[I can't speak for the package-level details; this review is just
regarding the general seed structure.]

On Fri, Jun 27, 2014 at 01:01:21AM -0000, Alex Chiang wrote:
> I also did not expect that I would have to manually add
> ubuntu-touch-core and ubuntu-touch-android as explicit Depends in
> debian/control...

Historically, we intentionally haven't made the metapackages depend on
each other, because it makes it excessively painful when you want to
vary from (say) ubuntu-standard but you're quite happy with the contents
of ubuntu-desktop. We usually deal with ensuring that we have all the
right metapackages in livecd-rootfs or similar instead.

However, in the particular case of touch, it's probably OK to do this
using explicit Depends.

> But I think germinate tries to be smart about overlapping packages or something, and that is why the debdiff is huge:
>
> http://pastebin.ubuntu.com/7708769/

This all seems as expected. If you have packages in an inner seed (or
its dependency-expansion), then they don't also need to show up in the
expansion of an outer seed. That's the whole point of seed inheritance.

> In any case, feedback is appreciated.

Please let's not have a seed called "core", anywhere. Vast confusion
with the existing core package set will result. Call it "touch-core",
perhaps. Ditto android -> touch-android. You're explicitly specifying
the task and metapackage names, so changing the seed names will be
harmless.

> -touch: minimal sdk-libs
> -desktop: minimal sdk-libs
> +android: minimal sdk-libs
> +core: minimal sdk-libs
> +touch: minimal sdk-libs android core
> +desktop: minimal sdk-libs core

There is no need to repeat the transitive inheritance, so this is
sufficient:

  touch: touch-android touch-core
  desktop: touch-core

Is it possible that touch-android should inherit from touch-core? If
you're never going to use touch-android without touch-core, then that
would save a lot of duplication of entries in Task fields.

> +Task-Per-Derivative: 1
> +Task-Section: user
> +Task-Description: Ubuntu touch android
> +Task-Extended-Description: This task provides the Android-to-Ubuntu bridge
> +Task-Key: ubuntu-touch-android
> +Task-Name: ubuntu-touch-android
> +Task-Metapackage: ubuntu-touch-android

I'm not entirely sure why we bother to create a task for the current
touch seed at all; as far as I can see it isn't used, so it's just bloat
in the Packages file for no reason. However, maybe I've missed
something, so it's probably OK to add tasks for these. I might go round
and clean this up later.

that split will not work in many areas due to code having dependencies on android bits so you wouldnt be able to run -core standalone at all (there are apps in core that user the android property system (see mtp-server) or talk to the android layer via sockets, etc etc). while i agree we should have a finer grained seed setup to make running bits and pieces without android possible i dont see why this has to happen before RTM.
if we actually want to make use of it there will be a lot of code changes and additional abstraction needed that nobody will be able to invest time into right now.
could this wait til after RTM ?

Colin, thanks for the review comments. I have made some changes in r210.

However, I now get behavior I don't entirely understand. As you can see in STRUCTURE, touch now inherits from both touch-android and touch-core. I also realized that no one would ever install touch-android as a standalone thing, so there is no need for it to be a metapackage. Therefore, I removed that stanza from d/control.

When I build the metapackage, I get the below debdiff. Note that neither the ubuntu-touch nor ubuntu-desktop-next stanzas appear in the debdiff; this is because I removed the explicit Depends on ubuntu-touch-core.

http://pastebin.ubuntu.com/7712228/

Upon inspecting the binary package though, I see:

http://pastebin.ubuntu.com/7712247/

None of the packages appear in the Depends: line (as I would have expected from seed inheritance; see line 12 of diff below). Nor does ubuntu-touch-core appear as a Depends (but that is expected, since I did not list it as an explicit Depends).

So what am I doing wrong?

###

Oliver, since touch-core now inherits from touch-android, I think that does make it possible to run touch-core standalone.

Regarding the mtp-server package, it was seeded into 'desktop-next' before I even touched it. Is that an issue?

Regarding waiting until post-RTM, that is not possible, as we have other projects (some commercial, some internal) in-flight that will be built upon this split. So we need to parallelize.

At the end of the day, this change *must* be transparent to the phone image; if it is not, that is a bug and must be fixed. I hesitate to say this is "just" refactoring, but the changes should be straightforward enough that we can inspect the diffs closely and reason about them in our minds, along with careful testing to ensure no regressions for phone.

Thanks.

The behaviour for touch-core seems fine. For touch-android, though, I see two mistakes:

 * You should not list touch-android in ubuntu-touch-meta/update.cfg if you don't want to build a metapackage for it.
 * You need to help out germinate-update-metapackage a little bit to tell it that it needs to include the entries from the touch-android seed in the metapackage corresponding to the touch seed (that is, ubuntu-touch). You can achieve this by putting "Task-Seeds: touch-android" in the header paragraph of the touch seed. Compare the desktop seed in the ubuntu.utopic seeds, which does a similar trick with desktop-common.

This should make the metapackage contents work out better. Let me know if it doesn't.

Colin, thank you for the comments. I made changes as you suggested, re-merged with upstream, and verified that adding the Task-Seeds: line to the header paragraph produced the desired output in the metapackage (ubuntu-touch-meta).

Oliver,

The utopic archive currently has ubuntu-touch-meta (1.163).

After my changes to the seed and rebuilding the metapackage, I produced this debdiff:

http://pastebin.ubuntu.com/7762568/

As you can see, the only real change was that the debug tools have been added to the ubuntu-desktop-next metapackage, as they are included in the touch-core seed. I believe this is harmless at worst and advantageous at best.

I also built the binary packages locally, and inspected the output, which is quite clean:

http://pastebin.ubuntu.com/7762576/

At this point, I believe everything looks sane, and the branch is ready for merge and the new metapackage is ready for upload.

Can a core-dev merge this in and do the upload for me?

Thanks.

To be complete, I also built and tested in an armhf chroot.

http://pastebin.ubuntu.com/7762724/

And here are the contents of one of the *.info files:

http://pastebin.ubuntu.com/7762736/

Note that the Android packages did indeed make it into the metapackage.

Thanks. I'm basing this review on your diff. The concept is sound and the only problem I notice is that we don't want to have ubuntu-touch-session on desktop since it overrides some conditions from the packages themselves. We decided to prefer an approach of figuring out how to integrate these changes into the packages themselves instead. So if you could move that into touch then I'll be happy.

Thank you, Iain. I have (again, re-)merged with upstream, and moved ubuntu-touch-session out of touch-core and into touch.

This is the debdiff:
http://pastebin.ubuntu.com/7799973/

This is inspecting the binary package (amd64):
http://pastebin.ubuntu.com/7800006/

This is inspecting the binary package (armhf):
http://pastebin.ubuntu.com/7800023/

I believe I've addressed all the comments now, and think this is ready to land. I'll need some help with that from a core-dev.

Please let me know.

Thanks all!

I've tested the branch and confirmed that with current meta it doesn't change any of the ubuntu-touch .deb deps; it only changes the desktop-next packages and the changes are ok (adding these debug / utils packages; we'll probably want to drop them eventually, but not a big deal for now).

The only thing which stopped me from merging is the "core" name when it actually lists a bunch of things that are common to desktop next / touch phone / images with UI but wouldn't necessarily be suitable on an image for UI less devices.

Perhaps the name core is confusing there and we should use touch-common or such?

Cheers,

To get this landed, I am +1 to s/core/common/ both in the developer-only
seed names and the user-visible metapackage

Thanks.

On Thu, Jul 17, 2014 at 12:49 PM, Loïc Minier <email address hidden>
wrote:

> I've tested the branch and confirmed that with current meta it doesn't
> change any of the ubuntu-touch .deb deps; it only changes the desktop-next
> packages and the changes are ok (adding these debug / utils packages; we'll
> probably want to drop them eventually, but not a big deal for now).
>
> The only thing which stopped me from merging is the "core" name when it
> actually lists a bunch of things that are common to desktop next / touch
> phone / images with UI but wouldn't necessarily be suitable on an image for
> UI less devices.
>
> Perhaps the name core is confusing there and we should use touch-common or
> such?
>
> Cheers,
> --
>
> https://code.launchpad.net/~achiang/ubuntu-seeds/ubuntu-touch.utopic/+merge/224738
> You are the owner of lp:~achiang/ubuntu-seeds/ubuntu-touch.utopic.
>