pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] Missing dep on Catalog.pm in meson rules
18+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2023-06-10 a1cd982098 meson: Add dependencies to perl modules to various script invocations
* [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-01 12:41 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 2 replies; 18+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-06-01 12:41 UTC (permalink / raw)
To: pgsql-hackers
Hi Hackers,
While hacking on Catalog.pm (over in
https://postgr.es/m/87y1l3s7o9.fsf%40wibble.ilmari.org) I noticed that
ninja wouldn't rebuild postgres.bki on changes to the module. Here's a
patch that adds it to depend_files for the targets I culd find that
invoke scripts that use it.
- ilmari
Attachments:
0001-meson-declare-dependency-on-Catalog.pm-for-targets-t.patch (text/x-diff, 2.1K) download
From 63e8cdbd2509feeb493bf7b52362e8b429e8279c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Thu, 1 Jun 2023 13:27:41 +0100
Subject: [PATCH] meson: declare dependency on Catalog.pm for targets that use
it
---
src/include/catalog/meson.build | 2 +-
src/include/nodes/meson.build | 1 +
src/include/utils/meson.build | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/include/catalog/meson.build b/src/include/catalog/meson.build
index 3179be09d3..01181ae1fc 100644
--- a/src/include/catalog/meson.build
+++ b/src/include/catalog/meson.build
@@ -111,7 +111,7 @@ generated_catalog_headers = custom_target('generated_catalog_headers',
output: output_files,
install_dir: output_install,
input: input,
- depend_files: bki_data_f,
+ depend_files: bki_data_f + files('../../backend/catalog/Catalog.pm'),
build_by_default: true,
install: true,
command: [
diff --git a/src/include/nodes/meson.build b/src/include/nodes/meson.build
index 9a8e85c4a5..dafad003ed 100644
--- a/src/include/nodes/meson.build
+++ b/src/include/nodes/meson.build
@@ -50,6 +50,7 @@ node_support_install = [
generated_nodes = custom_target('nodetags.h',
input: node_support_input,
output: node_support_output,
+ depend_files: files('../../backend/catalog/Catalog.pm'),
command: [
perl, files('../../backend/nodes/gen_node_support.pl'),
'-o', '@OUTDIR@',
diff --git a/src/include/utils/meson.build b/src/include/utils/meson.build
index 2fed1e3f8e..3cbe21350c 100644
--- a/src/include/utils/meson.build
+++ b/src/include/utils/meson.build
@@ -44,6 +44,7 @@ fmgrtab_output = ['fmgroids.h', 'fmgrprotos.h', 'fmgrtab.c']
fmgrtab_target = custom_target('fmgrtab',
input: '../catalog/pg_proc.dat',
output : fmgrtab_output,
+ depend_files: files('../../backend/catalog/Catalog.pm'),
command: [perl, '-I', '@SOURCE_ROOT@/src/backend/catalog/', files('../../backend/utils/Gen_fmgrtab.pl'), '--include-path=@SOURCE_ROOT@/src/include', '--output=@OUTDIR@', '@INPUT@'],
install: true,
install_dir: [dir_include_server / 'utils', dir_include_server / 'utils', false],
--
2.39.2
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-01 13:11 Michael Paquier <michael@paquier.xyz>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
1 sibling, 0 replies; 18+ messages in thread
From: Michael Paquier @ 2023-06-01 13:11 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
On Thu, Jun 01, 2023 at 01:41:40PM +0100, Dagfinn Ilmari Mannsåker wrote:
> While hacking on Catalog.pm (over in
> https://postgr.es/m/87y1l3s7o9.fsf%40wibble.ilmari.org) I noticed that
> ninja wouldn't rebuild postgres.bki on changes to the module. Here's a
> patch that adds it to depend_files for the targets I culd find that
> invoke scripts that use it.
Nice catch! Indeed, we would need to track the dependency in the
three areas that use this module.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-01 21:16 Tristan Partin <tristan@neon.tech>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
1 sibling, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2023-06-01 21:16 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Could you create a variable for the file instead of calling files() 3
times?
> catalog_pm = files('path/to/Catalog.pm')
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-01 21:22 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Tristan Partin <tristan@neon.tech>
0 siblings, 1 reply; 18+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-06-01 21:22 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; pgsql-hackers
On Thu, 1 Jun 2023, at 22:16, Tristan Partin wrote:
> Could you create a variable for the file instead of calling files() 3
> times?
>
>> catalog_pm = files('path/to/Catalog.pm')
Sure, but which meson.build file should it go in? I know nothing about meson variable scoping.
> --
> Tristan Partin
> Neon (https://neon.tech)
--
- ilmari
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-02 04:06 Tristan Partin <tristan@neon.tech>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2023-06-02 04:06 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
On Thu Jun 1, 2023 at 4:22 PM CDT, Dagfinn Ilmari Mannsåker wrote:
> On Thu, 1 Jun 2023, at 22:16, Tristan Partin wrote:
> > Could you create a variable for the file instead of calling files() 3
> > times?
> >
> >> catalog_pm = files('path/to/Catalog.pm')
>
> Sure, but which meson.build file should it go in? I know nothing about meson variable scoping.
Not a problem. In Meson, variables are globally-scoped. You can use
unset_variable() however to unset it.
In our case, we should add the ^line to src/backend/catalog/meson.build.
I would say just throw the line after the copyright comment. Hopefully
there isn't a problem with the ordering of the Meson file tree traversal
(ie the targets you are changing are configured after we get through
src/backend/catalog/meson.build).
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-02 13:00 Andres Freund <andres@anarazel.de>
parent: Tristan Partin <tristan@neon.tech>
0 siblings, 1 reply; 18+ messages in thread
From: Andres Freund @ 2023-06-02 13:00 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Hi,
On 2023-06-01 23:06:04 -0500, Tristan Partin wrote:
> In our case, we should add the ^line to src/backend/catalog/meson.build.
src/backend is only reached well after src/include, due to needing
dependencies on files generated in src/include.
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.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-02 13:10 Tristan Partin <tristan@neon.tech>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2023-06-02 13:10 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
On Fri Jun 2, 2023 at 8:00 AM CDT, Andres Freund wrote:
> Hi,
>
> On 2023-06-01 23:06:04 -0500, Tristan Partin wrote:
> > In our case, we should add the ^line to src/backend/catalog/meson.build.
>
> src/backend is only reached well after src/include, due to needing
> dependencies on files generated in src/include.
I was worried about this :(.
> 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.
perl_files = {
'Catalog.pm': files('path/to/Catalog.pm'),
...
}
Otherwise, manual as it is in the original patch seems like an alright
compromise for now.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-02 13:47 Andres Freund <andres@anarazel.de>
parent: Tristan Partin <tristan@neon.tech>
0 siblings, 1 reply; 18+ messages in thread
From: Andres Freund @ 2023-06-02 13:47 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
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.
> Otherwise, manual as it is in the original patch seems like an alright
> compromise for now.
Yea. I'm working on a more complete version, also dealing with dependencies on
PerfectHash.pm.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-02 15:13 Tristan Partin <tristan@neon.tech>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2023-06-02 15:13 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
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!
> > Otherwise, manual as it is in the original patch seems like an alright
> > compromise for now.
>
> Yea. I'm working on a more complete version, also dealing with dependencies on
> PerfectHash.pm.
Good to hear. Happy to review any patches :).
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-09 18:43 Andres Freund <andres@anarazel.de>
parent: Tristan Partin <tristan@neon.tech>
0 siblings, 2 replies; 18+ messages in thread
From: Andres Freund @ 2023-06-09 18:43 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Hi,
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.
> > > Otherwise, manual as it is in the original patch seems like an alright
> > > compromise for now.
> >
> > Yea. I'm working on a more complete version, also dealing with dependencies on
> > PerfectHash.pm.
>
> Good to hear. Happy to review any patches :).
Attached.
Greetings,
Andres Freund
Attachments:
v2-0001-meson-Add-dependencies-to-perl-modules-to-various.patch (text/x-diff, 7.4K) download
From e64cd6233ee8637e2e66f9e31085f5c2c5c5c388 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 9 Jun 2023 11:41:42 -0700
Subject: [PATCH v2] meson: Add dependencies to perl modules to various script
invocations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reported-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/87v8g7s6bf.fsf@wibble.ilmari.org
---
meson.build | 14 ++++++++++++++
src/include/catalog/meson.build | 2 +-
src/include/nodes/meson.build | 1 +
src/include/utils/meson.build | 1 +
src/common/meson.build | 4 ++--
src/common/unicode/meson.build | 3 +++
src/pl/plpgsql/src/meson.build | 7 ++++---
src/interfaces/ecpg/preproc/meson.build | 19 ++++---------------
8 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/meson.build b/meson.build
index 16b2e866462..82f2782673e 100644
--- a/meson.build
+++ b/meson.build
@@ -2681,6 +2681,20 @@ gen_export_kwargs = {
+###
+### Helpers for custom targets used across the tree
+###
+
+catalog_pm = files('src/backend/catalog/Catalog.pm')
+perfect_hash_pm = files('src/tools/PerfectHash.pm')
+gen_kwlist_deps = [perfect_hash_pm]
+gen_kwlist_cmd = [
+ perl, '-I', '@SOURCE_ROOT@/src/tools',
+ files('src/tools/gen_keywordlist.pl'),
+ '--output', '@OUTDIR@', '@INPUT@']
+
+
+
###
### windows resources related stuff
###
diff --git a/src/include/catalog/meson.build b/src/include/catalog/meson.build
index 3179be09d3d..c3fd05d0279 100644
--- a/src/include/catalog/meson.build
+++ b/src/include/catalog/meson.build
@@ -111,7 +111,7 @@ generated_catalog_headers = custom_target('generated_catalog_headers',
output: output_files,
install_dir: output_install,
input: input,
- depend_files: bki_data_f,
+ depend_files: bki_data_f + catalog_pm,
build_by_default: true,
install: true,
command: [
diff --git a/src/include/nodes/meson.build b/src/include/nodes/meson.build
index 9a8e85c4a5e..626dc696d51 100644
--- a/src/include/nodes/meson.build
+++ b/src/include/nodes/meson.build
@@ -50,6 +50,7 @@ node_support_install = [
generated_nodes = custom_target('nodetags.h',
input: node_support_input,
output: node_support_output,
+ depend_files: catalog_pm,
command: [
perl, files('../../backend/nodes/gen_node_support.pl'),
'-o', '@OUTDIR@',
diff --git a/src/include/utils/meson.build b/src/include/utils/meson.build
index 2fed1e3f8e9..c212c4091f7 100644
--- a/src/include/utils/meson.build
+++ b/src/include/utils/meson.build
@@ -44,6 +44,7 @@ fmgrtab_output = ['fmgroids.h', 'fmgrprotos.h', 'fmgrtab.c']
fmgrtab_target = custom_target('fmgrtab',
input: '../catalog/pg_proc.dat',
output : fmgrtab_output,
+ depend_files: catalog_pm,
command: [perl, '-I', '@SOURCE_ROOT@/src/backend/catalog/', files('../../backend/utils/Gen_fmgrtab.pl'), '--include-path=@SOURCE_ROOT@/src/include', '--output=@OUTDIR@', '@INPUT@'],
install: true,
install_dir: [dir_include_server / 'utils', dir_include_server / 'utils', false],
diff --git a/src/common/meson.build b/src/common/meson.build
index 41bd58ebdf1..9efc80ac024 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -54,8 +54,8 @@ endif
common_kwlist = custom_target('kwlist',
input: files('../include/parser/kwlist.h'),
output: 'kwlist_d.h',
- command: [perl, '-I', '@SOURCE_ROOT@/src/tools', files('../tools/gen_keywordlist.pl'),
- '--extern', '--output', '@OUTDIR@', '@INPUT@'])
+ depend_files: gen_kwlist_deps,
+ command: [gen_kwlist_cmd, '--extern'])
generated_sources += common_kwlist
common_sources += common_kwlist
diff --git a/src/common/unicode/meson.build b/src/common/unicode/meson.build
index 1ffece1550a..9033c4a3dcf 100644
--- a/src/common/unicode/meson.build
+++ b/src/common/unicode/meson.build
@@ -28,6 +28,7 @@ update_unicode_targets += \
custom_target('unicode_norm_table.h',
input: [unicode_data['UnicodeData.txt'], unicode_data['CompositionExclusions.txt']],
output: ['unicode_norm_table.h', 'unicode_norm_hashfunc.h'],
+ depend_files: perfect_hash_pm,
command: [
perl, files('generate-unicode_norm_table.pl'),
'--outdir', '@OUTDIR@', '@INPUT@'],
@@ -38,6 +39,7 @@ update_unicode_targets += \
custom_target('unicode_nonspacing_table.h',
input: [unicode_data['UnicodeData.txt']],
output: ['unicode_nonspacing_table.h'],
+ depend_files: perfect_hash_pm,
command: [perl, files('generate-unicode_nonspacing_table.pl'), '@INPUT@'],
build_by_default: false,
capture: true,
@@ -56,6 +58,7 @@ update_unicode_targets += \
custom_target('unicode_normprops_table.h',
input: [unicode_data['DerivedNormalizationProps.txt']],
output: ['unicode_normprops_table.h'],
+ depend_files: perfect_hash_pm,
command: [perl, files('generate-unicode_normprops_table.pl'), '@INPUT@'],
build_by_default: false,
capture: true,
diff --git a/src/pl/plpgsql/src/meson.build b/src/pl/plpgsql/src/meson.build
index e185a87024c..85e7293b374 100644
--- a/src/pl/plpgsql/src/meson.build
+++ b/src/pl/plpgsql/src/meson.build
@@ -25,11 +25,11 @@ pl_errcodes = custom_target('plerrcodes',
generated_sources += pl_errcodes
plpgsql_sources += pl_errcodes
-gen_keywordlist = files('../../../../src/tools/gen_keywordlist.pl')
pl_reserved = custom_target('pl_reserved_kwlist',
input: ['pl_reserved_kwlist.h'],
output: ['pl_reserved_kwlist_d.h'],
- command: [perl, '-I', '@SOURCE_ROOT@/src/tools', gen_keywordlist, '--output', '@OUTDIR@', '--varname', 'ReservedPLKeywords', '@INPUT@']
+ depend_files: gen_kwlist_deps,
+ command: [gen_kwlist_cmd, '--varname', 'ReservedPLKeywords'],
)
generated_sources += pl_reserved
plpgsql_sources += pl_reserved
@@ -37,7 +37,8 @@ plpgsql_sources += pl_reserved
pl_unreserved = custom_target('pl_unreserved_kwlist',
input: ['pl_unreserved_kwlist.h'],
output: ['pl_unreserved_kwlist_d.h'],
- command: [perl, '-I', '@SOURCE_ROOT@/src/tools', gen_keywordlist, '--output', '@OUTDIR@', '--varname', 'UnreservedPLKeywords', '@INPUT@']
+ depend_files: gen_kwlist_deps,
+ command: [gen_kwlist_cmd, '--varname', 'UnreservedPLKeywords'],
)
generated_sources += pl_unreserved
plpgsql_sources += pl_unreserved
diff --git a/src/interfaces/ecpg/preproc/meson.build b/src/interfaces/ecpg/preproc/meson.build
index 08d772d2614..eef8f1864fe 100644
--- a/src/interfaces/ecpg/preproc/meson.build
+++ b/src/interfaces/ecpg/preproc/meson.build
@@ -69,14 +69,8 @@ c_kwlist = custom_target('c_kwlist_d.h',
input: ['c_kwlist.h'],
output: ['c_kwlist_d.h'],
depends: check_rules,
- command: [
- perl,
- '-I', '@SOURCE_ROOT@/src/tools',
- '@SOURCE_ROOT@/src/tools/gen_keywordlist.pl',
- '--output', '@OUTDIR@',
- '--varname', 'ScanCKeywords',
- '--no-case-fold', '@INPUT0@',
- ],
+ depend_files: gen_kwlist_deps,
+ command: [gen_kwlist_cmd, '--varname', 'ScanCKeywords', '--no-case-fold'],
)
generated_sources += c_kwlist
ecpg_sources += c_kwlist
@@ -84,13 +78,8 @@ ecpg_sources += c_kwlist
ecpg_kwlist = custom_target('ecpg_kwlist_d.h',
input: ['ecpg_kwlist.h'],
output: ['ecpg_kwlist_d.h'],
- command: [
- perl, '-I',
- '@SOURCE_ROOT@/src/tools',
- '@SOURCE_ROOT@/src/tools/gen_keywordlist.pl',
- '--output', '@OUTDIR@',
- '--varname', 'ScanECPGKeywords', '@INPUT0@',
- ]
+ depend_files: gen_kwlist_deps,
+ command: [gen_kwlist_cmd, '--varname', 'ScanECPGKeywords'],
)
generated_sources += ecpg_kwlist
ecpg_sources += ecpg_kwlist
--
2.38.0
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-09 18:58 Tristan Partin <tristan@neon.tech>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2023-06-09 18:58 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Patch looks good to me!
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-10 03:16 Andres Freund <andres@anarazel.de>
parent: Tristan Partin <tristan@neon.tech>
0 siblings, 0 replies; 18+ messages in thread
From: Andres Freund @ 2023-06-10 03:16 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Hi,
On 2023-06-09 13:58:46 -0500, Tristan Partin wrote:
> Patch looks good to me!
Thanks for the report Ilmari and the review Tristan! Pushed the fix.
Regards,
Andres
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-14 19:32 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 2 replies; 18+ messages in thread
From: Andres Freund @ 2023-06-14 19:32 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
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.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-15 00:25 Tristan Partin <tristan@neon.tech>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 18+ messages in thread
From: Tristan Partin @ 2023-06-15 00:25 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
On Wed Jun 14, 2023 at 2:32 PM CDT, 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.
I would need to familiarize myself with perl, but since you've written
probably all or almost all that needs to be written, I can probably
scrape by :).
I definitely have the bandwidth to make this change though pending what
Ilmari says.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-16 20:20 Andrew Dunstan <andrew@dunslane.net>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 18+ messages in thread
From: Andrew Dunstan @ 2023-06-16 20:20 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Tristan Partin <tristan@neon.tech>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-16 21:16 Andres Freund <andres@anarazel.de>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 18+ messages in thread
From: Andres Freund @ 2023-06-16 21:16 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Tristan Partin <tristan@neon.tech>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers
Hi,
On 2023-06-16 16:20:14 -0400, Andrew Dunstan wrote:
> 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.
I was envisioning using %INC after the use/require block - I don't think our
scripts load additional modules after that point?
> And then you would also need to filter out things loaded that
> are not our artefacts (e.g. Catalog.pm loads File::Compare).
I don't think we would need to filter the output. This would just be for a
build dependency file. I don't see a problem with rerunning genbki.pl et al after
somebody updates File::Compare?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-16 22:10 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-06-16 22:10 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Tristan Partin <tristan@neon.tech>; pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> Hi,
>
> On 2023-06-16 16:20:14 -0400, Andrew Dunstan wrote:
>> 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.
>
> I was envisioning using %INC after the use/require block - I don't think our
> scripts load additional modules after that point?
I was thinking of a module for writing depfile entries that would append
`values %INC` to the list of source files for each target specified by
the script.
>> And then you would also need to filter out things loaded that
>> are not our artefacts (e.g. Catalog.pm loads File::Compare).
>
> I don't think we would need to filter the output. This would just be for a
> build dependency file. I don't see a problem with rerunning genbki.pl et al after
> somebody updates File::Compare?
As long as mason doesn't object to dep files outside the source tree.
Otherwise, and option would be to pass in @SOURCE_ROOT@ and only include
`grep /^\Q$source_root\E\b/, values %INC` in the depfile.
> Greetings,
>
> Andres Freund
- ilmari
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Missing dep on Catalog.pm in meson rules
@ 2023-06-16 22:11 Tristan Partin <tristan@neon.tech>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 0 replies; 18+ messages in thread
From: Tristan Partin @ 2023-06-16 22:11 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; Andres Freund <andres@anarazel.de>; +Cc: Andrew Dunstan <andrew@dunslane.net>; pgsql-hackers
On Fri Jun 16, 2023 at 5:10 PM CDT, Dagfinn Ilmari Mannsåker wrote:
> Andres Freund <andres@anarazel.de> writes:
>
> > Hi,
> >
> > On 2023-06-16 16:20:14 -0400, Andrew Dunstan wrote:
> >> 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.
> >
> > I was envisioning using %INC after the use/require block - I don't think our
> > scripts load additional modules after that point?
>
> I was thinking of a module for writing depfile entries that would append
> `values %INC` to the list of source files for each target specified by
> the script.
>
> >> And then you would also need to filter out things loaded that
> >> are not our artefacts (e.g. Catalog.pm loads File::Compare).
> >
> > I don't think we would need to filter the output. This would just be for a
> > build dependency file. I don't see a problem with rerunning genbki.pl et al after
> > somebody updates File::Compare?
>
> As long as mason doesn't object to dep files outside the source tree.
> Otherwise, and option would be to pass in @SOURCE_ROOT@ and only include
> `grep /^\Q$source_root\E\b/, values %INC` in the depfile.
Meson has no such restrictions.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2023-06-16 22:11 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 12:41 [PATCH] Missing dep on Catalog.pm in meson rules Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-06-01 13:11 ` Michael Paquier <michael@paquier.xyz>
2023-06-01 21:16 ` Tristan Partin <tristan@neon.tech>
2023-06-01 21:22 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-06-02 04:06 ` Tristan Partin <tristan@neon.tech>
2023-06-02 13:00 ` Andres Freund <andres@anarazel.de>
2023-06-02 13:10 ` Tristan Partin <tristan@neon.tech>
2023-06-02 13:47 ` Andres Freund <andres@anarazel.de>
2023-06-02 15:13 ` Tristan Partin <tristan@neon.tech>
2023-06-09 18:43 ` Andres Freund <andres@anarazel.de>
2023-06-09 18:58 ` Tristan Partin <tristan@neon.tech>
2023-06-10 03:16 ` Andres Freund <andres@anarazel.de>
2023-06-14 19:32 ` Andres Freund <andres@anarazel.de>
2023-06-15 00:25 ` Tristan Partin <tristan@neon.tech>
2023-06-16 20:20 ` Andrew Dunstan <andrew@dunslane.net>
2023-06-16 21:16 ` Andres Freund <andres@anarazel.de>
2023-06-16 22:10 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-06-16 22:11 ` Tristan Partin <tristan@neon.tech>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox