Обсуждение: Put genbki.pl output into src/include/catalog/ directly

Поиск
Список
Период
Сортировка

Put genbki.pl output into src/include/catalog/ directly

От
Peter Eisentraut
Дата:
With the makefile rules, the output of genbki.pl was written to
src/backend/catalog/, and then the header files were linked to
src/include/catalog/.

This patch changes it so that the output files are written directly to
src/include/catalog/.  This makes the logic simpler, and it also makes
the behavior consistent with the meson build system.  For example, a 
file like schemapg.h is now mentioned only in

src/include/catalog/{meson.build,Makefile,.gitignore}

where before it was mentioned in (checks ...)

src/backend/catalog/.gitignore
src/backend/catalog/Makefile
src/include/Makefile
src/include/catalog/.gitignore
src/include/catalog/meson.build

Also, the list of catalog files is now kept in parallel in
src/include/catalog/{meson.build,Makefile}, while before the makefiles
had it in src/backend/catalog/Makefile.

I think keeping the two build systems aligned this way will be useful 
for longer-term maintenance.

(There are other generated header files that are linked in a similar way 
and could perhaps be simplified.  But they don't all work the same way. 
Some of the scripts also generate .c files, for example, so they need to 
put some stuff under src/backend/.  So I restricted this patch to 
src/{backend,include}/catalog/, especially because it would be good to 
keep the catalog lists aligned.)
Вложения

Re: Put genbki.pl output into src/include/catalog/ directly

От
Tom Lane
Дата:
Peter Eisentraut <peter@eisentraut.org> writes:
> With the makefile rules, the output of genbki.pl was written to
> src/backend/catalog/, and then the header files were linked to
> src/include/catalog/.

> This patch changes it so that the output files are written directly to
> src/include/catalog/.

Didn't read the patch, but +1 for concept.

            regards, tom lane



Re: Put genbki.pl output into src/include/catalog/ directly

От
Andreas Karlsson
Дата:
On 2/8/24 8:58 AM, Peter Eisentraut wrote:
> I think keeping the two build systems aligned this way will be useful 
> for longer-term maintenance.

Agreed, so started reviewing the patch. Attached is a rebased version of 
the patch to solve a conflict.

Andreas
Вложения

Re: Put genbki.pl output into src/include/catalog/ directly

От
Andreas Karlsson
Дата:
On 3/13/24 12:41 PM, Andreas Karlsson wrote:
> On 2/8/24 8:58 AM, Peter Eisentraut wrote:
>> I think keeping the two build systems aligned this way will be useful 
>> for longer-term maintenance.
> 
> Agreed, so started reviewing the patch. Attached is a rebased version of 
> the patch to solve a conflict.

I have reviewed the patch now and would say it looks good. I like how we 
remove the symlinks plus make things more similar to the meson build so 
I think we should merge this.

I tried building, running tests, running make clean, running make 
install and tried building with meson (plus checking that meson really 
checks for the generated files and actually refuse to build). Everything 
worked as expected.

The code changes look clean and mostly consist of moving code. I 
personally think this is ready for committer.

Andreas



Re: Put genbki.pl output into src/include/catalog/ directly

От
Peter Eisentraut
Дата:
On 14.03.24 02:33, Andreas Karlsson wrote:
> On 3/13/24 12:41 PM, Andreas Karlsson wrote:
>> On 2/8/24 8:58 AM, Peter Eisentraut wrote:
>>> I think keeping the two build systems aligned this way will be useful 
>>> for longer-term maintenance.
>>
>> Agreed, so started reviewing the patch. Attached is a rebased version 
>> of the patch to solve a conflict.
> 
> I have reviewed the patch now and would say it looks good. I like how we 
> remove the symlinks plus make things more similar to the meson build so 
> I think we should merge this.
> 
> I tried building, running tests, running make clean, running make 
> install and tried building with meson (plus checking that meson really 
> checks for the generated files and actually refuse to build). Everything 
> worked as expected.
> 
> The code changes look clean and mostly consist of moving code. I 
> personally think this is ready for committer.

Committed, thanks.