Jondolf / avian

ECS-driven 2D and 3D physics engine for the Bevy game engine.
https://crates.io/crates/avian3d
Apache License 2.0
1.36k stars 108 forks source link

Naughty repository structure causes build issues #298

Open musjj opened 8 months ago

musjj commented 8 months ago

Building bevy_xpbd from a git source with cargo vendor fails with an error involving ../../src/lib.rs.

To reproduce:

cargo init foobar
cd foobar
cargo add --git https://github.com/Jondolf/bevy_xpbd bevy_xpbd_2d
mkdir -p .cargo
cargo vendor >.cargo/config.toml
cargo build

The error:

   ....
   Compiling bevy_xpbd_2d v0.3.2 (https://github.com/Jondolf/bevy_xpbd#7e7e14d2)
error: couldn't read /tmp/foobar/vendor/bevy_xpbd_2d/../../src/lib.rs: No such file or directory (os error 2)

error: could not compile `bevy_xpbd_2d` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

The error is probably caused by this:

https://github.com/Jondolf/bevy_xpbd/blob/7e7e14d210efb27177d975367bc47090cfa3652d/crates/bevy_xpbd_2d/Cargo.toml#L34-L37

I'm also getting the same error when running cargo package -p bevy_xpbd_2d locally within this repository. So I'm guessing that the package structure is actually just invalid. Do you manually copy src into each sub-crate every time you publish to the registry?

I think the structure of the repository should definitely be revised so that it is compliant with cargo.

EDIT:

It looks that there's this thing:

https://github.com/Jondolf/bevy_xpbd/blob/7e7e14d210efb27177d975367bc47090cfa3652d/publish.sh#L11-L14

There's gotta be a better way.

Jondolf commented 8 months ago

So, as you're aware, bevy_xpbd is currently split into separate crates for 2D and 3D. However, most of the actual logic and structs can still be shared, and we don't want to just duplicate all of the code. The (current) solution is to have a single src folder, and have both crates point to that, but with different feature flags.

The issue that you're also experiencing is that for e.g. publishing, cargo just packages the crate's own folder, ignoring external folders and files like the entire src in this case. This results in the "../path/to/src/lib.rs: No such file or directory" warning when running cargo publish (or cargo vendor).

To be able to publish the crate, I made that shell script to copy the src, licenses, and other important files into the crate folders. This way, Cargo can find them and release correctly. This is also the approach used by Rapier and related projects (I kinda copied the structure from them orginally).

Potential solutions

Eventually, I'd like to experiment with a single physics crate that supports both 2D and 3D at the same time by having e.g. separate components for 2D and 3D rigid bodies. This would be more flexible, get rid of the awkward crate structure, allow a single directory for all examples, and also make the dimension feature flags additive, which in general is just good practise and probably preferrable for Bevy. The challenge is avoiding code duplication and excessive trait verbosity, but I'll try to experiment with this when I have time.

For now, a potential solution could be to have a separate bevy_xpbd or bevy_xpbd_core crate that would contain the engine code. Then bevy_xpbd_2d and bevy_xpbd_3d could have that as a dependency, using different feature flags. We could also just get rid of the separate 2D and 3D crates and force people to specify the 2d/3d feature flag instead, but I'm not sure if it's good practise to have to add a feature for the crate to work, and you'd also need to pass the feature when running examples and such.

I think a separate bevy_xpbd crate with the engine code could be reasonable for now. Would that be okay?

musjj commented 8 months ago

Thanks for the detailed explanation. Separate crates sounds like a reasonable idea, I think I'd be down for that. Though it does sound like it'd make the development workflow a bit more annoying (for contributors).

On the other hand, I've also tried experimenting with a simpler idea using symlinks instead: https://github.com/musjj/bevy_xpbd/tree/symlink So far it works pretty well. My only concern is its compatibility with Windows (I've only tested it on Linux). I've heard that you need to toggle a specific feature gate in order to create symlinks without elevated priviledges, but I'm not sure if it has changed recently.

71rd commented 2 months ago

unfortunately i also ran into this issue packaging a game that vendored this dependency.

Both of your solutions seem good to me and if unifying xpbd into a single crate seems like a better solution for development reasons, having to enable features when adding the crate does not seem like a big problem.

But if you dont want that, you could set 2d and 3d as default features of the crate, so disabling one of them would become a way for people wanting to optimize compile time.