refactor: use SpirvBuilder directly, just manage rustc_backend_spirv dylibs

[Firestar99]

rust-gpu PR: Rust-GPU/rust-gpu#245

closes#55
closes#68
closesRust-GPU/rust-gpu#208

The current version of cargo gpu roughly works like this, imagine it as stack frames:

• cargo gpu
• spawns spirv-builder-cli process
• calls SpirvBuilder
• spawns cargo process
• spawns rustc process
• loads rustc-codegen-spirv

And then we back-propagate the results:

• rustc-codegen-spirv: writes spv files and json metadata of ShaderModules
• Spirvbuilder: reads and parses json metadata and returns ShaderModules
• spirv-builder-cli: converts rust types to version-independent ShaderModules and writes them to a json file
• cargo gpu: reads json file, turns ShaderModules to Linkages (basically the same) and pretty writes a json metadata for the end user to parse

The core idea: Why not have cargo gpu directly call cargo with the required rustc-codegen-spirv flags attached, like Spirvbuilder does currently? And remove the complicated setup of spawning a new spirv-builder-cli process to call Spirvbuilder and parsing it all back out?

The key observation is that SpirvBuilder doesn't care at all what toolchain you use to build it, only it's dependency rustc_codegen_spirv has to be built by a specific toolchain. In Rust-GPU/rust-gpu#245 I make this dependency optional and hide it behind the feature rustc_codegen_spirv, which allows us to directly depend on SpirvBuilder and still build on stable. We just need to supply our SpirvBuilder with a librustc_codegen_spirv.so or rustc_codegen_spirv.dll prebuilt using the specific toolchain it requires, which significantly simplifies the build process.

This requires SpirvBuilder to stay be backwards compatible for all future versions, and we need to add support for past versions.

User-facing changes

• Renames:
  • capability -> capabilities
  • extension -> extensions
  • shader-target -> target
  • debug -> release (clap: remains --debug)
  • no_default_features -> default_features (clap: remains --no-default-features)
  • --force-spirv-cli-rebuild -> --rebuild-codegen
  • removed rust_toolchain (as you can't use a different toolchain than the one required anyway)
• proper path support for rust-gpu
  • will build rustc_codegen_spirv in that local path, no need to compile twice
  • will always try to rebuild
• faster install: removed slow rust-gpu repo cloning, instead reuse cargo's cache
• faster build: fewer cmds need to be launched or things parsed
• less disk space: clear target dirs of rustc_codegen_spirv after successful build

Internal changes, including to SpirvBuilder

• removed spirv_builder_cli crate
• split mod spirv_cli into spirv_source, install_toolchain and lockfile
• moved most BuildArgs members to SpirvBuilder
  • made all fields pub on SpirvBuilder, replacing setters (but keeping them around)
  • made SpirvBuilder derive serde::Serialize, serde::Deserialize
  • moved clap macro args to SpirvBuilder (masked by clap feature)
  • BuildArgs has a new SpirvBuilder member with clap(flatten) and serde(flatten)
  • required some renames, see above
• replaced cargo tree calls with cargo_metadata crate, cherry-picked from Switch to cargo metadata #66 by @tmvkrpxl0, required to "retrieve path to manifest" during install
• new install procedure:

 * check spirv-std version
- * clone rust-gpu repo at revision (slow)- * check `toolchain.toml` for toolchain- * copy `spirv-builder-cli` to cache dir+ * write blank `rustc_codegen_spirv_dummy` project+ * only depends on `rustc_codegen_spirv`, version equal to `spirv-std`+ * check metadata for project+ * cargo creates `Cargo.lock`+ * cargo clones repo into it's cache, if necessary+ * retrieve path to manifest, either in crates cache or cloned repo cache+ * check `build.rs` of `rustc_codegen_spirv` for toolchain version+ * has a **string literal** with the exact same contents as the `toolchain.toml`+ * delete `Cargo.lock` to prevent lockfile version issues
* build dummy project with determined toolchain
* retrieve `librustc_codegen_spirv.so` or `rustc_codegen_spirv.dll`
- * retrieve `spirv-builder-cli` binary
* copy target-specs jsons

• new build procedure:

 * cargo gpu
- * spawns `spirv-builder-cli` process+ * set `rustc_codegen_spirv_location` and `toolchain_overwrite` on SpirvBuilder
* calls SpirvBuilder
(...)
* SpirvBuilder returns
- * converts `CompileResult` into `ShaderModule`s, writes to disk- * `spirv-builder-cli` process exits- * load `ShaderModule`s from disk
* convert to `Linkage`s
* write `Linkage`s to file

Todo

• local path changes Cargo.lock for some reason
• make modern SpirvBuilder work with old versions
  • 0.9.0 & 0.8.0: complains about invalid target-specs json
  • install upgrades shader lockfile to version 4
• debug windows spirv-tools build fail
• add back lockfile version mismatch check (force_overwrite_lockfiles_v4_to_v3)
• watch is likely broken
  • watch is broken on windows, see refactor: use SpirvBuilder directly, just manage rustc_backend_spirv dylibs #69 (comment)
• cache toolchain channel in a file instead of reextracting it every time
• ci testing:
  • test local path by cloning rust-gpu repo
  • test use within build script, env vars can cause issues Build fails when RUSTC environment variable is set to nightly #68

[Firestar99]

@schell@tombhcargo gpu install works! build still needs to be adjusted

[Firestar99]

I would like to see the Switch to cargo metadata PR to be merged first (or integrated into here), as you can't just use it for more efficient version querying. It also tells you where cargo has checked out the repo / downloaded the crate, which we can use to retrieve the toolchain version we need without having to clone it ourselves.

Sample:

{
"name": "rustc_codegen_spirv",
"version": "0.9.0",
"id": "git+https://github.com/Rust-GPU/rust-gpu?rev=82a0f69#rustc_codegen_spirv@0.9.0",
"license": "MIT OR Apache-2.0",
"license_file": null,
"description": "SPIR-V code generator backend for rustc",
"source": "git+https://github.com/Rust-GPU/rust-gpu?rev=82a0f69#82a0f69008414f51d59184763146caa6850ac588",
..."manifest_path": "/home/firestar99/.cargo/git/checkouts/rust-gpu-d06d15e2ba0f0ae2/82a0f69/crates/rustc_codegen_spirv/Cargo.toml",
...
},

[tombh]

This looks great! So let me see if I have a rough idea of what the change is here. In short, we're replacing spirv-builder-cli with rustc_codegen_spirv_dummy (which in turns builds the custom SPIRV codegen backend for rustc)? The main benefit of which is that we can now interact directly with rust-gpu, significantly simplifying and stabilising cargo-gpu?

[tombh]

I think this PR would also fix this Rust-GPU/rust-gpu#208

[Firestar99]

Ignore rustc_codegen_spirv_dummy that only exists to depend on rustc_codegen_spirv and have cargo build it for us. I'll try to write a better summary in the description of all the changes.

[Firestar99]

@tombh feel free to rereview and merge, but I got one followup question:

cargo-gpu/crates/xtask/src/main.rs

Lines 154 to 188 in 6408188

	log::info!("installing cargo gpu artifacts");
	cmd([
	"cargo",
	"gpu",
	"install",
	"--shader-crate",
	SHADER_CRATE_PATH,
	"--auto-install-rust-toolchain",
	"--force-overwrite-lockfiles-v4-to-v3",
	])
	.unwrap();
	let dir = tempdir::TempDir::new("test-shader-output").unwrap();
	letmut overwriter = ShaderCrateTemplateCargoTomlWriter::new();
	overwriter.replace_output_dir(dir.path()).unwrap();
	ifletSome(rust_gpu_version) = maybe_rust_gpu_version {
	if rust_gpu_version != "latest"{
	overwriter
	.replace_spirv_std_version(rust_gpu_version)
	.unwrap();
	}
	}
	cmd([
	"cargo",
	"gpu",
	"build",
	"--shader-crate",
	SHADER_CRATE_PATH,
	"--auto-install-rust-toolchain",
	"--force-spirv-cli-rebuild",
	"--force-overwrite-lockfiles-v4-to-v3",
	])
	.unwrap();

Why are we installing one rust-gpu version (whatever is in the repo, currently a rev), then replace the version with what was specified in the cmdline and when building, reinstall a different rust-gpu version? I was wondering why our 0.9.0 and 0.8.0 builds were taking twice as much as the latest does. Any specific reason it has been done like that, or should we unify to testing only one version?

[tombh]

Wow, that doesn't seem right. I can't think of any reason why that might be. I wonder if @schell does? I think as long as the tests pass then just a single cargo build ... should be enough?

[tmvkrpxl0]

I like how we are discovering what it does along the way

[schell]

No clue why it does that, but it's probably just cruft from having us all hack on it with abandon, as they say :)

[schell]

I wish there were a way we could keep the usage in the README in sync with the tool without having to manually insert it here. 🤔

[schell]

Thank you for doing it, though, obviously.

[schell]

Great work! I checked out the source and this compiles my project.

I only had some quick nitpicks about typos. Can't wait to see this merge.

[Firestar99]

The version 0.8.0 and 0.9.0 builds are now just as fast as latest on ubuntu (not to speak of windows taking twice as long...)

[Firestar99]

@tombh as you requested changes earlier, github wants your approval before we can merge

[Firestar99]

Added a few more things:

• remove the target dir of rustc_codegen_spirv build, about 200MiB of saved disk space
• rename --force-spirv-cli-rebuild to --rebuild-codegen and fixed other docs mentioning it

[tombh]

Apologies, was under the impression that we only needed one approval to merge. Noted.

I'll take the opportunity again to say how great this PR is. Thank you.

[Firestar99]

I can only squash and merge. Could we switch it to just merging (no squashing) for just this PR, as it does change a lot?

[tombh]

I don't appear to have permissions for that, I'm sure @schell does.

Just to throw in my 2 cents, I'm not totally convinced that these commits would be better merged as they are over being squashed. Whilst they do tell an important story, they also clutter up main, making reading said story harder in other contexts, say for bisecting a bug. But don't let that stop you merging! Just wanted to mention it.

[nazar-pc]

Generally better to use full argument names in places like this for improved readability:

Suggested change

	build_command.args(["-p","rustc_codegen_spirv","--lib"]);
	build_command.args(["--package","rustc_codegen_spirv","--lib"]);