Skip to content

pgx_ulid: init at 0.2.0#371463

Merged
wolfgangwalther merged 4 commits intoNixOS:masterfrom
myypo:init-pgx_ulid
Jan 12, 2025
Merged

pgx_ulid: init at 0.2.0#371463
wolfgangwalther merged 4 commits intoNixOS:masterfrom
myypo:init-pgx_ulid

Conversation

@myypo
Copy link
Contributor

@myypo myypo commented Jan 6, 2025 •
edited
Loading

pgx_ulid is a Postgres ULID extension developed with pgrx.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@myypo myypo requested a review from wolfgangwalther January 6, 2025 14:48
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 6, 2025
@myypo myypo mentioned this pull request Jan 6, 2025
Closed
13 tasks
@nix-owners nix-owners bot requested review from Ma27 and thoughtpolice January 6, 2025 14:50
@github-actions github-actions bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 6, 2025
wolfgangwalther
wolfgangwalther reviewed Jan 6, 2025
View reviewed changes
nixos/tests/postgresql/pgx_ulid.nix Outdated
Copy link
Contributor

@wolfgangwalther wolfgangwalther Jan 6, 2025

Choose a reason for hiding this comment

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

(possibly out of scope for here, but the resource usage is a problem, not sure whether blocking or not...)

A full VM test is pretty heavy for resource usage.. because we're not running a single VM test, but 10 of them. Running all postgresql related VM tests already takes way too much time / memory right now.

Also VM tests have the downside of not working on darwin, yet, IIRC (this might be outdated, not sure).

I don't see anything specific in those tests that might need a VM test - we could achieve the same with a test based on postgresqlTestExtension. There are a few examples of this in other extensions.

Those don't use any expectations, yet - just basic running of SELECT statements. I wonder whether we could extend postgresqlTextExtension to support pgtap-based basic tests.

(there could also be a problem with buildPgrxExtension, not sure if it supports finalAttrs, yet, which we'd need for that)

Copy link
Contributor Author

@myypo myypo Jan 6, 2025

Choose a reason for hiding this comment

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

I don't see anything specific in those tests that might need a VM test - we could achieve the same with a test based on postgresqlTestExtension. There are a few examples of this in other extensions.

Thanks for pointing out this function - the only reason I have used the VM tests is because I was not aware of it. Will replace them.

Those don't use any expectations, yet - just basic running of SELECT statements. I wonder whether we could extend postgresqlTextExtension to support pgtap-based basic tests.

It is not a big deal since the tests I have provided are meant to be rather smokish, so the lack of assertions should be fine for now.

Copy link
Contributor Author

@myypo myypo Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

(there could also be a problem with buildPgrxExtension, not sure if it supports finalAttrs, yet, which we'd need for that)

Yeah, it does not accept the argument yet. For now, I have just removed the VM tests as updating this function and its dependencies is out of scope.

@github-actions github-actions bot removed the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 6, 2025
@myypo myypo requested a review from wolfgangwalther January 7, 2025 07:25
@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jan 7, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 371463

x86_64-linux

⏩ 2 packages marked as broken and skipped:
  • postgresql13JitPackages.pgx_ulid
  • postgresql13Packages.pgx_ulid
✅ 8 packages built:
  • postgresql14JitPackages.pgx_ulid
  • postgresql14Packages.pgx_ulid
  • postgresql15JitPackages.pgx_ulid
  • postgresql15Packages.pgx_ulid
  • postgresql16JitPackages.pgx_ulid
  • postgresql16Packages.pgx_ulid
  • postgresqlJitPackages.pgx_ulid (postgresql17JitPackages.pgx_ulid)
  • postgresqlPackages.pgx_ulid (postgresql17Packages.pgx_ulid)

aarch64-darwin

⏩ 2 packages marked as broken and skipped:
  • postgresql13JitPackages.pgx_ulid
  • postgresql13Packages.pgx_ulid
❌ 8 packages failed to build:
  • postgresql14JitPackages.pgx_ulid
  • postgresql14Packages.pgx_ulid
  • postgresql15JitPackages.pgx_ulid
  • postgresql15Packages.pgx_ulid
  • postgresql16JitPackages.pgx_ulid
  • postgresql16Packages.pgx_ulid
  • postgresqlJitPackages.pgx_ulid (postgresql17JitPackages.pgx_ulid)
  • postgresqlPackages.pgx_ulid (postgresql17Packages.pgx_ulid)

Now we only need to figure out darwin.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jan 7, 2025

I get this build error for aarch64-darwin:

Executing cargo-pgrx buildPhase
   Validating /nix/store/gwc5lnhi8vr99g60kk1ziklkragfyhl2-postgresql-16.6-dev/bin/pg_config
 Initializing data directory at /private/tmp/nix-build-pgx_ulid-0.2.0.drv-5/tmp.wUR0Qd5vUs/data-16
Error:
   0: problem running initdb: "/nix/store/5wcha3xzmj759jcw9rwbk31gp13wj08k-postgresql-16.6/bin/initdb" "--locale=C" "--lc-ctype=UTF-8" "-D" "/private/tmp/nix-b>
      initdb: could not find suitable text search configuration for locale "UTF-8"
      2025-01-07 18:32:12.513 UTC [78697] FATAL:  could not create shared memory segment: Cannot allocate memory
      2025-01-07 18:32:12.513 UTC [78697] DETAIL:  Failed system call was shmget(key=53943773, size=56, 03600).
      2025-01-07 18:32:12.513 UTC [78697] HINT:  This error usually means that PostgreSQL's request for a shared memory segment exceeded your kernel's SHMALL p>
        The PostgreSQL documentation contains more information about shared memory configuration.
      child process exited with exit code 1
      initdb: removing data directory "/private/tmp/nix-build-pgx_ulid-0.2.0.drv-5/tmp.wUR0Qd5vUs/data-16"


Location:
   src/command/init.rs:615

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 5 frames hidden ⋮
   6: cargo_pgrx::command::init::initdb::h84cb1b3429b7780e
      at <unknown source file>:<unknown line>
   7: cargo_pgrx::command::init::init_pgrx::h8e3998d0b8a026a0
      at <unknown source file>:<unknown line>
   8: <cargo_pgrx::command::init::Init as cargo_pgrx::CommandExecute>::execute::h7658e9d43117ef33
      at <unknown source file>:<unknown line>
   9: <cargo_pgrx::command::pgrx::CargoPgrxSubCommands as cargo_pgrx::CommandExecute>::execute::hee2d01c882098c69
      at <unknown source file>:<unknown line>
  10: cargo_pgrx::main::h3061de081a02c961
      at <unknown source file>:<unknown line>
  11: std::sys::backtrace::__rust_begin_short_backtrace::h1e8f0f6a28220936
      at <unknown source file>:<unknown line>
  12: std::rt::lang_start::{{closure}}::h6cd3b3cdff1d7a96
      at <unknown source file>:<unknown line>
  13: std::rt::lang_start_internal::h60914846c59a19a9
      at <unknown source file>:<unknown line>
  14: _main<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jan 7, 2025

Hm, that might be unrelated to this PR though.. I saw something similar on #371242.

@wolfgangwalther wolfgangwalther mentioned this pull request Jan 7, 2025
Open
@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jan 12, 2025

Hm, that might be unrelated to this PR though.. I saw something similar on #371242.

Debugging this more leads me towards a problem with MacOS updates. So not blocking here.

@wolfgangwalther wolfgangwalther merged commit c79c52d into NixOS:master Jan 12, 2025
22 of 25 checks passed
@wolfgangwalther wolfgangwalther mentioned this pull request Feb 21, 2025
Closed
2 tasks
@wolfgangwalther wolfgangwalther mentioned this pull request Jul 7, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ma27 Ma27 Awaiting requested review from Ma27

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@wolfgangwalther wolfgangwalther Awaiting requested review from wolfgangwalther

1 more reviewer

@pksunkara pksunkara pksunkara approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@myypo @wolfgangwalther @pksunkara