Trigger SpiderMonkey builds when there are changes to libs which SM builds depend on



Release Engineering
a year ago
7 months ago


(Reporter: xidorn, Assigned: sfink)


Firefox Tracking Flags

(Not tracked)



(1 attachment)

Per bug 1238452 comment 0, currently SpiderMonkey builds are only triggered automatically when anything in js/src is changed. However, changes to libs it depends on can also break SM builds, so they should be included as well.

According to bug 1238452 comment 4, that should probably be mfbt, nspr, and mozglue.

Comment 1

a year ago
I don't know if we really need dependencies on *everything* spidermonkey uses. We have that list for the SM(pkg) job, and it includes a number of things that don't feel like they're worth it to me. We have to weigh the added resources against the probability of catching something that is not going to get caught in some other automation job.

But I'm not sure how to make the call for all of this stuff. mfbt obviously should be in. nsprpub ideally wouldn't be, but we haven't finished removing the nspr dependency, so I'd leave it in. mozglue is good. But what about the rest of the list:

                - build/**
                - config/**
                - dom/bindings/**
                - intl/icu/**
                - js/moz.configure
                - layout/tools/reftest/reftest/**
                - media/webrtc/trunk/tools/gyp/**
                - memory/**
                - modules/fdlibm/**
                - modules/zlib/src/**
                - moz.configure
                - python/**
                - taskcluster/
                - testing/mozbase/**
                - test.mozbuild
                - toolkit/mozapps/installer/
                - toolkit/mozapps/installer/

Is it a good idea to kick off the full set of spidermonkey jobs for any change to build/** config/** or python/**? It might be; I don't know how often those change. I'm inclined to say we should just use the full SM(pkg) files-changed list for all of the spidermonkey jobs to avoid thinking about it, but I'm not sure how many extra builds that would be. I know that *some* build/** or config/** changes *should* get spidermonkey builds, so using the full list is feeling like the simplest and most reasonable approach to me right now.

needinfo? glandium to see if he'd *like* to have the spidermonkey builds kicked off for the above sorts of build system changes. If he's having to manually touch js/src files right now, that'd be another argument in favor.
Flags: needinfo?(mh+mozilla)

Comment 2

a year ago
(Sorry; looks like I had one opinion at the beginning of that comment and a different one at the end. Which is exactly the case -- I changed my mind while writing it.)

Comment 3

11 months ago
> If he's having to manually touch js/src files right now, that'd be another argument in favor.

I do, and it's rather annoying when I need to, but on the other hand, it's also not that frequent that I need to. So I don't really have a strong opinion. It would surely be nicer if one didn't need to think too much about it.

>                - dom/bindings/**
>                - layout/tools/reftest/reftest/**
>                - media/webrtc/trunk/tools/gyp/**

Those don't actually impact the js build at all. gyp shouldn't be required at all (it should only be imported when gyp is actually used, which building js doesn't). The others are an unfortunate consequence of python imports. But for none of them does changes there affect js.
Flags: needinfo?(mh+mozilla)

Comment 4

8 months ago
Created attachment 8858445 [details] [diff] [review]
Run SM jobs whenever just anything that affects them changes

There was another failure that would have been caught had mfbt/ changes triggers SM build, so I really ought to land this. r?glandium simply because he's thought about this before. Some of these may be a little too conservative, but right now I"m feeling like it's easier to not think.
Attachment #8858445 - Flags: review?(mh+mozilla)


8 months ago
Assignee: nobody → sphink
Comment on attachment 8858445 [details] [diff] [review]
Run SM jobs whenever just anything that affects them changes

Review of attachment 8858445 [details] [diff] [review]:

::: taskcluster/ci/spidermonkey/kind.yml
@@ +27,5 @@
>          files-changed:
>              # any when.files-changed specified below in a job will be
>              # appended to this list
> +            - build/**
> +            - config/**

These two are broad, but meh.

@@ +46,2 @@
>              - taskcluster/ci/spidermonkey/kind.yml
> +            - taskcluster/

That doesn't seem necessary.

@@ +60,5 @@
>              files-changed:
>                  - build/**
>                  - config/**
>                  -
> +                - dom/bindings/mozwebidlcodegen

really, I think we should remove this dependency, it's wrong. Same with webrtc's gyp.
Attachment #8858445 - Flags: review?(mh+mozilla) → review+
Component: Tools → General
Product: Release Engineering → Release Engineering


7 months ago
Last Resolved: 7 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1365763
You need to log in before you can comment on or make changes to this bug.