Closed
Bug 1325936
Opened 7 years ago
Closed 7 years ago
Trigger SpiderMonkey builds when there are changes to libs which SM builds depend on
Categories
(Release Engineering :: General, defect)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1365763
People
(Reporter: xidorn, Assigned: sfink)
Details
Attachments
(1 file)
2.19 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•7 years 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/** - configure.py - dom/bindings/** - intl/icu/** - js/moz.configure - layout/tools/reftest/reftest/** - Makefile.in - media/webrtc/trunk/tools/gyp/** - memory/** - modules/fdlibm/** - modules/zlib/src/** - moz.build - moz.configure - python/** - taskcluster/moz.build - testing/mozbase/** - test.mozbuild - toolkit/mozapps/installer/package-name.mk - toolkit/mozapps/installer/upload-files.mk 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)
Assignee | ||
Comment 2•7 years 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•7 years 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)
Assignee | ||
Comment 4•7 years ago
|
||
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)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
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/moz.build That doesn't seem necessary. @@ +60,5 @@ > files-changed: > - build/** > - config/** > - configure.py > + - 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+
Updated•7 years ago
|
Component: Tools → General
Assignee | ||
Updated•7 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•