Skip to content

ci: drop really old trick and ensure MSRV for all feature combo #676

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

Merged
merged 14 commits into from
Mar 25, 2025
Merged

ci: drop really old trick and ensure MSRV for all feature combo #676

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e52ac8d
ci: drop really old trick and use new actions
tisonkun Mar 24, 2025
26f6625
serde requires 1.61+
tisonkun Mar 24, 2025
f1372a8
another way
tisonkun Mar 24, 2025
2cac1dd
everything requires 1.61.0
tisonkun Mar 24, 2025
828abbc
try patching
tisonkun Mar 24, 2025
6eb2731
undo pin runner name
tisonkun Mar 24, 2025
28026bf
ensure workflow permissions
tisonkun Mar 24, 2025
d70c00c
install toolchain by scripts
tisonkun Mar 24, 2025
5932509
sval 2.14.1
tisonkun Mar 24, 2025
5173090
fixup scripts
tisonkun Mar 24, 2025
faca8cb
update change log
tisonkun Mar 24, 2025
6f4bab1
Merge branch 'master' into drop-really-old-trick
tisonkun Mar 24, 2025
b9c84e8
Update CHANGELOG.md
tisonkun Mar 24, 2025
50c2018
fixup running IT cases
tisonkun Mar 24, 2025
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
123 changes: 47 additions & 76 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,101 +1,77 @@
name: CI
on: [push, pull_request]

permissions:
contents: read # to fetch code (actions/checkout)

jobs:
test:
name: Test
runs-on: ${{ matrix.os }}
strategy:
matrix:
build: [stable, beta, nightly, macos, win32, win64, mingw]
include:
- build: stable
os: ubuntu-latest
os: ubuntu-24.04
rust: stable
- build: beta
os: ubuntu-latest
os: ubuntu-24.04
rust: beta
- build: nightly
os: ubuntu-latest
os: ubuntu-24.04
rust: nightly
- build: macos
os: macos-latest
os: macos-14
rust: stable
- build: win32
os: windows-latest
os: windows-2022
rust: stable-i686-pc-windows-msvc
- build: win64
os: windows-latest
os: windows-2022
rust: stable-x86_64-pc-windows-msvc
- build: mingw
os: windows-latest
os: windows-2022
rust: stable-x86_64-pc-windows-gnu
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update ${{ matrix.rust }} --no-self-update
rustup default ${{ matrix.rust }}
cargo install cargo-hack
- uses: actions/checkout@v4
- name: Install toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
- uses: taiki-e/install-action@cargo-hack
- run: cargo hack test --feature-powerset --lib --exclude-features max_level_off,max_level_error,max_level_warn,max_level_info,max_level_debug,max_level_trace,release_max_level_off,release_max_level_error,release_max_level_warn,release_max_level_info,release_max_level_debug,release_max_level_trace
- run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml
- run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml --release

rustfmt:
name: Rustfmt
runs-on: ubuntu-latest
check:
name: Check Format and Clippy
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update stable --no-self-update
rustup default stable
rustup component add rustfmt
# log repo does not use Cargo workspaces, so `cargo fmt` will not check all the code
# perhaps this should be changed in the future
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
with:
components: clippy,rustfmt
- run: cargo fmt -- --check
- run: cargo fmt --manifest-path test_max_level_features/Cargo.toml -- --check
- run: cargo fmt --manifest-path tests/Cargo.toml -- --check

clippy:
name: Clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update stable --no-self-update
rustup default stable
rustup component add clippy
- run: cargo clippy --verbose
- run: cargo clippy --verbose --manifest-path test_max_level_features/Cargo.toml
- run: cargo clippy --verbose --manifest-path tests/Cargo.toml

doc:
name: Check Documentation
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update stable --no-self-update
rustup default stable
rustup component add rust-docs
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
with:
components: rust-docs
- name: Run rustdoc
run: RUSTDOCFLAGS="-D warnings" cargo doc --verbose --features std,serde,sval,sval_ref,value-bag,kv,kv_std,kv_sval,kv_serde
env:
RUSTDOCFLAGS: "-D warnings"
run: cargo doc --verbose --features std,serde,sval,sval_ref,value-bag,kv,kv_std,kv_sval,kv_serde

features:
name: Feature check
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update nightly --no-self-update
rustup default nightly
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- run: cargo build --verbose -Z avoid-dev-deps --features kv
- run: cargo build --verbose -Z avoid-dev-deps --features "kv std"
- run: cargo build --verbose -Z avoid-dev-deps --features "kv kv_sval"
Expand All @@ -105,13 +81,10 @@ jobs:

minimalv:
name: Minimal versions
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update nightly --no-self-update
rustup default nightly
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- run: cargo build --verbose -Z minimal-versions --features kv
- run: cargo build --verbose -Z minimal-versions --features "kv std"
- run: cargo build --verbose -Z minimal-versions --features "kv kv_sval"
Expand All @@ -121,26 +94,24 @@ jobs:

msrv:
name: MSRV
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update 1.60.0 --no-self-update
rustup default 1.60.0
- run: |
cargo test --verbose --manifest-path tests/Cargo.toml
cargo test --verbose --manifest-path tests/Cargo.toml --features kv
Copy link
Contributor Author

@tisonkun tisonkun Mar 24, 2025

Choose a reason for hiding this comment

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

Previously we check MSRV only for a subset.

Copy link
Contributor Author

@tisonkun tisonkun Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If we agree that MSRV is only valid for no feature / with kv, I can revert the test targets and undo MSRV -> 1.61 and the sval PR is unnecessary.

Copy link
Contributor

@KodrAus KodrAus Mar 24, 2025

Choose a reason for hiding this comment

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

Previously we only applied it to a subset, but now that the key-value API is stable I’m happy to add one for it. What do you think @Thomasdezeeuw?

Copy link
Contributor Author

@tisonkun tisonkun Mar 24, 2025

Choose a reason for hiding this comment

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

Thanks for your information! I'd emphasize that ensuring MSRV for sval/serde would require bumping MSRV to 1.61, mainly for const fn. You may take it into consideration whether it's possible, since I see:

This version is explicitly tested in CI and may be bumped in any release as needed. Maintaining compatibility with older compilers is a priority though, so the bar for bumping the minimum supported version is set very high. Any changes to the supported minimum version will be called out in the release notes.

Copy link
Contributor

@KodrAus KodrAus Mar 24, 2025

Choose a reason for hiding this comment

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

I think we haven't done an MSRV bump in some time, so it should be safe to move up to 1.61.0 and keep a single policy 👍

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw Mar 24, 2025

Choose a reason for hiding this comment

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

Technically a MSRV bump is a breaking change, but since 1.61 came out in May 2022, I think it's fine we increase it.

Copy link
Member

@BurntSushi BurntSushi Mar 24, 2025

Choose a reason for hiding this comment

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

I don't know why you say it is "technically" a breaking change. The log policy in the README even explicitly allows it, and that is the general practice of Rust libraries (although some do treat MSRV bumps as semver incompatible).

See also: https://doc.rust-lang.org/cargo/reference/rust-version.html#setting-and-updating-rust-version

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw Mar 24, 2025

Choose a reason for hiding this comment

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

I'm not sure why you're questioning whether or not bumping MSRV is a breaking change or not. It is. If I run Rust 1.60 then I can run log 0.4.26, but not 0.4.27, that's a breaking change.

Whether or not it's allowed by our own policy is a different question.

Copy link
Member

@BurntSushi BurntSushi Mar 24, 2025

Choose a reason for hiding this comment

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

The topic has been discussed to death. Search the libs team repo or the api guidelines repo. I'm on mobile or I would link you. Increasing toolchain requirements is broadly not considered a breaking change.

- uses: actions/checkout@v4
- name: Install toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: 1.61.0
- uses: taiki-e/install-action@cargo-hack
- run: cargo hack test --feature-powerset --lib --exclude-features max_level_off,max_level_error,max_level_warn,max_level_info,max_level_debug,max_level_trace,release_max_level_off,release_max_level_error,release_max_level_warn,release_max_level_info,release_max_level_debug,release_max_level_trace
- run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml
- run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml --release

embedded:
name: Embedded
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@master
- name: Install Rust
run: |
rustup update stable --no-self-update
rustup default stable
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: rustup target add thumbv6m-none-eabi riscv32imc-unknown-none-elf
- run: cargo build --verbose --target=thumbv6m-none-eabi
- run: cargo build --verbose --target=riscv32imc-unknown-none-elf
14 changes: 10 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ A lightweight logging facade for Rust
categories = ["development-tools::debugging"]
keywords = ["logging"]
exclude = ["rfcs/**/*"]
rust-version = "1.60.0"
rust-version = "1.61.0"
edition = "2021"

[package.metadata.docs.rs]
Expand Down Expand Up @@ -66,16 +66,22 @@ value-bag = { version = "1.7", optional = true, default-features = false, featur

[dev-dependencies]
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
serde_test = "1.0"
serde_json = { version = "1.0" }
serde_test = { version = "1.0" }
sval = { version = "2.1" }
sval_derive = { version = "2.1" }
value-bag = { version = "1.7", features = ["test"] }

# NOTE: log doesn't actually depent on this crate. However, our dependencies,
# NOTE: log doesn't actually depend on this crate. However, our dependencies,
# serde and sval, dependent on version 1.0 of the crate, which has problem fixed
# in 1.0.60, specifically in the following commit
# https://github.com/dtolnay/proc-macro2/commit/e31d61910049e097afdd3d27c37786309082bdcb.
# By defining the crate as direct dependency we can increase its minimal
# version making the minimal (crate) version CI happy.
proc-macro2 = { version = "1.0.63", default-features = false }

[patch.crates-io]
sval = { git = "https://github.com/tisonkun/sval.git", rev = "06aa86b" }
sval_buffer = { git = "https://github.com/tisonkun/sval.git", rev = "06aa86b" }
sval_nested = { git = "https://github.com/tisonkun/sval.git", rev = "06aa86b" }
sval_ref = { git = "https://github.com/tisonkun/sval.git", rev = "06aa86b" }
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ implementation that is most suitable for its use case.

## Minimum supported `rustc`

`1.60.0+`
`1.61.0+`

This version is explicitly tested in CI and may be bumped in any release as needed. Maintaining compatibility with older compilers is a priority though, so the bar for bumping the minimum supported version is set very high. Any changes to the supported minimum version will be called out in the release notes.

Expand Down
22 changes: 0 additions & 22 deletions tests/Cargo.toml
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions tests/src/build.rs
View file
Open in desktop
Copy link
Contributor Author

@tisonkun tisonkun Mar 24, 2025

Choose a reason for hiding this comment

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

yak shaving from here .. even from rustc 1.60.0 we don't need this trick I think.

Copy link
Contributor

@KodrAus KodrAus Mar 24, 2025

Choose a reason for hiding this comment

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

Glad to see this get cleaned up! I think we were previously only testing these in our MSRV job. To get them working this way we may need to pull that macros module into the regular log crate so its tests are still picked up.

Copy link
Contributor Author

@tisonkun tisonkun Mar 24, 2025

Choose a reason for hiding this comment

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

Do you provide any change here?

Cargo would detect tests folder and these two files are already tested by the after setup.

See https://doc.rust-lang.org/book/ch11-03-test-organization.html#integration-tests

This file was deleted.

14 changes: 0 additions & 14 deletions tests/src/lib.rs
View file
Open in desktop

This file was deleted.