Tests: fixup: explicitly unset SOURCE_DATE_EPOCH during 'test_html_multi_line_copyright'

[jayaddison]

Purpose

Sphinx's copyright substitution currently allows years identified as the current year to be downgraded to previous years when SOURCE_DATE_EPOCH is configured, to assist reproducibility of documentation builds.

However, we have a test case test_html_multi_line_copyright, written in Y2024, that mentioned a future year - Y2025.

Now that that year is current, it is eligible for substitution when SOURCE_DATE_EPOCH is configured. Many buildsystems, such as those used by Debian and Fedora, choose the most-recent packaging/commit timestamp to use as SOURCE_DATE_EPOCH's timestamp, since those corresponds sensibly to a time-of-build.

However, for the v8.1.3 Sphinx release including the updated substitution logic, the year-of-release/commit was Y2024. Thus, if a commit/packaging date from that year is chosen, and SOURCE_DATE_EPOCH is configured when the unit tests run, the test_html_multi_line_copyright test will fail.

The fix suggested here is to explicitly unset SOURCE_DATE_EPOCH within test_html_multi_line_copyright.

References

• Resolves test_html_multi_line_copyright fails when SOURCE_DATE_EPOCH is set for 2024 #13223
• Bug introduced by copyright: Apply substitutions only to current-year entries, and disallow future year insertion #12516
• Relates to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1091951 (cc @mitya57)

Edit: add a reference to the pull request that introduced the bug

[jayaddison]

cc also @cjwatson; I'll admit that I had scanread the Debian bug yesterday, albeit without looking at your patch in any detail; I wrote this suggestion separately today after investigating and understanding the bug, and.. it's near-identical. I'm happy to defer to yours instead if you'd prefer (you wrote it first, and strictly speaking I had read the bug thread).

[jayaddison]

(NB: I only compared the content of the patches after developing and testing this one.. difficult to prove that, though, I suppose)

[cjwatson]

I have no sense of proprietary ownership here, and the fix seemed fairly obvious after understanding the problem. Go ahead.

[jayaddison]

Thank you. Despite that, I'd recommend some caution from the maintainers before/if merging this; I think people without context could reasonably be skeptical that I authored it, and perhaps even if I think I did, I may have subconsciously picked up the details yesterday since it's a small patch.

[cjwatson]

It doesn't worry me, but if you want to, you could include something like Co-authored-by: Colin Watson <[email protected]> in your commit message and that should remove any doubt.

[jayaddison]

Possibly; in this context I'm currently skeptical about whether co-authorship is an accurate assessment. I think you authored the patch first, and then I apparently authored an equivalent patch independently (but as noted, I treat myself as an unreliable narrator).

[cjwatson]

I really don't mind, and while I appreciate your diligence, with all due respect I think you're probably overthinking this. This patch is straightforward enough, and the new fixture is clearly similar enough to the existing source_date_year fixture, that independent invention is entirely plausible. And even if you did subconsciously copy it, I don't think it's worth my time to resubmit my marginally-different version of the PR separately.

[cjwatson]

Also, the main reason I hadn't submitted it myself is that I hadn't got round to writing up my reasoning in a form suitable for a PR description. You did that in a way that's probably clearer than I would have done.

[AA-Turner]

If we only use this once, is it worth being a standalone function?

If we should keep it, could we add a docstring to describe why and when we should use the fixture?

A

[cjwatson]

When I experimented with this, I initially tried inlining it into the relevant test, but that didn't work since the monkeypatch needs to be applied before the app fixture is started.

[jayaddison]

It doesn't worry me, but if you want to, you could include something like Co-authored-by: ... in your commit message and that should remove any doubt.

@cjwatson If it's OK with you, could we instead present your original patch and credit you as author, with me in Co-authored-by? Note that a CHANGES.rst entry should be included too (as usual, I forgot to write one).

That would I hope be an honest representation, and would mean that if I did subconsciously copy, the output of that copying isn't merged.

[AA-Turner]

@jayaddison can you add a docstring to the fixture describing why it is needed? Then I will merge this. We don't need CHANGES here as it isn't a true bug (in sphinx/) but a problem with the test suite.

---

On attribution, I agree with Colin that it's best to simply merge this PR as it is open -- I will add a Authored-by trailer line if you would like. I highly doubt there's an issue with copyright, both as the patch was independently written by two people and as Colin has said he doesn't mind and is fine with this, even if there was an issue.

Note also that git history does not define the copyright holder, authorship, or any moral rights (if extant), it simply records who committed the patch.

A

[jayaddison]

I don't want to waste time, but I do prefer fairly strongly to apply the original patch; I've pushed that plus the docstring change (lint fixup required, which I'll apply momentarily).

[jayaddison]

I acknowledge that this entry may not be strictly required, but we do have a changelog section for Testing, and I think this fits within that topic.

[jayaddison]

NB: Since both of the patches required configuration-or-absence of SOURCE_DATE_EPOCH to demonstrate the problem/confirm-the-fix, I applied equivalent local testing for each of the patch variants (pytest tests/test_builders/test_build_html_copyright.py with and without SOURCE_DATE_EPOCH=1731456000 as reported in the linked bug). Again, pedantic of me since the only change was a variable name, but process-wise I felt more comfortable having applied that to both.

[AA-Turner]

Thanks both!

A