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

fix(docs): Change year in copyright footer automatically to current year #3079

Merged
merged 3 commits into from
Feb 13, 2023

Conversation

comawill
Copy link
Contributor

Sphinx operates on environment variable SOURCE_DATE_EPOCH to provide reproducible builds.
This PR intends to change the Makefile to set SOURCE_DATE_EPOCH to the latest git revision

Additionally sphinx will only do this correctly, when there are no spaces between the numbers and dash. [src]

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@comawill comawill requested review from smatting and jschaul February 13, 2023 12:20
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 13, 2023
@comawill comawill marked this pull request as draft February 13, 2023 12:37
@comawill
Copy link
Contributor Author

Do not merge: worked perfectly locally, but has no git in github action

@smatting
Copy link
Contributor

smatting commented Feb 13, 2023

@comawill Try adding pkgs.git to the list of available packages here https://github.com/wireapp/wire-server/blob/develop/nix/default.nix?plain=1#L28
I think @flokli mentioned before that adding git to nix envs could causes problems. But in this case maybe its fine?

@flokli
Copy link
Contributor

flokli commented Feb 13, 2023

This would mean the docs and all derivations that depend on them would not become binary reproducible anymore. Can we just drop the year numbers from the footer entirely?

@comawill comawill force-pushed the comawill/docs_copyright_year branch from 89963b5 to e10b685 Compare February 13, 2023 13:59
@smatting
Copy link
Contributor

@comawill I guess "date" inside the build returns unix epoch 0. I think the problem is that within a nix build you don't have access to the outside world's time. nix tries hard to make all builds deterministic on the input, so it forbids external inputs such as the time.
@flokli do you have an idea how to do this? pass the date from outside to script somehow?

@flokli
Copy link
Contributor

flokli commented Feb 13, 2023

As i wrote above, ideally we don't add a year number there at all. If we really want to, not make it automatic based on the current time, but manually, once per year.

@comawill
Copy link
Contributor Author

@smatting using date +%s actually seems to work, but I agree with @flokli that this compromises reproducible builds.

My attempt was to depend on the git history here, but within nix-build we don't have the history available anymore.

Using the current year or a manual set one does not make a difference as the problems are caused by sphinx here when SOURCE_DATE_EPOCH is set. My proposed workaround is now to ignore SOURCE_DATE_EPOCH within sphinx.

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to squash this before merge.

@comawill comawill marked this pull request as ready for review February 13, 2023 14:56
@comawill comawill merged commit c15881c into develop Feb 13, 2023
@comawill comawill deleted the comawill/docs_copyright_year branch February 13, 2023 14:57
@flokli
Copy link
Contributor

flokli commented Feb 28, 2023

@comawill this broke builds outside the nix sandbox (make dev-run for example), where SOURCE_DATE_EPOCH env var is not set:

~/src/wire-server/docs $ make dev-run
rm -rf "/home/mf/src/wire-server/docs/build"
sphinx-autobuild \
        --port 3000 \
        --host 127.0.0.1 \
        -b html \
        -q \
        "/home/mf/src/wire-server/docs/src" "/home/mf/src/wire-server/docs/build"
[sphinx-autobuild] > sphinx-build -b html -q /home/mf/src/wire-server/docs/src /home/mf/src/wire-server/docs/build

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/nix/store/6gcnx7220g1aaz7d9y32yv8y0f1jg7q7-python3-3.10.9-env/lib/python3.10/site-packages/sphinx/config.py", line 350, in eval_config_file
    exec(code, namespace)
  File "/home/mf/src/wire-server/docs/src/conf.py", line 17, in <module>
    del os.environ['SOURCE_DATE_EPOCH']
  File "/nix/store/sp5x6s8n36gjlwck74xhj1i61p66vcpa-python3-3.10.9/lib/python3.10/os.py", line 696, in __delitem__
    raise KeyError(key) from None
KeyError: 'SOURCE_DATE_EPOCH'

Would you mind sending a followup PR to fix this?

@comawill
Copy link
Contributor Author

@flokli wow, nice finding, sure, will make a PR

@flokli
Copy link
Contributor

flokli commented Feb 28, 2023

Already happened in #3110.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants