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

Add PGXN release workflow #1711

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add PGXN release workflow #1711

wants to merge 1 commit into from

Conversation

theory
Copy link

@theory theory commented Mar 22, 2024

It's triggered by a semver tag. I don't see any tags on this repo, but do notice that the version on PGXN appears to be a couple versions back. Use this workflow if you'd like to automate its release; change the on triggers to trigger a release by some other means.

Be sure to add workflow secrets to this project named PGXN_USERNAME and PGXN_PASSWORD to allow it to authenticate to publish the extension.

Other stuff:

  • Add .gitattributes so that the contents of .github and .git are excluded from the exported archive
  • Fix the extension name in the META.json file to the actual extension name (same as the control file name)
  • Updated the version in the META.json and shortened the abstract and description a bit (otherwise they take up a lot of space on the PGXN page.
  • Read the versions from the META.json file and set the extension name from MODULE_big.
  • Add a dist make target to produce a zip file to manually upload to PGXN.

@aked21
Copy link
Contributor

aked21 commented Mar 29, 2024

Hi @rafsun42 ... Can you look into this PR?

@rafsun42
Copy link
Member

rafsun42 commented Apr 3, 2024

@aked21 Sure, I am reviewing it.

@rafsun42 rafsun42 self-assigned this Apr 3, 2024
@aked21
Copy link
Contributor

aked21 commented Apr 10, 2024

Hi @rafsun42, can this get merged?

@rafsun42
Copy link
Member

@theory Hello. Is this PR tested?

@jrgemignani
Copy link
Contributor

jrgemignani commented Apr 18, 2024

I am a bit weary of modifications to the Makefile. Why are those modifications necessary? Especially, if someone could modify the META.json file and impact the Makefile builds.

@theory
Copy link
Author

theory commented Apr 18, 2024

Yes, I tested it with the exception of actually releasing it. See this build.

The changes to the Makefile are no strictly speaking required; I thought it useful to demonstrate how to use it to set the version in one place. pgrx projects often read the version from a Cargo.toml file, but many PGXS-based projects use the META.json file. The dist target is not required, but handy to manually check that that the PGXN zip file is generated properly.

We could probably remove it all if you'd like; the pgxn-bundle command used in the release workflow does pretty much the same thing, depending on the META.json file to read the version.

@jrgemignani
Copy link
Contributor

@theory My preference, for now, is to not touch the Makefile. I do get what you're trying to do, though, and it is appreciated.

@theory
Copy link
Author

theory commented Apr 18, 2024

Okay, removed the Makefile changes in 969842b.

Copy link
Contributor

@jrgemignani jrgemignani left a comment

Choose a reason for hiding this comment

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

Some questions

META.json Outdated
}
},
"prereqs": {
"runtime": {
"requires": {
"PostgreSQL": "14.0.0"
"PostgreSQL": "11.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the minimum PG? If so, while we do support 11, we've stopped most new additions to 11 & 12. 13 would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, we support up to PG16.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is the minimum supported version. I took it from the README, which says:

for now AGE supports Postgres 11, 12, 13, 14, 15 & 16.

Do you want to drop support for 11 and 12?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this cause it to build against 11? The main issue with 11 & 12 is that we have mostly dropped support of them beyond 1.5.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, a lot of this has to do with 11 and 12 being much different from 13 and above.

Copy link
Author

Choose a reason for hiding this comment

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

It just means that it supports PostgreSQL 11 and up. If someone downloaded it and looked, they'd know they could build it against Postgres 11. At some point a smoke test setup might depend on this config to decide what versions of Postgres to test builds against.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@theory An age release for PG16 (which is master branch currently) will not compile on Postgres 11, or any other Postgres version (at least it is not meant to). For context, we have different development branches for different PG versions. Given that, I believe the version in the meta file would depend on the branch this PR is being applied on. For example, if the PR is being applied on PG15, then the version can point to the latest minor version of PG15.

Copy link
Author

@theory theory Apr 19, 2024

Choose a reason for hiding this comment

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

Oooh, okay. Then you'd be best off with separate release names for each one. SO in 5bf40ed I set the name of the distribution to "name": "ApacheAGE16", and the Postgres version requirements to "PostgreSQL": ">= 16.0.0, < 17.0.0". See a sample build here, where instead of releasing it runs ls -lh and shows ApacheAGE16-1.5.0.zip.

This allows you to have separate distributions for 13, 14, 15, 16, by adjusting these two items to indicate the supported version. For 15, for example:

"name": "ApacheAGE15",

and

"PostgreSQL": ">= 15.0.0, < 16.0.0"

You would need to replicate this for each maintenance branch and release them separately.

Copy link
Author

Choose a reason for hiding this comment

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

Also, be aware that these will be considered separate distributions by PGXN. Wheres today you have https://pgxn.org/dist/apacheage/, future releases will be under https://pgxn.org/dist/apacheage16/, https://pgxn.org/dist/apacheage15/, etc.

META.json Show resolved Hide resolved
@jrgemignani
Copy link
Contributor

@theory

Be sure to add workflow secrets to this project named PGXN_USERNAME and PGXN_PASSWORD to allow it to authenticate to publish the extension.

I'm not sure what or where to add this information.

@theory
Copy link
Author

theory commented Apr 19, 2024

I'm not sure what or where to add this information.

Presumably you have access to the PGXN username and password, since Apache Age is on PGXN already. What you need to do is go to https://github.com/apache/age/settings/secrets/actions and add two repository secrets, one named PGXN_USERNAME and the other PGXN_PASSWORD. These will then be injected into the GitHub workflow and used to authenticate to PGXN and publish the release.

@jrgemignani
Copy link
Contributor

@theory Unfortunately, I don't have access to that. We will need to find out who does before moving forward and merging this.

@jrgemignani jrgemignani mentioned this pull request Apr 24, 2024
@MuhammadTahaNaveed MuhammadTahaNaveed added the override-stale To keep issues/PRs untouched from stale action label May 9, 2024
@MuhammadTahaNaveed MuhammadTahaNaveed linked an issue May 11, 2024 that may be closed by this pull request
@jrgemignani
Copy link
Contributor

@theory We're still waiting on access. @eyab can we get a status?

@theory
Copy link
Author

theory commented Sep 23, 2024

Hi there. Are you still unable to get the PGXN credentials? Could you use password reset for the age user?

Oh, it looks like user ibrar is the current owner. They would have to grant co-ownership. Shall I email them to ask about transferring ownership to the age user created by @eyab?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master override-stale To keep issues/PRs untouched from stale action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to Apache AGE's directory on PGXN
5 participants