On Thu, Apr 24, 2025 at 7:27 PM David E. Wheeler <da...@justatheory.com> wrote: > > On Apr 24, 2025, at 11:18, Matheus Alcantara <matheusssil...@gmail.com> wrote: > > > In v2 I've moved the logic to remove the /extension to > > parse_extension_control_file(), do you think that this Assert on this > > function would still be wrong? IIUC we should always have /extension at > > the end of "control_dir" at this place, because the > > extension_control_path GUC will omit the /extension at the end and we > > will force it to have the suffix on the path at > > find_extension_control_filename() and > > get_extension_control_directories() functions. I'm missing something > > here? > > I took this patch for a spin and managed to make it core dump. How? Well I > installed semver with this command: > > ```sh > make prefix=/Users/david/Downloads install > ``` > > Then set the search paths and restarted: > > ```ini > extension_control_path = '/Users/david/Downloads/share/extension:$system' > dynamic_library_path = '/Users/david/Downloads/lib:$libdir' > ``` > > Then I connected and ran `CREATE EXTENSION semver` and it segfaulted. I poked > around for a few minutes and realized that my prefix is not what I expected. > Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The > actual paths are: > > ```ini > extension_control_path = > '/Users/david/Downloads/share/postgresql/extension:$system' > dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir' > ``` > > With that fix it no longer segafulted. > > So I presume something crashes when a directory or file doesn’t exist. >
Yes, you are right. The problem was where I was asserting control->control_dir != NULL. I've moved the assert after the "if (!filename)" check that returns an error if the extension was not found. Attached v3 with this fix and also a TAP test for this scenario. I'm just a bit confused how you get it working using /extension at the end of extension_control_path since with this patch this suffix is not necessary and since we hard coded append this it should return an error when trying to search on something like /Users/david/Downloads/share/postgresql/extension/extension -- Matheus Alcantara
v3-0001-Make-directory-work-with-extension-control-path.patch
Description: Binary data
