Provide variant pkg-config file for multi-threaded static lib #4020
Conversation
|
Hi @Ansuel! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
lib/Makefile
Outdated
| @@ -319,6 +319,17 @@ libzstd.pc: libzstd.pc.in | |||
| -e 's|@LIBS_PRIVATE@|$(LDFLAGS_DYNLIB)|' \ | |||
| $< >$@ | |||
|
|
|||
| mt-libzstd.pc: mt-libzstd.pc.in | |||
There was a problem hiding this comment.
Both recipes look identical.
So they could be combined.
|
I understand the need, |
|
Do you mean a special env on calling make install-pc ? Also IMHO adding extra sed and token in the .in file might result in the file more bloated than keeping the thing separated but I understand it since I also hate duplicating things... It's just that makefile makes thing hard to handle with file stamp. |
Yes. It seems it would make sense to use the same variable both to create a MT static library, which would now depend on |
|
For static multi thread we have make lib-mt... Maybe something like MT=1
make install-pc ?
Il Sab 6 Apr 2024, 23:35 Yann Collet ***@***.***> ha scritto:
… Do you mean a special env on calling make install-pc ?
Yes.
It seems it would make sense to use the same variable both to create a MT
static library, which would now depend on -pthread, and then to update
the libzstd.pc with the correct link flags.
—
Reply to this email directly, view it on GitHub
<#4020 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE2ZMQW4DKGK2KTNH7MFE2DY4BTB7AVCNFSM6AAAAABF2N3SMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGIYTAOJRHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
5774737 to
8d88874
Compare
Multi-threaded static library require -pthread to correctly link and works. The pkg-config we provide tho only works with dynamic multi-threaded library and won't provide the correct libs and cflags values if lib-mt is used. To handle this, introduce an env variable MT to permit advanced user to install and generate a correct pkg-config file for lib-mt or detect if lib-mt target is called. With MT env set on calling make install-pc, libzstd.pc.in is a pkg-config file for a multi-threaded static library. On calling make lib-mt, a libzstd.pc is generated for a multi-threaded static library as it's what asked by the user by forcing it. libzstd.pc is changed to PHONY to force regeneration of it on calling lib targets or install-pc to handle case where the same directory is used for mixed compilation. This was notice while migrating from meson to make build system where meson generates a correct .pc file while make doesn't. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
|
@Cyan4973 I pushed a more advanced version of this... What do you think? I also think there was a very latent bug in how the .pc file was handled on dirty build. (since lib generates the .pc, any call to install-pc would ignore any custom values of the PREFIX env) should be fixed by forcing .pc regeneration every time. |
|
Yes, that looks much better. Give me some time to properly review the build/install flow, I should be able to comment this week. |
|
@Cyan4973 no problem and thanks for reviewing this. We recently switched from xz to zstd on openwrt and this is just an effort to upstream our downstream patch (we use multi-threaded static library on our build systems) |
Multi-threaded static library require -pthread to correctly link and works.
The pkg-config we provide tho only works with dynamic multi-threaded library
and won't provide the correct libs and cflags values if lib-mt is used.
To handle this, introduce an env variable MT to permit advanced user to
install and generate a correct pkg-config file for lib-mt or detect if
lib-mt target is called.
With MT env set on calling make install-pc, libzstd.pc.in is a
pkg-config file for a multi-threaded static library.
On calling make lib-mt, a libzstd.pc is generated for a multi-threaded
static library as it's what asked by the user by forcing it.
libzstd.pc is changed to PHONY to force regeneration of it on calling
lib targets or install-pc to handle case where the same directory is
used for mixed compilation.
This was notice while migrating from meson to make build system where
meson generates a correct .pc file while make doesn't.