Re: extension_control_path and "directory"

On Thu, Apr 24, 2025 at 7:27 PM David E. Wheeler <david@justatheory.com> wrote:
>
> On Apr 24, 2025, at 11:18, Matheus Alcantara <matheusssilv97@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
thatmy prefix is not what I expected. Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The
actualpaths 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