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

Put commands in their own files and some stricter lints #25

Merged
merged 4 commits into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/push.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
name: push

on:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised my editor is leaving this whitespace!

on:
push:
branches:
branches:
- main
pull_request:

Expand All @@ -13,18 +13,18 @@ env:
jobs:
install-and-build-shaders:
strategy:
matrix:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.os }}
defaults:
run:
run:
shell: bash
env:
env:
RUST_LOG: debug
steps:
- uses: actions/checkout@v2
- uses: moonrepo/setup-rust@v1
- run: rustup default stable
- run: rustup default stable
- run: rustup update
- run: cargo test
- run: cargo install --path crates/cargo-gpu
Expand Down
31 changes: 30 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[workspace]
members = [
"crates/cargo-gpu",
"crates/cargo-gpu",
"crates/shader-crate-template"
]

Expand All @@ -21,3 +21,32 @@ relative-path = "1.9.3"
serde = { version = "1.0.214", features = ["derive"] }
serde_json = "1.0.132"
toml = "0.8.19"

[workspace.lints.rust]
missing_docs = "warn"

[workspace.lints.clippy]
all = { level = "warn", priority = 0 }
pedantic = { level = "warn", priority = 0 }
nursery = { level = "warn", priority = 0 }
cargo = { level = "warn", priority = 0 }
restriction = { level = "warn", priority = 0 }
blanket_clippy_restriction_lints = { level = "allow", priority = 1 }

arithmetic_side_effects = { level = "allow", priority = 1 }
absolute_paths = { level = "allow", priority = 1 }
cargo_common_metadata = { level = "allow", priority = 1 }
implicit_return = { level = "allow", priority = 1 }
single_call_fn = { level = "allow", priority = 1 }
question_mark_used = { level = "allow", priority = 1 }
multiple_crate_versions = { level = "allow", priority = 1 }
pub_with_shorthand = { level = "allow", priority = 1 }
partial_pub_fields = { level = "allow", priority = 1 }
pattern_type_mismatch = { level = "allow", priority = 1 }
print_stdout = { level = "allow", priority = 1 }
std_instead_of_alloc = { level = "allow", priority = 1 }

# TODO: Try to not depend on these lints
unwrap_used = { level = "allow", priority = 1 }
panic = { level = "allow", priority = 1 }

6 changes: 5 additions & 1 deletion crates/cargo-gpu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ name = "cargo-gpu"
version = "0.1.0"
edition = "2021"
description = "Generates shader .spv files from rust-gpu shader crates"
repository = "https://github.com/Rust-GPU/cargo-gpu"
readme = "../../README.md"

keywords = ["gpu", "compiler"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Expand Down Expand Up @@ -33,3 +34,6 @@ codegen-units = 256
opt-level = 3
incremental = true
codegen-units = 256

[lints]
workspace = true
23 changes: 14 additions & 9 deletions crates/cargo-gpu/src/builder.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
use std::io::Write;
//! `cargo gpu build`, analogous to `cargo build`

use std::io::Write as _;

use clap::Parser;
use spirv_builder_cli::{Linkage, ShaderModule};

use crate::{install::Install, target_spec_dir};

/// `cargo build` subcommands
#[derive(Parser, Debug)]
pub(crate) struct Build {
pub struct Build {
/// Install the `rust-gpu` compiler and components
#[clap(flatten)]
install: Install,

Expand All @@ -32,6 +36,7 @@ pub(crate) struct Build {
}

impl Build {
/// Entrypoint
pub fn run(&mut self) {
let (dylib_path, spirv_builder_cli_path) = self.install.run();

Expand Down Expand Up @@ -93,7 +98,7 @@ impl Build {
entry,
path: filepath,
}| {
use relative_path::PathExt;
use relative_path::PathExt as _;
let path = self.output_dir.join(filepath.file_name().unwrap());
std::fs::copy(&filepath, &path).unwrap();
let path_relative_to_shader_crate =
Expand All @@ -109,19 +114,19 @@ impl Build {
linkage.sort();
// UNWRAP: safe because we know this always serializes
let json = serde_json::to_string_pretty(&linkage).unwrap();
let mut file = std::fs::File::create(&manifest_path).unwrap_or_else(|e| {
let mut file = std::fs::File::create(&manifest_path).unwrap_or_else(|error| {
log::error!(
"could not create shader manifest file '{}': {e}",
"could not create shader manifest file '{}': {error}",
manifest_path.display(),
);
panic!("{e}")
panic!("{error}")
});
file.write_all(json.as_bytes()).unwrap_or_else(|e| {
file.write_all(json.as_bytes()).unwrap_or_else(|error| {
log::error!(
"could not write shader manifest file '{}': {e}",
"could not write shader manifest file '{}': {error}",
manifest_path.display(),
);
panic!("{e}")
panic!("{error}")
});

log::info!("wrote manifest to '{}'", manifest_path.display());
Expand Down
51 changes: 32 additions & 19 deletions crates/cargo-gpu/src/install.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
use std::io::Write;
//! Install a dedicated per-shader crate that has the `rust-gpu` compiler in it.
use std::io::Write as _;

use crate::{cache_dir, spirv::Spirv, target_spec_dir};

const SPIRV_BUILDER_CLI_CARGO_TOML: &str = include_str!("../../spirv-builder-cli/Cargo.toml");
const SPIRV_BUILDER_CLI_MAIN: &str = include_str!("../../spirv-builder-cli/src/main.rs");
const SPIRV_BUILDER_CLI_LIB: &str = include_str!("../../spirv-builder-cli/src/lib.rs");
/// These are the files needed to create the dedicated, per-shader `rust-gpu` builder create.
const SPIRV_BUILDER_FILES: &[(&str, &str)] = &[
("Cargo.toml", SPIRV_BUILDER_CLI_CARGO_TOML),
("src/main.rs", SPIRV_BUILDER_CLI_MAIN),
("src/lib.rs", SPIRV_BUILDER_CLI_LIB),
(
"Cargo.toml",
include_str!("../../spirv-builder-cli/Cargo.toml"),
),
(
"src/main.rs",
include_str!("../../spirv-builder-cli/src/main.rs"),
),
(
"src/lib.rs",
include_str!("../../spirv-builder-cli/src/lib.rs"),
),
];

/// Metadata for the compile targets supported by `rust-gpu`
const TARGET_SPECS: &[(&str, &str)] = &[
(
"spirv-unknown-opengl4.0.json",
Expand Down Expand Up @@ -74,15 +83,16 @@ const TARGET_SPECS: &[(&str, &str)] = &[
),
];

/// `cargo gpu install`
#[derive(clap::Parser, Debug)]
pub(crate) struct Install {
pub struct Install {
/// spirv-builder dependency, written just like in a Cargo.toml file.
#[clap(long, default_value = Spirv::DEFAULT_DEP)]
spirv_builder: String,

/// Rust toolchain channel to use to build `spirv-builder`.
///
/// This must match the `spirv_builder` argument.
/// This must be compatible with the `spirv_builder` argument as defined in the `rust-gpu` repo.
#[clap(long, default_value = Spirv::DEFAULT_CHANNEL)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

rust_toolchain: String,

Expand All @@ -92,18 +102,20 @@ pub(crate) struct Install {
}

impl Install {
/// Returns a [`Spirv`] instance, responsible for ensuring the right version of the `spirv-builder-cli` crate.
fn spirv_cli(&self) -> Spirv {
Spirv {
dep: self.spirv_builder.clone(),
channel: self.rust_toolchain.clone(),
}
}

/// Create the `spirv-builder-cli` crate.
fn write_source_files(&self) {
let cli = self.spirv_cli();
let checkout = cli.cached_checkout_path();
std::fs::create_dir_all(checkout.join("src")).unwrap();
for (filename, contents) in SPIRV_BUILDER_FILES.iter() {
for (filename, contents) in SPIRV_BUILDER_FILES {
log::debug!("writing {filename}");
let path = checkout.join(filename);
let mut file = std::fs::File::create(&path).unwrap();
Expand All @@ -114,8 +126,9 @@ impl Install {
}
}

/// Add the target spec files to the crate.
fn write_target_spec_files(&self) {
for (filename, contents) in TARGET_SPECS.iter() {
for (filename, contents) in TARGET_SPECS {
let path = target_spec_dir().join(filename);
if !path.is_file() || self.force_spirv_cli_rebuild {
let mut file = std::fs::File::create(&path).unwrap();
Expand All @@ -124,14 +137,14 @@ impl Install {
}
}

// Install the binary pair and return the paths, (dylib, cli).
/// Install the binary pair and return the paths, (dylib, cli).
pub fn run(&self) -> (std::path::PathBuf, std::path::PathBuf) {
// Ensure the cache dir exists
let cache_dir = cache_dir();
log::info!("cache directory is '{}'", cache_dir.display());
std::fs::create_dir_all(&cache_dir).unwrap_or_else(|e| {
std::fs::create_dir_all(&cache_dir).unwrap_or_else(|error| {
log::error!(
"could not create cache directory '{}': {e}",
"could not create cache directory '{}': {error}",
cache_dir.display()
);
panic!("could not create cache dir");
Expand Down Expand Up @@ -178,7 +191,7 @@ impl Install {

command.args([
"--features",
&Self::get_required_spirv_builder_version(spirv_version.channel),
&Self::get_required_spirv_builder_version(&spirv_version.channel),
]);

log::debug!("building artifacts with `{:?}`", command);
Expand Down Expand Up @@ -209,8 +222,8 @@ impl Install {
} else {
log::error!("could not find {}", cli_path.display());
log::debug!("contents of '{}':", release.display());
for entry in std::fs::read_dir(&release).unwrap() {
let entry = entry.unwrap();
for maybe_entry in std::fs::read_dir(&release).unwrap() {
let entry = maybe_entry.unwrap();
log::debug!("{}", entry.file_name().to_string_lossy());
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 good naming :)

}
panic!("spirv-builder-cli build failed");
Expand All @@ -228,9 +241,9 @@ impl Install {
/// `spirv-builder` version from there.
/// * Warn the user that certain `cargo-gpu` features aren't available when building with
/// older versions of `spirv-builder`, eg setting the target spec.
fn get_required_spirv_builder_version(toolchain_channel: String) -> String {
fn get_required_spirv_builder_version(toolchain_channel: &str) -> String {
let parse_date = chrono::NaiveDate::parse_from_str;
let datetime = parse_date(&toolchain_channel, "nightly-%Y-%m-%d").unwrap();
let datetime = parse_date(toolchain_channel, "nightly-%Y-%m-%d").unwrap();
let pre_cli_date = parse_date("2024-04-24", "%Y-%m-%d").unwrap();

if datetime < pre_cli_date {
Expand Down
Loading
Loading