Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwrap pkg_config result in build script #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KarelPeeters
Copy link

Currently the build.rs script from coin_cbc_sys looks like this (I've added type annotations to clarify):

use pkg_config::{Error, Library};

fn main() {
    let _: Result<Library, Error> = pkg_config::probe_library("cbc");
}

The failure condition of the probe is being completely ignored! This means that a failure to find the library is only reported as confusing linking errors at the end of the build process:

[...] = note: LINK : fatal error LNK1181: cannot open input file 'CbcSolver.lib'

I think the build script should be changed to unwrap() the Result instead, which would cause cargo to show this more helpful error message, much sooner during the build process:

   --- stderr
  thread 'main' panicked at coin_cbc_sys\build.rs:5:38:
  called `Result::unwrap()` on an `Err` value: `"pkg-config" "--libs" "--cflags" "cbc"` did not exit successfully: exit code: 1
  error: could not find system library 'cbc' required by the 'coin_cbc_sys' crate

  --- stderr
  Package clp was not found in the pkg-config search path.
  Perhaps you should add the directory containing `clp.pc'
  to the PKG_CONFIG_PATH environment variable
  Package 'clp', required by 'cbc', not found

@TeXitoi
Copy link
Collaborator

TeXitoi commented Apr 17, 2024

That’s a feature. If pkg-config is not installed, it can still work with some other manual configuration. With unwrap, it would fail the build.

5487a82

I don’t really know if that’s good or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants