Bug 59456 - Adding a --sysroot like option to fc-cache
Summary: Adding a --sysroot like option to fc-cache
Status: RESOLVED FIXED
Alias: None
Product: fontconfig
Classification: Unclassified
Component: fc-cache (show other bugs)
Version: unspecified
Hardware: All All
: medium enhancement
Assignee: Akira TAGOH
QA Contact: Behdad Esfahbod
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-16 08:54 UTC by Laurentiu Palcu
Modified: 2014-06-18 02:50 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch implementing sysroot option (21.08 KB, text/plain)
2013-01-29 12:09 UTC, Laurentiu Palcu
Details
Added version 3 (21.51 KB, text/plain)
2013-01-30 19:09 UTC, Laurentiu Palcu
Details
Version 4 (21.33 KB, text/plain)
2013-02-01 12:30 UTC, Laurentiu Palcu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Laurentiu Palcu 2013-01-16 08:54:25 UTC
Where working on activating read-only rootfs support in our project (Yocto Project to be exact) and we aim to have all postinstall scriptlets run on host, instead of target. However, packages that provide fonts need to run fc-cache in the postinstall stage. And, even we can get around the architecture-dependent "problem" by running fc-cache inside qemu user emulator, and change the fonts.conf file to contain paths pointing to the target rootfs on host, the cache would contain the entire host path. This is not acceptable when deploying the image on target...

So, in this case, a --sysroot like option to fc-cache would be of great help. We could feed it to fc-cache and this path would be prefixed to all paths in fonts.conf so that the fonts scan can take place but have the final cache generated without this prefix.

If we implement this feature, would it be accepted?
Comment 1 Behdad Esfahbod 2013-01-16 10:56:09 UTC
Yes, I would take it.  Please add the SysRoot to FcConfig and propagate it from there.  You need two new APIs: FcConfigSetSysRoot and FcConfigGetSysRoot.  All utilities can then be updated to add this new option.
Comment 2 Laurentiu Palcu 2013-01-18 10:54:55 UTC
Thanks, I'll start working on this.
Comment 3 Laurentiu Palcu 2013-01-29 12:09:18 UTC
Created attachment 73824 [details]
Patch implementing sysroot option

Addressed Akira's comment on the ML.
Comment 4 Laurentiu Palcu 2013-01-30 19:09:15 UTC
Created attachment 73944 [details]
Added version 3

This new version fixes an issue with fc-lang compilation. It turns out that 'make distclean' did not erase fclang.h, and fc-lang was not rebuilt. Hence, did not see the issue before. This patch adresses this problem which was generated by calling the new FcConfigGetSysRoot() in FcStrSetAddFilename() in fcstr.c.
Comment 5 Akira TAGOH 2013-02-01 02:29:27 UTC
Hmm, after further reading, that looks like your patch changes the behavior of FcCacheSubdir(). it is supposed to return the constant string though, with FcConfigSetSysRoot(), it returns the malloc'd one. this means that new API introduces the memory leaks on other apps/libraries when they use it. it looks not a good idea to me. what was wrong with just hooking up when reading/storing caches?
Comment 6 Laurentiu Palcu 2013-02-01 12:30:31 UTC
Created attachment 74042 [details]
Version 4

Akira,

Thanks for the review. You have a good point. I shouldn't have touched the API. My mistake. However, I don't understand how could I hook up when reading the cache. As far as I saw, correct me if I'm wrong, the cache file is mmap'ed. FcCacheDir/FcCacheSubdir just return pointers to that area. I was able to do that when writing the cache though. Anyway,the new patch (v4) leaves those 2 functions untouched and the sysroot path will be added at a higher level.

Thanks,
Laurentiu
Comment 7 Akira TAGOH 2013-02-06 11:06:18 UTC
Well, it is just a serialized meta data. you can get rid of the sysroot when creating new cache. let me try to make a counterpart patch to address this when I have a time..

Anyway, as long as adding new API for sysroot, you shouldn't expect to do something outside fontconfig to support sysroot. IMHO it has to be done inside, no exception of even fc-* tools.
Comment 8 Behdad Esfahbod 2013-02-06 21:32:12 UTC
Also, FcConfig[SG]etSysroot should take a FcConfig * as first argument.
Comment 9 Akira TAGOH 2013-02-07 07:49:10 UTC
(In reply to comment #8)
> Also, FcConfig[SG]etSysroot should take a FcConfig * as first argument.

Yeah, that too, right.
Comment 10 Laurentiu Palcu 2013-02-08 11:25:28 UTC
I'll try to find a fontconfig only solution to the sysroot implementation. Regarding the FcConfig * argument to FcConfig[SG]etSysroot(), why is this needed? _FcConfigSysRoot is not part of the FcConfig structure, it's static. Is this just an API requirement?
Comment 11 Behdad Esfahbod 2013-02-08 16:48:07 UTC
(In reply to comment #10)
> _FcConfigSysRoot is not part of the FcConfig structure, it's static.

It should be.
Comment 12 Laurentiu Palcu 2013-02-11 08:10:42 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > _FcConfigSysRoot is not part of the FcConfig structure, it's static.
> 
> It should be.

Well, I started to implement it this way but I soon realized it cannot be done. If you guys know a way, I'm open to ideas. Anyhow, here's why I did it static: when FcInitLoadConfig() is called, it must be already aware of any sysroot in order to contain the config file search to a certain location. Otherwise I would have had to pass the sysroot to FcInitLoadConfig() but, I'm pretty certain, changing the existing API would have been even worse. So, I'm not sure I can make it part of FcConfig structure...
Comment 13 Behdad Esfahbod 2013-02-11 20:46:38 UTC
Humm.  I see.  Ok, let me think about it.
Comment 14 Akira TAGOH 2013-02-27 09:33:08 UTC
http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz59456

proposed patch to fix this.
Please test.
Comment 15 Behdad Esfahbod 2013-03-04 23:30:23 UTC
If it works for you, commit it.

I think we should clean up FcInitLoadConfig() later on.
Comment 16 Akira TAGOH 2013-03-05 10:25:55 UTC
pushed to master. please reopen if any further work is needed.
Comment 17 Arnaud Vrac 2014-06-11 17:24:48 UTC
IMO the current implementation of fc-cache --sysroot is flawed. The original patch was using the sysroot path for _all_ files, including the config file and the fonts. In practice this meant those commands were giving the same result:

from the host:
# fc-cache -sy /target

from the target:
# fc-cache -s

Now it's just a mix of sysrooted files (the generated cache) and the host file (the fonts and the config), so it's pretty useless. Is there any way the original behaviour could be implemented instead ?
Comment 18 Arnaud Vrac 2014-06-12 10:14:04 UTC
I've just found out that the cache binary format is arch specific, using intptr_t... This makes the sysroot option totally useless, even if it was working.
Comment 19 Behdad Esfahbod 2014-06-12 16:50:14 UTC
Right.  It's no good for cross-compiling.
Comment 20 Akira TAGOH 2014-06-13 02:45:05 UTC
looking at it now
Comment 21 Akira TAGOH 2014-06-13 10:30:03 UTC
We may need to have a new API to avoid API breakage to scan a font on the targeted directory because, FcFreeTypeQuery*() currently takes an argument to see what a file should be read though, it will be added into the cache as well. however there are no way to know the path contains the sysroot one or not because it isn't accessible to FcConfig.
Comment 22 Behdad Esfahbod 2014-06-13 16:15:21 UTC
FcFreeTypeQuery doesn't touch the cache.  It does put the filename into the pattern.  The higher level can modify to remove the sysroot from the pattern.  I don't think fcfreetype layer needs to know about sysroot.
Comment 23 Akira TAGOH 2014-06-16 07:09:50 UTC
(In reply to comment #22)
> FcFreeTypeQuery doesn't touch the cache.  It does put the filename into the
> pattern.  The higher level can modify to remove the sysroot from the
> pattern.  I don't think fcfreetype layer needs to know about sysroot.

just thought it may be an extra work. there seems no way to do it other than that then. will fix it that way shortly
Comment 24 Akira TAGOH 2014-06-17 11:13:23 UTC
Fixed in git.
Comment 25 Colin Walters 2014-06-17 18:57:18 UTC
This commit breaks gnome-continuous; we've temporarily "tagged" back to the previous commit.  See:

https://git.gnome.org/browse/gnome-continuous/commit/?id=64c7b16ca8bd19a14d74d1c0f798a5fad98b671b

Which links to the build log, but basically fc-cache is segfaulting.  I don't yet have a stack, but I could get one if it's necessary.
Comment 26 Akira TAGOH 2014-06-18 02:50:12 UTC
Doh. thanks. fixed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.