Closed Bug 1145583 Opened 9 years ago Closed 9 years ago

Add builder for lightsaber

Categories

(Release Engineering :: Applications: MozharnessCore, defect, P1)

All
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wcosta, Assigned: wcosta)

References

Details

User Story

We need to add one more setup step before build b2g images for lightsaber. See bug 1134226 comment 1 for details.

Attachments

(1 file, 1 obsolete file)

      No description provided.
Status: NEW → ASSIGNED
Summary: Add builder for aries device → Add builder for lightsabe
Summary: Add builder for lightsabe → Add builder for lightsaber
User Story: (updated)
Attached file MozReview Request: bz://1145583/wcosta (obsolete) —
/r/6563 - Bug 1145583: Add lightsaber builder script to mozharness.

Pull down this commit:

hg pull -r 28bff6246cdef700258aac267009a532280f9f86 https://reviewboard-hg.mozilla.org/build-mozharness
Attachment #8587600 - Flags: review?(bhearsum)
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

https://reviewboard.mozilla.org/r/6561/#review5509

I'm really not sure I'm the right person to review this. Here's some comments, but someone that understands what Lightsaber is and the intent should probably have a look too.

::: configs/b2g/taskcluster-lightsaber-nightly.py
(Diff revision 1)
> +            "upload_remote_symlink": "/srv/ftp/pub/mozilla.org/b2g/%(target)s/%(branch)s-%(target)s/latest",

Is /srv/ftp/pub the right path here? I don't know where these files are uploading, but it doesn't match a valid path on ftp.mozilla.org.

::: configs/b2g/taskcluster-lightsaber-nightly.py
(Diff revision 1)
> +#!/usr/bin/env python

I don't really understand this config. Based on reading https://github.com/fxos/lightsaber, it seems that Lightsaber is an alternative build system for FirefoxOS. If that's the case, why does it need a Mozharness config? Is this just a base that will never be used by itself?

::: scripts/b2g_lightsaber.py
(Diff revision 1)
> +from b2g_build import B2GBuild

Seems like it B2GBuild is due for a refactor into a Mixin, or something. This will work for now, but I don't think any guarantees are made about imports like this continuing to work (and there's certainly no guarantees about the methods in this class not changing). jlund might have some design thoughts here.

::: scripts/b2g_lightsaber.py
(Diff revision 1)
> +        self.run_command('rm -rf {}'.format(lightsaber_dir))

Mozharness provides wrappers for things like this that do much better logging, you should be using it instead of shelling out: https://github.com/mozilla/build-mozharness/blob/master/mozharness/base/script.py#L82

::: configs/b2g/taskcluster-lightsaber-nightly.py
(Diff revision 1)
> +            "upload_remote_host": "",

Actually, it looks like you're not uploading anything at all? If that's the case, why have an "upload" section at all?

Maybe I'm missing the big picture, but this config just doesn't make sense to me at all.
Attachment #8587600 - Flags: review?(bhearsum)
https://reviewboard.mozilla.org/r/6561/#review5587

> I don't really understand this config. Based on reading https://github.com/fxos/lightsaber, it seems that Lightsaber is an alternative build system for FirefoxOS. If that's the case, why does it need a Mozharness config? Is this just a base that will never be used by itself?

Meanwhile, lightsaber will have its own update channel and the 'checkout-lightsaber' action. Lightsaber will probably merge into main gaia in the future, so keeping things in separate files will make cleanup easier.

> Is /srv/ftp/pub the right path here? I don't know where these files are uploading, but it doesn't match a valid path on ftp.mozilla.org.

This is not actually used in taskcluster, it was inherited from taskcluster-phone-nightly (I am actually don't remember why I kept this config on that file either).

> Actually, it looks like you're not uploading anything at all? If that's the case, why have an "upload" section at all?
> 
> Maybe I'm missing the big picture, but this config just doesn't make sense to me at all.

hrm I think I can knock this out.

> Seems like it B2GBuild is due for a refactor into a Mixin, or something. This will work for now, but I don't think any guarantees are made about imports like this continuing to work (and there's certainly no guarantees about the methods in this class not changing). jlund might have some design thoughts here.

Lightsaber is just b2g build with one extra step, which is build lightsaber and replace gaia with it, but I think this won't last, soon lightsaber will merge into main gaia and this will not be needed anymore, so I thought would be better to keep things in a separate file to make future cleanup easier.

> Mozharness provides wrappers for things like this that do much better logging, you should be using it instead of shelling out: https://github.com/mozilla/build-mozharness/blob/master/mozharness/base/script.py#L82

Ops, didn't find this function, I am going to replace the call.
Attachment #8587600 - Flags: review?(jlund)
Attachment #8587600 - Flags: review?(bhearsum)
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

/r/6563 - Bug 1145583: Add lightsaber builder script to mozharness.

Pull down this commit:

hg pull -r 551350f53bca6b0c441a6121aaa3af9b5aad913f https://reviewboard-hg.mozilla.org/build-mozharness
Comments addressed and some more fixes applied. Also I added :jlund as a review.
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

https://reviewboard.mozilla.org/r/6561/#review5743

I guess this is fine. I'm not particularly fussy on it if it's merging back in with b2g_build.py and friends in the near future.
Attachment #8587600 - Flags: review?(bhearsum) → review+
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

https://reviewboard.mozilla.org/r/6561/#review5825

agree with ben that as a temporary thing (nice call stripping this out into its own script), this shouldn't be a big issue. I'll trust in you to determine what is needed for lightsaber but as far as mozharness idioms are concerned, looks great!

::: scripts/b2g_lightsaber.py
(Diff revision 2)
> +    all_actions = [

do you know which actions we will never need for b2g_lightsaber.py runs?

I'm guessing a few of these deprecated ones can be removed from the list.

::: scripts/b2g_lightsaber.py
(Diff revision 2)
> +        self.vcs_checkout_repos([lightsaber_repo])

would tc_vcs be appropriate here or should we stick with gittool/hgtool?
Attachment #8587600 - Flags: review?(jlund)
https://reviewboard.mozilla.org/r/6561/#review5829

> do you know which actions we will never need for b2g_lightsaber.py runs?
> 
> I'm guessing a few of these deprecated ones can be removed from the list.

hrm I am not intimate of all actions, I just copied from b2g_build. The actual actions are configured in taskcluster-lightsaber-nightly.py. I will strip out those I am sure are unnecessary.

> would tc_vcs be appropriate here or should we stick with gittool/hgtool?

The main advantage of tc_vcs is repo caching, but lightsaber it self is a small repo, so this is not a concern. tc_vcs also works with both git and hg, but lightsaber won't have a hg mirror for the forseeable future.
Attachment #8587600 - Flags: review+ → review?(jlund)
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

/r/6563 - Bug 1145583: Add lightsaber builder script to mozharness.

Pull down this commit:

hg pull -r eb6bcbbc992436857e9d60be0085e3d744075739 https://reviewboard-hg.mozilla.org/build-mozharness
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

https://reviewboard.mozilla.org/r/6561/#review5851

Ship It!
Attachment #8587600 - Flags: review?(jlund) → review+
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

/r/6563 - Bug 1145583: Typoe in vcs info. r=pmoore

Pull down this commit:

hg pull -r 39879dfcbd0a65f7c38ad206d9add5466ee14006 https://reviewboard-hg.mozilla.org/build-mozharness
Attachment #8587600 - Flags: review+ → review?(pmoore)
Comment on attachment 8587600 [details]
MozReview Request: bz://1145583/wcosta

/r/6563 - Bug 1145583: Use gittool to checkout lightsaber. r=pmoore

Pull down this commit:

hg pull -r 27b25791af9b3167c874978cca6a234094691f82 https://reviewboard-hg.mozilla.org/build-mozharness
Hey Wander,

I can't quite work out how to post comments in reviewboard, so I'll do it here. Have you had any successful runs on e.g. try/cypress using gittool? Could you point me to the treeherder job urls?

Thanks!
Pete
Flags: needinfo?(wcosta)
Attachment #8587600 - Flags: review?(pmoore) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #8587600 - Attachment is obsolete: true
Attachment #8619829 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: