LIHPC-Computational-Geometry / metis-rs

Idiomatic wrapper for METIS, the serial graph partitioner and fill-reducing matrix orderer
https://lihpc-computational-geometry.github.io/metis-rs/metis/
Apache License 2.0
10 stars 8 forks source link

Build library from source #15

Closed oisyn closed 11 months ago

oisyn commented 11 months ago

Hi!

Instead of just generating bindings, I've taken the liberty to build the whole library from source. The two dependent libraries, METIS and GKlib, are submodules in the vendor folder. This means you no longer have to figure out how to install METIS and set up the appropriate environment variables, it just works.

I haven't spent any time making things configurable. I can imagine someone might want to override compile flags, or override this build altogether and still use a preinstalled version of the library. If we can work out some spec, I'll be happy to add that.

Tested on Windows and Ubuntu (through WSL2).

cedricchevalier19 commented 11 months ago

Thanks for your contribution @oisyn .

It's an interesting feature but I do not think that it should replace the old behavior. We have use cases where we need to link against an already installed metis library and it is not possible anymore.

It should be a feature, perhaps we even can enable it by default (or as a failback), but I really think the old behavior should be available.

hhirtz commented 11 months ago

This looks good. Having things work out of the box is great. I went and made some changes there #16 (mainly what Cédric said). Many crates in the ecosystem default to dynamic linking [0-2], i'd rather metis-rs follow this convention.

Following metis' master branch seems fine since they don't seem to be tagging releases too much (no up-to-date changelog either), and their API is pretty much set in stone anyway.

[0] https://github.com/sfackler/rust-openssl/blob/f2217fd13903601ecd0e547cd5ce79d751c7c348/openssl-sys/Cargo.toml#L18 [1] https://github.com/alexcrichton/curl-rust/blob/4bf8eb2a87c8f9e4a6552a1fcf4f9695d7a600f3/curl-sys/Cargo.toml#L51 [2] https://github.com/rusqlite/rusqlite/blob/6176b11a3008d749cf501ff5d3782935e5348af3/libsqlite3-sys/Cargo.toml#L16

oisyn commented 11 months ago

I'm perfectly fine with that behavior 🙂. I think we can close this in favor of #16.