Bug 59456

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.

Thanks, I'll start working on this.

Created attachment 73824 [details]
Patch implementing sysroot option

Addressed Akira's comment on the ML.

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.

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?

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

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.

Also, FcConfig[SG]etSysroot should take a FcConfig * as first argument.

(In reply to comment #8)
> Also, FcConfig[SG]etSysroot should take a FcConfig * as first argument.

Yeah, that too, right.

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?

(In reply to comment #10)
> _FcConfigSysRoot is not part of the FcConfig structure, it's static.

It should be.

(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...

Humm.  I see.  Ok, let me think about it.

http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz59456

proposed patch to fix this.
Please test.

If it works for you, commit it.

I think we should clean up FcInitLoadConfig() later on.

pushed to master. please reopen if any further work is needed.

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 ?

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.

Right.  It's no good for cross-compiling.

looking at it now

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.

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.

(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

Fixed in git.

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.

Doh. thanks. fixed.