Support for library with different name than crate name

[azam]

This resolves #1099

[eeeebbbbrrrr]

This looks good.

I feel like we ought to document this somewhere, but pgx' "documentation" is kinda disorganized. We'll get this documented somewhere later.

[eeeebbbbrrrr]

Oh, one change, @azam: could you also update the .github/workflows/tests.yml file to include the new example? Unfortunately we hardcode those...

[eeeebbbbrrrr]

Also, looks like it needs a cargo fmt too. Thanks! That's the test current test failure.

[azam]

@eeeebbbbrrrr
Sure! Pushed the changes. Found the doc on lib.name, so I also added a comment on changed line referencing the doc.

[azam]

Got to put a hold on this, I am stuck at getting the lib name here. Should we read the cargo manifest to get it, since there is no env var set for lib name (there is for binaries though CARGO_BIN_NAME)?

[eeeebbbbrrrr]

Got to put a hold on this, I am stuck at getting the lib name here. Should we read the cargo manifest to get it, since there is no env var set for lib name (there is for binaries though CARGO_BIN_NAME)?

That's a sneaky line of code!

Yeah, so I think now it makes sense to centralize the manifest handling, and use it here. I think what might be okay is to add a PgxManifest type (or whatever) to the pgx-pg-config crate, which both cargo-pgx and pgx-tests already have a dependency for.

I'd go for the absolutely minimal implementation of this new PgxManifest type -- basically just a constructor that can parse the file and a public function to get the crate name we'd want everyone to use.

What do you think?

[eeeebbbbrrrr]

edit: and I suppose this means moving some of cargo-pgx/src/manifest.rs over too. So yeah, there'll be a little bit of design work here now.

[azam]

I could do that, thinking on the line of exporting these functions:

• get_crate_name(cargo_toml_path)
• get_crate__version(cargo_toml_path)
• get_extension_name(cargo_toml_path)

Should I do this?

[eeeebbbbrrrr]

Yeah, that seems fine. Thanks!

[azam]

@eeeebbbbrrrr
Pushed my changes. Hope nothing breaks.

[eeeebbbbrrrr]

Since we just renamed to pgrx, this PR needs to be rebased against the develop branch. Would you mind doing that?

And if there's any references to "pgx" in your changeset they'll now need to be "pgrx". Sorry for the trouble! Growing pains!

[azam]

@eeeebbbbrrrr
Rebased my repo!

[eeeebbbbrrrr]

This looks good. I'm letting CI run it now. Thanks @azam!

[eeeebbbbrrrr]

I'm not sure if you're allowed to see the test runner output? https://github.com/tcdi/pgrx/actions/runs/4738921279/jobs/8425118139

But basically the tests are failing with:

Error: 
   0: Could not get shared object file from Cargo output.

Location:
   cargo-pgrx/src/command/install.rs:380

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: cargo_pgrx::command::install::find_library_file
      at cargo-pgrx/src/command/install.rs:351
   1: cargo_pgrx::command::install::install_extension with pg_version=13.10 profile=Dev test=true features=["pg13", "pg_test", "cshim"]
      at cargo-pgrx/src/command/install.rs:94
   2: cargo_pgrx::command::install::execute
      at cargo-pgrx/src/command/install.rs:51

Oh, and this is for the pgrx-tests crate. Any ideas? Have you tried running cargo test --all --features pg13 --no-default-features locally?

[azam]

artifact.target.name is filled with crate name as-is (ie pgrx-tests) or lib name, depending on whether lib name has a name override or not, but cargo_manifest::Manifest auto fills lib name with crate name with hyphen replaced with underscore (ie pgrx_tests). Added target_name on PgrxManifestExt to return a value matching what cargo_metadata expects

[azam]

@eeeebbbbrrrr
Apologize for the failure; borked my cargo-pgrx after the rebase, it kept the old version.
Fixed the error, re-ran the test locally and passed.
test_out.log

[eeeebbbbrrrr]

No need to apologize! It's a complex system and that's why we have CI. We appreciate your efforts on this PR!