search

source-foundry / Hack

A typeface designed for source code
http://sourcefoundry.org/hack/
Other
16.4k stars 615 forks source link

Refactor build scripting to support Python 3 only builds, pinned build dependency versions, optional system installed build dependencies #416

Closed chrissimpkins closed 6 years ago

chrissimpkins commented 6 years ago

This PR updates the build shell scripts and Makefile to support:

TODO:

Addresses the following IR:

and represents the "backwards incompatible" set of build workflow changes that will be released as v4.000.

chrissimpkins commented 6 years ago

@paride I've implemented an approach for compiles of the desktop ttf files using either pinned versions of all build dependencies (in a pipenv venv with pinned Py3, pinned Python build dependencies, and pinned compiled dependencies) or system installed versions of Python 3, Python build packages, and compiled packages in this branch. I've broken the CI tests until all build artifact scripting is complete so ignore the CI testing results for now. The default approach with pinned versions is working for desktop font builds without issues for me in local testing. Can I ask you to confirm that the approach to build with system PATH installed build executables and Python 3 interpreter works for you and appears to be an approach that adequately addresses the following IR that you submitted:

Confirm that all necessary build dependencies are installed on PATH. The make target for the desktop (*.ttf) compile testing on your end will then be:

$ make ttf-system

*.woff compile scripting is available too if you want to give it a shot:

$ make woff-system

Once all of the builds are refactored to support this new approach, you will be able to use:

$ make build-system

to compile .ttf, .woff, *.woff2 (including all subset files) artifacts with a single make target using system installed build dependencies. The default build approach for the upstream project will remain make and will use pinned dependency versions.

This will not address https://github.com/source-foundry/Hack/issues/394 and we will not be making the ttfautohint adjustments to address that IR as part of the v4.000 work. That will require a fair bit of visual examination to push ttfautohint to v1.8.1 since we have had issues with that version in the past. Will be work for down the road. The flag is still supported in v1.8.1 of ttfautohint.

cc @anthrotype - this implements a number of your suggestions in various open IR if you are interested. I ended up going with a pipenv approach and everything is pinned (Python interpreter, Python packages, compiled projects) in the default build. I haven't come up with a way to connect commit times with compile time settings to eliminate the time stamp issue. If you have any thoughts about how to do this let me know.

cc @spstarr build changes that are in the works as previously discussed. No new build dependencies will be introduced but this will change the existing approach, hopefully in a way that supports how you build on Linux (based upon @paride suggestions) out of the box rather than requiring significant modification of our new default approach on your end. Let me know if this can be improved to better support your package.

paride commented 6 years ago

https://github.com/source-foundry/Hack/pull/416/files#diff-4dcc5008412e33290f6089c60c2e1579R68 fails, as $TTFAH does not contain the path to the file.

After bypassing that test the ttf-system target worked fine for me.

chrissimpkins commented 6 years ago

Woops. That shouldn't be tested when you use those make targets. I changed it in the woff builds. Will change it in that script too. Thanks Paride. Confirming that this is what you were looking to achieve in those IR?

paride commented 6 years ago

Yes, this solves both the issues.

As a side note, this part of the script:

# create directory if it does not exist
# (occurs with git + empty directories)
if ! [ -d build/ttf ]; then
        mkdir build/ttf
fi

makes me think that you assume that the built fonts are not part of the git repository. But they actually are (you are not .gitignoring them). This is not a problem at all, of course, I just want to be sure this is intended.

chrissimpkins commented 6 years ago

That was carried over from the old scripts. The comment that I left in the code suggests that I once had a problem when the dir was empty. Don't recall the issue. Will have a look at it and can eliminate if I can't reproduce the previous problem. Thanks Paride.

chrissimpkins commented 6 years ago

Will add system installed dependency build testing to our CI tests to confirm that future changes to the scripts do not break this support. Added as a TODO in the OP.

Actually not going to do this. The installation of the build dependencies (particularly the compiled ones) is not straightforward on the CI testing boxes, it increases the duration of our test times, and the system installed build dependency approach is not our default (or recommended) approach to builds. Will defer this testing to downstream projects that intend to use it. It is there if needed and we can modify as necessary for any problems that are found downstream.

chrissimpkins commented 6 years ago

Broken builds of ttf files with system installed dependencies fixed in https://github.com/source-foundry/Hack/pull/416/commits/353455d38eff8e9b72e7cf8056e9bbd86c81cc53

chrissimpkins commented 6 years ago

All steps are now implemented.

I can confirm that make builds .ttf, .woff full + subsets, *.woff2 full + subsets using pipenv venv on my local macOS machine. Build and script linting tests updated and now passing on Semaphore CI Linux boxes with the same approach. We seem to be gtg with the new default build approach.

If someone would be willing to give the full system installed build dependency builds a shot with:

$ make build-system

it would be greatly appreciated! That make target will produce all fonts above with system installed versions of all build dependencies.

paride commented 6 years ago

It seems that one target is still missing:

make: *** No rule to make target 'subsets-system', needed by 'webfonts-system'.  Stop.
chrissimpkins commented 6 years ago

Sorry! Just pushed it. I think this should be correct now :)

chrissimpkins commented 6 years ago

@anthrotype I think that the only missing version pin is now in the brotli git submodule of the woff2 repository. I don't have experience with pinning submodules to a specific version tag but based upon some reading, it appears that I can use the following to achieve this? 

git clone --recursive https://github.com/google/woff2.git
cd woff2/brotli
# checkout the desired brotli tag
git checkout v1.0.3
cd ..
# checkout the woff2 tag
git checkout v1.0.2
make clean all

Will this achieve the desired effect of pinning both the woff2 source and its brotli dependency at those version git tags?

Edit: It looks like woff2 fixes the brotli repository at the commit associated with v1.0.1 tag when I clone and perform a git log. I assume that this means that this does not change until it is redefined in the woff2 repo and that we are not pulling from the HEAD of master in brotli? i.e. we do not need to consider the brotli submodule version, we simply fix the woff2 version and a fixed brotli version comes with it.

Also, I did confirm that upstream woff2 now builds without issues on macOS as of the current HEAD commit on master. Added this to the open issue report so that it prompts someone to close or look into it further if it is open due to previous macOS version support issues.

paride commented 6 years ago

@chrissimpkins I did try build-system and it works fine now!

anthrotype commented 6 years ago

we simply fix the woff2 version and a fixed brotli version comes with it.

right, that's how git submodules work

chrissimpkins commented 6 years ago

@paride

I did try build-system and it works fine now!

Excellent! Are we at a place that addresses all of the recommendations in your issue reports and needs in your package?

chrissimpkins commented 6 years ago

@anthrotype

right, that's how git submodules work

OK, I must have misunderstood the SO article that I read. It discussed checking out specific version tags in the submodule but perhaps that was to modify the version specified in the top level git repo? I was interpreting it as an attempt to pin what is pulled in as the HEAD of the submodule.

For my own knowledge, is it possible to do what I suggested should you want to modify the git submodule relative to that defined in the top level git repo? I know, likely not advisable. Trying to understand how these work. Is it simply like an semi-isolated (other than top level project defines the commit where it is fixed) embedded git repository that can be manipulated as with a standard git repo when you are located within one of its directories/sub-directories?

Relevant to this project because it does not appear that woff2 is very actively developed whereas brotli is. I may toy with pushing the brotli version since more recent releases tout better compression ratios. Need to figure out how to test the woff2 files that are produced... I suppose that manual visual testing will suffice for now.

chrissimpkins commented 6 years ago

@paride

I also removed the from __future__ import statement in https://github.com/source-foundry/Hack/pull/416/commits/e20d78677fe0a17b2fc12bdeebb15caf891c2690. This was a request in your Python 3 transition IR. I believe that this is the only location where this is found in local scripts in the project.

chrissimpkins commented 6 years ago

And in case you missed it, when you build-system now, you are building with python3. This is explicit in the script. Both approaches use python3 only.

anthrotype commented 6 years ago

Is it simply like an semi-isolated (other than top level project defines the commit where it is fixed) embedded git repository that can be manipulated as with a standard git repo when you are located within one of its directories/sub-directories?

pretty much, yes.

it does not appear that woff2 is very actively developed

send a PR to update the brotli submodule and I can ping the right people ;)

paride commented 6 years ago

@chrissimpkins It seems to me that all my packaging related issues are now solved, and Hack 4 will be packaged without the need to patch anything. Thanks a lot!

chrissimpkins commented 6 years ago

@anthrotype

Thanks for all of the input Cosimo. I appreciate it! I will prompt woff2 with a new issue report (or PR if I can figure out development with submodules) :)

chrissimpkins commented 6 years ago

@paride

It seems to me that all my packaging related issues are now solved, and Hack 4 will be packaged without the need to patch anything. Thanks a lot!

Great to hear that! Happy to help do our part to make what you do easier.

@fabiangreffrath you mentioned that you would like to get to reproducible builds in the Debian package during our discussions about these changes. See the changes that we made here, in part based upon requests from Paride. There will be new optional make targets available as of v4.0 that allow you to build fonts with system PATH installed versions of all build dependencies in this project. We also moved to use whatever is defined as python3 as the python interpreter for all Python packages and local Python scripts. If you have any feedback on the approach, particularly if you suggest changes to help you support the desire for reproducible builds better, please weigh in.

chrissimpkins commented 6 years ago

For all other stakeholders in the build, now is your chance to weigh in on the changes. Once we push this I would like to avoid further build process breaking changes for some time. Let's have the conversations now.

My next planned major version release after these changes will include the Python bindings for brotli and zopfli so that we can transition to the use of Python tools to create our web fonts. This will be most important to me for our webfont subsets because it will simplify the process significantly relative to our current fontmake hack to create subsets. The introduction of these two (at least) new build dependencies will take a bit of time to propagate through the downstream packaging approval processes on Linux distros and I would like to allow projects some time (~ 6-12 months) to achieve this before we make this move. I am hoping to avoid other "backwards incompatible" changes to the build process during this period. Lots of design work to do and we will use this block of time to do it before we delve into build changes again.

Everything is open to feedback and debate. Make target names, transition to Python 3 default, other build needs that exist out there, etc. Let us know.

chrissimpkins commented 6 years ago

Converted to upstream woff2 in https://github.com/source-foundry/Hack/pull/416/commits/96a35cb528c3784e80326f466a99042be0f46938. This is pinned at v1.0.2 (woff2) and includes v1.0.1 of brotli.

Submitted PR https://github.com/google/woff2/pull/110 to woff2 project in order to bump brotli submodule to v1.0.3. Changelog states that compression ratios improved with brotli 1.0.2 --> 1.0.3 changes. Unclear impact on font compiles. We will see if this leads to smaller woff2 web fonts.

chrissimpkins commented 6 years ago

As of https://github.com/source-foundry/Hack/commit/bc6a683fba3170e16eb690f45bbce5043e19d10a updated build documentation is available that is current with all changes here.

chrissimpkins commented 6 years ago

Updated woff2_compress to use brotli v1.0.3 in https://github.com/source-foundry/Hack/pull/416/commits/2a46cad575da4d7c4c48acb9232d111636150e1e. Renders with updated build tooling 👍 c/w previous builds. Pre:post waterfall of woff2 renders (Firefox on macOS):

k7nka-image

chrissimpkins commented 6 years ago

No significant decrease in the woff2 file sizes with the brotli v1.0.3 updates.

chrissimpkins commented 6 years ago

Merged into dev branch and will be included in v4.000 release of Hack