Build and link Box2D in the build script

[Gohla]

Build and link Box2D as a Git submodule (pinned at version 2.3.1) in build.rs, not requiring users to have Box2D installed on their system. This significantly reduces the effort to use this library on Windows, since it does not have a package manager to install Box2D. This does require users to have CMake installed, because this is what Box2D uses as its build system.

Is this acceptable for merging, or would you rather have this as an optional feature?

[Gohla]

Thanks!

I don't think installing Box2D on the user's system is necessary

Do you mean the .define("BOX2D_INSTALL", "ON") line? This does not install Box2D on the system, because the cmake crate redirects the install prefix directory. It ends up being "installed" into a directory like target\debug\build\wrapped2d-1ee9bcd9113e85bb\out, so that println!("cargo:rustc-link-search=native={}/lib", box2d_install_prefix.display()); can link to it.

it would be nice if an already-installed Box2D (dynamic or static) could be specified or detected

I'm not sure how to detect an already installed Box2D, but I could put the environment variable back so that users can point to their own version of Box2D. What do you think the default (when env. variable is not set) should be? I'm in favor of the default being that it automatically builds and links Box2D, at least on non-linux systems where package management sucks.

Also the README will need to be updated and the version bumped :)

Right, I'll update the README and bump the version as well.

I would keep this custom build out of the user system like the frontend

I'm not entirely sure what you mean. Do you mean that the Box2D cmake build should be moved into a separate crate?

[Bastacyclop]

Do you mean the .define("BOX2D_INSTALL", "ON") line? This does not install Box2D on the system,

Oh I didn't know, all good then 👍

Do you mean that the Box2D cmake build should be moved into a separate crate?

That's not what I meant (I guess a box2d-sys crate would be idiomatic, but I personally don't see the benefits in this case).

I agree that the default should be the automatic local build, even on linux, an optional environment variable to a custom dynamic/static library should be enough flexibility.

[Gohla]

@Bastacyclop I've brought back the BOX2D_INCLUDE_PATH env variable, which should behave exactly the same as before when set. I also updated the README and version. I bumped the patch version because this should be backwards compatible, but maybe it is more appropriate to bump the minor version. Let me know if any more changes are needed.

[Bastacyclop]

I don't think we need a variable for the header files anymore. We have them at hand now, and modifying them would do no good IMO. I was more thinking about something like:

BOX2D=path/to/Box2D/lib_dir/lib.so cargo build

[Gohla]

You're right, setting the header files makes no sense (it has been a while since I have worked with C/C++ code 😄 ). I changed it to what you proposed, and bumped the minor version instead.

[Bastacyclop]

@Gohla There is no rush, but what's up ?

[Gohla]

@Bastacyclop no time for a while :) I can specify the library path on Windows now and that works, and I updated the readme. You should be able to squash the PR commits on merge

[Gohla]

👍

[Bastacyclop]

Nice bonus: docs.rs can build the documentation successfully now :)