On Mon, Apr 28, 2025 at 5:49 PM David E. Wheeler <david@justatheory.com> wrote:
>
> > To append the /extension at each path on extension_control_path would
> > require some changes on find_in_path() that
> > find_extension_control_filename() calls, which I think that it would
> > make the function more complicated. I've them created a similar
> > find_in_paths() function that works in the same way but it receives a
> > List of paths instead of the string of paths separated by ":". We can
> > get this List of paths using get_extension_control_directories() that
> > also handle the macro substitution like find_in_path().
> >
> > Attached v4 with these fixes. I hope that now you should be able to omit
> > the /extension from the GUC value.
>
> Yes! It now works with this configuration:
>
> ```ini
> extension_control_path = '/Users/david/Downloads/share/postgresql:$system'
> dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir’
> ```
>
> Which is nicely more consistent. Kind of want that first one to be called “share_path” now, though, since it’s not
justextensions. Although I guess it’s only extension control file searching that uses it (for now).
>
Thanks for testing!
> If I understand this bit correctly:
>
> ```c
> /* Substitute the path macro if needed */
> mangled = substitute_path_macro(piece, "$system", system_dir);
>
> /*
> * Append "extension" suffix in case is a custom extension control
> * path.
> */
> if (strcmp(piece, "$system") != 0)
> mangled = psprintf("%s/extension", mangled);
> ```
>
> The value of `piece` is a single path from the search path, right? If so, I think it’s either `$system` or something
else;I don’t it would ever be that `$system` is a substring of a single path. Is that right?
>
Yes, it is a single path from the search path, in your case it will be
"/Users/david/Downloads/share/postgresql" and "$system". We split these
paths based on the system path separator and get the next "piece" here:
char *piece = first_path_var_separator(ecp);
The first_path_var_separator() changes the "ecp" parameter on every call,
it returns the next path on "ecp" and changes it to have the remaining
paths to iterate over it.
> Other than that, I think this patch is good to go. Thanks!
>
Thanks for reviewing!
--
Matheus Alcantara