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 font duplication #888

Merged
merged 2 commits into from
Dec 14, 2017
Merged

Conversation

mihaitodor
Copy link
Contributor

@mihaitodor mihaitodor commented Dec 7, 2017

This is an attempt to partially address #865 and clean up some inconsistencies in the font-related verbs.

  • Add verb w_try_cp_font_files, which makes sure that we don't get the same font file but with different casing under C:\windows\Fonts. It also forces lower case for font names to help alleviate duplication issues.
  • Cache PowerPointViewer.exe in "$W_CACHE/PowerPointViewer" for consistency.
  • Remove "# FIXME: why is this commented out? Should be removed or enabled." comment. This was referring to the Andalé Mono font, which is no longer distributed with Windows. If you wish, I can create a separate verb for it since it's available here, but I think we don't need it at all, since it's almost identical to Courier.
  • Remove chmod +w "${W_FONTSDIR_UNIX}"/tahoma*.ttf since no app should be allowed to overwrite fonts. I'd rather have this posted as a workaround on https://appdb.winehq.org/ instead of maintaining it in Winetricks.
  • Place font package URLs in quotes and leave them hardcoded in the verb body because they're much easier to grep for and replace that way.
  • w_try_cabextract already passes -q to cabextract.

Note: Since eufonts only updates trebucbd.ttf, allfonts will skip installing eufonts after this PR, since trebucbd.ttf is installed in advance by corefonts. #885 needs to be addressed in order to fix this.

Note2: eufonts is an update to some of the fonts installed by corefonts, so corefonts should always be installed first. Luckily, allfonts installs them in the right order.

As a reference, here is the difference between installed font files based on this PR:
original_font_files.txt
updated_font_files.txt

And here is the difference in registry entries:
original_windows_nt.txt
updated_windows_nt.txt

The observed differences in the registry are due to the fact that eufonts was not installed as explained above.

As described in CONTRIBUTING.md (#871), I ran ./tests/shell-checks successfully and I skipped winetricks-test, since it didn't work as described. See here.

@mihaitodor mihaitodor force-pushed the fix-font-duplication branch from 8c6faaa to 3b093b3 Compare December 7, 2017 16:43
@mihaitodor
Copy link
Contributor Author

mihaitodor commented Dec 7, 2017

Looks like something went wrong on Travis for the OSX build:

/Users/travis/.travis/job_stages: line 57: pip: command not found

The Linux build was successful.

LE: pip got removed in the latest OSX Travis image: travis-ci/travis-ci#8829

LE2: found a potential fix here: https://github.com/google/nxt-standalone/pull/144/files

LE3: drat, looks like I need to sudo when invoking pip now...

LE4: I opened #890 to tackle this issue first. Once that gets merged, I will rebase this PR.

LE5: I rebased the PR.

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.

2 participants