Code review comment for lp:~jibel/ubuntu-cdimage/support_ubuntu_subproject

Revision history for this message
Ɓukasz Zemczak (sil2100) wrote :

Ok, after reviewing the change in more detail, I am not completely sure about this change. Let me elaborate a bit:

First of all, this will not fix the main problem we have with ubuntu-canary builds not getting published. Sure, this is a small part of the problem, but the main one is that ubuntu cdimage doesn't recognize the disco-desktop-canary-amd64.raw and other generated files because it's expecting disco-desktop-amd64 instead, without the subproject. This is why empty directories are getting created and nothing gets published to them. Getting this merged will not fix this issue, it will simply result in empty timestamp directories getting created in a separate directory.

We should fix this. I guess I can help with that.

Second thing: from what I see, this change will result in canary images getting published to canary/<image_type>/<timestamp> in the root directory instead of ubuntu/canary/... . This is because self.directory is the main cdimage root publish directory (as by default we publish ubuntu images to the root directory, which is why we have daily-live there). So the way it is right now, I don't think we'd want this merged. We'd have to make sure ubuntu is still published to the root directory but subprojects, if we want them elsewhere, getting published to some other nice place.

Last thing, possibly the most important: I have no idea how we actually want subprojects to be handled! What I mean by this: subprojects are this very strange entity that has no real meaning in the world of the cdimage tree. We never really supported publishing subproject images to separate directories. I remember in the past, during the ubuntu-touch times, when I wanted to make ubuntu-touch-custom a subproject, I have been told not to do that, since subprojects are... well... we shouldn't use subprojects as sub-projects. Or something.
So this is the tricky part really. So far the only way we were using subprojects was mostly for livefs, e.g. only like an additional flag to do something special during the livefs build of a system. I can't remember seeing a project here before which we were building separate images: one with a subproject and one without. It was always either a project's image-type had a subproject defined or not. So this is certainly something we'd have to have a design discussion about.

Now I don't know if a subproject is something we should use here if we really want the images to be published. If yes, we need to discuss where such images should go? Of course, we could 'one time special-case it' without having a final solution here, and just say: "if subproject == 'canary' -> publish to this special ubuntu-canary directory here".
But I don't know if subprojects aren't supposed to work (by principle) more like arches, meaning that if we have a subproject build for a project, we publish to the normal project directory and then forward copy the previous build non-subproject images to it. Doesn't seem to be the case though. Also, I don't like this as I don't think we have any way of saying: build this image for both subproject = "" and subproject = "canary" in one run.

Subprojects really suck in cdimage. Like, it's some weird thing that was implemented long ago and partially dropped from the design or something.
How should we proceed? Let's ask our architect! Steve?

review: Disapprove

« Back to merge proposal