Thread: Put genbki.pl output into src/include/catalog/ directly
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.)
Attachment
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
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
Attachment
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
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.