Confused about gram.y referencs in Makefile?

Started by Japin Liover 2 years ago5 messages
#1Japin Li
japinli@hotmail.com

Hi, hackers

I find src/backend/utils/mb/Unicode/Makefile has the following comments:

# Note that while each script call produces two output files, to be
# parallel-make safe we need to split this into two rules. (See for
# example gram.y for more explanation.)
#

I could not find the explanation in gram.y easily. Would someone point
it out for me? Thanks in advance!

--
Regrads,
Japin Li

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Japin Li (#1)
Re: Confused about gram.y referencs in Makefile?

Japin Li <japinli@hotmail.com> writes:

I find src/backend/utils/mb/Unicode/Makefile has the following comments:

# Note that while each script call produces two output files, to be
# parallel-make safe we need to split this into two rules. (See for
# example gram.y for more explanation.)

I could not find the explanation in gram.y easily. Would someone point
it out for me? Thanks in advance!

It's referring to this bit in src/backend/parser/Makefile:

-----
# There is no correct way to write a rule that generates two files.
# Rules with two targets don't have that meaning, they are merely
# shorthand for two otherwise separate rules. If we have an action
# that in fact generates two or more files, we must choose one of them
# as primary and show it as the action's output, then make all of the
# other output files dependent on the primary, like this. Furthermore,
# the "touch" action is essential, because it ensures that gram.h is
# marked as newer than (or at least no older than) gram.c. Without that,
# make is likely to try to rebuild gram.h in subsequent runs, which causes
# failures in VPATH builds from tarballs.

gram.h: gram.c
touch $@

gram.c: BISONFLAGS += -d
gram.c: BISON_CHECK_CMD = $(PERL) $(srcdir)/check_keywords.pl $< $(top_srcdir)/src/include/parser/kwlist.h
-----

This is indeed kind of confusing, because there's no explicit
reference to gram.y here --- the last two lines just tweak
the behavior of the default .y to .c rule.

Maybe we should adjust the comment in Unicode/Makefile, but
I'm not sure what would be a better reference.

regards, tom lane

#3Japin Li
japinli@hotmail.com
In reply to: Tom Lane (#2)
Re: Confused about gram.y referencs in Makefile?

On Mon, 25 Sep 2023 at 11:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Japin Li <japinli@hotmail.com> writes:

I find src/backend/utils/mb/Unicode/Makefile has the following comments:

# Note that while each script call produces two output files, to be
# parallel-make safe we need to split this into two rules. (See for
# example gram.y for more explanation.)

I could not find the explanation in gram.y easily. Would someone point
it out for me? Thanks in advance!

It's referring to this bit in src/backend/parser/Makefile:

-----
# There is no correct way to write a rule that generates two files.
# Rules with two targets don't have that meaning, they are merely
# shorthand for two otherwise separate rules. If we have an action
# that in fact generates two or more files, we must choose one of them
# as primary and show it as the action's output, then make all of the
# other output files dependent on the primary, like this. Furthermore,
# the "touch" action is essential, because it ensures that gram.h is
# marked as newer than (or at least no older than) gram.c. Without that,
# make is likely to try to rebuild gram.h in subsequent runs, which causes
# failures in VPATH builds from tarballs.

gram.h: gram.c
touch $@

gram.c: BISONFLAGS += -d
gram.c: BISON_CHECK_CMD = $(PERL) $(srcdir)/check_keywords.pl $< $(top_srcdir)/src/include/parser/kwlist.h
-----

This is indeed kind of confusing, because there's no explicit
reference to gram.y here --- the last two lines just tweak
the behavior of the default .y to .c rule.

Maybe we should adjust the comment in Unicode/Makefile, but
I'm not sure what would be a better reference.

regards, tom lane

Thank you!

Maybe be reference src/backend/parser/Makefile is better than current.

How about "See gram.h target's comment in src/backend/parser/Makefile"
or just "See src/backend/parser/Makefile"?

--
Regrads,
Japin Li

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Japin Li (#3)
Re: Confused about gram.y referencs in Makefile?

On 25 Sep 2023, at 05:34, Japin Li <japinli@hotmail.com> wrote:

Maybe be reference src/backend/parser/Makefile is better than current.

How about "See gram.h target's comment in src/backend/parser/Makefile"
or just "See src/backend/parser/Makefile"?

The latter seems more stable, if the Makefile is ever restructured it's almost
guaranteed that this comment will be missed with the location info becoming
stale.

--
Daniel Gustafsson

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#4)
Re: Confused about gram.y referencs in Makefile?

Daniel Gustafsson <daniel@yesql.se> writes:

On 25 Sep 2023, at 05:34, Japin Li <japinli@hotmail.com> wrote:

How about "See gram.h target's comment in src/backend/parser/Makefile"
or just "See src/backend/parser/Makefile"?

The latter seems more stable, if the Makefile is ever restructured it's almost
guaranteed that this comment will be missed with the location info becoming
stale.

I did it like this:

 # Note that while each script call produces two output files, to be
-# parallel-make safe we need to split this into two rules.  (See for
-# example gram.y for more explanation.)
+# parallel-make safe we need to split this into two rules.  (See notes
+# in src/backend/parser/Makefile about rules with multiple outputs.)
 #

There are a whole lot of other cross-references to that same comment,
and they all look like

# See notes in src/backend/parser/Makefile about the following two rules

I considered modifying all of those as well, but decided it wasn't
really worth the trouble. The Makefiles' days are numbered I think.

regards, tom lane