Skip to content

Commit

Permalink
[SM-1444] Fix Windows GNU Builds (#1053)
Browse files Browse the repository at this point in the history
## 🎟️ Tracking

https://bitwarden.atlassian.net/browse/SM-1444

## 📔 Objective

Windows GNU builds are currently broken due to a bug in
[rustls-platform-verifier](https://github.com/rustls/rustls-platform-verifier)
when LTO is turned on.

I have submitted a GH Issue for this here:
rustls/rustls-platform-verifier#141

## ⏰ Reminders before review

- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or
informed the documentation
  team

## 🦮 Reviewer guidelines

<!-- Suggested interactions but feel free to use (or not) as you desire!
-->

- 👍 (`:+1:`) or similar for great changes
- 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info
- ❓ (`:question:`) for questions
- 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry
that's not quite a confirmed
  issue and could potentially benefit from discussion
- 🎨 (`:art:`) for suggestions / improvements
- ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or
concerns needing attention
- 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or
indications of technical debt
- ⛏ (`:pick:`) for minor or nitpick changes
  • Loading branch information
coltonhurst authored Sep 19, 2024
1 parent 2f1717d commit 7641717
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
12 changes: 11 additions & 1 deletion .github/workflows/build-rust-cross-platform.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,23 @@ jobs:
- name: Add build architecture
run: rustup target add ${{ matrix.settings.target }}

# Build Rust for musl
- name: Build Rust for - ${{ matrix.settings.target }}
if: ${{ contains(matrix.settings.target, 'musl') }}
env:
RUSTFLAGS: "-D warnings"
run: cargo zigbuild -p bitwarden-c --target ${{ matrix.settings.target }} --release

# Build Rust for windows-gnu
- name: Build Rust for - ${{ matrix.settings.target }}
if: ${{ !contains(matrix.settings.target, 'musl') }}
if: ${{ matrix.settings.target == 'x86_64-pc-windows-gnu' }}
env:
RUSTFLAGS: "-D warnings"
run: cargo build -p bitwarden-c --target ${{ matrix.settings.target }} --profile=release-windows

# Build Rust for !musl && !windows-gnu
- name: Build Rust for - ${{ matrix.settings.target }}
if: ${{ !contains(matrix.settings.target, 'musl') && matrix.settings.target != 'x86_64-pc-windows-gnu' }}
env:
RUSTFLAGS: "-D warnings"
MACOSX_DEPLOYMENT_TARGET: "10.14" # allows using new macos runner versions while still supporting older systems
Expand All @@ -79,3 +88,4 @@ jobs:
name: libbitwarden_c_files-${{ matrix.settings.target }}
path: |
target/${{ matrix.settings.target }}/release/*bitwarden_c*
target/${{ matrix.settings.target }}/release-windows/*bitwarden_c*
7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ opt-level = 1
[profile.release]
lto = "thin"
codegen-units = 1

# Turn off LTO on release mode for windows
# This is a workaround until this is fixed: https://github.com/rustls/rustls-platform-verifier/issues/141
[profile.release-windows]
inherits = "release"
lto = "off"

# Stripping the binary reduces the size by ~30%, but the stacktraces won't be usable anymore.
# This is fine as long as we don't have any unhandled panics, but let's keep it disabled for now
# strip = true

0 comments on commit 7641717

Please sign in to comment.