-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: add option to use system directories #23288
fontconfig: add option to use system directories #23288
Conversation
The fontconfig package currently tries to load fonts from some directory within the conan package directory. This results in the following error when this directory is not present: ``` Fontconfig error: Cannot load default config file: No such file: (null) ``` This commit adds an option `system_dirs` to use system directories instead. Fixes conan-io#5782
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 fixes what seems like an important oversight. Thanks!
Should use project_options
instead, though.
if self.settings.os in ("Linux", "FreeBSD") and self.options.get_safe("system_dirs", False): | ||
self._replace_configuration_string(build_script, "CONFIGDIR", "fc_configdir", "'/etc/fonts/conf.d'") | ||
self._replace_configuration_string(build_script, "FC_CACHEDIR", "fc_cachedir", "'/var/cache/fontconfig'") | ||
self._replace_configuration_string(build_script, "FC_TEMPLATEDIR", "fc_templatedir", "'/usr/share/fontconfig/conf.avail'") | ||
self._replace_configuration_string(build_script, "FONTCONFIG_PATH", "fc_baseconfigdir", "'/etc/fonts'") |
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.
You should be able to accomplish the same without patching simply via project_options in generate(), I think:
tc.project_options["fc_configdir"] = "/etc/fonts/conf.d"
tc.project_options["fc_cachedir"] = "/var/cache/fontconfig"
tc.project_options["fc_templatedir"] = "/usr/share/fontconfig/conf.avail"
tc.project_options["fc_baseconfigdir"] = "/etc/fonts"
Also, you should probably override xml-dir
as well? https://gitlab.freedesktop.org/fontconfig/fontconfig/-/blob/main/meson_options.txt?ref_type=heads#L32-45
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.
I thought I had tried that previously, but you are right, it works on Conan 2.
Unfortunately, on Conan 1, Meson attempts to install something to /usr and fails. Looks like it is due to missing this change: conan-io/conan#15706
Could setting either of these options get the best of both worlds without introducing a recipe option, maybe? |
This comment has been minimized.
This comment has been minimized.
Incidentally I noticed that the paths get different defaults on Conan 2 compared to Conan 1 (that is, when Conan 2:
Conan 1:
|
Looks like those options are used to specify the location of fonts, not the config/cache directories. |
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
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.
@johningve Thank you for your suggestion of fixing that case. I would not go for adding a new option only change an environment variable, it's too much. Conan offers mechanisms to do it without touching the recipe. Please, read #5782 (comment) and check if fits your case.
I see a fragility here as you are hardcoding system paths that we can not guarantee. Conan only knows its own cache, outside of that is risky. Still, in case that option is not enough, we could add a Conan configuration to handle those variables, they fit better because are not part of the package ID.
Conan v1 pipeline ✔️All green in build 3 (
Conan v2 pipeline ✔️
All green in build 3 ( |
It is not about changing an environment variable, that is a workaround to this issue. What this is about is changing the location where fontconfig will look for fonts by default. I want to be able to include fontconfig in my application and have it work without the user having to worry about environment variables.
I would say that at least on Linux, these paths are well known and relied upon. I get that Conan doesn't want to rely upon the outside too much, but the purpose of fontconfig is to enable applications to find and use the fonts available on the local system. |
I would ask, at which point would anyone want fontconfig to use the configuration within conan's cache? |
Hello again! I’m following up on the discussion regarding the default system directories used by fontconfig. I’ve provided a more detailed explanation of how to parameterize the system folders in my comment here: #5782 (comment). Thank you once again for your pull request and for proposing an alternative to address this limitation. However, the current implementation seems to introduce additional complexity to the Conan recipe. This includes modifying the package ID with a new recipe option, requiring several custom user configurations, and applying patches dynamically. For these reasons, I’ll be closing this pull request, as I’m not inclined to accept the proposed changes. I encourage you to explore using Conan’s native configuration to extend the Meson configuration for this use case. Please feel free to share your feedback or ask for help on how to use Conan configuration for your particular case. Thank you for your understanding! |
The fontconfig package currently tries to load fonts from some directory within the conan package directory. This results in the following error when this directory is not present:
This commit adds an option
system_dirs
to use system directories instead.Fixes #5782
Specify library name and version: fontconfig/2.13.93+