Closed
Bug 1403519
Opened 7 years ago
Closed 7 years ago
Only run Sphinx tasks when Sphinx docs change
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla59
People
(Reporter: gps, Assigned: dustin)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
The Sphinx docs are essentially independent from everything else in mozilla-central. They don't impact builds or tests. We should be able to change scheduling to reflect this exclusive nature of the Sphinx tasks.
This may be a good test for SCHEDULES because the Sphinx docs are spread out all over the repo!
Reporter | ||
Comment 1•7 years ago
|
||
Better summary :)
Summary: Don't run builds, tests when Sphinx docs change → Only run Sphinx tasks when Sphinx docs change
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dustin
Assignee | ||
Comment 2•7 years ago
|
||
(I can work on this but probably in the "week or two" range. If it's more urgent, or you've already started, please feel free to reassign)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8914447 [details]
Bug 1403519 - only build docs when necessary
https://reviewboard.mozilla.org/r/185768/#review190692
::: browser/moz.build:12
(Diff revision 1)
> CONFIGURE_SUBST_FILES += ['installer/Makefile']
>
> SPHINX_TREES['browser'] = 'docs'
>
> +with Files('docs/**'):
> + SCHEDULES.exclusive = ['docs']
I dislike that these need to be kept in sync. I'm open to better ways of handling this! I'm still very new to the fancy moz.build frontend technology.
Assignee | ||
Updated•7 years ago
|
Attachment #8914447 -
Flags: review?(ted)
Assignee | ||
Updated•7 years ago
|
Attachment #8914447 -
Flags: review?(ted) → review?(core-build-config-reviews)
Updated•7 years ago
|
Attachment #8914447 -
Flags: review?(core-build-config-reviews) → review?(gps)
Assignee | ||
Comment 5•7 years ago
|
||
I'd prefer a WONTFIX over the silent treatment here.......
Comment 6•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> I'd prefer a WONTFIX over the silent treatment here.......
I don't understand the SCHEDULES stuff so I'm not going to review, but this shouldn't be WONTFIX. gps, bump this up your queue!
Flags: needinfo?(gps)
Reporter | ||
Comment 7•7 years ago
|
||
I'm behind in my queue due to work week and head cold. I'm working through things this morning...
Flags: needinfo?(gps)
Reporter | ||
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8914447 [details]
Bug 1403519 - only build docs when necessary
https://reviewboard.mozilla.org/r/185768/#review204978
This seems reasonable.
I think there is room to mark ``**/*.py`` as inclusive as well. But it isn't strictly necessary.
Also, since we define the directories containing Sphinx docs in moz.build, we could probably change the moz.build layer to automagically emit equivalent Files() SCHEDULES data structures when those are defined. That would help avoid redundancy and ensure everything stays in sync. That's definitely follow-up territory though.
Attachment #8914447 -
Flags: review?(gps) → review+
Assignee | ||
Comment 9•7 years ago
|
||
I think this can land, but I want to wait until bug 1426254 and bug 1426255 land, then bug 1403322, then check that I have all of the semantics right. This is the first component that is used both exclusively (for the .rst) and inclusively (for the .py).
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•7 years ago
|
||
This is a rebase so mozreview will show lots of junk. I discovered a few more SPHINX_TREES and added corresponding `Files(..): SCHEDULES.exclusive = ['docs']` and otherwise didn't change anything. So I'll consider the r+ carried forward.
Some checks:
# some random file does not build docs..
(sandbox) dustin@lamport ~/p/m-c (bug1403519) $ ./mach file-info schedules LEGAL
robocop, geckoview, reftest, web-platform-tests, macosx, talos, cppunittest, awsy, mochitest, web-platform-tests-reftests, web-platform-tests-wdspec, mozmill, telemetry-tests-client, linux, windows, marionette, firefox-ui, gtest, xpcshell, android, xpcshell-coverage
# Python change adds docs to the list of exclusive components..
(sandbox) dustin@lamport ~/p/m-c (bug1403519) $ ./mach file-info schedules python/mach_commands.py
robocop, mozmill, mochitest, marionette, web-platform-tests, docs, macosx, talos, cppunittest, awsy, geckoview, web-platform-tests-reftests, web-platform-tests-wdspec, linux, telemetry-tests-client, firefox-ui, reftest, windows, gtest, xpcshell, android, xpcshell-coverage
# Docs files only schedule docs tasks...
(sandbox) dustin@lamport ~/p/m-c (bug1403519) $ ./mach file-info schedules build/docs/locales.rst
docs
(sandbox) dustin@lamport ~/p/m-c (bug1403519) $ ./mach file-info schedules taskcluster/docs/kinds.rst
docs
Comment 14•7 years ago
|
||
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d539e4a02bd
only build docs when necessary r=gps
Comment 15•7 years ago
|
||
Backed out changeset 6d539e4a02bd (bug 1403519) for lint failures
Push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=6d539e4a02bd5a819c798c16ffc2fdbe9fb5fab2&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&filter-resultStatus=success&selectedJob=155724623
Log failure: https://treeherder.mozilla.org/logviewer.html#?job_id=155724623&repo=autoland&lineNumber=216
Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0524b54f7f9dc0568a55b5d70fef4180d4cb1838&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&filter-resultStatus=success
Flags: needinfo?(dustin)
Assignee | ||
Comment 16•7 years ago
|
||
From the Bugzilla lint (which, I guess, traverses all moz.build's?)
[task 2018-01-11T20:20:44.572Z] ValueError: Two Files sections have set SCHEDULES.exclusive to differentvalues; these cannot be combined: [u'android'] and [u'docs']
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dustin)
Assignee | ||
Comment 17•7 years ago
|
||
So the problem is in mobile/android/moz.build:
---
with Files('**'):
BUG_COMPONENT = ('Firefox for Android', 'Build Config & IDE Support')
SCHEDULES.exclusive = ['android']
...
with Files('docs/**'):
SCHEDULES.exclusive = ['docs']
---
I tried re-ordering them and using FINALLY, but it didn't seem to help (I think FINALLY is for inheritance *between* moz.build files?)
Is there a way to say Files('**', except=['docs/**']), without explicitly listing all files and directories in this dir?
Flags: needinfo?(gps)
Reporter | ||
Comment 18•7 years ago
|
||
We don't have a mechanism to declare exclusions, no. What we can do is overwrite values since last write generally wins. Although IIRC SCHEDULES.exclusive might be implemented in a way that makes that difficult?
Flags: needinfo?(gps)
Assignee | ||
Comment 19•7 years ago
|
||
I suppose it's posisble to make that work for SCHEDULES.exclusive, but it feels strange for later declarations of that variable to override earlier, while SCHEDULES.inclusive doesn't work that way. I could do something like
SCHEDULES.exclusive.reset()
SCHEDULES.exclusive = ['docs']
to be more explicit, but that starts to look functional and not like a config file. I think the least adventurous option is just to list the files in mobile/android..
Assignee | ||
Comment 20•7 years ago
|
||
I changed my mind -- it doesn't feel strange, so I'll take that first option.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8943050 [details]
Bug 1403519: reset SCHEDULES.exclusive if set multiple times;
https://reviewboard.mozilla.org/r/213318/#review219076
Attachment #8943050 -
Flags: review?(gps) → review+
Assignee | ||
Comment 23•7 years ago
|
||
Assignee | ||
Comment 24•7 years ago
|
||
The 'Bugzilla' task passed, meaning there are no more conflicts of this sort.
Comment 25•7 years ago
|
||
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b293bcd3fc7
only build docs when necessary r=gps
https://hg.mozilla.org/integration/autoland/rev/f53f8adcd578
reset SCHEDULES.exclusive if set multiple times; r=gps
Comment 26•7 years ago
|
||
Backed out 2 changesets (bug 1403519) for bustage on \python\mozbuild\mozbuild\test\frontend\test_reader.py on a CLOSED TREE
Push that was backed out:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f53f8adcd578c38a432e58f482a1c486f2c2dd55&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running
https://treeherder.mozilla.org/logviewer.html#?job_id=156779132&repo=autoland
https://hg.mozilla.org/integration/autoland/rev/46f721d7988bcc0bd1cbf22854d40abd0cf1c2b2
:dustin Could you please take a look?
Flags: needinfo?(dustin)
Comment hidden (mozreview-request) |
Comment 28•7 years ago
|
||
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0f0395687d36
only build docs when necessary r=gps
https://hg.mozilla.org/integration/autoland/rev/ce4e64aa7ea0
reset SCHEDULES.exclusive if set multiple times; r=gps
Comment 29•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0f0395687d36
https://hg.mozilla.org/mozilla-central/rev/ce4e64aa7ea0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dustin)
Updated•7 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•