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.

Sign up for GitHub

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

Disable debug info compilation in CI to boost performance. #779

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Disable debug info compilation in CI to boost performance. #779

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca459f3
Disable debug info compilation in CI to boost performance.
xStrom Dec 9, 2024
2af1b01
Use the `ci` Cargo profile in more CI steps.
xStrom Dec 9, 2024
41d2fb5
Update `strip` comment.
xStrom Dec 9, 2024
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
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ jobs:
save-if: ${{ github.event_name != 'merge_group' }}

- name: Run cargo nextest
run: cargo nextest run --workspace --locked --all-features --no-fail-fast
run: cargo nextest run --workspace --locked --cargo-profile ci --all-features --no-fail-fast
env:
# We do not run the masonry render tests on platforms without a working GPU,
# because those require Vello rendering to be working
Expand All @@ -294,7 +294,7 @@ jobs:
path: masonry/src/widget/screenshots

- name: Run cargo test --doc
run: cargo test --doc --workspace --locked --all-features --no-fail-fast
run: cargo test --doc --workspace --locked --profile ci --all-features --no-fail-fast

test-stable-wasm:
name: cargo test (wasm32)
Expand All @@ -316,7 +316,7 @@ jobs:

# TODO: Find a way to make tests work. Until then the tests are merely compiled.
- name: Run cargo test --no-run
run: cargo test --workspace ${{ env.NO_WASM_PKGS }} --locked --target wasm32-unknown-unknown --all-features --no-run
run: cargo test --workspace ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --all-features --no-run

check-stable-android:
name: cargo check (aarch64-android)
Expand Down Expand Up @@ -434,7 +434,7 @@ jobs:

# We test documentation using nightly to match docs.rs.
- name: Run cargo doc
run: cargo doc --workspace --locked --all-features --no-deps --document-private-items -Zunstable-options -Zrustdoc-scrape-examples
run: cargo doc --workspace --locked --profile ci --all-features --no-deps --document-private-items -Zunstable-options -Zrustdoc-scrape-examples
env:
RUSTDOCFLAGS: '--cfg docsrs -D warnings'

Expand Down
2 changes: 2 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,7 @@ time = "0.3.36"

[profile.ci]
inherits = "dev"
debug = 0 # Don't compile debug info to reduce compilation artifact size for cache benefits.
strip = "debuginfo" # Implied with debug = 0 since Rust 1.77, still useful for older.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence doesn't scan to me - older doesn't really work as a subject. Maybe something like:

Suggested change
strip = "debuginfo" # Implied with debug = 0 since Rust 1.77, still useful for older.
strip = "debuginfo" # Implied by debug = 0 since Rust 1.77 - but our MSRV is older.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's meant to be copy-pastable and in Xilem's case the MSRV is actually newer than 1.77. I adjusted the comment but made the MSRV reference a bit more conditional.

Copy link
Contributor

Choose a reason for hiding this comment

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

MSRV will be 1.82 as soon as this gets updated Peniko or Vello.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just not have the comment at all? The redundancy doesn't have any real cost

[profile.ci.package."*"]
debug-assertions = true # Keep always on for dependencies for cache reuse.
Loading