ENH: Explain rebuilds

[larsoner]

Subject: Explain why rebuilds are being triggered

Feature or Bugfix

• Feature

Purpose

Give reasons why a full build is being done.

Detail

On long sphinx builds, full rebuilds are painful. With this PR, the rebuild is explained in logging:

Running Sphinx v3.0.0+/2c826795b
loading pickled environment... Config changed (item numpydoc_xref_aliases)... done
[autosummary] generating autosummary for:
...
updating environment: (build configuration changed) 663 added, 0 changed, 0 removed

The "Config changed" and parenthetical for updating environment are new.

[codecov]

Codecov Report

Merging #6419 into 2.0 will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              2.0    #6419      +/-   ##
==========================================
+ Coverage   83.73%   83.74%   +<.01%     
==========================================
  Files         271      271              
  Lines       40977    40999      +22     
  Branches     6001     6002       +1     
==========================================
+ Hits        34314    34336      +22     
  Misses       5335     5335              
  Partials     1328     1328

Impacted Files	Coverage Δ
sphinx/builders/__init__.py	83.18% <100%> (ø)	⬆️
tests/test_environment.py	100% <100%> (ø)	⬆️
sphinx/environment/__init__.py	77.77% <100%> (+0.46%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbc11a0...c0e46ad. Read the comment docs.

[tk0miya]

Sorry for late reviewing.

Once, reading phase were consisted of both builder and environment. And it was hard to understand how building process works.

After some refactoring, builder only goes reading phase now. The environment object behaves like a database. According to the design, environment does not show any console messages (there are some exceptions, I know). So I'd like to move these implementations to Builder.read().

I think outputting messages across components is hard to understand. Also hard to control. Especially, this PR outputs partial some messages. It makes hard to call this method when you like. For example, creating another instance of environment will output message. Is this expected behavior?

[larsoner]

According to the design, environment does not show any console messages (there are some exceptions, I know). So I'd like to move these implementations to Builder.read().

I can look to see how to do this. But it will probably require creating a new env property, is this okay? If so, any preferred name for it?

For example, creating another instance of environment will output message. Is this expected behavior?

This part was intentional, yes. But I thought it was better to always explain why there is a rebuild (current behavior) than to only sometimes explain why there is a rebuild (i.e., if I got rid of the new environment message).

[tk0miya]

I can look to see how to do this. But it will probably require creating a new env property, is this okay? If so, any preferred name for it?

config_diff or old_config? I don't have good idea for it because I still don't understand why is the changed item needed.

This part was intentional, yes. But I thought it was better to always explain why there is a rebuild (current behavior) than to only sometimes explain why there is a rebuild (i.e., if I got rid of the new environment message).

I think creating an instance of environment does not related with rebuild always. In future, it might be helpful for parallel build. This message adds restriction for Sphinx to create only one environment during build.
(We don't have any plan to do that at this moment. But I'd not like to add new restriction)

[larsoner]

Okay I've refactored the code so that the printing is done in Builder.read and for:

• repeated/unchanged config, says nothing new (just says which are actually changed):

  updating environment: 0 added, 1 changed, 0 removed
  

• new config: says its a new config

  updating environment: [new config] 671 added, 0 changed, 0 removed
  

• changed extensions:

  updating environment: [extensions changed ('gen_commands')] 671 added, 0 changed, 0 removed
  

• changed config['env']:

  updating environment: [config changed ('numpydoc_xref_aliases')] 671 added, 0 changed, 0 removed
  

[tk0miya]

LGTM with nits.

I'm thinking about we should add a testcase for this or not. What do you think?

[larsoner]

Comments addressed and tests added

[larsoner]

Rebased to latest 2.0 branch, changed my PR to be against that branch instead of master, and moved my tests to just add to the new ones from #6532. I assumed this is what you wanted based on the request to rebase against 2.0, let me know if that's wrong and I can cherry-pick the commit and make the PR against master again.

[larsoner]

There was a merge conflict so I rebased and force-pushed

[larsoner]

Travis failure looks like an unrelated apt-get problem

[tk0miya]

Sorry for late response. Merging!
Thank you for your contribution!