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

pgx_ulid: init at 0.1.5 #312398

Closed
wants to merge 2 commits into from
Closed

pgx_ulid: init at 0.1.5 #312398

wants to merge 2 commits into from

Conversation

k0001
Copy link
Contributor

@k0001 k0001 commented May 17, 2024

Description of changes

Initial packaging of the pgx_ulid extension for PostgreSQL.

https://github.com/pksunkara/pgx_ulid

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@k0001 k0001 requested a review from thoughtpolice as a code owner May 17, 2024 08:28
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels May 17, 2024
@k0001
Copy link
Contributor Author

k0001 commented May 27, 2024

Version 0.1.5 of pgx_ulid came out, so I updated this pull-request to use that newer version.

@k0001 k0001 changed the title pgx_ulid: init at 0.1.4 pgx_ulid: init at 0.1.5 May 27, 2024
@steadygaze
Copy link

This PR seems stalled; is there anything I can do to help get it merged?

@pksunkara
Copy link

What's blocking this from being merged? Who needs to approve?

Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Since the "init" commit is not merged, yet, you can squash both commits and "init at 0.1.5" instead of updating immediately after.

src = fetchFromGitHub {
owner = "pksunkara";
repo = "pgx_ulid";
rev = "29a037e7e2dd8b18e4bb5e5ec4f9a6fe270e84bc";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rev = "29a037e7e2dd8b18e4bb5e5ec4f9a6fe270e84bc";
rev = "v${version}";

maintainers = [ maintainers.renzo ];
platforms = postgresql.meta.platforms;
license = licenses.mit;
broken = versionOlder postgresql.version "11";
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to mark versions as broken which are not part of nixpkgs anymore. The oldest one we currently have is 12, so:

Suggested change
broken = versionOlder postgresql.version "11";

doCheck = false;

meta = with lib; {
description = "A PostgreSQL extension to support ULID";
Copy link
Contributor

Choose a reason for hiding this comment

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

There was a PR a while ago to remove all articles at the beginning of descriptions, so:

Suggested change
description = "A PostgreSQL extension to support ULID";
description = "PostgreSQL extension to support ULID";

@FliegendeWurst FliegendeWurst added the awaiting_changes (old Marvin label, do not use) label Nov 6, 2024

meta = with lib; {
description = "A PostgreSQL extension to support ULID";
homepage = "https://github.com/pksunkara/pgx_ulid";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add a changelog entry, e.g. to https://github.com/pksunkara/pgx_ulid/blob/v${version}/CHANGELOG.md

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@steadygaze
Copy link

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@pksunkara
Copy link

As mentioned here, the build process needs to be updated.

@myypo
Copy link
Contributor

myypo commented Jan 2, 2025

Minimal building example for the new version. Could not trick pgrx tests into using the already built extension with the running postgres instance.

{
  buildPgrxExtension,
  fetchFromGitHub,
  postgresql,
}:
buildPgrxExtension rec {
  inherit postgresql;

  pname = "pgx_ulid";
  version = "0.2.0";

  src = fetchFromGitHub {
    owner = "pksunkara";
    repo = "pgx_ulid";
    tag = "v${version}";
    hash = "sha256-VdLWwkUA0sVs5Z/Lyf5oTRhcHVzPmhgnYQhIM8MWJ0c=";
  };

  cargoHash = "sha256-Gn+SjzGaxnGKJYI9+WyE1+TzlF/2Ne43aKbXrSzfQKM=";

  # Upstream renames the extension when packaging
  # https://github.com/pksunkara/pgx_ulid/blob/084778c3e2af08d16ec5ec3ef4e8f345ba0daa33/.github/workflows/release.yml#L81
  postInstall =
    let
      installDir = "$out/share/postgresql/extension";
    in
    ''
      mv ${installDir}/${pname}.control ${installDir}/ulid.control
      mv ${installDir}/${pname}--${version}.sql ${installDir}/ulid--${version}.sql
    '';

  # pgrx tests try to install the extension into nix store
  doCheck = false;
}

@pksunkara
Copy link

mv ${installDir}/${pname}--${version}.sql ${installDir}/ulid--${version}.sql

I think it should move all ${pname}--*.sql. That is how postgres upgrades extensions. We currently don't have them because I have an open issue to add those. pksunkara/pgx_ulid#49

@myypo
Copy link
Contributor

myypo commented Jan 4, 2025

@k0001 If you don't have time, I could pick up this package in a new PR as I am currently using it myself either way

@SigmaSquadron SigmaSquadron removed the awaiting_changes (old Marvin label, do not use) label Jan 5, 2025
@myypo
Copy link
Contributor

myypo commented Jan 6, 2025

Superseded by #371463

@myypo myypo closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants