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

Build various target libraries from source #7

Merged
merged 11 commits into from
Jan 8, 2021

Conversation

GrygrFlzr
Copy link
Contributor

@GrygrFlzr GrygrFlzr commented Jan 4, 2021

Increment version to 0.2.2
Add dependency on cmake and cc crates
Updated readme with additional instructions

Supports:

  • i686-pc-windows-msvc
  • i686-pc-windows-gnu
  • x86_64-pc-windows-gnu

Increment version to 0.2.2
Add dependency on cmake and cc crates
Updated readme with additional instructions
build/build.rs Outdated Show resolved Hide resolved
build/build.rs Outdated Show resolved Hide resolved
- Removed git submodule initialization
- Move all library outputs to OUT_DIR
Cargo.toml Outdated Show resolved Hide resolved
Libraries were built ahead-of-time for the following platforms:
- x86_64-pc-windows-msvc
- x86_64-unknown-linux-gnu
- x86_64-apple-darwin
- aarch64-linux-android
- armv7-linux-androideabi

Libraries are built from source for the following platform(s):
- i686-pc-windows-msvc
@GrygrFlzr
Copy link
Contributor Author

@cart @bjorn3 I've come up with a novel solution to our issue!

By making use of The links Manifest Key and Overriding Build Scripts, we can remove all build dependencies for all other supported platforms:

  • x86_64-pc-windows-msvc
  • x86_64-unknown-linux-gnu
  • x86_64-apple-darwin
  • aarch64-linux-android
  • armv7-linux-androideabi

.cargo/config.toml supplies the build metadata ahead of time and skips running the build script altogether, and so skips compiling the build dependencies.

@@ -0,0 +1,64 @@
[target.x86_64-pc-windows-msvc.glslang]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only applies when directly compiling this crate. It doesn't have any effect when using this crate as a dependency unfortunately. .cargo/config.toml is only read for the directory from which cargo was invoked, parent dirs and the home dir, not from arbitrary dependencies.

Properly skips optional dependencies pending rust-lang/cargo#7914
More extensible for addition of new platforms
@GrygrFlzr
Copy link
Contributor Author

GrygrFlzr commented Jan 5, 2021

Okay since the config.toml solution doesn't actually work, I've made the build require a feature flag instead, automatically enabled by platform.

Currently, on stable cargo, it will eagerly activate the feature for all targets regardless:

$ cargo tree
bevy-glsl-to-spirv v0.2.2 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv)
└── glslang_actual_builder v0.1.0 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv\glslang_actual_builder)
    [build-dependencies]
    ├── cc v1.0.66
    │   └── jobserver v0.1.21
    └── cmake v0.1.45
        └── cc v1.0.66 (*)

On nightly with the itarget feature enabled, the extraneous dependency will be removed:

$ cargo +nightly tree -Z features=itarget
bevy-glsl-to-spirv v0.2.2 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv)
└── glslang_actual_builder v0.1.0 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv\glslang_actual_builder)

$ cargo +nightly tree -Z features=itarget --target i686-pc-windows-msvc
bevy-glsl-to-spirv v0.2.2 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv)
└── glslang_actual_builder v0.1.0 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv\glslang_actual_builder)
    [build-dependencies]
    ├── cc v1.0.66
    │   └── jobserver v0.1.21
    └── cmake v0.1.45
        └── cc v1.0.66 (*)

Fortunately, this isn't so bad, because cc and jobserver are already part of bevy's dependency tree at the moment, so the only new external dependency is cmake.

$ cargo +nightly tree -Z features=itarget -i cc
cc v1.0.66
[build-dependencies]
├── minimp3-sys v0.3.2
│   └── minimp3 v0.5.1
│       └── ...
├── spirv-reflect v0.2.3
│   └── bevy_render v0.4.0 (C:\Users\GrygrFlzr\Documents\projects\bevy\crates\bevy_render)
│       ├── ...
└── spirv_cross v0.22.2
    ├── ...


$ cargo +nightly tree -Z features=itarget -i cmake
(blank)

$ cargo tree -i cmake
cmake v0.1.45
[build-dependencies]
└── glslang_actual_builder v0.1.0 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv\glslang_actual_builder)
    └── bevy-glsl-to-spirv v0.2.2 (C:\Users\GrygrFlzr\Documents\projects\glsl-to-spirv)
        └── ...

@cart
Copy link
Owner

cart commented Jan 5, 2021

Ok the cmake dependency seems acceptable, provided that it won't break builds on systems that (1) don't have cmake and (2) can use one of the precompiled binaries in this crate.

@GrygrFlzr
Copy link
Contributor Author

Systems that already have precompiled libraries will continue as normal and never call the cmake crate, so real cmake never gets called.

  • x86_64-pc-windows-msvc
  • x86_64-unknown-linux-gnu
  • x86_64-apple-darwin
  • aarch64-linux-android
  • armv7-linux-androideabi

Systems that are explicitly opted into building will build once and reuse the compiled libraries.

  • i686-pc-windows-msvc

Other systems will panic (without calling cmake), but before this PR they would either:

  • panic anyway
    • aarch64-unknown-linux-gnu
    • i686-pc-windows-gnu
    • i686-unknown-linux-gnu
    • x86_64-pc-windows-gnu
    • A bunch of Tier 2 and Tier 3 platforms
  • not depend on this crate
    • wasm32-unknown-emscripten
    • wasm32-unknown-unknown
    • wasm32-wasi
    • aarch64-apple-ios and other iOS variants
    • aarch64-apple-darwin

It might be possible to use GitHub workflows to check if the four remaining Tier 1 platforms can be built from source.

@GrygrFlzr GrygrFlzr changed the title Build i686-pc-windows-msvc libraries from source Build various target libraries from source Jan 6, 2021
@GrygrFlzr
Copy link
Contributor Author

I've managed to add support for i686-pc-windows-gnu and x86_64-pc-windows-gnu (you can run the test and see it works).
Bad news for i686-pc-windows-gnu, it breaks on another bevy dependency: spirv_cross, but we can deal with that another time.
Good news for x86_64-pc-windows-gnu, you can run bevy on it now!

image

@GrygrFlzr
Copy link
Contributor Author

@cart Anything left you want me to tick off?

@cart
Copy link
Owner

cart commented Jan 8, 2021

Awesome work! I'll boot into windows and give it a whirl.

@cart
Copy link
Owner

cart commented Jan 8, 2021

This worked as expected on windows (msvc 64, msvc 32, gnu 64) and on Linux. Nice!

@cart cart merged commit 973f5e5 into cart:master Jan 8, 2021
@cart
Copy link
Owner

cart commented Jan 8, 2021

To publish this we'll need to publish glslang-actual-builder separately / update the dependency in the main crate. Additionally, I think we should probably rename glslang-actual-builder to something like bevy-glsl-to-spirv-builder.

@cart
Copy link
Owner

cart commented Jan 8, 2021

Feel free to do the rename in a followup pr. If not ill get to it (and publish the crates) before the next bevy release.

@GrygrFlzr
Copy link
Contributor Author

To publish this we'll need to publish glslang-actual-builder separately / update the dependency in the main crate. Additionally, I think we should probably rename glslang-actual-builder to something like bevy-glsl-to-spirv-builder.

How are bevy subcrates handled?

[dependencies]
bevy_dylib = {path = "crates/bevy_dylib", version = "0.4.0", default-features = false, optional = true}
bevy_internal = {path = "crates/bevy_internal", version = "0.4.0", default-features = false}

I made the assumption that since bevy did it, it was doable to publish it as a single crate, does it have to do with defining a workspace?

Agreed on the rename if we need to publish it as a separate crate.

@GrygrFlzr
Copy link
Contributor Author

@cart PR #9 is ready to go.

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.

3 participants