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.

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

Check minimal version of Rust that compiles #421

Merged
merged 1 commit into from
Aug 18, 2024

Conversation

jongiddy
Copy link
Contributor

Specify a minimum version of the Rust compiler that successfully builds the crate. This allows us to be aware if the minimal version increases and to avoid issues such as #370

The minimum version is a best-effort measured value and is different to the MSRV which is a support guarantee. The minimum version can be incremented by a PR in order to pass tests, as long as the MSRV holds. When the minimum version increases, the next release should be a minor version, to allow any affected users to pin to a previous minor version.

@jongiddy
Copy link
Contributor Author

The minimum version for the updated miniz_oxide is higher than I expected. Since the rust-version in Cargo.toml can prevent Cargo from selecting the crate, it needs to be the minimum version supported by a common feature-set. We do not want to prevent someone using Rust 1.56.1 from downloading the crate if they are planning to use zlib-* feature.

My current thinking is to remove the rust-version directive from Cargo.toml but keep the test, and use it to document behavior. (e.g. if using Rust < 1.60 a different backend must be selected).

@oyvindln
Copy link
Contributor

The current version of miniz_oxide works with 1.56.0 as of now if I test it on it's own locally though I don't want to give any guarantees that it will stay at that.

It looks like you may need to pin some other crates to older versions to make flate2 work with that version though judging by the CI build results.

@jongiddy
Copy link
Contributor Author

Ah it's ppv-lite86 which is a transitive dependency in the dev-dependencies. v0.2.17 in my lock file has no dependencies, but the latest v0.2.20 used in GitHub Actions adds zerocopy which requires 1.60.

[dev-dependencies]
├── quickcheck v1.0.3
│   └── rand v0.8.5
│       ├── libc v0.2.153
│       ├── rand_chacha v0.3.1
│       │   ├── ppv-lite86 v0.2.17

@jongiddy jongiddy force-pushed the test-minimal-version branch 2 times, most recently from cc5a458 to 8475fed Compare August 16, 2024 21:18
@jongiddy
Copy link
Contributor Author

Changed the CI workflow to only test that code builds, without running the tests.

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for making clearer the defacto way the MSRV was handled.

I'd also hope that adding the rust-version can't break anyone. In theory, those who use even more ancient compiler versions will benefit from those not respecting that field yet. So it should be safe especially with a value as low as that.

There is just one improvement that I think could be made to the MSRV paragraph, which would have helped me to understand it more quickly.

README.md Outdated

Older stables may work, but failures may not be treated as a `flate2` bug.

The `Cargo.toml` file specifies a minimum version for which builds passed at some point.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `Cargo.toml` file specifies a minimum version for which builds passed at some point.
The `Cargo.toml` file specifies a minimum version for which builds passed at some point using the `rust-version` field.

Copy link
Member

Choose a reason for hiding this comment

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

Since 'minimum version' is really the rust-version field, I wonder if that name should rather be used to reduce the level of abstraction.

Specify a minimum version of the Rust compiler that successfully
builds the crate. This allows us to be aware if the minimal version
increases and to avoid issues such as
rust-lang#370

The minimum version is a best-effort measured value and is different to
the MSRV which is a support guarantee. The minimum version can be
incremented by a PR in order to pass tests, as long as the MSRV holds.
When the minimum version increases, the next release should be a minor
version, to allow any affected users to pin to a previous minor version.
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Byron Byron merged commit d616177 into rust-lang:main Aug 18, 2024
5 checks passed
@@ -7,8 +7,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
# I don't really understand the build matrix here...
build: [stable, beta, nightly, macos, windows, mingw]
build: [stable, beta, nightly, minimal, macos, windows, mingw]
Copy link
Member

Choose a reason for hiding this comment

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

CI now fails on main, and I have a feeling that this entry might be the cause of it?

It's surprising that it didn't fail CI before.

In any case, your help for a fix will be welcome.

@jongiddy
Copy link
Contributor Author

jongiddy commented Aug 18, 2024

Oh wow, CI should complain more noisily if that happens!

#422 should be the fix.

renovate bot referenced this pull request in oxc-project/oxc Aug 26, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [flate2](https://togithub.com/rust-lang/flate2-rs) |
workspace.dependencies | patch | `1.0.31` -> `1.0.33` |
| [prettyplease](https://togithub.com/dtolnay/prettyplease) |
workspace.dependencies | patch | `0.2.20` -> `0.2.22` |
| [quote](https://togithub.com/dtolnay/quote) | workspace.dependencies |
patch | `1.0.36` -> `1.0.37` |
| [serde](https://serde.rs)
([source](https://togithub.com/serde-rs/serde)) | workspace.dependencies
| patch | `1.0.208` -> `1.0.209` |
| [serde_json](https://togithub.com/serde-rs/json) |
workspace.dependencies | patch | `1.0.125` -> `1.0.127` |

---

### Release Notes

<details>
<summary>rust-lang/flate2-rs (flate2)</summary>

###
[`v1.0.33`](https://togithub.com/rust-lang/flate2-rs/releases/tag/1.0.33):
- fix minimal manifest versions

[Compare
Source](https://togithub.com/rust-lang/flate2-rs/compare/1.0.32...1.0.33)

#### What's Changed

- Fix msrv: Run msrv checks with minimal versions by
[@&#8203;NobodyXu](https://togithub.com/NobodyXu) in
[https://github.com/rust-lang/flate2-rs/pull/425](https://togithub.com/rust-lang/flate2-rs/pull/425)

#### New Contributors

- [@&#8203;NobodyXu](https://togithub.com/NobodyXu) made their first
contribution in
[https://github.com/rust-lang/flate2-rs/pull/425](https://togithub.com/rust-lang/flate2-rs/pull/425)

**Full Changelog**:
rust-lang/flate2-rs@1.0.32...1.0.33

###
[`v1.0.32`](https://togithub.com/rust-lang/flate2-rs/releases/tag/1.0.32):
- turn panic into error

[Compare
Source](https://togithub.com/rust-lang/flate2-rs/compare/1.0.31...1.0.32)

#### What's Changed

##### Fix

- Return error instead of packing on Z_MEM_ERROR by
[@&#8203;crazymerlyn](https://togithub.com/crazymerlyn) in
[https://github.com/rust-lang/flate2-rs/pull/423](https://togithub.com/rust-lang/flate2-rs/pull/423)

##### Other

- prepare new release by [@&#8203;Byron](https://togithub.com/Byron) in
[https://github.com/rust-lang/flate2-rs/pull/416](https://togithub.com/rust-lang/flate2-rs/pull/416)
- update miniz_oxide dependency to 0.8.x by
[@&#8203;oyvindln](https://togithub.com/oyvindln) in
[https://github.com/rust-lang/flate2-rs/pull/420](https://togithub.com/rust-lang/flate2-rs/pull/420)
- update maintenance guide with recent news by
[@&#8203;Byron](https://togithub.com/Byron) in
[https://github.com/rust-lang/flate2-rs/pull/419](https://togithub.com/rust-lang/flate2-rs/pull/419)
- Check minimal version of Rust that compiles by
[@&#8203;jongiddy](https://togithub.com/jongiddy) in
[https://github.com/rust-lang/flate2-rs/pull/421](https://togithub.com/rust-lang/flate2-rs/pull/421)
- Remove non-existent build in CI by
[@&#8203;jongiddy](https://togithub.com/jongiddy) in
[https://github.com/rust-lang/flate2-rs/pull/422](https://togithub.com/rust-lang/flate2-rs/pull/422)

#### New Contributors

- [@&#8203;crazymerlyn](https://togithub.com/crazymerlyn) made their
first contribution in
[https://github.com/rust-lang/flate2-rs/pull/423](https://togithub.com/rust-lang/flate2-rs/pull/423)

**Full Changelog**:
rust-lang/flate2-rs@1.0.31...1.0.32

</details>

<details>
<summary>dtolnay/prettyplease (prettyplease)</summary>

###
[`v0.2.22`](https://togithub.com/dtolnay/prettyplease/releases/tag/0.2.22)

[Compare
Source](https://togithub.com/dtolnay/prettyplease/compare/0.2.21...0.2.22)

- Support formatting precise capture `use<>` syntax
([#&#8203;80](https://togithub.com/dtolnay/prettyplease/issues/80))

###
[`v0.2.21`](https://togithub.com/dtolnay/prettyplease/releases/tag/0.2.21)

[Compare
Source](https://togithub.com/dtolnay/prettyplease/compare/0.2.20...0.2.21)

- Support formatting tail-call `become` expressions
([#&#8203;79](https://togithub.com/dtolnay/prettyplease/issues/79))

</details>

<details>
<summary>dtolnay/quote (quote)</summary>

### [`v1.0.37`](https://togithub.com/dtolnay/quote/releases/tag/1.0.37)

[Compare
Source](https://togithub.com/dtolnay/quote/compare/1.0.36...1.0.37)

- Implement ToTokens for CStr and CString
([#&#8203;283](https://togithub.com/dtolnay/quote/issues/283))

</details>

<details>
<summary>serde-rs/serde (serde)</summary>

###
[`v1.0.209`](https://togithub.com/serde-rs/serde/releases/tag/v1.0.209)

[Compare
Source](https://togithub.com/serde-rs/serde/compare/v1.0.208...v1.0.209)

- Fix deserialization of empty structs and empty tuples inside of
untagged enums
([#&#8203;2805](https://togithub.com/serde-rs/serde/issues/2805), thanks
[@&#8203;Mingun](https://togithub.com/Mingun))

</details>

<details>
<summary>serde-rs/json (serde_json)</summary>

###
[`v1.0.127`](https://togithub.com/serde-rs/json/releases/tag/1.0.127)

[Compare
Source](https://togithub.com/serde-rs/json/compare/1.0.126...1.0.127)

- Add more removal methods to OccupiedEntry
([#&#8203;1179](https://togithub.com/serde-rs/json/issues/1179), thanks
[@&#8203;GREsau](https://togithub.com/GREsau))

###
[`v1.0.126`](https://togithub.com/serde-rs/json/releases/tag/1.0.126)

[Compare
Source](https://togithub.com/serde-rs/json/compare/1.0.125...1.0.126)

- Improve string parsing on targets that use 32-bit pointers but also
have fast 64-bit integer arithmetic, such as
aarch64-unknown-linux-gnu_ilp32 and x86\_64-unknown-linux-gnux32
([#&#8203;1182](https://togithub.com/serde-rs/json/issues/1182), thanks
[@&#8203;CryZe](https://togithub.com/CryZe))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 10am on monday" in timezone
Asia/Shanghai, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job log](https://developer.mend.io/github/oxc-project/oxc).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants