Only run Sphinx tasks when Sphinx docs change

RESOLVED FIXED in mozilla59

Status

task
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: gps, Assigned: dustin)

Tracking

(Blocks 2 bugs)

unspecified
mozilla59
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
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

2 years ago
Better summary :)
Summary: Don't run builds, tests when Sphinx docs change → Only run Sphinx tasks when Sphinx docs change
Reporter

Updated

2 years ago
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 hidden (mozreview-request)
Assignee

Comment 4

2 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.
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)
Reporter

Comment 7

2 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

2 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+
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)
Comment hidden (mozreview-request)
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
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']
Flags: needinfo?(dustin)
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

Last year
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.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..
I changed my mind -- it doesn't feel strange, so I'll take that first option.
Reporter

Comment 22

Last year
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+
The 'Bugzilla' task passed, meaning there are no more conflicts of this sort.

Comment 25

Last year
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 hidden (mozreview-request)

Comment 28

Last year
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

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/0f0395687d36
https://hg.mozilla.org/mozilla-central/rev/ce4e64aa7ea0
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Flags: needinfo?(dustin)

Updated

Last year
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.