[gucci-on-fleek]

The internal function ltx_read explicitly support supports a table specification:

luaotfload/src/luaotfload-loaders.lua

Line 134 in f0d45e3

spec = type(spec) == 'string' and fonts.definers.analyze(spec, size) or spec

However, the luaotfload.rewrite_fontname callback handler raises an error if you pass a table specification to luaotfload.define_font. All the other default callbacks work correctly with a table argument though, so you can currently work around the problem by running luaotfload.aux.stop_rewrite_fontname() or by setting config.luaotfload.run.definer = "read", but the better solution would be to fix luaotfload.rewrite_fontname to correctly handle this case.

[gucci-on-fleek]

Looks like the CI is currently pinned at TL24:

luaotfload/.github/workflows/main.yaml

Line 56 in f0d45e3

texlive_version: "2024"

Do you want me to change the new test to only use l3build features available back then, or do you want me to bump the version?

[zauguin]

For me it seems rewrite_fontname not working with table specifications is an inherent incompatibility between rewrite_fontname's purpose and non-string based font requests. Why would rewrite_fontname still be useful with this change?

[gucci-on-fleek]

@zauguin

For me it seems rewrite_fontname not working with table specifications is an inherent incompatibility between rewrite_fontname's purpose and non-string based font requests.

Correct. Well, the

luaotfload/src/luaotfload-auxiliary.lua

Line 57 in 74f1152

tfmdata.name = specification.name

part is needed because otherwise LuaTeX segfaults, but otherwise this PR makes rewrite_fontname completely ignore table arguments since it can't handle them properly.

Why would rewrite_fontname still be useful with this change?

It would still work with every string specification (which is presumably every specification before this patch), so it would still be useful in that case. It wouldn't do anything useful with a table argument, but that seems fine, because there aren't any backwards compatibility concerns here, so we can just require the table users to manually quote the name if needed.

I think that what you're getting at is that I should just use luaotfload.aux.stop_rewrite_fontname() here, but that's a global setting that breaks fontspec, so that's not really something that a package can use. Whereas this PR won't affect any pre-existing code, so it should be safe for packages to use.

[zauguin]

Whereas this PR won't affect any pre-existing code, so it should be safe for packages to use.

The point of rewriting fontnames is that multiple places (microtype immediately comes to mind, I think it used to be even the kernel but that should be gone by now assume that the \fontname result can be passed back t \font (potentially with prior modification) and the same font gets loaded. This promise would completely bread for any font loaded this way. That's inherently a global change for the behavior and meaning of \fontname which breaks expectation of packages.

I don't see how this would be safe for any package unless they first ensure that no package depends on \fontname's anymore (and in that case we have stop_rewrite_fontname)

[gucci-on-fleek]

@zauguin

Hmm, ok, so it sounds like I completely misunderstood what rewrite_fontname does, sorry about that. I've just pushed a new version that should be safer (I think).