Merge lp:~jibel/ubuntu-cdimage/support_ubuntu_subproject into lp:ubuntu-cdimage

Proposed by Jean-Baptiste Lallement
Status: Rejected
Rejected by: Steve Langasek
Proposed branch: lp:~jibel/ubuntu-cdimage/support_ubuntu_subproject
Merge into: lp:ubuntu-cdimage
Diff against target: 12 lines (+2/-1)
1 file modified
lib/cdimage/tree.py (+2/-1)
To merge this branch: bzr merge lp:~jibel/ubuntu-cdimage/support_ubuntu_subproject
Reviewer Review Type Date Requested Status
Łukasz Zemczak Disapprove
Review via email: mp+365598@code.launchpad.net

Description of the change

Fixed publication of subproject for Ubuntu Desktop images

For ubuntu desktop images publish subproject to
ubuntu/<subproject>/<image_type>/<timestamp>
Currently when a canary (subproject of Ubuntu) image is built then published,
the publication creates an empty directory under ubuntu/daily-live.

To post a comment you must log in.
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Thanks for the merge request! I just took a quick look at it for now and I need to still think about all the consequences of the change. For now one thing that needs fixing: tests. There's one test failing right now, might need some tweaking.

To run the tests, just execute ./run-tests .

Let me get back to you with a final review tomorrow

review: Needs Fixing (tests)
1799. By Jean-Baptiste Lallement

tree.py: strip trailing slash when there is a subproject

Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Tests are passing.
On disco 5 tests are still failing but they are also failing without the patch.

Revision history for this message
Łukasz Zemczak (sil2100) wrote :
Download full text (3.3 KiB)

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 tha...

Read more...

review: Disapprove

Unmerged revisions

1799. By Jean-Baptiste Lallement

tree.py: strip trailing slash when there is a subproject

1798. By Jean-Baptiste Lallement

Support subproject publication directory for Ubuntu

For Ubuntu Desktop images, when a subproject is defined, publish the image to
cdimage.u.c under ubuntu/<subproject/<image_type>/<timestamp>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/cdimage/tree.py'
2--- lib/cdimage/tree.py 2019-03-22 15:55:28 +0000
3+++ lib/cdimage/tree.py 2019-04-09 09:58:53 +0000
4@@ -155,7 +155,8 @@
5 def project_base(self):
6 """Return the per-project base directory within this tree."""
7 if self.config.project == "ubuntu":
8- return self.directory
9+ return os.path.join(self.directory,
10+ self.config.subproject).rstrip('/')
11 else:
12 return os.path.join(self.directory, self.config.project)
13

Subscribers

People subscribed via source and target branches