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

Please make .OTF generation reproducible. #219

Merged
merged 1 commit into from Feb 14, 2018

Conversation

lamby
Copy link
Contributor

@lamby lamby commented Feb 12, 2018

Whilst working on the Reproducible Builds effort, we noticed that ufo2ft generates .otf files that are not reproducible.

For example, here is showotf output of the league-spartan font:

│ │ │ │ │  HEAD table (at 188)
│ │ │ │ │  	Version=1
│ │ │ │ │  	fontRevision=2
│ │ │ │ │ -	checksumAdj=e32dbbe4
│ │ │ │ │ +	checksumAdj=e3309724
│ │ │ │ │  	magicNumber=5f0f3cf5 (0x5f0f3cf5, diff=0)
│ │ │ │ │  	flags=3 baseline_at_0 lsb_at_0
│ │ │ │ │  	unitsPerEm=1250
│ │ │ │ │  	create[0]=0
│ │ │ │ │ -	 create[1]=d580d7b0
│ │ │ │ │ -	File created: Tue Jul  4 05:27:12 2017
│ │ │ │ │ +	 create[1]=d57f6a10
│ │ │ │ │ +	File created: Mon Jul  3 03:27:12 2017

… which shows that it varies on the local timezone.

This has also been filed in Debian as #890280

@lamby
Copy link
Contributor Author

lamby commented Feb 12, 2018

Python 2 failing.. can't recall the incantation there! :)

@jamesgk
Copy link
Contributor

jamesgk commented Feb 13, 2018

I don't think there's a one-line way to do this in Python 2. Easiest way may be to implement timezone.utc locally, as in the example here: https://docs.python.org/2/library/datetime.html#tzinfo-objects (under "Example tzinfo classes:").

Whilst working on the Reproducible Builds effort [0], we noticed
that ufo2ft generates .otf files that are not reproducible.

For example, here is showotf output of the league-spartan fonts:

│ │ │ │ │  HEAD table (at 188)
│ │ │ │ │  	Version=1
│ │ │ │ │  	fontRevision=2
│ │ │ │ │ -	checksumAdj=e32dbbe4
│ │ │ │ │ +	checksumAdj=e3309724
│ │ │ │ │  	magicNumber=5f0f3cf5 (0x5f0f3cf5, diff=0)
│ │ │ │ │  	flags=3 baseline_at_0 lsb_at_0
│ │ │ │ │  	unitsPerEm=1250
│ │ │ │ │  	create[0]=0
│ │ │ │ │ -	 create[1]=d580d7b0
│ │ │ │ │ -	File created: Tue Jul  4 05:27:12 2017
│ │ │ │ │ +	 create[1]=d57f6a10
│ │ │ │ │ +	File created: Mon Jul  3 03:27:12 2017

… which shows that it varies on the local timezone.

Patch attached. This has also been filed in Debian [1].

 [0] https://reproducible-builds.org/
 [1] https://bugs.debian.org/890280
@lamby
Copy link
Contributor Author

lamby commented Feb 14, 2018

@jamesgk Thanks for that. I have updated the PR to match and now I read:

All checks have passed

:)

@jamesgk
Copy link
Contributor

jamesgk commented Feb 14, 2018

@anthrotype and/or @moyogo, want to review before merging? And maybe we should add a unit test.

@jamesgk jamesgk merged commit 62738d6 into googlefonts:master Feb 14, 2018
@lamby lamby deleted the reproducible-build branch February 15, 2018 08:35
@lamby
Copy link
Contributor Author

lamby commented Feb 15, 2018

Thanks!

@anthrotype
Copy link
Member

Thanks @lamby for this PR! (hi @jamesgk! )

Yeah, this is a problem with the UFO spec, which doesn't make it clear whether dates and times should be interpreted as UTC or local time:
unified-font-object/ufo-spec#54

Both you and I would interpret such "naive" UFO datetimes (without explicit timezone info) as UTC, however the author of ufo2fdk (from which ufo2ft was forked) seems to have interpreted them as local times. I only infer this from the fact that the code is using time.mktime, and the latter expects a time tuple that is expressed as local time (it's the inverse of time.localtime).

With your patch, you made this datetime object "aware" by giving it an explicit UTC tzinfo.

But if we agree that all dates and times in UFO (lacking an explicit +HHMM offset) should be understood as UTC, I think an even simpler solution is to avoid using time.mktime altogether and instead use calendar.timegm to convert the time struct to a timestamp. This is the inverse of time.gmtime (for historic reason it happens to be in the calendar module), it takes a time struct or tuple expressed as UTC, and converts it to a UNIX timestamp (integer):
https://docs.python.org/3.6/library/calendar.html#calendar.timegm

I'll send a PR for that.

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

4 participants