Re: Postgresql for cygwin - 3rd

Il 3/6/2013 11:46 PM, Andrew Dunstan ha scritto:
>
> Excellent. Will test it out soon.
>
> cheers
>
> andrew
>

Andrew,
please find attached a full patch for cygwin relative to 9.3beta1 :

- DLLTOLL/DLLWRAP are not used anymore, replaced
by gcc also for postgres.exe (*)
- DLL versioning is added

Check failures:
- prepared_xacts is still freezing
The cygwin failure you highlighted was solved,
so it should be something else
- attached the 2 regressions diffs
tsearch ... FAILED
without_oid ... FAILED
The second one seems a new one, not sure cygwin specific

Regards
Marco

*) http://www.cygwin.com/ml/cygwin/2013-03/msg00032.html

On Mon, Jun 10, 2013 at 11:08:36AM +0200, marco atzeri wrote:
> Il 3/6/2013 11:46 PM, Andrew Dunstan ha scritto:
> >
> >Excellent. Will test it out soon.
> >
> >cheers
> >
> >andrew
> >
>
> Andrew,
> please find attached a full patch for cygwin relative to 9.3beta1 :
>
> - DLLTOLL/DLLWRAP are not used anymore, replaced
> by gcc also for postgres.exe (*)
> - DLL versioning is added
>
> Check failures:
> - prepared_xacts is still freezing
> The cygwin failure you highlighted was solved,
> so it should be something else
> - attached the 2 regressions diffs
> tsearch ... FAILED
> without_oid ... FAILED
> The second one seems a new one, not sure cygwin specific

Andrew, should this configuration/code patch be applied to 9.4?

http://www.postgresql.org/message-id/51B59794.3000500@gmail.com

I think we would have to make Cygwin-specific regression output to
handle the regression failures, but frankly I am not even sure if they
are right.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Andrew, should this configuration/code patch be applied to 9.4?

> http://www.postgresql.org/message-id/51B59794.3000500@gmail.com

> I think we would have to make Cygwin-specific regression output to
> handle the regression failures, but frankly I am not even sure if they
> are right.

Those regression failures certainly say there is something broken in
the submitter's build, so this needs to be taken with a grain of salt.
I'm not qualified to evaluate the proposed changes, but I wonder why
they're needed given that we have successful cygwin builds in the
buildfarm.

regards, tom lane

On Thu, Jan 23, 2014 at 10:48:01PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Andrew, should this configuration/code patch be applied to 9.4?
>
> > http://www.postgresql.org/message-id/51B59794.3000500@gmail.com
>
> > I think we would have to make Cygwin-specific regression output to
> > handle the regression failures, but frankly I am not even sure if they
> > are right.
>
> Those regression failures certainly say there is something broken in
> the submitter's build, so this needs to be taken with a grain of salt.
> I'm not qualified to evaluate the proposed changes, but I wonder why
> they're needed given that we have successful cygwin builds in the
> buildfarm.

Yes, that confuses me too. Unless we get more details, we should ignore
the patches. Thanks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 01/23/2014 10:50 PM, Bruce Momjian wrote:
> On Thu, Jan 23, 2014 at 10:48:01PM -0500, Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>>> Andrew, should this configuration/code patch be applied to 9.4?
>>> http://www.postgresql.org/message-id/51B59794.3000500@gmail.com
>>> I think we would have to make Cygwin-specific regression output to
>>> handle the regression failures, but frankly I am not even sure if they
>>> are right.
>> Those regression failures certainly say there is something broken in
>> the submitter's build, so this needs to be taken with a grain of salt.
>> I'm not qualified to evaluate the proposed changes, but I wonder why
>> they're needed given that we have successful cygwin builds in the
>> buildfarm.
> Yes, that confuses me too. Unless we get more details, we should ignore
> the patches. Thanks.
>

AFAICT the regression is in Cygwin. The buildfarm passes because it's
using an oldish Cygwin release, 1.7.7 rather than the current 1.7.27. I
have brought the regression the athe attention of the Cygwin people in
the past, but without response.

The build system changes have slipped off my radar, unfortunately. Not
sure when I can get to them.

cheers

andrew

On 24/01/2014 05:28, Andrew Dunstan wrote:
>
> On 01/23/2014 10:50 PM, Bruce Momjian wrote:
>> On Thu, Jan 23, 2014 at 10:48:01PM -0500, Tom Lane wrote:
>>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>>>> Andrew, should this configuration/code patch be applied to 9.4?
>>>> http://www.postgresql.org/message-id/51B59794.3000500@gmail.com
>>>> I think we would have to make Cygwin-specific regression output to
>>>> handle the regression failures, but frankly I am not even sure if they
>>>> are right.
>>> Those regression failures certainly say there is something broken in
>>> the submitter's build, so this needs to be taken with a grain of salt.
>>> I'm not qualified to evaluate the proposed changes, but I wonder why
>>> they're needed given that we have successful cygwin builds in the
>>> buildfarm.
>> Yes, that confuses me too. Unless we get more details, we should ignore
>> the patches. Thanks.

Andrew,
nice to see, the question rises again.
I dropped from the pgsql-hackers(at)postgresql(dot)org some time ago,
as no one was following the issue; I just rejoined.

As explained here:
http://cygwin.com/ml/cygwin/2013-03/msg00217.html
http://cygwin.com/ml/cygwin/2013-03/msg00050.html

1) Using DLLTOOL/DLLWRAP

"postgresql dll's allocation table are partially wrong,
so they fail at load after a rebase."

the build farm can not test this rebase failure, as it will happen after
installation at any rebase.
DLLTOOL/DLLWRAP usage is "really" deprecated on cygwin as it produces
damaged binaries that

2) I am the currently package mantainer for cygwin
last I packged was postgresql-9.2.4
9.3.2 is on my TODO list

> AFAICT the regression is in Cygwin. The buildfarm passes because it's
> using an oldish Cygwin release, 1.7.7 rather than the current 1.7.27. I
> have brought the regression the athe attention of the Cygwin people in
> the past, but without response.

which issue ?
During my package tests I have only two issues:

tsearch ... FAILED
and
test: prepared_xacts
must be skipped as it never completes

> The build system changes have slipped off my radar, unfortunately. Not
> sure when I can get to them.

Regars
Marco

On 01/24/2014 01:20 AM, Marco Atzeri wrote:
>
>> AFAICT the regression is in Cygwin. The buildfarm passes because it's
>> using an oldish Cygwin release, 1.7.7 rather than the current 1.7.27. I
>> have brought the regression the athe attention of the Cygwin people in
>> the past, but without response.
>
> which issue ?
> During my package tests I have only two issues:
>
> tsearch ... FAILED
> and
> test: prepared_xacts
> must be skipped as it never completes
>
>

Those two issues need to be fixed. And yes, they are regressions from my
Cygwin 1.7.7 setup where they pass consistently, just about every day.
See <http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=brolga&br=HEAD>

You don't get to choose which regression tests you're going to pass and
which you're not. Disabling the tests or providing nonsensical results
files are unacceptable. This is a Cygwin behavior issue and needs to be
fixed.

cheers

andrew

On 24/01/2014 12:56, Andrew Dunstan wrote:
>
> On 01/24/2014 01:20 AM, Marco Atzeri wrote:
>>
>>> AFAICT the regression is in Cygwin. The buildfarm passes because it's
>>> using an oldish Cygwin release, 1.7.7 rather than the current 1.7.27. I
>>> have brought the regression the athe attention of the Cygwin people in
>>> the past, but without response.
>>
>> which issue ?
>> During my package tests I have only two issues:
>>
>> tsearch ... FAILED
>> and
>> test: prepared_xacts
>> must be skipped as it never completes
>>
>>
>
> Those two issues need to be fixed. And yes, they are regressions from my
> Cygwin 1.7.7 setup where they pass consistently, just about every day.
> See <http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=brolga&br=HEAD>

1.7.7 is 3.5 years hold.
In the mean time we added 64 bit and moved to gcc-4.8.x

> You don't get to choose which regression tests you're going to pass and
> which you're not. Disabling the tests or providing nonsensical results
> files are unacceptable. This is a Cygwin behavior issue and needs to be
> fixed.

Distributing broken binary that crashes after standard rebase, it is
also not acceptable and it is also worst.
Your farm is not testing this case, I presume.

Until I took over there was NO recent cygwin package at all in
the distribution, so do not complain that my binary is not perfect,
I already know, but for me almost working is better than NOT available
or BROKEN.

> cheers
>
> andrew

I am available to work on tests regression, but I am not a postgresql
expert so do not expect all the job from my side.

Regards
Marco

On 01/24/2014 07:50 AM, Marco Atzeri wrote:
>
>> Those two issues need to be fixed. And yes, they are regressions from my
>> Cygwin 1.7.7 setup where they pass consistently, just about every day.
>> See
>> <http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=brolga&br=HEAD>
>
> 1.7.7 is 3.5 years hold.
> In the mean time we added 64 bit and moved to gcc-4.8.x

No doubt, but that doesn't mean that extra things failing is OK.

>
>> You don't get to choose which regression tests you're going to pass and
>> which you're not. Disabling the tests or providing nonsensical results
>> files are unacceptable. This is a Cygwin behavior issue and needs to be
>> fixed.
>
> Distributing broken binary that crashes after standard rebase, it is
> also not acceptable and it is also worst.
> Your farm is not testing this case, I presume.

Quite so. But this is not a case of either/or.

I have now tested the central part of the proposed changes on both old
and new Cygwin installations, and they appear to work.

I'm going to commit them and backpatch back to 9.0, which is where we
currently have buildfarm coverage (and 8.4 will be at EOL in a few
months anyway). That will take care of your rebase issue.

That leaves several issues to be handled:

* LDAP libraries - the way you have proposed surely isn't right. What
we want is something more like this in the Makefile.global.in:
ifeq ($(PORTNAME), cygwin)
libpq_pgport += $(LDAP_LIBS_FE)
endif
* isolation tests fail with an indefinite hang on newer Cygwin
* prepared_xacts test fails with an indefinite hang on newer Cygwin if
run in parallel with other tests
* tsearch tests fail on non-C locale (or at least on en_US.utf8). It
turns out this is actually an old bug, and can be reproduced on my
old Cygwin instance. I wonder if it's caused by faulty locale files?

Really, for a properly working port all these need to be fixed.

>
>
> I am available to work on tests regression, but I am not a postgresql
> expert so do not expect all the job from my side.

We can help you some, but very few people in the community run Cygwin,
and my time to spend on it is fairly limited.

cheers

andrew

On 25/01/2014 19:23, Andrew Dunstan wrote:
>
> On 01/24/2014 07:50 AM, Marco Atzeri wrote:
>>
>>> Those two issues need to be fixed. And yes, they are regressions from my
>>> Cygwin 1.7.7 setup where they pass consistently, just about every day.
>>> See
>>> <http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=brolga&br=HEAD>
>>
>> 1.7.7 is 3.5 years hold.
>> In the mean time we added 64 bit and moved to gcc-4.8.x
>
> No doubt, but that doesn't mean that extra things failing is OK.

Andrew,
I never wrote that.

>>
>>> You don't get to choose which regression tests you're going to pass and
>>> which you're not. Disabling the tests or providing nonsensical results
>>> files are unacceptable. This is a Cygwin behavior issue and needs to be
>>> fixed.

some indication where to look for in the code will help.

>> Distributing broken binary that crashes after standard rebase, it is
>> also not acceptable and it is also worst.
>> Your farm is not testing this case, I presume.
>
> Quite so. But this is not a case of either/or.

No, but I spent a lot of time on understanding that DLLTOLL/DLLWRAP
produce buggy binaries, and that was a CRITICAL failure.

> I have now tested the central part of the proposed changes on both old
> and new Cygwin installations, and they appear to work.
>
> I'm going to commit them and backpatch back to 9.0, which is where we
> currently have buildfarm coverage (and 8.4 will be at EOL in a few
> months anyway). That will take care of your rebase issue.
>
> That leaves several issues to be handled:
>
> * LDAP libraries - the way you have proposed surely isn't right. What
> we want is something more like this in the Makefile.global.in:
> ifeq ($(PORTNAME), cygwin)
> libpq_pgport += $(LDAP_LIBS_FE)
> endif

I will test in this way. I have no preferance on
the implemented solution.

> * isolation tests fail with an indefinite hang on newer Cygwin
> * prepared_xacts test fails with an indefinite hang on newer Cygwin if
> run in parallel with other tests

It hangs also stand alone. I guess some race or deadlock issue.

> * tsearch tests fail on non-C locale (or at least on en_US.utf8). It
> turns out this is actually an old bug, and can be reproduced on my
> old Cygwin instance. I wonder if it's caused by faulty locale files?

Tested tsearch with
"export LANG=C" works
"export LANG=C.utf8" fails
"export LANG=it_IT" works
"export LANG=it_IT.utf8" fails

faulty locale or wrong assumption on utf8 implementation ?

> Really, for a properly working port all these need to be fixed.

No objection. Step by step

>> I am available to work on tests regression, but I am not a postgresql
>> expert so do not expect all the job from my side.
>
>
> We can help you some, but very few people in the community run Cygwin,
> and my time to spend on it is fairly limited.

For the time being, I can run tests and builds.

> cheers
>
> andrew

cheers
Marco

On 25/01/2014 22:42, Marco Atzeri wrote:
> On 25/01/2014 19:23, Andrew Dunstan wrote:
>>
>> On 01/24/2014 07:50 AM, Marco Atzeri wrote:

>>
>> * LDAP libraries - the way you have proposed surely isn't right. What
>> we want is something more like this in the Makefile.global.in:
>> ifeq ($(PORTNAME), cygwin)
>> libpq_pgport += $(LDAP_LIBS_FE)
>> endif
>
> I will test in this way. I have no preferance on
> the implemented solution.
>

your proposal builds fine.
----------------------------------------------------------------
--- origsrc/postgresql-9.3.2/src/Makefile.global.in 2013-12-02
21:57:48.000000000 +0100
+++ src/postgresql-9.3.2/src/Makefile.global.in 2014-01-25
22:46:36.484816700 +0100
@@ -508,6 +508,11 @@ ifeq ($(PORTNAME),win32)
LIBS += -lws2_32 -lshfolder
endif

+# missing for link on cygwin ?
+ifeq ($(PORTNAME),cygwin)
+libpq_pgport += $(LDAP_LIBS_FE)
+endif
+
# Not really standard libc functions, used by the backend.
----------------------------------------------------------------

Of course no difference on test results, as expected

Attached full patch as I am currently testing on 9.3.2

Regards
Marco

On 01/25/2014 01:23 PM, Andrew Dunstan wrote:
>
>
> I have now tested the central part of the proposed changes on both old
> and new Cygwin installations, and they appear to work.
>
> I'm going to commit them and backpatch back to 9.0, which is where we
> currently have buildfarm coverage (and 8.4 will be at EOL in a few
> months anyway). That will take care of your rebase issue.

That part is done. Reliance on dllwrap is a thing of the past.

>
> That leaves several issues to be handled:
>
> * LDAP libraries - the way you have proposed surely isn't right. What
> we want is something more like this in the Makefile.global.in:
> ifeq ($(PORTNAME), cygwin)
> libpq_pgport += $(LDAP_LIBS_FE)
> endif

Unless someone comes up with a better answer than this I'm going to
commit it too.

> * isolation tests fail with an indefinite hang on newer Cygwin
> * prepared_xacts test fails with an indefinite hang on newer Cygwin if
> run in parallel with other tests
> * tsearch tests fail on non-C locale (or at least on en_US.utf8). It
> turns out this is actually an old bug, and can be reproduced on my
> old Cygwin instance. I wonder if it's caused by faulty locale files?
>

And these are where we need help, especially from the Cygwin community.
The fact that things that work on older Cygwins now fail is annoying.

cheers

andrew

On 01/02/2014 22:57, Andrew Dunstan wrote:
>
> On 01/25/2014 01:23 PM, Andrew Dunstan wrote:
>>
>>

>> * isolation tests fail with an indefinite hang on newer Cygwin
>> * prepared_xacts test fails with an indefinite hang on newer Cygwin if
>> run in parallel with other tests

>> * tsearch tests fail on non-C locale (or at least on en_US.utf8). It
>> turns out this is actually an old bug, and can be reproduced on my
>> old Cygwin instance. I wonder if it's caused by faulty locale files?

Andrew,
is it possible the tsearch test never worked on en_US.utf8
but only on C locale ?

See my finding on
http://www.postgresql.org/message-id/52ED5627.4070005@gmail.com

>
> And these are where we need help, especially from the Cygwin community.
> The fact that things that work on older Cygwins now fail is annoying.
>
> cheers
>
> andrew

Marco

On 02/01/2014 05:12 PM, Marco Atzeri wrote:
>
>
> On 01/02/2014 22:57, Andrew Dunstan wrote:
>>
>> On 01/25/2014 01:23 PM, Andrew Dunstan wrote:
>>>
>>>
>
>
>>> * isolation tests fail with an indefinite hang on newer Cygwin
>>> * prepared_xacts test fails with an indefinite hang on newer Cygwin if
>>> run in parallel with other tests
>
>>> * tsearch tests fail on non-C locale (or at least on en_US.utf8). It
>>> turns out this is actually an old bug, and can be reproduced on my
>>> old Cygwin instance. I wonder if it's caused by faulty locale files?
>
> Andrew,
> is it possible the tsearch test never worked on en_US.utf8
> but only on C locale ?

Yes, that's more or less what I said, I thought.

Maybe we need to test this on other Windows systems in non-C encodings.
Let's make sure it's only a Cygwin problem.

I'll work on that. You should try to concentrate on the thinks like
prepared_xacts and isolation_test that we know don't work ONLY on Cygwin.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 02/01/2014 05:12 PM, Marco Atzeri wrote:
>> is it possible the tsearch test never worked on en_US.utf8
>> but only on C locale ?

> Yes, that's more or less what I said, I thought.

> Maybe we need to test this on other Windows systems in non-C encodings.
> Let's make sure it's only a Cygwin problem.

> I'll work on that. You should try to concentrate on the thinks like
> prepared_xacts and isolation_test that we know don't work ONLY on Cygwin.

Please test the patch I just posted in the pgsql-bugs thread. It looks
to me like there are bugs in both the C and non-C locale cases, but only
the latter case would be exercised by our regression tests, since we
don't use any non-ASCII characters in the tests.

regards, tom lane

On 02/01/2014 05:46 PM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On 02/01/2014 05:12 PM, Marco Atzeri wrote:
>>> is it possible the tsearch test never worked on en_US.utf8
>>> but only on C locale ?
>> Yes, that's more or less what I said, I thought.
>> Maybe we need to test this on other Windows systems in non-C encodings.
>> Let's make sure it's only a Cygwin problem.
>> I'll work on that. You should try to concentrate on the thinks like
>> prepared_xacts and isolation_test that we know don't work ONLY on Cygwin.
> Please test the patch I just posted in the pgsql-bugs thread. It looks
> to me like there are bugs in both the C and non-C locale cases, but only
> the latter case would be exercised by our regression tests, since we
> don't use any non-ASCII characters in the tests.
>
>

This is commit 082c0dfa140b5799bc7eb574d68610dcfaa619ba and friends, right?

If so, it appears to have done the trick. brolga is now happy running
tsearch with en_US.utf8:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=brolga&dt=2014-02-03%2011%3A26%3A10&stg=install-check-en_US.utf8>

Cool.

cheers

andrew