From 261b3e9ee78f3edb7e8d02debfab233689f87814 Mon Sep 17 00:00:00 2001 From: Jon C Date: Wed, 20 Mar 2024 13:21:00 +0100 Subject: [PATCH] CI: Add windows clippy job and fix clippy errors (#330) * CI: Run clippy on windows * Update cargo-clippy-before-script.sh for Windows * Pacify clippy --- .github/scripts/cargo-clippy-before-script.sh | 4 +++ .github/workflows/cargo.yml | 2 ++ accounts-db/src/hardened_unpack.rs | 3 ++ .../src/geyser_plugin_manager.rs | 6 +++- install/src/command.rs | 10 ++----- programs/sbf/benches/bpf_loader.rs | 7 +++-- rpc/src/rpc_service.rs | 29 +++++++++---------- 7 files changed, 35 insertions(+), 26 deletions(-) diff --git a/.github/scripts/cargo-clippy-before-script.sh b/.github/scripts/cargo-clippy-before-script.sh index b9426203a..bba030608 100755 --- a/.github/scripts/cargo-clippy-before-script.sh +++ b/.github/scripts/cargo-clippy-before-script.sh @@ -6,6 +6,10 @@ os_name="$1" case "$os_name" in "Windows") + vcpkg install openssl:x64-windows-static-md + vcpkg integrate install + choco install protoc + export PROTOC='C:\ProgramData\chocolatey\lib\protoc\tools\bin\protoc.exe' ;; "macOS") brew install protobuf diff --git a/.github/workflows/cargo.yml b/.github/workflows/cargo.yml index 3d7b1371b..b52a543e2 100644 --- a/.github/workflows/cargo.yml +++ b/.github/workflows/cargo.yml @@ -31,6 +31,7 @@ jobs: matrix: os: - macos-latest-large + - windows-latest runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -53,6 +54,7 @@ jobs: matrix: os: - macos-latest-large + - windows-latest runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 diff --git a/accounts-db/src/hardened_unpack.rs b/accounts-db/src/hardened_unpack.rs index ebdafe675..cff22fde8 100644 --- a/accounts-db/src/hardened_unpack.rs +++ b/accounts-db/src/hardened_unpack.rs @@ -205,6 +205,9 @@ where #[cfg(windows)] fn set_perms(dst: &Path, _mode: u32) -> std::io::Result<()> { let mut perm = fs::metadata(dst)?.permissions(); + // This is OK for Windows, but clippy doesn't realize we're doing this + // only on Windows. + #[allow(clippy::permissions_set_readonly_false)] perm.set_readonly(false); fs::set_permissions(dst, perm) } diff --git a/geyser-plugin-manager/src/geyser_plugin_manager.rs b/geyser-plugin-manager/src/geyser_plugin_manager.rs index d88814d88..d5521c9ad 100644 --- a/geyser-plugin-manager/src/geyser_plugin_manager.rs +++ b/geyser-plugin-manager/src/geyser_plugin_manager.rs @@ -451,9 +451,13 @@ mod tests { plugin: P, config_path: &'static str, ) -> (LoadedGeyserPlugin, Library, &'static str) { + #[cfg(unix)] + let library = libloading::os::unix::Library::this(); + #[cfg(windows)] + let library = libloading::os::windows::Library::this().unwrap(); ( LoadedGeyserPlugin::new(Box::new(plugin), None), - Library::from(libloading::os::unix::Library::this()), + Library::from(library), config_path, ) } diff --git a/install/src/command.rs b/install/src/command.rs index fe7617af6..ad6c1ea1f 100644 --- a/install/src/command.rs +++ b/install/src/command.rs @@ -333,9 +333,7 @@ pub fn string_from_winreg_value(val: &winreg::RegValue) -> Option { let words = unsafe { slice::from_raw_parts(val.bytes.as_ptr() as *const u16, val.bytes.len() / 2) }; - let mut s = if let Ok(s) = String::from_utf16(words) { - s - } else { + let Ok(mut s) = String::from_utf16(words) else { return None; }; while s.ends_with('\u{0}') { @@ -392,11 +390,9 @@ fn add_to_path(new_path: &str) -> bool { }, }; - let old_path = if let Some(s) = + let Some(old_path) = get_windows_path_var().unwrap_or_else(|err| panic!("Unable to get PATH: {}", err)) - { - s - } else { + else { return false; }; diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index 47c552450..1dd827bbe 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -2,7 +2,10 @@ #![cfg(feature = "sbf_c")] #![allow(clippy::uninlined_format_args)] #![allow(clippy::arithmetic_side_effects)] -#![cfg_attr(not(target_arch = "x86_64"), allow(dead_code, unused_imports))] +#![cfg_attr( + any(target_os = "windows", not(target_arch = "x86_64")), + allow(dead_code, unused_imports) +)] use { solana_rbpf::memory_region::MemoryState, @@ -103,7 +106,7 @@ fn bench_program_create_executable(bencher: &mut Bencher) { } #[bench] -#[cfg(target_arch = "x86_64")] +#[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] fn bench_program_alu(bencher: &mut Bencher) { let ns_per_s = 1000000000; let one_million = 1000000; diff --git a/rpc/src/rpc_service.rs b/rpc/src/rpc_service.rs index 303a1e94b..10580b471 100644 --- a/rpc/src/rpc_service.rs +++ b/rpc/src/rpc_service.rs @@ -878,24 +878,21 @@ mod tests { panic!("Unexpected RequestMiddlewareAction variant"); } - #[cfg(unix)] + std::fs::remove_file(&genesis_path).unwrap(); { - std::fs::remove_file(&genesis_path).unwrap(); - { - let mut file = std::fs::File::create(ledger_path.path().join("wrong")).unwrap(); - file.write_all(b"wrong file").unwrap(); - } - symlink::symlink_file("wrong", &genesis_path).unwrap(); + let mut file = std::fs::File::create(ledger_path.path().join("wrong")).unwrap(); + file.write_all(b"wrong file").unwrap(); + } + symlink::symlink_file("wrong", &genesis_path).unwrap(); - // File is a symbolic link => request should fail. - let action = rrm.process_file_get(DEFAULT_GENESIS_DOWNLOAD_PATH); - if let RequestMiddlewareAction::Respond { response, .. } = action { - let response = runtime.block_on(response); - let response = response.unwrap(); - assert_ne!(response.status(), 200); - } else { - panic!("Unexpected RequestMiddlewareAction variant"); - } + // File is a symbolic link => request should fail. + let action = rrm.process_file_get(DEFAULT_GENESIS_DOWNLOAD_PATH); + if let RequestMiddlewareAction::Respond { response, .. } = action { + let response = runtime.block_on(response); + let response = response.unwrap(); + assert_ne!(response.status(), 200); + } else { + panic!("Unexpected RequestMiddlewareAction variant"); } } }