fix cargo-pgrx and pgrx-tests on Windows #1934

usamoi · 2024-11-01T11:07:37Z

Notes:

postgres must run without administrator privileges, so switch calling postgres directly to starting postgres by pg_ctl in testing
Windows CI is added
known issue: pgrx-cshim.c is compiled with -flto, because, without this flag, pgrx_embed.exe is linked to postgres.exe
known issue: cargo-pgrx downloads, instead of building, postgres binaries from EDB website
known issue: cargo pgrx run and cargo pgrx test always print logs for pg_ctl start on Windows (pipes generated by std::process::Command are leaked to postgres, a command with Stdio::piped() hangs, so we use Stdio::inherit() on Windows)
breaking change: this pull request fixes PgLwLock and PgAtomic but introduces a breaking change about PgLwLock and PgAtomic: name must be provided at PgLwLock/PgAtomic::new, PgLwLock::from_named is removed and a parameter of PgLwLock::attach changes
breaking change: this pull request makes everything C-unwind and it's necessary for downstream to switch from C to C-unwind
regression: cargo-pgrx users need to execute sudo sysctl fs.protected_fifos=0 before using --runas on Linux

usamoi · 2024-11-01T11:12:52Z

~~raw syntax is stablized in 1.82 but it seems that CI does not know it~~ fixed

usamoi · 2024-11-01T15:53:59Z

It seems that https://patchwork.kernel.org/project/linux-hardening/patch/20180416175918.GA13494@beast/ breaks --runas on linux. I think the best solution is just telling user to execute sudo sysctl fs.protected_fifos=0 before using --runas.

YohDeadfall · 2024-11-03T19:14:46Z

postgres binaries are download from EDB website, because I don't know how to compile postgres on Windows targeting msvc

The process is explained in the official documentation: https://www.postgresql.org/docs/16/install-windows.html

Not a Windows user for years, but can help if you're stuck with something (:

usamoi · 2024-11-04T17:12:42Z

postgres binaries are download from EDB website, because I don't know how to compile postgres on Windows targeting msvc

The process is explained in the official documentation: https://www.postgresql.org/docs/16/install-windows.html

Not a Windows user for years, but can help if you're stuck with something (:

I'm curious if there are really any Windows developers. So, I'm planning to leave this feature here to see if anyone fixes it.

usamoi · 2024-11-29T12:37:26Z

👀 reviews?

cAttte · 2025-02-23T03:42:40Z

this branch works like a charm on windows with MSVC installed and the updated line in .cargo/config.toml, so thank you! would be great if this got reviewed + merged!

workingjubilee · 2025-02-23T19:37:37Z

oh cool!

workingjubilee · 2025-02-23T19:37:57Z

That's basically the real review this thing needed, let me check the code

workingjubilee

This looks good overall but it should use the std::env::consts instead of all these cfgs.

workingjubilee · 2025-02-23T19:42:26Z

cargo-pgrx/src/command/install.rs

+    let so_ext = 'so_ext: {
+        if cfg!(target_os = "macos") {
+            break 'so_ext "dylib";
+        }
+        if cfg!(target_os = "windows") {
+            break 'so_ext "dll";
+        }
+        "so"
+    };


Suggested change

let so_ext = 'so_ext: {

if cfg!(target_os = "macos") {

break 'so_ext "dylib";

}

if cfg!(target_os = "windows") {

break 'so_ext "dll";

}

"so"

};

let so_ext = std::env::consts::DLL_EXTENSION;

workingjubilee · 2025-02-23T19:43:12Z

cargo-pgrx/src/command/test.rs

+    if runas.is_some() {
+        eyre::bail!("`--runas` is not supported on Windows");
+    }


Yeah, this seems better than trying to reimplement the equivalent for now.

workingjubilee · 2025-02-23T19:45:45Z

pgrx-pg-config/src/cargo.rs

+            }
+            ("lib", "so")
+        };
+        Ok(format!("{prefix}{}.{}", lib_name.replace('-', "_"), extension))


Suggested change

Ok(format!("{prefix}{}.{}", lib_name.replace('-', "_"), extension))

use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};

Ok(format!("{DLL_PREFIX}{name}{DLL_SUFFIX}", name = lib_name.replace('-', "_")))

workingjubilee · 2025-02-23T19:47:55Z

pgrx-pg-config/src/lib.rs

+        #[cfg(not(target_os = "windows"))]
        path.push("dropdb");
+        #[cfg(target_os = "windows")]
+        path.push("dropdb.exe");


Instead of this, can we get format!("dropdb{EXE_SUFFIX}") and so on?

Suggested change

#[cfg(not(target_os = "windows"))]

path.push("dropdb");

#[cfg(target_os = "windows")]

path.push("dropdb.exe");

#[cfg(not(target_os = "windows"))]

path.push("dropdb");

#[cfg(target_os = "windows")]

path.push("dropdb.exe");

workingjubilee · 2025-02-23T19:58:47Z

Exactly how you thread in the consts I'm not inclined to argue... you'll note I used two different forms in the above suggestions, you can pick one you prefer or use both and add a third... just asking to use them when they're available.

usamoi · 2025-02-24T10:59:45Z

~~cee-scape fails to compile due to rust-lang/rust#137512. Ha...~~

Windows CI fails. I'll see it later.

workingjubilee

great! just needs to resolve conflicts.

workingjubilee · 2025-02-25T02:12:30Z

cargo-pgrx/src/command/init.rs

+    if bytes.starts_with(b"PK\x03\x04")
+        || bytes.starts_with(b"PK\x05\x06")
+        || bytes.starts_with(b"PK\x07\x08")


one question: is this magic a true byte string or is it a uint32_t, in true C style?

it's copied from wikipedia directly, so this should be a true byte string?

It's the zip based file format magic bits: 50 4B 03 04, 50 4B 05 06, and 50 4B 07 08. The last two bytes are designators for the archive type.

usamoi · 2025-02-25T11:11:12Z

Windows CI fails due to rust-lang/rust#129582 (Rust 1.84) before. The pull request changes the behavior of "an extern "C" Rust function meets longjmp" from letting it go to aborting the process. In order to avoid it, all PostgreSQL functions and Rust external functions need to be extern "C-unwind". It's a little annoying. Do you have any ideas? @workingjubilee

@cAttte I’m really sorry to say that if you compiled the PGRX extension with a Rust version higher than 1.83 on my previous branch and ran into the 0xC0000409 error, please try recompiling it with Rust 1.83.

usamoi · 2025-02-25T14:26:31Z

~~MacOS CI fails due to rust-lang/rust-bindgen#3155. For now, just make do with string replacement and hope it breaks nothing.~~

workingjubilee · 2025-03-13T18:48:12Z

Windows CI fails due to rust-lang/rust#129582 (Rust 1.84) before. The pull request changes the behavior of "an extern "C" Rust function meets longjmp" from letting it go to aborting the process. In order to avoid it, all PostgreSQL functions and Rust external functions need to be extern "C-unwind". It's a little annoying. Do you have any ideas? @workingjubilee

@cAttte I’m really sorry to say that if you compiled the PGRX extension with a Rust version higher than 1.83 on my previous branch and ran into the 0xC0000409 error, please try recompiling it with Rust 1.83.

time to make everything C-unwind!

usamoi · 2025-03-15T07:00:26Z

So I think it's ready to be merged?

pgrx-bindgen/src/build.rs

workingjubilee · 2025-03-16T09:55:25Z

Thanks!

cAttte · 2025-03-16T13:08:36Z

yay 🚀 thank you both very much

Fixes a known issue (cshim requires LTO) in #1934.

…2014) `pgrx` is switched from "C" to "C-unwind" in #1934 so it's fine to emit an error here Without this error, someone who updates dependencies without reading changelog, would break extensions silently, if their only usage of `#[pg_guard]` is `_PG_init`. It's bad.

Notes: 1. postgres must run without administrator privileges, so switch calling `postgres` directly to starting postgres by `pg_ctl` in testing 2. Add Windows CI 3. known issue: `pgrx-cshim.c` is compiled with `-flto`, because, without this flag, `pgrx_embed.exe` is linked to `postgres.exe` 4. known issue: `cargo-pgrx` downloads, instead of building, postgres binaries from EDB website 5. known issue: `cargo pgrx run` and `cargo pgrx test` always print logs for `pg_ctl start` on Windows (pipes generated by `std::process::Command` are leaked to postgres, a command with `Stdio::piped()` hangs, so we use `Stdio::inherit()` on Windows) 6. breaking change: this pull request fixes `PgLwLock` and `PgAtomic` but introduces a breaking change about `PgLwLock` and `PgAtomic`: name must be provided at `PgLwLock/PgAtomic::new`, `PgLwLock::from_named` is removed and a parameter of `PgLwLock::attach` changes 7. breaking change: this pull request makes everything `C-unwind` and it's necessary for downstream to switch from `C` to `C-unwind` 8. regression: `cargo-pgrx` users need to execute `sudo sysctl fs.protected_fifos=0` before using `--runas` on Linux

…oundation#2006) Fixes a known issue (cshim requires LTO) in pgcentralfoundation#1934.

…gcentralfoundation#2014) `pgrx` is switched from "C" to "C-unwind" in pgcentralfoundation#1934 so it's fine to emit an error here Without this error, someone who updates dependencies without reading changelog, would break extensions silently, if their only usage of `#[pg_guard]` is `_PG_init`. It's bad.

usamoi force-pushed the win branch 2 times, most recently from 43cc5ca to e07355c Compare November 1, 2024 13:06

usamoi force-pushed the win branch 6 times, most recently from 7a70740 to 8befc9f Compare November 2, 2024 18:24

usamoi force-pushed the win branch from 8befc9f to 37d8f44 Compare November 16, 2024 01:26

eeeebbbbrrrr added the Needs Review label Nov 20, 2024

usamoi force-pushed the win branch from 37d8f44 to ea251b2 Compare November 22, 2024 20:17

workingjubilee reviewed Feb 23, 2025

View reviewed changes

usamoi force-pushed the win branch from ea251b2 to 2b6b598 Compare February 24, 2025 10:32

usamoi force-pushed the win branch from 2b6b598 to 6fa3947 Compare February 24, 2025 11:10

YohDeadfall mentioned this pull request Feb 24, 2025

Updated expected UI test outputs #1981

Merged

workingjubilee approved these changes Feb 25, 2025

View reviewed changes

usamoi force-pushed the win branch from 6fa3947 to fc1c18f Compare February 25, 2025 11:00

usamoi force-pushed the win branch 2 times, most recently from cce6c68 to e4c6c4e Compare February 25, 2025 14:25

usamoi force-pushed the win branch 2 times, most recently from eb76045 to 13c1abe Compare February 27, 2025 15:36

workingjubilee reviewed Mar 15, 2025

View reviewed changes

pgrx-bindgen/src/build.rs Outdated Show resolved Hide resolved

workingjubilee approved these changes Mar 15, 2025

View reviewed changes

usamoi force-pushed the win branch from 13c1abe to 9a37011 Compare March 16, 2025 09:18

usamoi added 8 commits March 16, 2025 17:19

fix cargo-pgrx and pgrx-tests on Windows

3ae2cca

fix shmem API on Windows

65e392b

fix runas.yml

ccc2971

add Windows CI

fd4ea55

workaround strange linker errors for PostgreSQL 14 on Windows

6760f11

use std::env::consts, instead of cfg and hardcored constants

91dd26f

switch to C-unwind

851fa23

workaround for bindgen bug

3cfdc6c

usamoi force-pushed the win branch from 9a37011 to 3cfdc6c Compare March 16, 2025 09:19

workingjubilee merged commit e35a74a into pgcentralfoundation:develop Mar 16, 2025
16 checks passed

usamoi mentioned this pull request Mar 18, 2025

tell C compiler to put code and data in separate sections #2006

Merged

workingjubilee pushed a commit that referenced this pull request Mar 18, 2025

tell C compiler to put code and data in separate sections (#2006)

121c254

Fixes a known issue (cshim requires LTO) in #1934.

usamoi mentioned this pull request Mar 22, 2025

force using extern "C-unwind" for functions marked with #[pg_guard] #2014

Merged

eeeebbbbrrrr added the breaks-API label Mar 28, 2025

eeeebbbbrrrr mentioned this pull request Apr 12, 2025

Update version to 0.14.0 #2040

Merged

BrewTestBot mentioned this pull request Apr 12, 2025

pgrx 0.14.0 Homebrew/homebrew-core#219829

Merged

KenjiBrown pushed a commit to SoftwareLibreMx/pgrx that referenced this pull request May 27, 2025

tell C compiler to put code and data in separate sections (pgcentralf…

a4cf55e

…oundation#2006) Fixes a known issue (cshim requires LTO) in pgcentralfoundation#1934.

	Ok(format!("{prefix}{}.{}", lib_name.replace('-', "_"), extension))
	use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
	Ok(format!("{DLL_PREFIX}{name}{DLL_SUFFIX}", name = lib_name.replace('-', "_")))

Uh oh!

fix cargo-pgrx and pgrx-tests on Windows #1934

fix cargo-pgrx and pgrx-tests on Windows #1934

Uh oh!

Conversation

usamoi commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

usamoi commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

usamoi commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YohDeadfall commented Nov 3, 2024

Uh oh!

usamoi commented Nov 4, 2024

Uh oh!

usamoi commented Nov 29, 2024

Uh oh!

cAttte commented Feb 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Feb 23, 2025

Uh oh!

workingjubilee commented Feb 23, 2025

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

workingjubilee commented Feb 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

usamoi commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

usamoi commented Feb 25, 2025

Uh oh!

usamoi commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Mar 13, 2025

Uh oh!

usamoi commented Mar 15, 2025

Uh oh!

Uh oh!

workingjubilee commented Mar 16, 2025

Uh oh!

Uh oh!

cAttte commented Mar 16, 2025

Uh oh!

Uh oh!

usamoi commented Nov 1, 2024 •

edited

Loading

usamoi commented Nov 1, 2024 •

edited

Loading

usamoi commented Nov 1, 2024 •

edited

Loading

cAttte commented Feb 23, 2025 •

edited

Loading

workingjubilee commented Feb 23, 2025 •

edited

Loading

usamoi commented Feb 24, 2025 •

edited

Loading

usamoi commented Feb 25, 2025 •

edited

Loading