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

use the full cargo version string in the version match check. #1443

Merged

Conversation

Lokathor
Copy link
Contributor

Rather than cargo-pgrx using just the rustc minor version when checking for a toolchain version mismatch, this now uses the full cargo version string. cargo uses the same version number as rustc, so it's a sufficient stand in for checking the version of rustc.

By checking the full version string instead of only the minor version, we can now detect a mismatch between two nightly versions of rust issued in the same 6 week period.

Example:
cargo 1.76.0-nightly (9b13310ca 2023-11-24)
vs
cargo 1.76.0-nightly (1aa9df1a5 2023-12-12)

These detect as the same minor version, but using the new full string check they will correctly show up as different and flag the mismatch error.

Also: the error message now alerts people how they can override the version check when necessary.

@workingjubilee
Copy link
Member

As memory serves, we had been avoiding this kind of granularity because it doesn't actually matter this much in practice (we had also imagined we would solve the problem that inflicts ABI Hell on ourselves sooner, but then other things came up). However, the improvement to the error message is making me consider accepting this anyways. I'm going to roll this around in my head a little bit.

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Yeah, I slept on it and I feel confident accepting this is better than the status quo.

@workingjubilee workingjubilee merged commit e7112ed into pgcentralfoundation:develop Dec 29, 2023
8 checks passed
@thomcc
Copy link
Contributor

thomcc commented Dec 29, 2023

This was deliberate at the time, because I update nightly almost every day, and don’t necessarily want to reinstall cargo-pgrx every day (especially given that ABI breaks are fairly rare). That said, I’m not on pgrx that much atm, so I don’t care a ton.

workingjubilee pushed a commit that referenced this pull request Jan 24, 2024
Rather than `cargo-pgrx` using just the `rustc` minor version when
checking for a toolchain version mismatch, this now uses the full
`cargo` version string. `cargo` uses the same version number as `rustc`,
so it's a sufficient stand in for checking the version of `rustc`.

By checking the full version string instead of only the minor version,
we can now detect a mismatch between two nightly versions of rust issued
in the same 6 week period.

Example:
`cargo 1.76.0-nightly (9b13310ca 2023-11-24)`
vs
`cargo 1.76.0-nightly (1aa9df1a5 2023-12-12)`

These detect as the same minor version, but using the new full string
check they will correctly show up as different and flag the mismatch
error.

Also: the error message now alerts people how they can override the
version check when necessary.
workingjubilee added a commit that referenced this pull request Jan 25, 2024
The pgrx 0.11.3 release addresses a few UB risks in pgrx, updates its
dependencies on many points, and includes many additional headers. It
should also now be easier to use cargo-pgrx on more-complicated network
configurations.

## New Bindings!

New bindings added thanks to
- @burmecia in #1432
- @daamien in
  - #1431
  - #1485
- @rebasedming in #1486
- @usamoi in #1436
- @workingjubilee in
#1453

## "...wait, that's UB?"

Two UB fixes!
- Thanks to @Lokathor in
#1443
- Thanks to @usamoi in
#1466

## Ergonomics

- A better `ereport!` macro in
#1472

## Less transport-level security problems in cargo-pgrx

- We no longer secretly require rustls! Thanks to @jirutka in
#1448
- We now use native certs if possible, even with rustls, since
#1449

Together these should mean it's possible to actually use cargo-pgrx on
whatever your network configuration is, but you might have to use `cargo
install --no-default-features --features native-tls` to install with
native-tls (which, on Linux, means OpenSSL). By default, you will use
rustls.

## Many dependency updates

These address some largely-hypothetical security risks, but one is
particularly important: the bindgen update means we now should be
compatible with some aarch64 builds that might have failed.

- #1492
- #1493
- #1494
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