Divide project into library and binary parts

[tuguzT]

Fixes #105

[tombh]

Would a little comment here describing the clap-the-feature, be useful here?

[schell]

Yes, I agree a little note would be good.

[tombh]

So this allows using cargo-gpu as a library? I wonder if there is simple e2e test for that? And/or perhaps an example in the README of how to do that?

[schell]

We need to restructure this to handle the case where the watch feature is disabled to let the else case happen. I'm pretty sure that with this change if the watch feature is missing the entire if else won't be compiled.

[schell]

Something like:

#[cfg(feature = "watch")]
let watching = self.build.watch;
#[cfg(not(feature = "watch"))]
let watching = false;

if watching {
...
} else {
...
}

[tuguzT]

I agree with that, I'll rewrite this code soon.

[tuguzT]

The main difficulty here is not the flag itself, but the code inside of is watching block, which is valid only if watch feature of spirv-builder is enabled.
I really want to move this logic inside of private method to not to clutter run method with conditional compilation handling.

[schell]

I don't think we want this cfg here, because we still want to parse compilation results if we're not watching.

[tuguzT]

I placed this just because this was marked by linter as unused (because I've cfg-featured the code block above).
Rewriting an if above with removing this cfg will resolve it.

[schell]

Just needs a few tweaks to the cfg stuff. Good work!

[nazar-pc]

So this allows using cargo-gpu as a library? I wonder if there is simple e2e test for that? And/or perhaps an example in the README of how to do that?

It is already possible on main. This PR just allows you to not compile binary-only dependencies (like clap) when you only use it as a library, see linked issue.

[tuguzT]

I have a strong feeling that this line means watch method should return ! or anyhow::Result<!>, but I'm not sure which one was in mind when this code were written.

[schell]

Without looking at the code I'm thinking it makes most sense to use anyhow::Result<!>. Conceptually, watching is a never-ending loop, and there could be errors along the way.

[schell]

Ah, I see, that's on SpirvBuilder...

[schell]

So it looks like SpirvBuilder spawns a thread and calls the given callback with the compilation result. But it doesn't look like there's any way to know when/if that spawned thread exits. It seems expected we loop forever until the user exits explicitly (hits control-c), so I don't think SpirvBuilder::watch needs to communicate anything, and I think we can provide the anyhow::Result<!> in our watch function by doing:

loop {
    std::thread::park();
}

@tombh, @Firestar99 how do you two feel about this?

[tombh]

Even though I originally added the watch code in cargo-gpu I don't remember all of the failure cases I'm afraid. I do personally use --watch often and it works well (just a small annoyance that it compiles twice on first run). So all I'd want to see is that the compilation logs still display and that CTRL+C exits.

[schell]

I think in practice the loop above should never actually loop, as thread::park should never return. But the loopgives us the return type we want.

I haven't actually written any of this code I'm suggesting though, so the real compiler (as opposed to my head compiler) might have something else to say about it ;).

[tuguzT]

I noticed that if shader crate compiles with errors in the first time, it returns an error, so it cannot be anyhow::Result<!>.

I also decided to turn panics into errors.
I removed loop and thread park entirely: I don't know if this behavior is Windows-specific, but watching continued without explicit waiting.

Additionally, I have a feeling that watch could return JoinHandle or its wrapper to the caller. Then we could just join it inside of this private method and return an error if watching ends somehow.
This is impossible to do, because JoinHandle<!> cannot be returned. But we can return Infallible, which hopefully will be the never type after its stabilization.

I've made a PR for that.

[tuguzT]

Also I'm not sure about APIs of library and binary parts:

• Should Cli, Command structs and some binary-only features (for example, Config and dump usage) be moved inside of binary part?
• What should the minimal API of library part be (for it to be usable inside of a build script)?
  Is it just all the pub contents of install module & spirv-builder re-export (as it is currently), or is there something else what could be useful inside of a build script?

This could be resolved in some separate PR, but I think this needs to be addressed at least.

[tuguzT]

I'll try to reorder commits of this branch via rebase now to de-clutter merge history.

P.S.: success! 🎊

[tuguzT]

So this allows using cargo-gpu as a library? I wonder if there is simple e2e test for that? And/or perhaps an example in the README of how to do that?

It is already possible on main. This PR just allows you to not compile binary-only dependencies (like clap) when you only use it as a library, see linked issue.

I think it would be nice to add build.rs script for shader-crate-template showing the usage of cargo-gpu as a library, but should this be done here or in a separate PR?

[nazar-pc]

Certainly a separate PR

[schell]

So far so good, we just need to address the feature comment and the watch situation. Thank you!

[Firestar99]

Wait what, how did I miss this one? Will have a look tomorrow morning.
(I should probably be on the default-reviewer list as well)

[Firestar99]

One huge red flag: We cannot rename the cli tool. It must stay cargo-gpu, otherwise you'd have to call it with cargo gpu-cli instead of just cargo gpu (iirc). Especially since we want to (more strongly) mirror cargo commands in the future, like cargo gpu check or cargo gpu clippy.

Otherwise, I strongly support the split up. I just haven't gotten around to it, or figured out a good alternative name for the "cargo-gpu-lib". Some ideas, feel free to suggest more:

• rustc_codegen_spirv_cache
• cargo_gpu_cache
• rust_gpu_cache

[nazar-pc]

One huge red flag: We cannot rename the cli tool.

The binary is still cargo-gpu (see [[bin]] section), only the crate name has changed.

[schell]

Also I'm not sure about APIs of library and binary parts:

* Should `Cli`, `Command` structs and some binary-only features (for example, `Config` and dump usage) be moved inside of binary part?

* What should the minimal API of library part be (for it to be usable inside of a build script)?
  Is it just all the pub contents of `install` module & `spirv-builder` re-export (as it is currently), or is there something else what could be useful inside of a build script?

This could be resolved in some separate PR, but I think this needs to be addressed at least.

I would just make most of it pub and call it a day. We can always go back and clean up the API if it's too complex.

[nazar-pc]

In case it is useful as a reference, here is the minimal API I'm using right now:

        let backend = cargo_gpu::Install::from_shader_crate(shader_crate.clone()).run()?;

        let spirv_builder = backend
            .to_spirv_builder(shader_crate, "spirv-unknown-vulkan1.2")

https://github.com/nazar-pc/abundance/blob/f8ec952de139c87509a5080c71142e43f351b3e9/crates/farmer/ab-proof-of-space-gpu/build.rs#L37-L40

Plus spirv_builder crate re-export so I don't have to guess its version.

P.S. Noticed just now that for some reason path to shader crate (shader_crate) is needed twice, feels redundant 🤷

The goal in the issue #105 was to remove dependencies from the library crate. This PR is a step in the right direction, which makes dependencies optional. But it still doesn't remove them, which causes interesting side-effects when Cargo's feature unification kicks in. So I'd say eventually those dependencies should be removed, which likely means all data structures that depend on them (like CLI arguments/options) will need to move as well.

[tuguzT]

One huge red flag: We cannot rename the cli tool. It must stay cargo-gpu, otherwise you'd have to call it with cargo gpu-cli instead of just cargo gpu (iirc). Especially since we want to (more strongly) mirror cargo commands in the future, like cargo gpu check or cargo gpu clippy.

Otherwise, I strongly support the split up. I just haven't gotten around to it, or figured out a good alternative name for the "cargo-gpu-lib". Some ideas, feel free to suggest more:

• rustc_codegen_spirv_cache
• cargo_gpu_cache
• rust_gpu_cache

I honestly do not want to rename cargo-gpu crate into cargo-gpu-cli too (and binary name cargo-gpu must remain the same, as it is in this PR), but I haven't got any ideas about naming of library part.
Am I right to assume that cache in the suggested names means caching of fixed nightly toolchain and codegen shared library?

[tuguzT]

In case it is useful as a reference, here is the minimal API I'm using right now:

let backend = cargo_gpu::Install::from_shader_crate(shader_crate.clone()).run()?;

let spirv_builder = backend
    .to_spirv_builder(shader_crate, "spirv-unknown-vulkan1.2")

https://github.com/nazar-pc/abundance/blob/f8ec952de139c87509a5080c71142e43f351b3e9/crates/farmer/ab-proof-of-space-gpu/build.rs#L37-L40

Plus spirv_builder crate re-export so I don't have to guess its version.

Inside of my project (which I mentioned in #93) I moved to cargo-gpu library as soon as it became possible to do, because std's Command API worked only sometimes.
I use similar symbols, too (namely cargo_gpu::Install and spirv_builder reexport).

[schell]

So long as cargo-gpu works for the binary name, and you said it did, then cargo-gpu-cli is fine for the crate name. It's a nice and descriptive name, and folks won't really interact with it anyway.

Thanks for your great work @tuguzT!

[tuguzT]

Some ideas, feel free to suggest more:

• rustc_codegen_spirv_cache
• cargo_gpu_cache
• rust_gpu_cache

I think rustc_codegen_spirv_cache or spirv-builder-cache might suite quite well.

The former suits because, as stated in README,

cargo gpu compiles a custom codegen backend for rustc

The latter - because cargo-gpu directly re-exports spirv-builder and enhances its capabilities by managing its codegen.

---

P.S.: also these names could contain toolchain in them to signal that the library manages toolchain installation.

But for now, I prefer rustc_codegen_spirv_cache over other variants.

[tuguzT]

Another point against cargo-gpu-cli with its binary renamed to cargo-gpu is the following issue with documentation:

warning: output filename collision.
The bin target `cargo-gpu` in package `cargo-gpu-cli v0.1.0 (C:\...\cargo-gpu\crates\cargo-gpu-cli)` has the same output filename as the lib target `cargo_gpu` in package `cargo-gpu v0.1.0 (C:\...\cargo-gpu\crates\cargo-gpu)`.
Colliding filename is: C:\...\cargo-gpu\target\doc\cargo_gpu\index.html
The output filenames should be unique.
This is a known bug where multiple crates with the same name use
the same path; see <https://github.com/rust-lang/cargo/issues/6313>.
If this looks unexpected, it may be a bug in Cargo. Please file a bug report at
https://github.com/rust-lang/cargo/issues/ with as much information as you
can provide.
cargo 1.90.0 (840b83a10 2025-07-30) running on `x86_64-pc-windows-msvc` target `x86_64-pc-windows-msvc`
First unit: Unit { pkg: Package { id: PackageId { name: "cargo-gpu-cli", version: "0.1.0", source: "C:\\...\\cargo-gpu\\crates\\cargo-gpu-cli" }, ..: ".." }, target: TargetInner { name: "cargo-gpu", doc: true, ..: with_path("C:\\...\\cargo-gpu\\crates\\cargo-gpu-cli\\src\\main.rs", Edition2021) }, profile: Profile { ..: default_dev() }, kind: Host, mode: Doc, features: [], rustflags: [], rustdocflags: [], links_overrides: {}, artifact: false, artifact_target_for_features: None, is_std: false, dep_hash: 755822672195230307 }
Second unit: Unit { pkg: Package { id: PackageId { name: "cargo-gpu", version: "0.1.0", source: "C:\\...\\cargo-gpu\\crates\\cargo-gpu" }, ..: ".." }, target: TargetInner { name_inferred: true, ..: lib_target("cargo_gpu", ["lib"], "C:\\...\\cargo-gpu\\crates\\cargo-gpu\\src\\lib.rs", Edition2021) }, profile: Profile { ..: default_dev() }, kind: Host, mode: Doc, features: ["clap", "watch"], rustflags: [], rustdocflags: [], links_overrides: {}, artifact: false, artifact_target_for_features: None, is_std: false, dep_hash: 15945473915602456875 }

This means that cargo-gpu library docs could be inacessible, because its doc page could be rewritten with cargo-gpu-cli binary doc page, and vice versa.

[Firestar99]

Have you changed something CI related that lints is failing, or is it just failing in general? (I'll go fix it if it's on us)

[tuguzT]

I've moved reworked, more aggressive lib/bin split in a separate draft, #117