Обсуждение: patch for minor Win32 makefile bug

Greetings,

While working on a patch that allows postmaster to run as an NT service
with Cygwin, I found minor problem in backend/Makefile.  In order to add
an additional library to the final link/build of postgres.exe, I changed
the DLLLIBS variable in makefiles/Makefile.win.  But this change had no
effect until I removed the line in backend/Makefile that also sets
DLLLIB, hiding the value from Makefile.win.  Note that backend/Makefile
includes Makefile.global, which includes Makefile.port, which is a
symlink to Makefile.win for a Cygwin/Win32 build.

The patch for this change to backend/Makefile is attached.

--
Fred Yankowski           fred@OntoSys.com      tel: +1.630.879.1312
Principal Consultant     www.OntoSys.com       fax: +1.630.879.1370
OntoSys, Inc             38W242 Deerpath Rd, Batavia, IL 60510, USA

--
Index: Makefile
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/Makefile,v
retrieving revision 1.68
diff -u -p -r1.68 Makefile
--- Makefile    2000/11/30 20:36:10     1.68
+++ Makefile    2001/03/30 17:56:22
@@ -64,7 +64,6 @@ $(DIRS:%=%-recursive): $(top_builddir)/s

 ifeq ($(MAKE_DLL), true)
 DLLOBJS := $(OBJS)
-DLLLIBS := -L/usr/local/lib -lcygipc -lcrypt -lcygwin -lkernel32

 postgres.def: $(DLLOBJS)
        $(DLLTOOL) --export-all --output-def $@ $(DLLOBJS)

Fred Yankowski <fcy@acm.org> writes:
> While working on a patch that allows postmaster to run as an NT service
> with Cygwin, I found minor problem in backend/Makefile.  In order to add
> an additional library to the final link/build of postgres.exe, I changed
> the DLLLIBS variable in makefiles/Makefile.win.  But this change had no
> effect until I removed the line in backend/Makefile that also sets
> DLLLIB, hiding the value from Makefile.win.

Hm.  Clearly we should have only one definition, so I've applied your
patch.  But if the one in backend/Makefile was the controlling one,
shouldn't we make the entry in makefiles/Makefile.win match what was
in Makefile?  Seems like that is the value that's been tested so far.

            regards, tom lane

Fred Yankowski <fred@ontosys.com> writes:
> You may be right, if I understand you, that the exact value of DLLLIBS
> that was in backend/Makefile should be used in makefiles/Makefile.win.
> Should I submit a new patch to that effect?

A patch is hardly necessary, we just need consensus about which value of
DLLLIBS is the (more) correct one.

            regards, tom lane

Tom,

On Sun, Apr 01, 2001 at 11:23:54PM -0400, Tom Lane wrote:
> Fred Yankowski <fcy@acm.org> writes:
> > While working on a patch that allows postmaster to run as an NT service
> > with Cygwin, I found minor problem in backend/Makefile.  In order to add
> > an additional library to the final link/build of postgres.exe, I changed
> > the DLLLIBS variable in makefiles/Makefile.win.  But this change had no
> > effect until I removed the line in backend/Makefile that also sets
> > DLLLIB, hiding the value from Makefile.win.
>
> Hm.  Clearly we should have only one definition, so I've applied your
> patch.  But if the one in backend/Makefile was the controlling one,
> shouldn't we make the entry in makefiles/Makefile.win match what was
> in Makefile?  Seems like that is the value that's been tested so far.

Fred and I had discussed this issue.  It was my suggestion that he post
a patch regarding this issue.  However after more reflection, I don't
think that we got it quite right.

I now think that backend/Makefile is meant to override the definition
of DLLLIBS in makefiles/Makefile.win so that the build of the backend
will not try to link with itself (i.e., -L$(top_builddir)/src/backend
-lpostgres).

However, the definition of DLLLIBS in backend/Makefile should not
completely overwrite the one from makefiles/Makefile.win.  Instead,
I think that the backend/Makefile definition should use the one
from makefiles/Makefile.win and remove the undesirable parts (i.e.,
-L$(top_builddir)/src/backend -lpostgres).

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Jason Tishler <Jason.Tishler@dothill.com> writes:
> I now think that backend/Makefile is meant to override the definition
> of DLLLIBS in makefiles/Makefile.win so that the build of the backend
> will not try to link with itself (i.e., -L$(top_builddir)/src/backend
> -lpostgres).

It seems like the real problem here is that DLLLIBS is being used for
two different purposes.  How about offering a patch that splits it into
two symbols, with no redefinition/overriding necessary?

I've also been wondering why we bother with DLLOBJS, which appears never
to be anything different from OBJS ...

            regards, tom lane

You may be right, if I understand you, that the exact value of DLLLIBS
that was in backend/Makefile should be used in makefiles/Makefile.win.
Should I submit a new patch to that effect?

On Sun, Apr 01, 2001 at 11:23:54PM -0400, Tom Lane wrote:
> Fred Yankowski <fcy@acm.org> writes:
> > While working on a patch that allows postmaster to run as an NT service
> > with Cygwin, I found minor problem in backend/Makefile.  In order to add
> > an additional library to the final link/build of postgres.exe, I changed
> > the DLLLIBS variable in makefiles/Makefile.win.  But this change had no
> > effect until I removed the line in backend/Makefile that also sets
> > DLLLIB, hiding the value from Makefile.win.
>
> Hm.  Clearly we should have only one definition, so I've applied your
> patch.  But if the one in backend/Makefile was the controlling one,
> shouldn't we make the entry in makefiles/Makefile.win match what was
> in Makefile?  Seems like that is the value that's been tested so far.

--
Fred Yankowski           fred@OntoSys.com      tel: +1.630.879.1312
Principal Consultant     www.OntoSys.com       fax: +1.630.879.1370
OntoSys, Inc             38W242 Deerpath Rd, Batavia, IL 60510, USA

Fred Yankowski <fred@ontosys.com> writes:
> I experimented with different ways to do this patch and discussed this
> with Jason Tischler, and I now conclude that the meaning of DLLLIBS in
> the two makefiles is different enough that it doesn't make sense to
> try to factor-out common information to share between them.

Well, in that case we DEFINITELY ought to replace them with two
differently-named symbols.  However, I'm pretty confused about
which is which and what gets used where.  Suggestions?

            regards, tom lane

Fred Yankowski <fred@ontosys.com> writes:
> I don't understand the several uses of DLLLIBS as well as I'd like,
> but here's what I think is going on.

> In makefiles/Makefile.win, DLLLIBS lists the libraries needed to build
> the various DLLs associated with the interfaces/* and pl/plpgsql
> directories.  As such it includes '-L$(top_builddir)/src/backend -lpostgres'
> as well as several Cygwin utility libraries.

Hmm.  It seems a little bit weird (no, a lot weird) to be referencing
-lpostgres for the client-side interface library builds.  I can see that
the PL-language DLLs might need to reference -lpostgres during their
links, but I've got severe doubts that this is a good idea anyplace
else.

My thought is that Makefile.win ought to have

DLLLIBS=-lcygipc -lcygwin -lcrypt -lkernel32

which is what will be used by Makefile.shlib to build the interfaces
libraries.  Then in the makefiles for the PL directories we should write

DLLLIBS:= -L$(top_builddir)/src/backend -lpostgres $(DLLLIBS)

so that -lpostgres is added just for the links of those shlibs.  And
finally backend/Makefile could use the Makefile.win definition as-is.

Comments?  If this seems plausible, could you test it?

BTW, I see that the prior version of backend/Makefile actually defined
DLLLIBS as

DLLLIBS := -L/usr/local/lib -lcygipc -lcrypt -lcygwin -lkernel32

as compared to what Makefile.win offers (shorn of the backend):

DLLLIBS=-lcygipc -lcygwin -lcrypt -lkernel32

Any comments on whether -L/usr/local/lib is appropriate here or not?
What about the ordering of these libraries, does that matter?

            regards, tom lane

Tom,

On Tue, Apr 03, 2001 at 06:03:45PM -0400, Tom Lane wrote:
> Hmm.  It seems a little bit weird (no, a lot weird) to be referencing
> -lpostgres for the client-side interface library builds.  I can see that
> the PL-language DLLs might need to reference -lpostgres during their
> links, but I've got severe doubts that this is a good idea anyplace
> else.

You are correct.  I just verified by using MS's dumpbin that none
of the above DLLs except for plpgsql.dll actually import any symbols
from libpostgres.a.  Hence, linking the client-side interface libraries
with libpostgres.a is superfluous.

However, you missed a few regression test related DLLs.  See below for
details.

> My thought is that Makefile.win ought to have
>
> DLLLIBS=-lcygipc -lcygwin -lcrypt -lkernel32
>
> which is what will be used by Makefile.shlib to build the interfaces
> libraries.

I agreed with the above except that the -lcygwin and -lkernel32 are
unnecessary.  Cygwin's ld always supplies these options.  This is true
for both the Net and b20.1 releases.

> Then in the makefiles for the PL directories we should write
>
> DLLLIBS:= -L$(top_builddir)/src/backend -lpostgres $(DLLLIBS)
>
> so that -lpostgres is added just for the links of those shlibs.

I agree with the above too.

> And finally backend/Makefile could use the Makefile.win definition as-is.

I agree with the above too.  Additionally, when Fred is ready with his
NT service patch, then he can add the following to backend/Makefile:

    DLLLIBS:= $(DLLLIBS) -lpopt

> Comments?  If this seems plausible, could you test it?

Yes, I have tested this on the Net release.  Is someone else willing to
test on b20.1 -- I'm not confident of my b20.1 setup.

When I executed make check, I noticed that the following DLLs are also
dependent on libpostgres.a:

    contrib/spi/autoinc.dll
    contrib/spi/refint.dll
    src/test/regress/regress.dll

So I used the same change that you proposed for plpgsql.dll for these too.

Did we miss any others?

With the attached patch applied to my CVS working directory, I was able
to build PostgreSQL without errors.

When I run make check, I am getting consistent failures with horology.
I don't believe that this is related to this build related patch.  See
attached for details.  Am I correct?

> BTW, I see that the prior version of backend/Makefile actually defined
> DLLLIBS as
>
> DLLLIBS := -L/usr/local/lib -lcygipc -lcrypt -lcygwin -lkernel32
>
> as compared to what Makefile.win offers (shorn of the backend):
>
> DLLLIBS=-lcygipc -lcygwin -lcrypt -lkernel32
>
> Any comments on whether -L/usr/local/lib is appropriate here or not?

We have already determined that the -L/usr/local/lib option is
superfluous for both the Cygwin Net and b20.1 releases.  See the
following for details:

    http://www.postgresql.org/mhonarc/pgsql-ports/2001-03/msg00049.html

> What about the ordering of these libraries, does that matter?

I believe that -lcygipc and -lcrypt are independent of each other and
that the -lcygwin and -lkernel32 are unnecessary, so no the order doesn't
matter.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Jason Tishler <Jason.Tishler@dothill.com> writes:
> On Tue, Apr 03, 2001 at 06:03:45PM -0400, Tom Lane wrote:
>> Hmm.  It seems a little bit weird (no, a lot weird) to be referencing
>> -lpostgres for the client-side interface library builds.  I can see that
>> the PL-language DLLs might need to reference -lpostgres during their
>> links, but I've got severe doubts that this is a good idea anyplace
>> else.

> You are correct.  I just verified by using MS's dumpbin that none
> of the above DLLs except for plpgsql.dll actually import any symbols
> from libpostgres.a.  Hence, linking the client-side interface libraries
> with libpostgres.a is superfluous.
> However, you missed a few regression test related DLLs.  See below for
> details.

Good point; those DLLs link into the backend.

Maybe Makefile.win should define FE_DLLLIBS (for frontend libraries)
and BE_DLLLIBS (for backend libraries).  That would require any
particular Makefile that's building a DLL to select one or the other
to define DLLLIBS as, before it could include Makefile.shlib.  Is that
approach good for clarity, or too much of a pain-in-the-neck?

            regards, tom lane

Tom,

On Wed, Apr 04, 2001 at 12:44:08AM -0400, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > On Tue, Apr 03, 2001 at 06:03:45PM -0400, Tom Lane wrote:
> >> Hmm.  It seems a little bit weird (no, a lot weird) to be referencing
> >> -lpostgres for the client-side interface library builds.  I can see that
> >> the PL-language DLLs might need to reference -lpostgres during their
> >> links, but I've got severe doubts that this is a good idea anyplace
> >> else.
>
> > You are correct.  I just verified by using MS's dumpbin that none
> > of the above DLLs except for plpgsql.dll actually import any symbols
> > from libpostgres.a.  Hence, linking the client-side interface libraries
> > with libpostgres.a is superfluous.
> > However, you missed a few regression test related DLLs.  See below for
> > details.
>
> Good point; those DLLs link into the backend.
>
> Maybe Makefile.win should define FE_DLLLIBS (for frontend libraries)
> and BE_DLLLIBS (for backend libraries).  That would require any
> particular Makefile that's building a DLL to select one or the other
> to define DLLLIBS as, before it could include Makefile.shlib.  Is that
> approach good for clarity, or too much of a pain-in-the-neck?

I'm not sure that I like the above proposal, because:

    1. I'm not crazy about the name FE_DLLLIBS -- I don't think that it
       accurately represents its semantics.  (I presume that you meant
       that FE_DLLLIBS = -lcygipc -lcrypt; otherwise, I'm not grokking
       your proposal.)
    2. Then all DLL makefiles would have to define DLLLIBS, instead of
       just the exceptions.

But, to be fair, I also don't completely like my current solution because
of the

    -L$(top_builddir)/src/backend -lpostgres

littered in more that one makefile.

What about leaving DLLLIBS as defined (in Makefile.win) in my patch, but
also defining BE_DLLLIBS as follows:

    BE_DLLLIBS = -L$(top_builddir)/src/backend -lpostgres

Then one can use this definition is in src/pl/plpgsql/src/Makefile as
follows:

    DLLLIBS:= $(BE_DLLLIBS) $(DLLLIBS)

The above would also be used where ever else was necessary (e.g.,
regress.dll).

How does this proposal sound?

Thanks,
Jason
--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Jason Tishler <Jason.Tishler@dothill.com> writes:
>     1. I'm not crazy about the name FE_DLLLIBS -- I don't think that it
>        accurately represents its semantics.

Okay.

>        (I presume that you meant
>        that FE_DLLLIBS = -lcygipc -lcrypt; otherwise, I'm not grokking
>        your proposal.)

Right, given that the other two need not be mentioned.  What about
-L/usr/local/lib, did that make any sense to include or not?

> What about leaving DLLLIBS as defined (in Makefile.win) in my patch, but
> also defining BE_DLLLIBS as follows:
>     BE_DLLLIBS = -L$(top_builddir)/src/backend -lpostgres
> Then one can use this definition is in src/pl/plpgsql/src/Makefile as
> follows:
>     DLLLIBS:= $(BE_DLLLIBS) $(DLLLIBS)
> The above would also be used where ever else was necessary (e.g.,
> regress.dll).

> How does this proposal sound?

Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
to get this done ASAP.  Do you have time to work up and test a patch
today, or shall I risk making untested changes myself?

            regards, tom lane

Tom,

On Wed, Apr 04, 2001 at 03:01:17PM -0400, Tom Lane wrote:
> Right, given that the other two need not be mentioned.  What about
> -L/usr/local/lib, did that make any sense to include or not?

Hmm...I guess that my emails are too long so that you lose interest while
reading them... :,)

Anyway, see the following:

    On Tue, Apr 03, 2001 at 10:38:51PM -0400, Jason Tishler wrote:
    > On Tue, Apr 03, 2001 at 06:03:45PM -0400, Tom Lane wrote:
    > > BTW, I see that the prior version of backend/Makefile actually defined
    > > DLLLIBS as
    > >
    > > DLLLIBS := -L/usr/local/lib -lcygipc -lcrypt -lcygwin -lkernel32
    > >
    > > as compared to what Makefile.win offers (shorn of the backend):
    > >
    > > DLLLIBS=-lcygipc -lcygwin -lcrypt -lkernel32
    > >
    > > Any comments on whether -L/usr/local/lib is appropriate here or not?
    >
    > We have already determined that the -L/usr/local/lib option is
    > superfluous for both the Cygwin Net and b20.1 releases.  See the
    > following for details:
    >
    >     http://www.postgresql.org/mhonarc/pgsql-ports/2001-03/msg00049.html

> > How does this proposal sound?
>
> Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
> to get this done ASAP.  Do you have time to work up and test a patch
> today, or shall I risk making untested changes myself?

I'm doing a build right now.  The patch should follow shortly.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Tom Lane writes:

> Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
> to get this done ASAP.  Do you have time to work up and test a patch
> today, or shall I risk making untested changes myself?

AFAIU, this doesn't actually fix a problem, it only cleans up a
long-standing oddity.  So why the rush?

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/

Tom,

On Wed, Apr 04, 2001 at 03:27:43PM -0400, Jason Tishler wrote:
> On Wed, Apr 04, 2001 at 03:01:17PM -0400, Tom Lane wrote:
> > Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
> > to get this done ASAP.  Do you have time to work up and test a patch
> > today, or shall I risk making untested changes myself?
>
> I'm doing a build right now.  The patch should follow shortly.

See attached for the updated patch.

Built fine but I'm still getting horology regression test failures.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Peter,

On Wed, Apr 04, 2001 at 09:51:53PM +0200, Peter Eisentraut wrote:
> Tom Lane writes:
>
> > Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
> > to get this done ASAP.  Do you have time to work up and test a patch
> > today, or shall I risk making untested changes myself?
>
> AFAIU, this doesn't actually fix a problem, it only cleans up a
> long-standing oddity.  So why the rush?

I don't know if this is a problem or not, but Tom had already
checked in a different patch that removed the DLLLIBS definition
from src/backend/Makefile.  Hence, when the backend is built it will
try to link with itself.  This is due to the DLLLIBS definition in
src/makefiles/Makefile.win no longer being overriden by the one in
src/backend/Makefile.  May be this is a no-op.  I don't know.

Nevertheless, I have generated, tested, and submitted my updated patch.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Works for me.  Given that we'd like to package RC3 tonight, it'd be nice
>> to get this done ASAP.  Do you have time to work up and test a patch
>> today, or shall I risk making untested changes myself?

> AFAIU, this doesn't actually fix a problem, it only cleans up a
> long-standing oddity.  So why the rush?

I think that current CVS sources are actually broken on Cygwin, because
I removed the DLLLIBS setting in backend/Makefile a day or two ago,
and it now appears that was wrong.  I have to either back that out or
install a proper fix, and I'd sooner install a fix.

            regards, tom lane

Jason Tishler writes:

> See attached for the updated patch.

It seems we're now down to

DLLLIBS= -lcygipc -lcrypt

but I think it can be removed completely.

We have three uses of this variable:

1) Makefile.win, for building extension modules.  No module in existance
needs -lcygipc, and if one needs -lcrypt it can put it into SHLIB_LINK.

2) Makefile.shlib, see 1)

3) -lcrypt already comes in via LIBS (like all the other libraries),
-lcygipc can do the same, or be hard-coded in place.

The idea behind naming a variable DLLLIBS seems to have been "libraries
necessary to create a DLL".  But ISTM that there are no such libraries,
strictly speaking.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/

Peter Eisentraut <peter_e@gmx.net> writes:
> It seems we're now down to

> DLLLIBS= -lcygipc -lcrypt

> but I think it can be removed completely.

> We have three uses of this variable:

> 1) Makefile.win, for building extension modules.  No module in existance
> needs -lcygipc, and if one needs -lcrypt it can put it into SHLIB_LINK.

> 2) Makefile.shlib, see 1)

> 3) -lcrypt already comes in via LIBS (like all the other libraries),
> -lcygipc can do the same, or be hard-coded in place.


But you forgot about -lpostgres for the backend libraries (plpgsql etc).

I suppose that we might consider adding these libraries to LIBS not
DLLLIBS, but that's cleanup work that I'd rather not try to do under
time pressure (especially since I can't test it here).

            regards, tom lane

Jason Tishler <Jason.Tishler@dothill.com> writes:
>> I suppose that we might consider adding these libraries to LIBS not
>> DLLLIBS, but that's cleanup work that I'd rather not try to do under
>> time pressure (especially since I can't test it here).

> Can we commit my updated patch for 7.1RC3?  I promise to help clean up
> the above for RC4 or final.  Is this acceptable?

I just did commit your patch (plus some editorializing of my own, which
you should test ;-)).

I'd say that getting rid of DLLLIBS entirely is cleanup that we can
leave for the 7.2 cycle, if we do it at all.  It does seem like merging
DLLLIBS into LIBS might be a good idea, but perhaps we'd best see what
emerges from Fred's NT service work before we take that step.

Current CVS also has a fix for the busted horology test, btw.

            regards, tom lane

Tom,
Peter,

On Wed, Apr 04, 2001 at 05:04:40PM -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > It seems we're now down to
>
> > DLLLIBS= -lcygipc -lcrypt
>
> > but I think it can be removed completely.
>
> > We have three uses of this variable:
>
> > 1) Makefile.win, for building extension modules.  No module in existance
> > needs -lcygipc, and if one needs -lcrypt it can put it into SHLIB_LINK.
>
> > 2) Makefile.shlib, see 1)
>
> > 3) -lcrypt already comes in via LIBS (like all the other libraries),
> > -lcygipc can do the same, or be hard-coded in place.
>
>
> But you forgot about -lpostgres for the backend libraries (plpgsql etc).
>
> I suppose that we might consider adding these libraries to LIBS not
> DLLLIBS, but that's cleanup work that I'd rather not try to do under
> time pressure (especially since I can't test it here).

Can we commit my updated patch for 7.1RC3?  I promise to help clean up
the above for RC4 or final.  Is this acceptable?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Tom,

On Wed, Apr 04, 2001 at 05:19:38PM -0400, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> >> I suppose that we might consider adding these libraries to LIBS not
> >> DLLLIBS, but that's cleanup work that I'd rather not try to do under
> >> time pressure (especially since I can't test it here).
>
> > Can we commit my updated patch for 7.1RC3?  I promise to help clean up
> > the above for RC4 or final.  Is this acceptable?
>
> I just did commit your patch (plus some editorializing of my own, which
> you should test ;-)).

I just tested the latest CVS and all looks good -- even horology.

> I'd say that getting rid of DLLLIBS entirely is cleanup that we can
> leave for the 7.2 cycle, if we do it at all.  It does seem like merging
> DLLLIBS into LIBS might be a good idea, but perhaps we'd best see what
> emerges from Fred's NT service work before we take that step.

Sounds like a good idea to me.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Tom,

I experimented with different ways to do this patch and discussed this
with Jason Tischler, and I now conclude that the meaning of DLLLIBS in
the two makefiles is different enough that it doesn't make sense to
try to factor-out common information to share between them.

So I wish to withdraw my proposed patch entirely, leaving the two
definitions of DLLLIBS as is.  My apologies for the confusion.

On Mon, Apr 02, 2001 at 01:48:23PM -0400, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > I now think that backend/Makefile is meant to override the definition
> > of DLLLIBS in makefiles/Makefile.win so that the build of the backend
> > will not try to link with itself (i.e., -L$(top_builddir)/src/backend
> > -lpostgres).
>
> It seems like the real problem here is that DLLLIBS is being used for
> two different purposes.  How about offering a patch that splits it into
> two symbols, with no redefinition/overriding necessary?

--
Fred Yankowski           fred@OntoSys.com      tel: +1.630.879.1312
Principal Consultant     www.OntoSys.com       fax: +1.630.879.1370
OntoSys, Inc             38W242 Deerpath Rd, Batavia, IL 60510, USA

I don't understand the several uses of DLLLIBS as well as I'd like,
but here's what I think is going on.

In makefiles/Makefile.win, DLLLIBS lists the libraries needed to build
the various DLLs associated with the interfaces/* and pl/plpgsql
directories.  As such it includes '-L$(top_builddir)/src/backend -lpostgres'
as well as several Cygwin utility libraries.

In backend/Makefile, DLLLIBS lists the libraries needed to build
postgres.exe.  This does not include '-lpostgres' since postgres.exe
is built by linking exactly the backend object files that also go into
libpostgres.a.  (I don't understand this organization.)

So, for example, in my work to add a wrapper layer to postgres.exe to
run as an NT service, I intend to use a function provided by libpopt,
referencing it from code in the backend/main/ directory.  It worked
(AFAICT) to add '-lpopt' to DLLLIBS in Makefile.win and remove the
DLLLIBS definition in backend/Makefile so that it picks up that value
from Makefile.win, because the appearance of '-lpostgres' is
extraneous but harmless when building postgres.exe.  Similarly,
'-lpopt' is not needed when building the DLLs in interfaces/* and
pl/plpgsql, but causes no problems.  It might make sense to use
'-lpopt' only when building postgres.exe.

I could live with the above scheme, but I admit that I don't
understand the postgres build structure well enough to be sure that
having these extra library names appearing in the final link steps
won't cause problems.


On Tue, Apr 03, 2001 at 04:04:54PM -0400, Tom Lane wrote:
> Fred Yankowski <fred@ontosys.com> writes:
> > I experimented with different ways to do this patch and discussed this
> > with Jason Tischler, and I now conclude that the meaning of DLLLIBS in
> > the two makefiles is different enough that it doesn't make sense to
> > try to factor-out common information to share between them.
>
> Well, in that case we DEFINITELY ought to replace them with two
> differently-named symbols.  However, I'm pretty confused about
> which is which and what gets used where.  Suggestions?

--
Fred Yankowski           fred@OntoSys.com      tel: +1.630.879.1312
Principal Consultant     www.OntoSys.com       fax: +1.630.879.1370
OntoSys, Inc             38W242 Deerpath Rd, Batavia, IL 60510, USA