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.

Sign up for GitHub

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

Jump to bottom

Make compressed sitemap deterministic #2100

Merged
merged 7 commits into from May 12, 2020
Merged

Make compressed sitemap deterministic #2100

merged 7 commits into from May 12, 2020

Conversation

ofek
Copy link
Contributor

@ofek ofek commented May 9, 2020 •
edited

Our deployment step for https://datadoghq.dev/integrations-core/ should only trigger when a change occurs in the built site. However, every build was different because gzip includes the timestamp in its header.

cc @waylan since you were involved with #1130

@waylan
Copy link
Member

waylan commented May 10, 2020

Since #1010 (4 years ago) we support the SOURCE_DATE_EPOCH environment variable for this very purpose. The value of that variable is used to set the build_date_utc variable in the template context. It seems to me that the same should be used for the gzip timestamp. After all, depending in which theme you use, you may need it set for deterministic builds anyway.

That said, I'm curious if perhaps the gzip lib already makes use of the env variable. After all, our users who use the variable have not complained about the sitemap.

Finally, I see that the docs for the build_date_utc template context variable does not make mention of SOURCE_DATE_EPOCH. That should probably be updated.

@ofek
Copy link
Contributor Author

ofek commented May 10, 2020

Interesting. Where does that date get rendered? Removing the archive makes our builds reproducible without setting that environment variable.

@waylan
Copy link
Member

waylan commented May 11, 2020 •
edited

In the builtin themes, the build_date_utc gets included in an HTML comment on the homepage only.

mkdocs theme:

mkdocs/mkdocs/themes/mkdocs/base.html

Lines 197 to 204 in a4eb4eb

</body>
</html>
{% if page and page.is_homepage %}
<!--
MkDocs version : {{ mkdocs_version }}
Build Date UTC : {{ build_date_utc }}
-->
{% endif %}

readthedocs theme:

mkdocs/mkdocs/themes/readthedocs/base.html

Lines 156 to 163 in a4eb4eb

</body>
</html>
{% if page and page.is_homepage %}
<!--
MkDocs version : {{ mkdocs_version }}
Build Date UTC : {{ build_date_utc }}
-->
{% endif %}

In both cases, the comment is the past thing after the closing <html> tag. On https://www.mkdocs.org we see that currently rendered as:

<!--
MkDocs version : 1.1
Build Date UTC : 2020-02-26 23:50:49
-->

That is the actual date and time the site was built and pushed live, which roughly corresponds with the time of the last release.

@ofek
Copy link
Contributor Author

ofek commented May 11, 2020

Ah I see, we minify the html so we have no comments.

@ofek
Copy link
Contributor Author

ofek commented May 12, 2020

@waylan Addressed! FYI a new version of flake8 was released, hence the errors.

@waylan
Copy link
Member

waylan commented May 12, 2020 •
edited

FYI a new version of flake8 was released, hence the errors.

Sigh. Thanks for the heads-up. We can ignore that here as it is unrelated to this change.

However, we do have a failing test in your code. Looks like you beat me to it.

Also, we need a note added to the release notes (under version 1.1.1).

Otherwise, this looks great. Thanks for the work on this.

@ofek
Copy link
Contributor Author

ofek commented May 12, 2020

Done, thank you! Any idea when 1.1.1 will be released?

@waylan waylan merged commit fa5aa4a into mkdocs:master May 12, 2020
@ofek ofek deleted the patch-1 branch May 12, 2020 14:06
@waylan
Copy link
Member

waylan commented May 12, 2020

Any idea when 1.1.1 will be released?

I expect very soon.

@waylan
Copy link
Member

waylan commented May 12, 2020

FYI, version 1.1.1 has been released.

@ofek
Copy link
Contributor Author

ofek commented May 12, 2020

Thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ofek @waylan