-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Use parseable buildinfo #12741
ENH: Use parseable buildinfo #12741
Conversation
Thank you! Immediate thoughts:
Please could this be a different PR? That will be quicker to merge! A |
Bumped to 2 and split off #12746 !
So far I have a
Regarding how we store the info, note that the info has to be stored somehow in a more complete way than it is currently (i.e., as a hash) -- the But beyond storage, we could compute differences in Python after loading the buildinfo. However, we'd have to make decisions like what to do in cases where the diff is quite large. We probably wouldn't want to show the whole thing by default at |
# Conflicts: # sphinx/builders/html/__init__.py
Perhaps we could just print out which keys have changed? That might be more manageable. A |
Okay I implemented that and now we see this when I force a config key to change:
In my case that wouldn't have been enough to fully diagnose the problem, though, so I think it's still worth keeping the |
I agree on keeping the old version of the file to allow for manual inspection. I did have a thought -- is there a risk of leaking secrets here? As I might be over-worrying, but I wanted to raise the concern and make sure we are comfortable here. A |
Argh yes I think you're right. Would it make sense to have this be opt in with a config var saying whether to use hash or this new string representation? |
Agreed on opt-in, but I don't think this can be part of buildinfo (directly) now. The setting should enable writing Would that work for you? A |
Do we get a convenient command line option to specify the output file (or print to the terminal) if we're just looking to check what files triggered a rebuild?
I've been having a problem where changes to external SVGs don't trigger rebuilds... Thus I'm forced to do a complete rebuild that takes minutes (for lack of an easier/evident sphinx-build solution). It'd be desirable to not make this a default overhead in regular builds as users will only want the info when they need to diagnose something. |
@AA-Turner yes that sounds reasonable. One problem with that approach is that such files will pile up, but that's probably okay if you're just generally trying to diagnose a problem. Any thoughts on what the config value would be called?
Under @AA-Turner 's idea you could do |
I would suggest something like A |
Closing for #12949 |
Subject: Help people figure out why rebuilds happen by:
.buildinfo["config"]
parseable rather than a hashAdding info about what template is problematic when rebuilds happen.I was going to open this as a feature request but then got started on a proof-of-concept so figured I'd open it as a proposal.
Feature or Bugfix
Purpose
When Sphinx encounters a difference in the build information, the current level of information provided is just:
If you use
-vv
and look closely (there are a ton of lines printed!) there is this bit of extra information:This PR changes it so that it has this extra line even in non-verbose-verbose mode by
logger.info
ing this:But then also making the
.buildinfo
no longer just store a hash, but rather ajson.dumps
of the representation of the config. This makes it possible to much more easily diagnose the problem. In my case, here's what I could see looking at adiff
between the.buildinfo
and.buildinfo_old
:And if this isn't immediately readable -- i.e., not obvious that
exclude_explicit_doc_regex
was what changed -- in principle you couldjson.loads(...)
and re-dump with nicer spacing and then look at the diff, etc. In this sense it's much more informative than a hash mismatch. For my build, this file was only ~23kB so not too big/painful I think.In my particular case, this allowed me to figure out that the rebuild is being caused by sphinx-gallery doing an
re.compile
on astr
created by joining elements of aset
, whose order will not be stable build-to-build. But I think it's generally more broadly useful in cases where thebuildinfo
changes happen.Then I also hit another problem, namely that we were generating a template on the fly which also was causing everything to be out of date. So I added a logging message for that:
allowing me to easily see that the
avatars.html
that we generate at the start of each doc build should more carefully be updated (or perhaps not be a template file actually!).In any case, if this PR's functionality is desired, probably lots of text and tests etc. need to be updated but I wanted to see if people agree this is useful first!