-
-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
fontconfig_210: remove package #93562
Conversation
There's a lot more of (now possibly unused) code - there's This is also exposed as For now I decide to keep this, to not change too much at once, but I assume we might want to remove the configVersion parts in there, too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great but I would like to hold off on merging this until we have dealt with the possible fallback from #73795
Yeah, the XSL stylesheet does two things at once. The reason why it is used in the fontconfig derivation is precisely to introduce the versioned |
@jtojnar has there been any breakage reported already, or do you mean you just want to await one staging cycle? |
I'd propose removing the |
Yeah, I would like to wait one staging cycle, or maybe slightly more since the channels lag a bit behind.
The current fontconfig needs it too, since it is the mechanism that makes it load the global config. |
f8e0b5b
to
36741d7
Compare
This fontconfig version isn't used anywhere inside nixpkgs anymore.
This isn't used anymore anywhere, and vulnerable to CVE-2016-5384.
36741d7
to
bab13cc
Compare
We had enough some staging rounds since #93562 (review). Rebased to latest master. |
Edit: moved over into #95319. |
NixOS module |
Apparently, edf2541 was missed while rebasing NixOS#93562. Provide 50-user.conf in fontconfig if includeUserConf is true (the default), and don't try removing the non-existent one if it's disabled Fixes NixOS#95685 Fixes NixOS#95712
Apparently, edf2541 was missed while rebasing NixOS#93562. Provide 50-user.conf in fontconfig if includeUserConf is true (the default), and don't try removing the non-existent one if it's disabled Fixes NixOS#95685 Fixes NixOS#95712
# update 51-local.conf path to look at local.conf | ||
rm $dst/51-local.conf | ||
|
||
substitute ${pkg.out}/etc/fonts/conf.d/51-local.conf \ | ||
$dst/51-local.conf \ | ||
--replace local.conf /etc/fonts/${pkg.configVersion}/local.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another section that should remain removed from edf2541. See #86601 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I grok this. Can you file a PR to master on what's needed to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These six lines should have remained removed. I will not get to filing PR before tomorrow.
Another part of edf2541 was missed while rebasing NixOS#93562, resulting in incorrect path as described by NixOS#86601 (comment)
This removes the
fontconfig_210
package, as well as the only remaining usage, the rendering of 2.10.x config and cache.I switched my system to this, and didn't spot any breakages.
Motivation for this change
Closes #88289
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)