Horton

Thread

Commits

  1. Doc: improve cross-reference in Makefile comment.

  1. Confused about gram.y referencs in Makefile?

    Japin Li <japinli@hotmail.com> — 2023-09-25T02:41:05Z

    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

  2. Re: Confused about gram.y referencs in Makefile?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-09-25T03:17:50Z

    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

  3. Re: Confused about gram.y referencs in Makefile?

    Japin Li <japinli@hotmail.com> — 2023-09-25T03:34:27Z

    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

  4. Re: Confused about gram.y referencs in Makefile?

    Daniel Gustafsson <daniel@yesql.se> — 2023-09-25T07:29:16Z

    > 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

  5. Re: Confused about gram.y referencs in Makefile?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-09-25T15:29:54Z

    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