search

r-lib / ragg

Graphic Devices Based on AGG
https://ragg.r-lib.org
Other
172 stars 24 forks source link

Ensure library link flags are unique, fixing build for Wasm/webR #152

Closed georgestagg closed 9 months ago

georgestagg commented 9 months ago

Fixes compiling with Emscripten for webR, where linking is static and so multiple instances of the same library link flag, such as -lz, leads to duplicate symbol errors.

jeroen commented 9 months ago

I think this is not a bug in the package configure but the build tooling. The package simply forwards the flags from pkg-config to the linker. Lots of packages will have this issue. Perhaps we should shim pkg-config to filter duplicates.

georgestagg commented 9 months ago

Perhaps we should shim pkg-config to filter duplicates.

Yes, good idea. As discussed, we should indeed handle this in r-wasm/rwasm. That also means our tweaks will only take effect for Emscripten builds, which is a nice safety net.

The package simply forwards the flags from pkg-config to the linker

I don't think that's strictly true here. Even after shimming pkg-config I'm getting a duplicate symbol error, I think due to the extra -ljpeg added at the end here:

https://github.com/r-lib/ragg/blob/792c1e10fa0ebcd7d89ff3e5227f51e8d299ec82/configure#L37-L39

jeroen commented 9 months ago

Ah you are right. I think the problem was actually introduced recently here: https://github.com/r-lib/ragg/commit/fa87a16a1bd19ce2680ddc22ee3bd576e189ddd7

Previously we hardcoded libjpeg libs as -ljpeg because it did not used to have a pc file. But then https://github.com/r-lib/ragg/commit/fa87a16a1bd19ce2680ddc22ee3bd576e189ddd7 was added, so that results in duplicates and also the configuration failing on systems without a jpeg.pc.

I think https://github.com/r-lib/ragg/commit/fa87a16a1bd19ce2680ddc22ee3bd576e189ddd7 was a mistake and should better be reverted, or otherwise we can remove the hardcoded -ljpeg.

georgestagg commented 9 months ago

I'm going to close this in PR favour of r-wasm/rwasm#9, but we should continue the discussion about what to do with -ljpeg. So, I'll open an issue instead and link it here.