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

fira-code: don't use variable font #123220

Closed
wants to merge 2 commits into from

Conversation

dotlambda
Copy link
Member

Motivation for this change

fixes #118891
Also, Arch Linux's ttf package installs the same files.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

It doesn't work well with LaTeX because the filename is wrong.
@r-rmcgibbo
Copy link

r-rmcgibbo commented May 16, 2021

Result of nixpkgs-review pr 123220 at 856969f run on aarch64-linux 1

2 packages built successfully:
  • fira-code
  • fira-mono

Result of nixpkgs-review pr 123220 at 856969f run on x86_64-linux 1

2 packages built successfully:
  • fira-code
  • fira-mono

@midchildan
Copy link
Member

It looks to me the real issue lies in luaotfload. luaotfload isn't properly matching the font family name, so it is falling back to finding the font by filename instead.

luaotfload-tool --find 'Fira Code' --fuzzy
luaotfload | db : Reload initiated (formats: otf,ttf,ttc); reason: Font "Fira Code" not found.
luaotfload | resolve : sequence of 3 lookups yielded nothing appropriate.
luaotfload | resolve : Cannot find "Fira Code" in index.
luaotfload | resolve : Looking for close matches, this may take a while ...
luaotfload | query : Distance from "firacode": 5
    Fira Code Light
    file-icons

luaotfload is mistakenly thinking that "Fira Code Light" is the family name, so the following works:

\usepackage{fontspec}
\setmainfont{Fira Code Light}

@midchildan
Copy link
Member

I'd prefer keeping the variable font unless it causes more problems because it's more customizable while being smaller in size. The non-variable ttfs are 1.8 MB combined while the variable one is only 260 KB.

@dotlambda
Copy link
Member Author

The non-variable ttfs are 1.8 MB combined while the variable one is only 260 KB.

That's a stark difference indeed.

It looks to me the real issue lies in luaotfload. luaotfload isn't properly matching the font family name, so it is falling back to finding the font by filename instead.

Do you have an idea how to fix that? As a workaround, can we maybe change the filename from FiraCode-VF.ttf to FiraCode.ttf?

@midchildan
Copy link
Member

midchildan commented May 17, 2021

For a complete fix, I believe this would have to be reported upstream at latex3/luaotfload. We could rename the file for Fira Code, but fixing it upstream would solve the problem for other fonts that may suffer from the same issue.

I'm not familiar with Lua but from a cursory look, it seems that luaotfload rolls its own parser for font files so any fixes would probably have to be made there.

@stale
Copy link

stale bot commented Nov 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@rycee
Copy link
Member

rycee commented Nov 16, 2021

I think installing FiraCode-VF.ttf as FiraCode.ttf sounds like a good idea. It would keep the variable font while, I guess, also work with lualatex.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 19, 2022
@minijackson
Copy link
Member

What about the solution proposed by #196087, keeping them both? The fira-code variable font also makes xdvipdfmx fail when I use xelatex

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 1, 2023
@drupol
Copy link
Contributor

drupol commented Jun 6, 2023

Dear @dotlambda,

The original issue this PR has been created for is fixed. Is this PR still relevant?

If not, can you please close it?

Thanks!

@minijackson
Copy link
Member

I can confirm that the issue that I had is fixed on nixpkgs master.

@drupol
Copy link
Contributor

drupol commented Jun 12, 2023

Could you please fix the merge conflict ?

@minijackson
Copy link
Member

Note: I'm confirming that for my usecase, this PR was not needed.

@drupol
Copy link
Contributor

drupol commented Jun 12, 2023

Oh, ok.

@dotlambda Are you ok to close this PR ?

@drupol drupol closed this Jun 13, 2023
@minijackson
Copy link
Member

Sorry for necrobumping the pull request, but I can reproduce the issue on master again.

\documentclass{article}
\usepackage{fontspec}\setmonofont{Fira Code}
\begin{document}
Let's test the “programming ligatres” \verb|hello <> === ok nein|.
\end{document}
nix shell 'nixpkgs/master#texlive.combined.scheme-full'
latexmk -pdf -xelatex test.tex

Fails with:

------------
Running 'xdvipdfmx -E -o "test.pdf"  "test.xdv"'
------------
test.xdv -> test.pdf
[1
xdvipdfmx:warning: Invalid TTC index (not TTC font): /nix/store/3nvjhw65a3vqf411g0w3nyyjmmwlxc75-fira-code-6.2/share/fonts/truetype/FiraCode-VF.ttf
xdvipdfmx:fatal: Invalid font: -1 (1)

No output PDF file written.
Latexmk: Errors, so I did not complete making targets
Collected error summary (may duplicate other messages):
  xdvipdfmx: Command for 'xdvipdfmx' gave return code 256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lualatex cannot find Fira Code
7 participants