-
Notifications
You must be signed in to change notification settings - Fork 0
ci: add Windows 7 builds with custom packaging #19
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis change adds UNIX distribution support and Windows‑7 artifact entries to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dist-workspace.toml (1)
28-68: Suggest defensive error handling improvements for the PowerShell script.To enhance reliability and troubleshooting, consider these refinements:
- Add explicit checks after rustup operations to confirm targets were installed successfully.
- Verify that the executable file exists before copying (e.g.,
if (-not (Test-Path ...)) { throw "..." }).- Document the script's assumptions regarding file locations and build output paths.
Apply this diff to add defensive checks:
# Install nightly toolchain and components rustup toolchain install nightly rustup component add rust-src --toolchain nightly rustup target add x86_64-win7-windows-msvc --toolchain nightly rustup target add i686-win7-windows-msvc --toolchain nightly # Build 64-bit Win7 binary cargo +nightly build -Z build-std --target x86_64-win7-windows-msvc --release New-Item -ItemType Directory -Force -Path "dist\x86_64" | Out-Null # Copy files and package artifact + if (-not (Test-Path "target\x86_64-win7-windows-msvc\release\elnpack.exe")) { + throw "Build artifact not found: target\x86_64-win7-windows-msvc\release\elnpack.exe" + } Copy-Item "target\x86_64-win7-windows-msvc\release\elnpack.exe" "dist\x86_64\elnpack.exe" + if (-not (Test-Path "LICENSE")) { + throw "Required file not found: LICENSE" + } Copy-Item "LICENSE" "dist\x86_64\LICENSE" + if (-not (Test-Path "README.md")) { + throw "Required file not found: README.md" + } Copy-Item "README.md" "dist\x86_64\README.md" Compress-Archive -Path "dist\x86_64\elnpack.exe","dist\x86_64\LICENSE","dist\x86_64\README.md" ` -DestinationPath "dist\x86_64\elnpack-x86_64-win7-windows-msvc.zip" -Force(Apply similar checks to the i686 section as well.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dist-workspace.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-03T16:16:45.363Z
Learnt from: CR
Repo: Athemis/ELNPack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T16:16:45.363Z
Learning: Use `cargo run` to build and launch the desktop app; use `cargo check` before pushing; use `cargo fmt` to format Rust sources (enforced style); use `cargo clippy --all-targets --all-features` to lint for warnings; use `cargo test` to run tests; use `cargo build --release` for optimized binary distribution.
Applied to files:
dist-workspace.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
dist-workspace.toml (1)
14-15: Clarify the purpose of theunix-archivesetting addition.This line introduces a new configuration directive unrelated to the Windows 7 build objective. While perfectly valid, I would like to understand: Is this addition necessary for the Windows 7 build pipeline, or does it address a separate concern? A brief explanatory comment would make the intent manifest.
| [[dist.extra-artifacts]] | ||
| # Builds for tier 3 target win7 | ||
| artifacts = ["dist/x86_64/elnpack-x86_64-win7-windows-msvc.zip", "dist/i686/elnpack-i686-win7-windows-msvc.zip"] | ||
|
|
||
| # Needs custom cargo flags, bundling of assets; builds both x86_64 and i686 | ||
| build = [ | ||
| "powershell", | ||
| "-NoProfile", | ||
| "-ExecutionPolicy", "Bypass", | ||
| "-Command", | ||
| ''' | ||
| $ErrorActionPreference = "Stop" | ||
| # Install nightly toolchain and components | ||
| rustup toolchain install nightly | ||
| rustup component add rust-src --toolchain nightly | ||
| rustup target add x86_64-win7-windows-msvc --toolchain nightly | ||
| rustup target add i686-win7-windows-msvc --toolchain nightly | ||
| # Build 64-bit Win7 binary | ||
| cargo +nightly build -Z build-std --target x86_64-win7-windows-msvc --release | ||
| New-Item -ItemType Directory -Force -Path "dist\x86_64" | Out-Null | ||
| # Copy files and package artifact | ||
| Copy-Item "target\x86_64-win7-windows-msvc\release\elnpack.exe" "dist\x86_64\elnpack.exe" | ||
| Copy-Item "LICENSE" "dist\x86_64\LICENSE" | ||
| Copy-Item "README.md" "dist\x86_64\README.md" | ||
| Compress-Archive -Path "dist\x86_64\elnpack.exe","dist\x86_64\LICENSE","dist\x86_64\README.md" ` | ||
| -DestinationPath "dist\x86_64\elnpack-x86_64-win7-windows-msvc.zip" -Force | ||
| # Build 32-bit Win7 binary | ||
| cargo +nightly build -Z build-std --target i686-win7-windows-msvc --release | ||
| New-Item -ItemType Directory -Force -Path "dist\i686" | Out-Null | ||
| # Copy files and package artifact | ||
| Copy-Item "target\i686-win7-windows-msvc\release\elnpack.exe" "dist\i686\elnpack.exe" | ||
| Copy-Item "LICENSE" "dist\i686\LICENSE" | ||
| Copy-Item "README.md" "dist\i686\README.md" | ||
| Compress-Archive -Path "dist\i686\elnpack.exe","dist\i686\LICENSE","dist\i686\README.md" ` | ||
| -DestinationPath "dist\i686\elnpack-i686-win7-windows-msvc.zip" -Force | ||
| ''' | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find the dist-workspace.toml file and verify its location and content
find . -name "dist-workspace.toml" -type fRepository: Athemis/ELNPack
Length of output: 80
🏁 Script executed:
# Check if LICENSE and README.md exist in the repository root
ls -la LICENSE README.md 2>&1 | head -20Repository: Athemis/ELNPack
Length of output: 174
🏁 Script executed:
# Look for cargo-dist configuration and version specifications
find . -maxdepth 3 -name "Cargo.toml" -o -name "Cargo.lock" -o -name "rust-toolchain.toml" | head -20Repository: Athemis/ELNPack
Length of output: 106
🏁 Script executed:
# Search for cargo-dist version references in configuration files
rg "cargo-dist|cargo_dist" --type toml -A 3 -B 3 | head -50Repository: Athemis/ELNPack
Length of output: 405
🏁 Script executed:
# Read full dist-workspace.toml to understand the build setup
cat -n dist-workspace.tomlRepository: Athemis/ELNPack
Length of output: 3123
🏁 Script executed:
# Check rust-toolchain.toml configuration
cat -n rust-toolchain.tomlRepository: Athemis/ELNPack
Length of output: 145
🏁 Script executed:
# Check Cargo.toml for any related configurations
cat -n Cargo.toml | head -50Repository: Athemis/ELNPack
Length of output: 2076
🏁 Script executed:
# Search for any existing nightly usage or tier-3 target references in the codebase
rg "nightly|tier-3|win7|build-std" --type toml -iRepository: Athemis/ELNPack
Length of output: 1343
🌐 Web query:
Rust tier-3 targets x86_64-win7-windows-msvc i686-win7-windows-msvc build-std nightly
💡 Result:
Short answer: those two targets are Tier 3 and Rust does not ship prebuilt std for them — build with nightly + -Z build-std or build rustc with the targets enabled. Example commands and important notes below.
What they are
- x86_64-win7-windows-msvc and i686-win7-windows-msvc are Tier‑3 Windows (Win7) targets. Rust does not ship std for them. [1]
Option A — use nightly + cargo -Z build-std (quicker)
- Install a nightly toolchain: rustup toolchain install nightly
- Build your crate and the standard library with cargo’s unstable build-std:
cargo +nightly build -Z build-std=std,core,alloc --target x86_64-win7-windows-msvc --release
(replace x86_64... with i686-win7-windows-msvc as needed). [1][2] - Notes:
- Using +nightly and -Z enables nightly features for the whole build; some dependencies may behave differently on nightly. [2][3]
- You will likely need a Windows MSVC toolchain or a cross toolchain (clang-cl + lld-link) and an appropriate Windows SDK; the rustc docs show xwin/clang-cl/lld-link examples for cross-compiling. [1]
Option B — build rustc with the target (full, reproducible)
- Add the target in your bootstrap/config (bootstrap.toml or config.toml) and build rustc with that target enabled so rustup-distributable components are produced. Example snippet:
[build]
build-stage = 1
target = ["x86_64-win7-windows-msvc"]
And set target-specific linker/cc if cross-compiling. Then build rustc. [1]
References
- rustc platform support: *-win7-windows-msvc (explains Tier 3, build-std option, cross toolchain examples). [1]
- rustc/platform-support general Windows MSVC guidance. [2]
- Community discussion about build-std/nightly caveats. [3]
If you want, tell me your host OS and whether you’ll cross-compile from Linux or Windows and I’ll give the exact toolchain/linker/Windows SDK steps and the precise cargo command for i686 or x86_64.
🌐 Web query:
Rust -Z build-std flag stability documentation tier-3 targets
💡 Result:
Short answer: -Z build-std is an unstable (nightly-only) Cargo flag that tells Cargo to compile the Rust standard-library crates from source (requires the rust-src component, a nightly toolchain, and passing --target). It's commonly used when targeting tier‑3 triples (which Rust does not ship prebuilt std for), because tier‑3 targets often require you to build your own std. See the Cargo unstable docs for details and the rustc platform-support docs for the tier‑3 definition. [1][2]
Key points:
- -Z build-std is unstable — requires nightly Cargo/rustc and the -Z flag on every invocation. [1]
- You must install rust-src (rustup component add rust-src) and pass --target. You can limit which std crates to build (e.g. -Z build-std=core,alloc). [1]
- Tier 3 = supported in source but not built/tested or distributed by the Rust project; std may be absent and you generally must build it yourself (hence using -Z build-std). [2]
Sources:
[1] Cargo “Unstable Features” (–Z build-std) documentation.
[2] rustc Platform Support — Target tiers (Tier 3 explanation).
Verify tier-3 target viability and PowerShell script robustness.
Your approach demonstrates commendable initiative in resurrecting Windows 7 support through custom build artifacts. However, several matters require attention:
Primary Concerns:
-
Tier-3 Target Stability: The targets
x86_64-win7-windows-msvcandi686-win7-windows-msvcare confirmed as Rust tier-3 targets with no prebuilt standard library. They require the nightly toolchain and the unstable-Z build-stdflag. While this is the correct and documented approach for these targets, ensure your CI infrastructure is prepared for potential breakage when nightly updates introduce incompatibilities or when dependencies behave differently under nightly. -
PowerShell Script Robustness: While
$ErrorActionPreference = "Stop"provides baseline error handling, consider adding explicit error handling around the cargo build commands (lines 43, 56). Cargo build failures can surface in unexpected ways, and defensive checks would improve reliability. -
Nightly Toolchain Risk: Your project uses stable by default (rust-toolchain.toml), which is sensible, but this build requires dynamically installing nightly on each CI run. Document and communicate to contributors that this extra-artifact build carries the inherent risk of nightly toolchain instability.
- added to test new build pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
dist-workspace.toml (2)
44-55: PowerShell build orchestration is functional; consider more explicit error signaling.Your script relies on
$ErrorActionPreference = "Stop"to halt on failure, which provides baseline protection. However, the sequence from build through packaging—copying files that depend on a successful binary output—would benefit from explicit error checks.While
cargo buildtypically surfaces failures clearly, adding an explicit check likeif ($LASTEXITCODE -ne 0) { throw "Build failed" }immediately after line 45 would make failure modes unambiguous and aid debugging in CI logs. Similarly, wrapping the Copy-Item and Compress-Archive operations in try-catch blocks would guard against surprising edge cases.For example:
cargo +nightly build -Z build-std --target x86_64-win7-windows-msvc --release if ($LASTEXITCODE -ne 0) { throw "x86_64 Win7 build failed" } New-Item -ItemType Directory -Force -Path "dist\x86_64" | Out-Null Copy-Item "target\x86_64-win7-windows-msvc\release\elnpack.exe" "dist\x86_64\elnpack.exe" -ErrorAction Stop Copy-Item "LICENSE" "dist\x86_64\LICENSE" -ErrorAction Stop Copy-Item "README.md" "dist\x86_64\README.md" -ErrorAction Stop Compress-Archive -Path "dist\x86_64\elnpack.exe","dist\x86_64\LICENSE","dist\x86_64\README.md" ` -DestinationPath "dist\x86_64\elnpack-x86_64-win7-windows-msvc.zip" -Force -ErrorAction StopThis pattern is more explicit and aids troubleshooting when issues do arise.
57-68: Symmetrical i686 build mirrors x86_64 logic; same error-handling considerations apply.The i686 build (lines 57–68) mirrors the x86_64 structure cleanly. The same recommendation for explicit error signaling and defensive error handling applies: insert
if ($LASTEXITCODE -ne 0)checks after each cargo invocation, and consider-ErrorAction Stopon file operations.This parallel structure ensures consistency across architectures but also means identical risk patterns. Strengthening error diagnostics for one naturally strengthens both.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)dist-workspace.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-03T16:16:45.363Z
Learnt from: CR
Repo: Athemis/ELNPack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T16:16:45.363Z
Learning: Use `cargo run` to build and launch the desktop app; use `cargo check` before pushing; use `cargo fmt` to format Rust sources (enforced style); use `cargo clippy --all-targets --all-features` to lint for warnings; use `cargo test` to run tests; use `cargo build --release` for optimized binary distribution.
Applied to files:
.github/workflows/release.ymldist-workspace.toml
📚 Learning: 2025-12-03T16:16:45.363Z
Learnt from: CR
Repo: Athemis/ELNPack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T16:16:45.363Z
Learning: Applies to Cargo.toml : Maintain Rust 2024 crate with no build script needed; egui compiles directly with the Rust code.
Applied to files:
dist-workspace.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build-local-artifacts (aarch64-apple-darwin)
- GitHub Check: build-local-artifacts (aarch64-unknown-linux-gnu)
- GitHub Check: build-local-artifacts (x86_64-apple-darwin)
- GitHub Check: build-local-artifacts (x86_64-unknown-linux-gnu)
- GitHub Check: build-local-artifacts (i686-unknown-linux-gnu)
- GitHub Check: build-local-artifacts (i686-pc-windows-msvc)
- GitHub Check: build-local-artifacts (x86_64-pc-windows-msvc)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
.github/workflows/release.yml (1)
130-133: Cache initialization step integrates smoothly into the build pipeline.The addition of
swatinem/rust-cache@v2is well-positioned between Rust installation and the dist toolchain setup. Usingmatrix.targetsas part of the cache key ensures that separate architectures maintain isolated caches, which is precisely what you need for multi-target builds.dist-workspace.toml (4)
14-17: Distribution and PR run configuration is well-reasoned.The
unix-archiveandpr-run-modesettings align with best practices for distribution workflows. Enabling pull request artifact uploads provides immediate feedback during development and testing cycles.
25-28: Artifact declarations properly specify Win7 build outputs.The
[[dist.extra-artifacts]]section correctly maps the expected build outputs. The naming schemeelnpack-{arch}-win7-windows-msvc.zipis explicit and aligns with Rust's tier-3 target naming conventions, making the distribution manifest clear to end users.
30-42: Nightly toolchain initialization follows documented best practices for tier-3 targets.The targets x86_64-win7-windows-msvc and i686-win7-windows-msvc are tier-3 targets targeting Windows 7, and your setup mirrors the standard approach: cargo's build-std feature has reached feature parity with prior tools and your explicit installation of the toolchain, rust-src component, and target triples aligns with documented guidance. The
$ErrorActionPreference = "Stop"ensures early termination on unexpected conditions—a prudent defensive measure for CI environments.Minor clarification: Ensure you understand that when using -Z build-std without specifying which crates, std, core, and alloc are compiled by default. This is appropriate for building a complete binary, but document this assumption if your project ever needs to strip down the standard library surface.
25-70: Nightly toolchain dependency requires clear team communication and CI resilience strategy.These Win7 artifact builds are commendably forward-thinking, but they carry a meaningful operational risk: tier-3 targets receive no automated testing from the Rust project, so they may or may not work, and the Rust team may temporarily disable tier-3 targets in nightly, expecting maintainers to update their targets, which can result in demotion if support is not forthcoming in time for a stable release.
Your project uses stable by default (per your learnings), yet this extra-artifact build requires nightly on every CI invocation. Nightly updates can introduce incompatibilities at any moment—this is not a defect in your code but an inherent property of consuming unstable features.
Recommended actions:
- Document this risk in your contributor or release notes so the team understands why these builds might intermittently fail.
- Consider marking this job as non-blocking in your CI orchestration (e.g.,
continue-on-error: truein GitHub Actions) so that a nightly breakage does not block releases of the main artifacts.- Monitor nightly upgrades during your development cycle and have a plan to adapt quickly if the Win7 targets destabilize.
Verify that your team is aware that nightly failures for these targets are expected occasional occurrences and have agreed on a response strategy (continue releases without Win7 artifacts, delay release, or invest in rapid fixes).
Summary
Testing
cargo fmtcargo clippy --all-targets --all-featurescargo testNotes for reviewers
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.