cargo-c icon indicating copy to clipboard operation
cargo-c copied to clipboard

Implement pkg-config flag deduplication

Open amyspark opened this issue 6 months ago • 8 comments

Should reduce a lot of the noise in Cflags and Libraries.

See #455

amyspark avatar Jun 14 '25 17:06 amyspark

if it doesn't explode horribly once you have the usual permutation of -Wl,-z,relro -Wl,-z,now -Wl,--as-needed...

lu-zero avatar Jun 14 '25 19:06 lu-zero

if it doesn't explode horribly once you have the usual permutation of -Wl,-z,relro -Wl,-z,now -Wl,--as-needed...

The only corner case I have so far (tested with Windows, macOS ongoing) is that the self.libs entry coming from the cache fingerprint isn't split like pkg-config's, so I need to sort it out.

That one case of relro etc. sounds prime for testing with Android, I'll do that later today.

amyspark avatar Jun 14 '25 20:06 amyspark

Speaking as the guy who implemented Meson's de-dup of link args and cflags, this is very easy to get wrong. Did you use Meson's de-dup class as a reference when building this?

nirbheek avatar Jun 16 '25 07:06 nirbheek

@nirbheek do you have a test suite we can share?

lu-zero avatar Jun 16 '25 12:06 lu-zero

Meson's test case won't exactly map here, because reordering link args needs to be done carefully. You can have undefined-symbol errors if you get the order wrong, or pick up the library from the wrong prefix. Meson does these things to alleviate this risk:

  1. We wrap library args with -Wl,--start-group ... -Wl,--end-group
  2. We preserve the order of -L -l args when de-duping
  3. We resolve -L -l sets from each .pc file immediately to an on-disk file (static or shared, depending on config)

(1) (2) can be done by cargo-c, but not (3) since cargo-c doesn't know which one to pick.

Anyway, the implementation is here: https://github.com/mesonbuild/meson/blob/master/mesonbuild/arglist.py

Cflags-related tests are here: https://github.com/mesonbuild/meson/blob/master/unittests/internaltests.py#L114-L279

Some of the library find/arg tests are here: https://github.com/mesonbuild/meson/blob/master/unittests/internaltests.py#L537-L675. The others are integration tests, not unit tests, so not useful for cargo-c.

nirbheek avatar Jun 16 '25 12:06 nirbheek

thank you a lot :)

lu-zero avatar Jun 16 '25 17:06 lu-zero

Did you use Meson's de-dup class as a reference when building this?

Not at all, I was not thinking of any downstream build system in particular when doing this. I relied on pkg_config's own resolution logic for this, and made sure that flags did not lose their semantics until concatenation was needed e.g. -Wl,--start-group ... -Wl,--end-group is meant to stay as a single piece of link flag if it was supplied as such.

The big blocker for doing a Meson-Rust port here is that pkg_config is the only one that knows which modules are being fetched under the hood (managing cross sysroots, PKG_CONFIG_PATH, etc.), and there are no ways for this PoC to access just the run function and then parse the output manually; it's why I added a step that reconstitutes the flags from the Cargo build-script lines.

amyspark avatar Jun 16 '25 21:06 amyspark

I'm conflicted about this PR, it would be better if this kind of logic is part of cargo itself and possibly not rely on pkg-config being stable on dealing with it.

lu-zero avatar Jun 28 '25 07:06 lu-zero