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

Prepare pgrx 0.11.3 #1496

Merged
merged 23 commits into from
Jan 25, 2024
Merged

Prepare pgrx 0.11.3 #1496

merged 23 commits into from
Jan 25, 2024

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented 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

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

Two UB fixes!

Ergonomics

Less transport-level security problems in cargo-pgrx

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.

burmecia and others added 21 commits January 24, 2024 14:44
The rule utils header contains useful fonctions such as
`deparse_expression()`.
this explains the version match requirements of `cargo-pgrx` and also
breaks up the initial "one step" into two steps with slightly more
explanation.
This fixes issue #1437.

The problem here is that we were blindly converting the entire
`syn::Variant` value to the Postgres enum variant name when in fact all
we want is its `Ident` value.
Discussed was also adding `libpq.h` but that would expand development requirements to Kerberos development headers and that isn't a good situation for existing users.
…1448)

ureq defines `default = ["tls", "gzip"]`, so "tls" feature (rustls) is
currently always enabled even when native-tls is preferred, i.e. both
rustls and native-tls is built if openssl headers are available and the
user cannot opt-out from using rustls. This also disallows building
cargo-pgrx on platforms not supported by the ring crate (dependency of
rustls).

This should also solve #1430 if `cargo-pgrx` is built with
`--no-default-features --features native-tls`.
I first settled down to write up something using `rustls-native_certs`
but then I actually noticed ureq simply enables this with a feature.

This should fully solve #1430 for both rustls and native-tls.
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.
- `allow(unused)` is redundant with `pub`
- `link_name` is redundant with `no_mangle`
#911 introduced using the wrong signature and
this UB was missed in review amidst the thousands of lines of changes.

Signed-off-by: usamoi <usamoi@outlook.com>
I use `ereport!` because I like to include a `detail` entry but it comes
with some inconveniences. When I would write the following snippet in my
`Err` match arms, the compiler would complain about mismatch type output
from the match arms thus requiring the inclusion of `unreachable!`

```rust
pgrx::ereport!(
    PgLogLevel::ERROR,
    PgSqlErrorCode::ERRCODE_INTERNAL_ERROR,
    format!("{} failed", stringify!(snowflake_pool_stats)),
    err.to_string()
);

// necessary to satisfy the compiler
unreachable!()
```

After looking at the macro src for `ereport!` I realized why I started
writing `PgLogLevel::ERROR` rather than simply `ERROR`:
the `ERROR` case didn't support a `detail` argument so let's add that.
Adds header files for multixact.h and its dependencies.
There's already a pgrx::varlena module but utils/varlena.h adds
functions to manipulate GUC variables and identifiers.

Includes utils/sortsupport.h as a transitive inclusion, along
with varatt.h which is already transitively included.
This documents our current state of support, and adds a note for people
hoping for 32-bit support: it unfortunately would not be cheap, neither
in terms of the incurred technical debt, nor in terms of running the
additional build and test infra to test on 32-bit CPUs, which is not
something GitHub Actions helps with.
Tweaking owo-color features required due to transitive deps.

Addresses a Windows-only, deeply hypothetical security alert for atty.
levkk and others added 2 commits January 24, 2024 19:44
`fs::canonicalize` calls `realpath` which requires the dest path to
exist. It doesn't when installing the extension for the first time (and
a new version of the extension for subsequent times). So...best we can
[1] do is trust that @eeeebbbbrrrr 's canonicalization works. 🚀

[1]: https://stackoverflow.com/questions/68231306/stdfscanonicalize-for-files-that-dont-exist
Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

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

LGTM

@workingjubilee workingjubilee merged commit 2244e62 into develop-v0.11 Jan 25, 2024
@workingjubilee workingjubilee deleted the prepare-0.11.3 branch January 26, 2024 18:39
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.

10 participants