patch for minor Win32 makefile bug

Greetings,

While working on a patch that allows postmaster to run as an NT service

Sounds great! How far are you?

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.

Michael Moehle

On Sun, Apr 01, 2001 at 09:17:46PM +0200, Michael Moehle wrote:

While working on a patch that allows postmaster to run as an NT service

Sounds great! How far are you?

It's working pretty well on my NT4SP6 machine.

Alpha version 0.2 of this patch is available at
<http://greatbridge.org/project/pgntservice/projdisplay.php&gt;. This
code has only been tested by me, AFAIK. Use at your own risk. See
the news items on the website, and the README file included in the
archive.

--
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 <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

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:

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

-----Ursprungliche Nachricht-----
Von: pgsql-ports-owner@postgresql.org
[mailto:pgsql-ports-owner@postgresql.org]Im Auftrag von Fred Yankowski
Gesendet: Montag, 2. April 2001 00:06
An: Michael Moehle
Cc: pgsql-ports@postgresql.org
Betreff: [PORTS] Re: patch for minor Win32 makefile bug

On Sun, Apr 01, 2001 at 09:17:46PM +0200, Michael Moehle wrote:

While working on a patch that allows postmaster to run as an NT service

Sounds great! How far are you?

It's working pretty well on my NT4SP6 machine.

Alpha version 0.2 of this patch is available at
<http://greatbridge.org/project/pgntservice/projdisplay.php&gt;. This
code has only been tested by me, AFAIK. Use at your own risk. See
the news items on the website, and the README file included in the
archive.

Witch version of Postgres must I juse ?
Michael

--
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

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

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

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

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

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

Fred Yankowski 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.

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.)

ISTM that $(DLLLIBS) in backend/Makefile should be replaced by

$(LDFLAGS) -lcygipc -lcygwin -lkernel32 $(LIBS)

(Presuming that these three libraries are actually needed. Why isn't
every program linked against -lcygwin?)

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

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

Peter,

On Wed, Apr 04, 2001 at 12:16:52AM +0200, Peter Eisentraut wrote:

$(LDFLAGS) -lcygipc -lcygwin -lkernel32 $(LIBS)

(Presuming that these three libraries are actually needed. Why isn't
every program linked against -lcygwin?)

Every Cygwin program *is* implicitly linked against -lcygwin, -lkernel32,
and others too.

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

Tom,

On Tue, Apr 03, 2001 at 10:38:51PM -0400, Jason Tishler wrote:

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?

Please comment on the above.

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