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

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

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 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 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 waylan commented May 11, 2020

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

mkdocs theme:

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

readthedocs theme:

</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 ofek commented May 11, 2020

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

@ofek
Copy link
Contributor Author

@ofek ofek commented May 12, 2020

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

@waylan
Copy link
Member

@waylan waylan commented May 12, 2020

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 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
1 check failed
@ofek ofek deleted the patch-1 branch May 12, 2020
@waylan
Copy link
Member

@waylan waylan commented May 12, 2020

Any idea when 1.1.1 will be released?

I expect very soon.

@waylan
Copy link
Member

@waylan waylan commented May 12, 2020

FYI, version 1.1.1 has been released.

@ofek
Copy link
Contributor Author

@ofek 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants