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

Retrieving all aggregated include/flags/etc for the entire probe #28

Closed
pbor opened this issue Feb 7, 2021 · 8 comments · Fixed by #32
Closed

Retrieving all aggregated include/flags/etc for the entire probe #28

pbor opened this issue Feb 7, 2021 · 8 comments · Fixed by #32

Comments

@pbor
Copy link

pbor commented Feb 7, 2021

I need to build a small C library from build.rs (see below for an example), so I am using the cc crate, however there is no "simple" way to get all the include_paths. I have to use libs.get("foo").unwrap().include_paths.clone(); but this has various issues:

  1. I have to know that "foo" pulls in the other deps, or manually get all includes and aggregate them
  2. The hashtable returned by probe() mangles the library name, so for instance get("gdk-pixbuf-2.0") fails, because the actual key in the map is gdk_pixbuf_2_0, but the caller has no way to know this.

Maybe probe should actually return a new type rather than the raw hash table.

    let libs = system_deps::Config::new().probe();
    if let Err(s) = libs {
        println!("cargo:warning={}", s);
        process::exit(1);
    }

    let libs = libs.unwrap();
    let includes = libs.get("foo").unwrap().include_paths.clone();

    let mut cc = cc::Build::new();

    cc.file("tests/constant.c");
    cc.file("tests/layout.c");

     for i in includes {
        cc.include(i);
    }

    cc.compile("cabitests");
@gdesmott
Copy link
Owner

gdesmott commented Feb 8, 2021

Hi Paolo, thanks for your feedback.

I have to know that "foo" pulls in the other deps, or manually get all includes and aggregate them

I'm not sure to understand what you mean here. The libs hash table will contain only the dependencies listed in your Cargo.toml. It won't recursively extend it to their own dependencies.

The hashtable returned by probe() mangles the library name, so for instance get("gdk-pixbuf-2.0") fails, because the actual key in the map is gdk_pixbuf_2_0, but the caller has no way to know this.

The key should always be the entry name in Cargo.toml which is defined here as gdk_pixbuf_2_0 in your example.
That's probably something I could make clearer in the doc indeed.

That said, returning a custom objects rather than an hash table sounds like a good idea. Having helpers returning aggregated values of the different settings would make sense, I think.

@gdesmott
Copy link
Owner

gdesmott commented Feb 8, 2021

That's probably something I could make clearer in the doc indeed.

Done in #31

@pbor
Copy link
Author

pbor commented Feb 8, 2021

Sorry I was not clear. Yes, basically I had two requests/suggestions:

  1. return a custom object so that it can have methods to get the list of all includes etc (what I meant was that right now requires either walking the whole table, or know that if I ask the headers of gdk-pixbuf, that in turns pulls in glib etc)
  2. Thanks for clarifying the documentation. I see now that gtk-rs has an utility to do the mangling when it generates the Cargo.toml file, so I can use the same. With that said, if we return a wrapper object, maybe we can also consider an utility to get_by_name

@gdesmott
Copy link
Owner

gdesmott commented Feb 9, 2021

I just implemented the API we discussed in #32 . Can you please give it a shot and check if it fits your needs?

@pbor
Copy link
Author

pbor commented Feb 9, 2021

That was fast! It looks like what I need, but I will give that a try as soon as possible.

Maybe call the object Dependencies instead of Libraries since it is a bit more generic?

@gdesmott
Copy link
Owner

gdesmott commented Feb 9, 2021

Maybe call the object Dependencies instead of Libraries since it is a bit more generic?

Makes sense, I renamed it.

@pbor
Copy link
Author

pbor commented Feb 10, 2021

I confirm the branch works for me

@gdesmott
Copy link
Owner

I just released 3.0.0 with this new API.

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 a pull request may close this issue.

2 participants