Обсуждение: gram.y => preproc.y

Hi,

since my last email seems to have disappeared, here we go again. Here's my
current patch that includes the changes to the build system. Thanks to Magnus
for the Windows part.

Comments anyone?

Michael

--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

Michael Meskes <meskes@postgresql.org> writes:
> since my last email seems to have disappeared, here we go again. Here's my
> current patch that includes the changes to the build system. Thanks to Magnus
> for the Windows part.

> Comments anyone? 

+ $(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
+     perl parse.pl < $< > $@ 

Use $(PERL) here.

(I'm not sure about the equivalent in the Windows world; it looks like
there are already places in Solution.pm that invoke perl via just
system("perl ..."), but is that really a good idea?  Anyway, not
directly your problem.)
 clean distclean:
!     rm -f keywords.c *.o ecpg$(X) preproc.y

Actually, we want to fix it so that preproc.y is treated like preproc.c,
ie, it's part of the shipped tarballs even though it's no longer in CVS.
For the same reason, you want to generate it in $(srcdir) even in a
VPATH build.  (Parts of this patch have that right and part don't.
You might want to test in a VPATH build before committing.)

Can't comment on the MSVC change.
        regards, tom lane

Michael Meskes <meskes@postgresql.org> writes:
> That's what I did first, but Magnus had a good reasoning to not keep preproc.y
> if we keep preproc.c in our tarball. And I agreed that there doesn't seem to be
> an advantage.

Other than whether it *works*, you mean?

make will not be happy if it has a dependency from preproc.c to preproc.y
and preproc.y is not there.  Please don't mess with this.
        regards, tom lane

On Thu, Nov 13, 2008 at 03:10:04PM -0500, Tom Lane wrote:
>   clean distclean:
> !     rm -f keywords.c *.o ecpg$(X) preproc.y
> 
> Actually, we want to fix it so that preproc.y is treated like preproc.c,
> ie, it's part of the shipped tarballs even though it's no longer in CVS.

That's what I did first, but Magnus had a good reasoning to not keep preproc.y
if we keep preproc.c in our tarball. And I agreed that there doesn't seem to be
an advantage.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

On Thu, Nov 13, 2008 at 03:57:49PM -0500, Tom Lane wrote:
> Michael Meskes <meskes@postgresql.org> writes:
> > That's what I did first, but Magnus had a good reasoning to not keep preproc.y
> > if we keep preproc.c in our tarball. And I agreed that there doesn't seem to be
> > an advantage.
> 
> Other than whether it *works*, you mean?
> 
> make will not be happy if it has a dependency from preproc.c to preproc.y
> and preproc.y is not there.  Please don't mess with this.

It doesn't complain but rebuild preproc.y which makes no sense at all. Removed
preproc.y from make clean.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

On Thu, Nov 13, 2008 at 03:10:04PM -0500, Tom Lane wrote:
> VPATH build.  (Parts of this patch have that right and part don't.
> You might want to test in a VPATH build before committing.)

Did that and fixed the remaining problems. Attached you'll find the latest
version. I already committed the new files, so the patch is way smaller.
However, the new files are not used without the attached changes to the build
system.

Does anyone see a problem with these changes? Or else I will commit.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

Michael Meskes wrote:

> +$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
> +    $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@ 

Why are you passing $(srcdir) as a parameter here?  It doesn't look like
the script uses it at all, or does it?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Fri, Nov 14, 2008 at 09:03:34AM -0300, Alvaro Herrera wrote:
> > +$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
> > +    $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@ 
> 
> Why are you passing $(srcdir) as a parameter here?  It doesn't look like
> the script uses it at all, or does it?

The new version does. I need to tell the script where to find the other files
in a VPATH build system.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

Michael Meskes <meskes@postgresql.org> writes:
> Does anyone see a problem with these changes? Or else I will commit.

preproc.y should be removed by make maintainer-clean.  For consistency
it probably ought to be listed in the distprep target too, even though
it would be indirectly updated by the preproc.c target.  I suspect it
would be a good idea to list preproc.c's antecedent as
$(srcdir)/preproc.y not just preproc.y.  Also, you'll need to add
preproc.y to .cvsignore.
        regards, tom lane

Michael Meskes wrote:
>
> Does anyone see a problem with these changes? Or else I will commit.
>
>   
>   

FWIW, I have looked at this a bit. I want to refactor it at some stage 
because it's ugly and fragile and rather obtuse, but the refactoring 
won't be happening for a while.

cheers

andrew

On Fri, Nov 14, 2008 at 10:44:08AM -0500, Tom Lane wrote:
> preproc.y should be removed by make maintainer-clean.  For consistency
> it probably ought to be listed in the distprep target too, even though
> it would be indirectly updated by the preproc.c target.  I suspect it
> would be a good idea to list preproc.c's antecedent as
> $(srcdir)/preproc.y not just preproc.y.  Also, you'll need to add
> preproc.y to .cvsignore.

Added.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

Michael Meskes <meskes@postgresql.org> writes:
> Added.

Looks alright to me now --- you might as well commit and let the
buildfarm find any remaining problems.
        regards, tom lane