Thread: Re: [COMMITTERS] 'pgsql/src/backend/parser parse.h gram.c'

> Modified Files:
>         parse.h gram.c
> Log Message:
> Someone forgot to commit gram.c and parse.h after his latest
> set of updates to gram.y.

I didn't forget! istm that we risk cvs bloat by checking in derived
files like gram.c, when there are probably minor differences in how each
system generates them. So I don't usually check in those files when we
are deep in the middle of hacker development. Of course, that could be
the wrong thing to (not) do...
                   - Tom

On Wed, 3 Mar 1999, Thomas G. Lockhart wrote:

> > Modified Files:
> >         parse.h gram.c
> > Log Message:
> > Someone forgot to commit gram.c and parse.h after his latest
> > set of updates to gram.y.
> 
> I didn't forget! istm that we risk cvs bloat by checking in derived
> files like gram.c, when there are probably minor differences in how each
> system generates them. So I don't usually check in those files when we
> are deep in the middle of hacker development. Of course, that could be
> the wrong thing to (not) do...

Would have to agree here...developers *should* have the tools on their
systems required to generate this...these should only be
generated/committed as part of our pre-release checklist...

I think there are slightly more important things to worry about during
development...no? :)

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 

The Hermit Hacker <scrappy@hub.org> writes:
> On Wed, 3 Mar 1999, Thomas G. Lockhart wrote:
>>>> Someone forgot to commit gram.c and parse.h after his latest
>>>> set of updates to gram.y.
>> 
>> I didn't forget! istm that we risk cvs bloat by checking in derived
>> files like gram.c,

> Would have to agree here...developers *should* have the tools on their
> systems required to generate this...these should only be
> generated/committed as part of our pre-release checklist...
> I think there are slightly more important things to worry about during
> development...no? :)

Well, if you want to know why I was annoyed, I'll explain.

On my machine, gram.c/parse.h appeared to be newer than gram.y.
Just a chance artifact of when I happened to do cvs updates,
no doubt.  (It doesn't help that cvs nearly always updates gram.y
first when it has to update them all --- it probably gave me
version N of gram.y and version N-1 of the derived files in the
same update run, while managing to make the derived files look
newer.)

Since gram.c/parse.h were in fact *not* up-to-date with respect
to header-file changes elsewhere, they didn't compile.

I thought this was breakage due to Bruce's removal of recipe.h/.c,
and griped accordingly last Tuesday or so.  It wasn't till Saturday
that I realized the guys whom I was expecting to fix it were on
vacation and I had better do my own digging.  Now it didn't take me
a lot of hours to realize what was wrong, but it very easily could've
--- and in any case I'd already spent a couple of evenings doing other
stuff when I had wanted to work on Postgres.

My feeling is this: if you want to remove gram.c/parse.h from the
CVS file set entirely, expecting developer types to have the
necessary tools, that's a defensible approach.  (Tarball drops
could and should include freshly-generated copies, of course.)
Or, if you want to keep them in CVS and *keep them up to date*,
that works for me too.  But don't be wishy-washy.  You're just
wasting download time and developer time if you don't treat
these files consistently.
        still fairly annoyed, tom lane

Then <lockhart@alumni.caltech.edu> spoke up and said:
> > Modified Files:
> >         parse.h gram.c
> > Log Message:
> > Someone forgot to commit gram.c and parse.h after his latest
> > set of updates to gram.y.
> 
> I didn't forget! istm that we risk cvs bloat by checking in derived
> files like gram.c, when there are probably minor differences in how each
> system generates them. So I don't usually check in those files when we
> are deep in the middle of hacker development. Of course, that could be
> the wrong thing to (not) do...

You *never* want to check in derived files when using CVS.  Instead,
create checkout rules (see the cvs docs) to automatically create those
files upon checkin/checkout, whichever is more appropriate.

-- 
=====================================================================
| JAVA must have been developed in the wilds of West Virginia.      |
| After all, why else would it support only single inheritance??    |
=====================================================================
| Finger geek@andrew.cmu.edu for my public key.                     |
=====================================================================

> >> I didn't forget! istm that we risk cvs bloat by checking in derived
> >> files like gram.c,
> Well, if you want to know why I was annoyed, I'll explain.
> On my machine, gram.c/parse.h appeared to be newer than gram.y.
> Since gram.c/parse.h were in fact *not* up-to-date with respect
> to header-file changes elsewhere, they didn't compile.

Oh! I haven't seen that behavior before, and am not sure why you did :(
If I updated gram.y, but did not update gram.c, then cvs and CVSup
should never bollix up the times on the files afaik.

Sorry for the diversion, but I'm confused as to how you got this
symptom. Been doing this for a couple of years now, and doing a *lot*
with gram.y so if it was a checkin or sync problem I would have expected
to see it before this.

btw, one of the changes I made was to move the backend/parser/ build to
earlier in the build list, since when debugging is enabled the node
printing functions (now) need to see the definitions of some of the
parser nodes. I wonder if somehow that was involved??
                     - Tom

"Thomas G. Lockhart" <lockhart@alumni.caltech.edu> writes:
>> Well, if you want to know why I was annoyed, I'll explain.
>> On my machine, gram.c/parse.h appeared to be newer than gram.y.
>> Since gram.c/parse.h were in fact *not* up-to-date with respect
>> to header-file changes elsewhere, they didn't compile.

> Oh! I haven't seen that behavior before, and am not sure why you did :(
> If I updated gram.y, but did not update gram.c, then cvs and CVSup
> should never bollix up the times on the files afaik.

I don't think it's cvs' fault.  The recent checkin times for gram.y are:
2.57 1999/02/23 07:42:41;  author: thomas
2.56 1999/02/21 03:49:00;  author: scrappy
2.55 1999/02/13 23:17:03;  author: momjian
2.54 1999/02/08 14:14:12;  author: wieck
2.53 1999/02/07 19:02:19;  author: wieck

and gram.c:
2.75 1999/02/27 21:33:53;  author: tgl
2.74 1999/02/22 05:26:33;  author: momjian
2.73 1999/02/14 05:14:09;  author: momjian
2.72 1999/02/13 23:16:54;  author: momjian
2.71 1999/02/09 06:30:40;  author: momjian
2.70 1999/02/09 03:51:30;  author: momjian
2.69 1999/02/07 19:04:59;  author: wieck

Unfortunately I do not have a log of when I recently did "cvs update"s
(unless someone knows a way to extract that info from cvs itself)?
But I will bet that when I updated on 2/24, it had been more than two
days since my previous update, and so cvs updated both gram.y (to 2.57)
and gram.c (to 2.74, that being Bruce's latest commit) in the same
update cycle.  Since cvs timestamps updated files with the time of
*retrieval*, gram.c ended up having a slightly newer timestamp locally
than gram.y, and so make didn't think it needed to be rebuilt.

I'm using cvs 1.10 here, but I don't think its behavior has changed
in this respect in any recent version.  If it did not timestamp updates
with time of retrieval, it would create synchronization bugs with
respect to locally created files, which'd be no fun either.

I still stand by my observation: either gram.c is a derived file and
shouldn't be in the CVS repository at all, or else it is a master file
and must be maintained just as faithfully as any other source file.
There is no middle ground that will work reliably with CVS.  Especially
not in the face of multiple authors some of whom commit the derived file
and some of whom don't.
        regards, tom lane

PS: treating gram.c as a derived file that is made while building
a tarball would also eliminate our recurring problem with gram.c
appearing to be out-of-date in releases, when it really isn't.

> I don't think it's cvs' fault.
> I'm using cvs 1.10 here, but I don't think its behavior has changed
> in this respect in any recent version.  If it did not timestamp 
> updates with time of retrieval, it would create synchronization bugs 
> with respect to locally created files, which'd be no fun either.

I use CVSup, and it seems to keep the creation date of files consistant
with the source cvs tree. Does anonymous cvs not do that? Again, I
*don't* see the behavior that you do, at least given my CVSup/cvs-1.9
system.

> PS: treating gram.c as a derived file that is made while building
> a tarball would also eliminate our recurring problem with gram.c
> appearing to be out-of-date in releases, when it really isn't.

Sure. Who wants to work that out?

btw, could all of this be traced to bad dependencies on parse.h? Our
current Makefile system does not do this correctly. I applied some small
patches recently to help, but it did not fix the fundamental problem
that all the backend/* directories refer to backend/parse.h, but they do
not know that backend/parse.h is a copy of backend/parser/parse.h.
                       - Tom

"Thomas G. Lockhart" <lockhart@alumni.caltech.edu> writes:
>> I'm using cvs 1.10 here, but I don't think its behavior has changed
>> in this respect in any recent version.  If it did not timestamp 
>> updates with time of retrieval, it would create synchronization bugs 
>> with respect to locally created files, which'd be no fun either.

> I use CVSup, and it seems to keep the creation date of files consistant
> with the source cvs tree. Does anonymous cvs not do that?

I don't know anything about CVSup, but plain cvs works the way I
described, and *should* work the way I described.  Otherwise, when
you update a ".c" file from the repository, it might not look like
it's newer than the corresponding ".o" file that you generated locally
(since that could have been made later than someone else committed a
change to the .c file).

If CVSup timestamps files with their repository timestamps, then the
only safe way to use it is to "make distclean" and rebuild from scratch
after every update.  (Of course that's probably a good idea anyway since
we aren't maintaining reliable dependency info in the makefiles, but we
shouldn't get forced into it because of a misdesigned tool.)

> btw, could all of this be traced to bad dependencies on parse.h?

No.  The problem was that parse.h itself was not up-to-date.
        regards, tom lane

> > btw, could all of this be traced to bad dependencies on parse.h?
> No.  The problem was that parse.h itself was not up-to-date.

Yeah, I guess that was what I was asking. There is a problem in that
most of the backend/* subsystems have a dependency on ../parse.h, rather
than ../parser/parse.h. So they never know that the parse.h is out of
date :(

If you can fix up this dependency so it does the right thing, that would
be A Good Thing...
                    - Tom