Skip to content
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

Always say why something is out of date #11572

Merged
merged 12 commits into from Aug 10, 2023
Merged

Conversation

larsoner
Copy link
Contributor

@larsoner larsoner commented Aug 9, 2023

Adds logging in two branches that was missing from the logger.debug. Gives output like:

[build target] outdated 'auto_examples/future/plot_future_imports' from dependency '/home/larsoner/python/sphinx-gallery/sphinx_gallery/tests/tinybuild/doc/auto_examples/future/images/thumb/sphx_glr_plot_future_imports_broken_thumb.png': 2023-08-09 18:36:25.911416 -> 2023-08-09 18:36:34.238679

Perhaps it's a bit verbose but it at least indicates that this file is marked outdated because a dependency of it was created after the file that uses it.

BUT this brings up an issue -- the dependency itself was not changed the two calls to make html. The conditional if depmtime > mtime: around line 501 checks if the mtime of the file during previous build is older than that of the dependency in this build, when it seems like it should check if the mtime of the dependency during previous build is older than that of the dependency in this build. In other words, if I create index.rst, then create whatever.inc, and .. include: whatever.inc, index.rst will always show up as out of date just because of the order in which I created index and whatever.inc.

Should I try to fix that here as well? It seems like a potentially tough thing to fix because it would mean adding all dependency files to self.all_docs (where self is the BuildEnvironment) -- or maybe better adding a self.all_deps that is a str->float mtime mapping for all dependencies.

@larsoner
Copy link
Contributor Author

larsoner commented Aug 9, 2023

BUT this brings up an issue -- the dependency itself was not changed the two calls to make html

Looks like this has actually been fixed on master sometime so updating fixed this part for me!

sphinx/environment/__init__.py Outdated Show resolved Hide resolved
sphinx/environment/__init__.py Outdated Show resolved Hide resolved
@larsoner
Copy link
Contributor Author

https://github.com/sphinx-doc/sphinx/actions/runs/5821558864/job/15784236771?pr=11572#step:9:2434

                        if depmtime > mtime:
>                           depmtime_dt = datetime.utcfromtimestamp(depmtime)
E                           ValueError: year 53608964 is out of range
...
=========================== short test summary info ============================
FAILED tests/test_intl.py::test_gettext_dont_rebuild_mo - ValueError: year 53608965 is out of range
FAILED tests/test_intl.py::test_html_rebuild_mo - ValueError: year 53608964 is out of range
FAILED tests/test_intl.py::test_xml_footnotes - ValueError: year 53608964 is out of range
====== 3 failed, 1957 passed, 22 skipped, 2 warnings in 192.82s (0:03:12) ======

@picnixz
Copy link
Member

picnixz commented Aug 10, 2023

Wow. Ok, so maybe utcfromtimestamp is not the correct way to do it? I think on line 492 we are using fromtimestamp (we divide the timestamp by 1_000_000 before but I'm not sure that it's ok to do the same here, so you'll definitely need to check whether this should apply or not). I also think it's the preferred way to do it (see https://docs.python.org/3/library/datetime.html#datetime.datetime.fromtimestamp).

@larsoner
Copy link
Contributor Author

we divide the timestamp by 1_000_000 before

Ahhh okay this is indeed probably the problem -- I'll simplify the code and push soon!

@larsoner
Copy link
Contributor Author

Okay finally green!

@AA-Turner AA-Turner added the type:enhancement enhance or introduce a new feature label Aug 10, 2023
@AA-Turner AA-Turner changed the title ENH: Always say why something is out of date Always say why something is out of date Aug 10, 2023
@AA-Turner
Copy link
Member

Please may you add an entry to CHANGES?

A

@larsoner
Copy link
Contributor Author

Done @AA-Turner !

@larsoner
Copy link
Contributor Author

Windows failure looks probably unrelated... is it a flakey test?

@AA-Turner
Copy link
Member

is it a flakey test?

Yes, see #11232; #11500.

A

@AA-Turner AA-Turner merged commit 7758e01 into sphinx-doc:master Aug 10, 2023
26 of 27 checks passed
@AA-Turner
Copy link
Member

Thank you @larsoner!

A

@larsoner larsoner deleted the why branch August 10, 2023 23:22
@AA-Turner AA-Turner added this to the 7.2.0 milestone Aug 11, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants