gdesmott / system-deps

Run pkg-config from declarative dependencies in Cargo.toml
Apache License 2.0
89 stars 21 forks source link

Don't ignore transitive imports when probing a static library #77

Closed amyspark closed 8 months ago

amyspark commented 1 year ago

This is needed towards building GStreamer's Rust plugins statically on Windows.

CC @nirbheek

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main@a80ea92). Click here to learn what that means.

:exclamation: Current head d3dc609 differs from pull request most recent head a89df41. Consider uploading reports for the commit a89df41 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #77 +/- ## ======================================= Coverage ? 75.14% ======================================= Files ? 3 Lines ? 3022 Branches ? 0 ======================================= Hits ? 2271 Misses ? 751 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gdesmott commented 1 year ago

We stopped relying on pkg-config to print metadata as we do it manually, see 331bab3e90687b3892acf2e8868668448611766b

Shouldn't we fix that manual code instead then?

amyspark commented 1 year ago

@gdesmott I assume you mean this? https://github.com/gdesmott/system-deps/blob/331bab3e90687b3892acf2e8868668448611766b/src/lib.rs#L152

If so, I'll see what I can do.

amyspark commented 1 year ago

@gdesmott I found the code, it needs a copy of rust-lang/pkg-config-rs#154 applied. Let me know if this looks OK now.

amyspark commented 9 months ago

@gdesmott Hi again, just updated the PR. Could this be reviewed now?

gdesmott commented 9 months ago

The change seems safe enough now.

Which actual use case is it fixing? Did you check it build and is usable now?

sdroege commented 9 months ago

FWIW the pkg-config-rs change that does basically the same is merged and released

gdesmott commented 8 months ago

@amyspark : sorry I missed your latest update.

Can you please rebase on top of main (I just fixed the CI) so we can merge this?

amyspark commented 8 months ago

@gdesmott done, let me know if anything else's needed.

gdesmott commented 8 months ago

Released in 6.2.1, sorry for the delay.