Closed Bug 1403519 Opened 7 years ago Closed 6 years ago

Only run Sphinx tasks when Sphinx docs change


(Firefox Build System :: Task Configuration, task)

Not set


(Not tracked)



(Reporter: gps, Assigned: dustin)


(Blocks 1 open bug)



(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!
Better summary :)
Summary: Don't run builds, tests when Sphinx docs change → Only run Sphinx tasks when Sphinx docs change
Blocks: fastci
Assignee: nobody → dustin
(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 on attachment 8914447 [details]
Bug 1403519 - only build docs when necessary

::: browser/
(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 frontend technology.
Attachment #8914447 - Flags: review?(ted)
Attachment #8914447 - Flags: review?(ted) → review?(core-build-config-reviews)
Attachment #8914447 - Flags: review?(core-build-config-reviews) → review?(gps)
I'd prefer a WONTFIX over the silent treatment here.......
(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)
I'm behind in my queue due to work week and head cold. I'm working through things this morning...
Flags: needinfo?(gps)
Comment on attachment 8914447 [details]
Bug 1403519 - only build docs when necessary

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, we could probably change the 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+
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).
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/
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 
(sandbox) dustin@lamport ~/p/m-c (bug1403519) $ ./mach file-info schedules taskcluster/docs/kinds.rst 
Pushed by
only build docs when necessary r=gps
From the Bugzilla lint (which, I guess, traverses all'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']
Flags: needinfo?(dustin)
So the problem is in mobile/android/
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* files?)

Is there a way to say Files('**', except=['docs/**']), without explicitly listing all files and directories in this dir?
Flags: needinfo?(gps)
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)
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 = ['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..
I changed my mind -- it doesn't feel strange, so I'll take that first option.
Comment on attachment 8943050 [details]
Bug 1403519: reset SCHEDULES.exclusive if set multiple times;
Attachment #8943050 - Flags: review?(gps) → review+
The 'Bugzilla' task passed, meaning there are no more conflicts of this sort.
Pushed by
only build docs when necessary r=gps
reset SCHEDULES.exclusive if set multiple times; r=gps
Pushed by
only build docs when necessary r=gps
reset SCHEDULES.exclusive if set multiple times; r=gps
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Flags: needinfo?(dustin)
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.