increase chances of successful local builds for the passers-by#178
Conversation
| # Shaders that REQUIRE GStreamer (media features) | ||
| - group: "media" | ||
| features: "" | ||
| features: "--features media" |
There was a problem hiding this comment.
Just wanted to point this out - due to the lack of default features, the media feature now has to be added here. I didn't validate the workflow, but at least it looks reasonable.
|
|
||
| - If you want to try your own shaders, check out the [usage.md](usage.md) and see [BUILD.md](BUILD.md). | ||
| - **Optional Media Support**: GStreamer dependencies are optional - use `--no-default-features` for lightweight builds with pure GPU compute shaders. | ||
| - **Optional Media Support**: GStreamer dependencies are optional and disabled by default. Use `--features media` for video, webcam, and audio examples. |
There was a problem hiding this comment.
This change might go against what you had in mind. If so, the last commit can be dropped and the default-feature-change vanishes.
| [[example]] | ||
| name = "audiovis" | ||
| required-features = ["media"] |
There was a problem hiding this comment.
I didn't validate that all of these truly need the media feature, and assumed Codex would get this right merely by compiling.
|
hey, thanks for this I like the actual goal here (a passer by being able to clone and build without a gst (GStreamer) setup getting in the way) :-) but I don't want to get there by flipping the default off. gst is intentionally a core part of cuneus and I'm leaning more into media shaders, so default = [] would change the primary cargo run --example … workflow for existing projects and me every media example would suddenly need --features media. (same reasoning I had on #41 last year). but i like the required-features = ["media"] gating, the build.rs guard that skips setup when media is off, and the no-default-features CI just on top of default = ["media"]. that way the default stays full featured (with gst still), and a passer by can still do cargo run --no-default-features --example blackhole for a quick "pure" (without gst) build, with a clean "needs media" message instead of a confusing compile error on the media ones... so can you please close this, but the shape of the fix is yours thanks for it of course :-) One small note also: thanks for being transparent about the LLM usage <3. I recently added a short contribution policy that just asks to skip the Co-authored-by: codex claude etc like trailers/LLM company ads on commits (I borrowed recently from GTK) |
… without media GStreamer is already modeled as an optional dependency, but all-targets builds still tried to compile examples that call media-only APIs. That made the no-default-features path unusable for contributors and downstream users who do not have GStreamer installed. Marking media examples with required-features and skipping build-script GStreamer setup when media is disabled keeps the lightweight build path working while preserving the existing opt-in media feature.
The no-default-features build is the supported path for systems without GStreamer, so it needs direct CI coverage instead of being implied by release packaging jobs. Running the exact all-targets check in its own workflow catches feature-gating regressions without requiring media dependencies.
|
Thanks a lot for the review and sorry for not following the contribution guideline! I completely missed it, and didn't expect it either for some reason. In any case, the commits are now authored by me, and I removed the last commit that changed the default features.
I assumed you don't want me to close the PR and that I didn't get the context 😅. PS: I failed to express earlier that I also think what you are doing here is art (without the quotes), because all of the shaders are very pleasing. That makes them more than math to me, and I am way more interested to help me run them more easily. |
|
Thank you for the kind words and for your contributions of course 🙂 |
Thanks for this amazing project, I always love to have my computer show me pretty graphics. And as the cherry on top, these ones are very hackable should I ever be inclined.
This PR is motivated to fix the initial issues I had building it, which nearly drove me away. It's a minimal way of increasing the chances of successful builds, by better handling the already optional "media" feature.
I added CI checks for good measure. Something I left out is a CI check that includes "media" examples, as it needs the installation of dependencies and I didn't want to spend time fiddling with CI.
The most contention commit might the the last one which removes "media" from default features so it's trivial to build examples that don't need it, while showing a message that makes clear what to do for where it is needed.
Now running the blackhole shader works as expected:
❯ cargo run --release --example blackhole Compiling cuneus v0.6.2 (/Users/byron/dev/github.com/altunenes/cuneus) Finished [`release` profile [optimized]](https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles) target(s) in 4.98s Running `target/release/examples/blackhole`Whereas other example will be explicit about what they need:
AI Disclosure: The PR text is written by me, the commits and commit messages are by Codex. I reviewed each one hunk by hunk, and possibly edited the commit messages.
Notes for the review
I encourage you to edit everything at will with
gh pr checkout 178and just force-push into the PR to make it fit your needs. No need for a lot of back and forth, and closing it is also fine of course. This is a flyby, which cost me no more than 15 minutes.