Re: [PATCH] Missing dep on Catalog.pm in meson rules

On 2023-06-14 We 15:32, Andres Freund wrote:
> Hi,
>
> On 2023-06-09 11:43:54 -0700, Andres Freund wrote:
>> On 2023-06-02 10:13:44 -0500, Tristan Partin wrote:
>>> On Fri Jun 2, 2023 at 8:47 AM CDT, Andres Freund wrote:
>>>> Hi,
>>>>
>>>> On 2023-06-02 08:10:43 -0500, Tristan Partin wrote:
>>>>>> I wonder if we instead could just make perl output the files it loads and
>>>>>> handle dependencies automatically that way? But that's more work, so it's
>>>>>> probably the right thing to go for the manual path for now.
>>>>> I am not familar with Perl enough (at all haha) to know if that is
>>>>> possible. I don't know exactly what these Perl files do, but perhaps it
>>>>> might make sense to have some global lookup table that is setup near the
>>>>> beginning of the script.
>>>> It'd be nice to have something more general - there are other perl modules we
>>>> load, e.g.
>>>> ./src/backend/catalog/Catalog.pm
>>>> ./src/backend/utils/mb/Unicode/convutils.pm
>>>> ./src/tools/PerfectHash.pm
>>>>
>>>>
>>>>> perl_files = {
>>>>> 'Catalog.pm': files('path/to/Catalog.pm'),
>>>>> ...
>>>>> }
>>>> I think you got it, but just to make sure: I was thinking of generating a
>>>> depfile from within perl. Something like what you propose doesn't quite seems
>>>> like a sufficient improvement.
>>> Whatever I am proposing is definitely subpar to generating a depfile. So
>>> if that can be done, that is the best option!
>> I looked for a bit, but couldn't find an easy way to do so. I would still like
>> to pursue going towards dep files for the perl scripts, even if that requires
>> explicit support in the perl scripts, but that's a change for later.
> Took a second look - sure looks like just using values %INC should suffice?
>
> Ilmari, you're the perl expert, is there an issue with that?
>
> Tristan, any chance you're interested hacking that up for a bunch of the
> scripts? Might be worth adding a common helper for, I guess?
>
> Something like
>
> for (values %INC)
> {
> print STDERR "$kw_def_file: $_\n";
> }
>
> seems to roughly do the right thing for gen_keywordlist.pl. Of course for
> something real it'd need an option where to put that data, instead of printing
> to stderr.
>

Unless I'm misunderstanding, this doesn't look terribly feasible to me.
You can only get at %INC by loading the module, which in many cases will
have side effects. And then you would also need to filter out things
loaded that are not our artefacts (e.g. Catalog.pm loads File::Compare).

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com