Re: Enderbury Island disappeared from timezone database

[Victor Wagner]

В Fri, 27 Oct 2023 14:00:38 -0400
Tom Lane  пишет:

> This means that if we substitute Kanton for Enderbury, things
> will work fine against tzdata 2021b or later, but will fail in
> the reverse way against older tzdata sets.  Do we want to
> bet that everybody in the world has up-to-date tzdata installed?

You are right. When nightly builds came, they showed problems with
Pacific/Kanton in
Debian 10, 11 and Ubuntu 20.04 (we do not more test ubuntu 18.04 as 5
year support period is ended). 

I haven't applied 'fix' to rpm-based disitrubutions, because none of
them as I'm aware of split tzdata into two packages.

> I guess the contract for using --with-system-tzdata is that it's
> up to you to maintain that, but still I don't like the odds.
> 
> The alternative I'm wondering about is whether to just summarily
> remove the PHOT entry from timezonesets/Default.  It's a made-up
> zone abbreviation in the first place, and per the above NEWS entry,
> there's only a couple dozen people in the world who might even
> be candidates to consider using it.  It seems highly likely that
> nobody would care if we just dropped it from the Default list.
> (We could keep the Pacific.txt entry, although re-pointing it
> to Pacific/Kanton seems advisable.)
> 
>   regards, tom lane
> 
> 



-- 
   Victor Wagner 

Re: Enderbury Island disappeared from timezone database

[Victor Wagner]

В Fri, 27 Oct 2023 11:17:03 -0400
Tom Lane  пишет:

> Victor Wagner  writes:
> > Tom Lane  пишет:  
> >> Did Ubuntu decide to remove *all* backzone links from their data?
> >> Or just that one?  Either way, I think they're going to get a
> >> tsunami of pushback pretty quickly.  People like their obsolete
> >> zone names.  
> 
> > They split tzdata packages into tzdata and tzdata-legacy (just for
> > those who like obsolete zone names), and into latter one gone 121
> > links, not counting "right" subdirectory.  
> 
> Fun.  I bet that breaks more than just Pacific/Enderbury.
> Can you try changing that entry to Pacific/Kanton, and repeat?

I did. No more problems. 

I.e. I've invoked

sed -i 's/Enderburry/Kanton/' $prefix/share/timezonesets/* 

and rerun tests. No failures.

It seems that Pacific/Enerberry was only one obsolete name which got
its way into abbreviations list.


> And then check the non-Default timezonesets lists too?

Enderbury аppears in two files in the timezonesets - Default
and Pacific.txt.

-- 
   Victor Wagner 

Re: Enderbury Island disappeared from timezone database

[Victor Wagner]

В Fri, 27 Oct 2023 10:25:57 -0400
Tom Lane  пишет:

> Victor Wagner  writes:
> > I've encountered following problem compiling PostgreSQL 15.4 with
> > just released Ubuntu 23.10.  
> 
> > I'm compiling postgres with --with-system-tzdata and then regression
> > test sysviews fails with following diff:
> > +ERROR:  time zone "Pacific/Enderbury" not recognized
> > +DETAIL:  This time zone name appears in the configuration file for
> > time zone abbreviation "phot".  
> 
> Hmph.  Pacific/Enderbury is still defined according to tzdata 2023c,
> which is the latest release:
> 
> $ grep Enderbury src/timezone/data/tzdata.zi
> L Pacific/Kanton Pacific/Enderbury
>
> Did Ubuntu decide to remove *all* backzone links from their data?
> Or just that one?  Either way, I think they're going to get a tsunami
> of pushback pretty quickly.  People like their obsolete zone names.

They split tzdata packages into tzdata and tzdata-legacy (just for
those who like obsolete zone names), and into latter one gone 121 links,
not counting "right" subdirectory. It is actually Debian unstable
feature that got impored into ubuntu. But my
test machines with debian testing do not use --with-system-tzdata, so
I've not noticed this earlier.

It has following entry in changelog:

tzdata (2023c-8) unstable; urgency=medium

  * Update Dutch debconf translation.
Thanks to Frans Spiesschaert 
(Closes: #1041278)
  * Ship only timezones in tzdata that follow the current rules of
geographical
region (continent or ocean) and city name. Move all legacy timezone
symlinks
(that are upgraded during package update) to tzdata-legacy. This
includes
dropping the special handling for US/* timezones. (Closes: #1040997)

 -- Benjamin Drung   Mon, 07 Aug 2023 15:02:14 +0200

I.e. they move obsolete timezones into separate package just for people
who like them.

Description of that package ends with:

 This package also contains legacy timezone symlinks that are not
 following
 the current rule of using the geographical region (continent or ocean)
 and
 city name.
 .
 You do not need this package if you are unsure.

Really I think that if at least some distirbutions don't like this
names, it is better to have postgres pass its regression tests without
these names as well as with them.







-- 
   Victor Wagner 

Enderbury Island disappeared from timezone database

[Victor Wagner]

Collegues,

I've encountered following problem compiling PostgreSQL 15.4 with just
released Ubuntu 23.10.

I'm compiling postgres with --with-system-tzdata and then regression
test sysviews fails with following diff:


--- /home/test/pg-tests/postgresql-15.4/src/test/regress/expected/sysviews.out  
2023-10-26 19:06:02.0 +
+++ /home/test/pg-tests/postgresql-15.4/src/test/regress/results/sysviews.out   
2023-10-27 07:10:22.214698986 +
@@ -147,23 +147,14 @@
 (1 row)
 
 select count(distinct utc_offset) >= 24 as ok from pg_timezone_abbrevs;
- ok 
-
- t
-(1 row)
-
+ERROR:  time zone "Pacific/Enderbury" not recognized
+DETAIL:  This time zone name appears in the configuration file for time zone 
abbreviation "phot".


with more such errors follows.

Investigation shows, that this timezone was long ago declared
deprecated, and eventually disappeared from tzdata package in Ubuntu
even as symlink to Pasific/Kanton (which is equivalent).

But this timezone present in src/timezone/tznames/Default, so this
error message is appears any time one access pg_timezone_abbrevs
regardless of Pacific region is included in results or not.

May be, Enderbury should be replaced by Kanton in
src/timezone/tznames/Default and src/timezone/tznames/Pacific.txt?

-- 
       Victor Wagner 

Re: OpenSSL deprectation warnings in Postgres 10-13

[Victor Wagner]

В Wed, 6 Apr 2022 15:01:42 +0200
Peter Eisentraut  пишет:

> On 06.04.22 11:55, Victor Wagner wrote:
> > And found out that versions 10-13 produce a lot of warnings about
> > deprecated OpenSSL functions.
> > 
> > I've found discussion about  this problem
> > 
> > https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se
> > 
> > and commit 4d3db13621be64fbac2faf which fixes problem in the 14
> > version and above. (it seems to me that declaring wish to use
> > outdated API is ugly workaround, but anyway it works)
> > 
> > But this commit seems not to be backpatched to versions 13-10.  
> 
> As was discussed in that thread, we don't actually have precise 
> knowledge of what OpenSSL versions we support in versions before
> PG13. So backpatching this could alter or break things that worked
> before.
> 
> It seems easier to just disable deprecation warnings if you don't
> want to see them.
> 

As far as my understanding goes, "just disable deprication warning" it
is what commit 4d3db13621be64fbac2faf does.

Real fix would be rewrite corresponding parts of code so they wouldn't
use deprecated API. (as it was done with sha2 code in PostgreSQL 14.
which doesn't use openssl version of sha2 at all).

It is quite simple to rewrite digest code to use generic EVP API, but a
bit more complicated with DH parameters, because if EVP API appeared in
0.9.x series and we can rely on it in any version of OpenSSL,
non-deprecated replacement for PEM_read_DHparams is 3.0 only. So we
have to keep 1.1.x compatible version for a while until all major
distribution would change to 3.x. 

Really I think that PostgreSQL does something wrong with TLS if it need
to use deprecated functions in be-secure-openssl.c. But it might be
that just OpenSSL team was to slow to catch up new things in the
transport security world, so application (including PostgreSQL) have to
work around it and use too low-level functions, which are now
deprecated.

-- 
   Victor Wagner 

OpenSSL deprectation warnings in Postgres 10-13

[Victor Wagner]

Collegues!

I've tried to build all supported versions of PostgresSQL in the Ubuntu
22.04 which is soon to be released.

And found out that versions 10-13 produce a lot of warnings about
deprecated OpenSSL functions.

I've found discussion about  this problem 


https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se

and commit 4d3db13621be64fbac2faf which fixes problem in the 14
version and above. (it seems to me that declaring wish to use outdated
API is ugly workaround, but anyway it works)

But this commit seems not to be backpatched to versions 13-10.
In 2020 it probably haven't be a problem,  as OpenSSL 3.0.0 was in
alpha stage, but this month first mainline Linux distribution which uses
openssl 3.0.x is going to be released.

It seems that we need to backpatch this commit into all supported
versions of PostgreSQL, because there are more and more distributions
switched to OpenSSL 3.0.x. Apart from Ubuntu I can see at least RH 9
beta and experimental packages for Debian which probably would be ready
in time for Debian 12.

-- 
   Victor Wagner 

Re: Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

В Wed, 6 May 2020 21:23:57 -0300
Ranier Vilela  пишет:

> 
> The perl is:
> Win32 strawberry-perl 5.30.1.1
> 

This perl would have problems when compiling PL/Perl (see my letter
about week ago), but it have no problems running various build scripts
for Postgres. I'm using it with MSVisualStudio 2019 and only unexpected
thing I've encountered is that it comes with its own  patch.exe, which
doesn't like unix-style end-of-lines in patches (but OK with them in
patched files).


> regards,
> Ranier VIlela
-- 

Re: Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

В Thu, 7 May 2020 09:14:33 +0900
Michael Paquier  пишет:

> On Wed, May 06, 2020 at 03:58:15PM -0300, Ranier Vilela wrote:
> > Hacking pgbison.pl, to print PATH, shows that the path inside
> > pgbison.pl, returned to being the original, without the addition of
> > c:\perl\bin;c:\bin. my $out = $ENV{PATH};
> > print "Path after system call=$out\n";
> > Path after system
> > call=...C:\Users\ranier\AppData\Local\Microsoft\WindowsApps;;
> > The final part lacks: c:\perl\bin;c:\bin
> > 
> > Now I need to find out why the path is being reset, within the perl
> > scripts.  
> 
> FWIW, we have a buildfarm animal called drongo that runs with VS 2019,
> that uses Python, and that is now happy.  One of my own machines uses
> VS 2019 as well and I have yet to see what you are describing here.
> Perhaps that's related to a difference in the version of perl you are
> using and the version of that any others?


I doubt so. I have different machines with perl from 5.22 to 5.30 but
none of tham exibits such weird behavoir. 

Perhaps problem is that Ranier calls vcvars64.bat from the menu, and
then calls msbuild such way that is becames unrelated process.

Obvoisly buildfarm animal doesn't use menu and then starts build
process from same CMD.EXE process, that it called vcvarsall.but into.

It is same in all OSes - Windows, *nix and even MS-DOS - there is no
way to change environment of parent process. You can change environment
of current process (and if this process is command interpreter you can
do so by sourcing script into it. In windows this misleadingly called
'CALL', but it executes commands from command file in the current
shell, not in subshell) you can pass enivronment to the child
processes. But you can never affect environment of the parent or
sibling process.

The only exception is - if you know that some process would at startup
read environment vars from some file or registry, you can modify this
source in unrelated process.

So, if you want perl in path of msbuild, started from Visual Studio,
you should either first set path in CMD.EXE, then type command to start
Studio from this very  command window, or set path via control panel
dialog (which modified registry). Later is what I usially do on machines
wher I compile postgres.




--



> --
> Michael

Re: Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

В Wed, 6 May 2020 10:21:41 -0300
Ranier Vilela  пишет:

> Em qua., 6 de mai. de 2020 às 09:53, Michael Paquier
>  escreveu:
> 
> > On Tue, May 05, 2020 at 10:16:23AM +0300, Victor Wagner wrote:  
> > > I agree, it is better.  
> >
> > Thanks, applied and back-patched down to 9.5.  Now for the second
> > problem of this thread..
> >  
> Sorry, no clue yet.
> I hacked the perl sources, to hardcoded perl, bison and flex with
> path.It works like this.

Perl has "magic" variable $^X which expands to full path to perl
executable, I wonder why build.pl doesn't  use it to invoke secondary 
perl scripts.

--

Re: Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

В Tue, 5 May 2020 15:45:48 +0900
Michael Paquier  пишет:

> On Fri, May 01, 2020 at 12:48:17PM +0300, Victor Wagner wrote:
> > Maybe. But probably original author of this code was afraid of using
> > too long chain of ->{} in the string substitution. 
> > 
> > So, I left this style n place.
> > 
> > Nonetheless, using qq wouldn't save us from doubling backslashes.  
> 
> Looking at this part in more details, I find the attached much more
> readable.  I have been able to test it on my own Windows environment

I agree, it is better.


> and the problem gets fixed (I have reproduced the original problem as
> well).
--

Postgresql Windows build and modern perl (>=5.28)

[Victor Wagner]

Collegues,

Postgresql embeded perl, plperl contain code long time ago copied
from POSIX.xs file in the perl distribution.
It is function setlocale_perl, which does some allocation of
perl-specific locale data using functions(or macros) new_ctype,
new_collate and new_numeric.

This is used only for WIN32, because as comment in the code said:

   /*
 * The perl library on startup does horrible things like call
 * setlocale(LC_ALL,""). We have protected against that on most platforms
 * by setting the environment appropriately. However, on Windows,
 * setlocale() does not consult the environment, so we need to save the
 * existing locale settings before perl has a chance to mangle them and
 * restore them after its dirty deeds are done.
 *
 * MSDN ref:
 * http://msdn.microsoft.com/library/en-us/vclib/html/_crt_locale.asp
 *
 * It appears that we only need to do this on interpreter startup, and
 * subsequent calls to the interpreter don't mess with the locale
 * settings.
 *
 * We restore them using setlocale_perl(), defined below, so that Perl
 * doesn't have a different idea of the locale from Postgres.
 *
 */


This worked up to perl 5.26. But in perl 5.28 these macros and
corresponding functions became strictly private. However public
function Perl_setlocale appeared in libperl, which from the quick
glance to the code does the same thing as setlocale_perl in plperl code.

Attached patch makes use of this function if PERL_VERSION >= 28. 
It makes plperl compile with ActiveStatePerl 5.28 and StrawberryPerl
5.30.2.1.

However, I'm not sure that I've choose correct approach. May be perl
just no more does horrible things with locale at startup?

-- 

diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 2db13d30308..d2787b50beb 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -300,8 +300,12 @@ static OP  *pp_require_safe(pTHX);
 static void activate_interpreter(plperl_interp_desc *interp_desc);
 
 #ifdef WIN32
+#if PERL_VERSION >= 28
+#define setlocale_perl(a,b)  Perl_setlocale(a,b)
+#else
 static char *setlocale_perl(int category, char *locale);
 #endif
+#endif
 
 /*
  * Decrement the refcount of the given SV within the active Perl interpreter
@@ -4159,6 +4163,7 @@ plperl_inline_callback(void *arg)
  * (needed because of the calls to new_*())
  */
 #ifdef WIN32
+#if PERL_VERSION < 28
 static char *
 setlocale_perl(int category, char *locale)
 {
@@ -4226,5 +4231,6 @@ setlocale_perl(int category, char *locale)
 
 	return RETVAL;
 }
+#endif 			/* PERL_VERSION < 28 */
 
 #endif			/* WIN32 */
diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h
index 3748158a86d..0044983ed1b 100644
--- a/src/pl/plperl/plperl.h
+++ b/src/pl/plperl/plperl.h
@@ -51,11 +51,11 @@
  */
 #ifdef _MSC_VER
 #define __inline__ inline
+#define __builtin_expect(a, b)  (a == b)
 #ifdef isnan
 #undef isnan
 #endif
 #endif
-
 /*
  * Regarding bool, both PostgreSQL and Perl might use stdbool.h or not,
  * depending on configuration.  If both agree, things are relatively harmless.

Re: Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

В Fri, 1 May 2020 17:52:15 +0900
Michael Paquier  пишет:

> On Thu, Apr 30, 2020 at 03:06:08PM +0300, Victor Wagner wrote:
> > Fix is very simple, see attach.
> > 
> > Patch is made against REL_12_STABLE, but probably applicable to
> > other versions as well.  
> 
> Indeed, thanks.
> 
> > my $pythonprog = "import sys;print(sys.prefix);"
> >   .
> > "print(str(sys.version_info[0])+str(sys.version_info[1]))"; my
> > $prefixcmd =
> > - $solution->{options}->{python} . "\\python -c
> > \"$pythonprog\"";
> > +   '"' . $solution->{options}->{python} . "\\python\"
> > -c \"$pythonprog\""; my $pyout = `$prefixcmd`;
> > die "Could not query for python version!\n" if $?;
> > my ($pyprefix, $pyver) = split(/\r?\n/, $pyout);  
> 
> This reminds me of ad7595b.  Wouldn't it be better to use qq() here?

Maybe. But probably original author of this code was afraid of using
too long chain of ->{} in the string substitution. 

So, I left this style n place.

Nonetheless, using qq wouldn't save us from doubling backslashes.

--

Postgres Windows build system doesn't work with python installed in Program Files

[Victor Wagner]

Collegues,

Accidently I've come over minor bug in the Mkvcbuild.pm.
It happens, that it doesn't tolerate spaces in the $config->{python}
path, because it want to call python in order to find out version,
prefix and so on, and doesn't properly quote command.

Fix is very simple, see attach.

Patch is made against REL_12_STABLE, but probably applicable to other
versions as well.
--
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 99f39caa522..6c95b7068d8 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -497,7 +497,7 @@ sub mkvcbuild
 		my $pythonprog = "import sys;print(sys.prefix);"
 		  . "print(str(sys.version_info[0])+str(sys.version_info[1]))";
 		my $prefixcmd =
-		  $solution->{options}->{python} . "\\python -c \"$pythonprog\"";
+		'"' . $solution->{options}->{python} . "\\python\" -c \"$pythonprog\"";
 		my $pyout = `$prefixcmd`;
 		die "Could not query for python version!\n" if $?;
 		my ($pyprefix, $pyver) = split(/\r?\n/, $pyout);

Re: make check crashes on POWER8 machine

[Victor Wagner]

В Fri, 13 Mar 2020 10:56:15 -0400
Tom Lane  пишет:

> Victor Wagner  writes:
> > Justin Pryzby  wrote:  
> >> On Fri, Mar 13, 2020 at 10:29:13AM +0300, Victor Wagner wrote:  
> >>> I've encountered a problem with Postgres on PowerPC machine.  
> 
> >> Is it related to
> >> https://www.postgresql.org/message-id/20032.1570808731%40sss.pgh.pa.us
> >> https://bugzilla.kernel.org/show_bug.cgi?id=205183  
> 
> > I don't think so. At least I cannot see any signal handler-related
> > stuff in the trace, but see lots of calls to stored procedure
> > executor instead.  
> 
> Read the whole thread.  We fixed the issue with recursion in the
> postmaster (9abb2bfc0); but the intermittent failure in
> infinite_recurse is exactly the same as what we've been seeing for a
> long time in the buildfarm, and there is zero doubt that it's that
> kernel bug.

I've tried to cherry-pick commit 9abb2bfc8 into REL_12_STABLE and rerun
make check in loop. Oops, on 543 run it segfaults with same symptoms
as before.

Here is link to new core and logs

https://drive.google.com/file/d/1oF-0fKHKvFn6FaJ3u-v36p9W0EBAY9nb/view?usp=sharing

I'll try to do this simple test (run make check repeatedly) with 
master. There is some time until end of weekend when this machine is
non needed by anyone else, so I have time to couple of thousands runs.





-- 
   Victor Wagner 

Re: make check crashes on POWER8 machine

[Victor Wagner]

On Fri, 13 Mar 2020 07:43:59 -0500
Justin Pryzby  wrote:

> On Fri, Mar 13, 2020 at 10:29:13AM +0300, Victor Wagner wrote:
> > Hi,
> > 
> > I've encountered a problem with Postgres on PowerPC machine.
> > Sometimes  
> 
> Is it related to
> https://www.postgresql.org/message-id/20032.1570808731%40sss.pgh.pa.us
> https://bugzilla.kernel.org/show_bug.cgi?id=205183

I don't think so. At least I cannot see any signal handler-related stuff
in the trace, but see lots of calls to stored procedure executor
instead.

Although several different stack traces show completely different parts
of code when signal SIGSEGV arrives, which may point to asynchronous
nature of the problem.

Unfortunately I've not kept all the cores I've seen.

It rather looks like that in some rare circumstances Postgres is unable
to properly determine end of stack condition.
-- 

pg_basebackup from REL_12_STABLE hands on solaris/sparch

[Victor Wagner]

Collegues,

I've encountered following problem on some old Sparc64 machine running
solaris 10:

When I compile postgresql 12 with --enable-tap-tests and run make check
in src/bin, test src/bin/pg_basebackup/t/010_pg_basebackup.pl
hangs and hangs infinitely. 

I've tried to attach gdb to the hanging process, but it attempt to 
do backtrace in it, gdb reports that stack is corrupt

Attaching to program 
`/home/vitus/postgrespro/src/bin/pg_basebackup/pg_basebackup', process 1467
[New process 1467]
Retry #1:
Retry #2:
Retry #3:
Retry #4:
Reading symbols from /usr/lib/sparcv9/ld.so.1...(no debugging symbols 
found)...done.
Loaded symbols for /usr/lib/sparcv9/ld.so.1
---Type  to continue, or q  to quit---
0xff2cca38 in ?? ()
(gdb) bt
#0  0xff2cca38 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)


When afterword I kill hanged process with
kill -SEGV to get core, I get following stack trace from core file:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x7d5d94ac in ___sigtimedwait () from /lib/64/libc.so.1
(gdb) bt
#0  0x7d5d94ac in ___sigtimedwait () from /lib/64/libc.so.1
#1  0x7d5c8c8c in __sigtimedwait () from /lib/64/libc.so.1
#2  0x7d5c0628 in __posix_sigwait () from /lib/64/libc.so.1
#3  0x7f3362b4 in pq_reset_sigpipe (osigset=0x7fffeb1c,
sigpipe_pending=false, got_epipe=true) at fe-secure.c:529 #4
0x7f336084 in pqsecure_raw_write (conn=0x100135e90,
ptr=0x10013a370, len=5) at fe-secure.c:399 #5  0x7f335e28 in
pqsecure_write (conn=0x100135e90, ptr=0x10013a370, len=5) at
fe-secure.c:316 #6  0x7f326c54 in pqSendSome (conn=0x100135e90,
len=5) at fe-misc.c:876 #7  0x7f326e84 in pqFlush
(conn=0x100135e90) at fe-misc.c:1004 #8  0x7f316584 in
sendTerminateConn (conn=0x100135e90) at fe-connect.c:4031 #9
0x7f3165a4 in closePGconn (conn=0x100135e90) at
fe-connect.c:4049 #10 0x7f31663c in PQfinish (conn=0x100135e90)
at fe-connect.c:4083 #11 0x0001bc64 in BaseBackup () at
pg_basebackup.c:2136 #12 0x0001d7ec in main (argc=4,
argv=0x7808) at pg_basebackup.c:2547

This happens on random tests in this test file with probablity about
1/10, but because there is more than 100 tests, hanging has 100%
probablity. But other two test files in src/bin/pg_basebackup directory
don't hang.

As far as I can notice, there is only two machines with Solaris in
pgbuildfarm now, and neither of them has any records of running
REL_12_STABLE branch. (not to mention that both don't run tap tests).

-- 

Re: PostgreSQL12 and older versions of OpenSSL

[Victor Wagner]

On Tue, 24 Sep 2019 18:49:17 +0900
Michael Paquier  wrote:

> On Tue, Sep 24, 2019 at 10:18:59AM +0300, Victor Wagner wrote:
> > PostgreSQL 12 documentation states, that minimum required version of
> > OpenSSL is 0.9.8. However, I was unable to сompile current
> > PGPRO_12_STABLE with OpenSSL 0.9.8j (from SLES 11sp4).
> 
> I can reproduce that with REL_12_STABLE and the top of
> OpenSSL_0_9_8-stable fromx OpenSSL's git.
> 
> > Replacing all 
> > 
> > #ifdef TLS1_1_VERSION
> > 
> > with
> > 
> > #if defined(TLS1_1_VERSION) && TLS1_1_VERSION <= TLS_MAX_VERSION
> > 
> > and analogue for TLS1_2_VERSION fixes the problem.
> 
> That sounds like a plan.  
[skip] 
> > ...
> > (line 1290). In this case check for TLS1_1_VERSION <=
> > TLS_MAX_VERSION seems to be more self-explanatory, than check for
> > somewhat unrelated symbol SSL_OP_NO_TLSv1_1
> 
> That sounds right.  Victor, would you like to write a patch?

I'm attaching patch which uses solution mentioned above.
It seems that chedk for SSL_OP_NO_TLSvX_Y is redundant if 
we are checking for TLS_MAX_VERSION.
-- 
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index c97c811..e24d7de 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1287,19 +1287,19 @@ ssl_protocol_version_to_openssl(int v, const char *guc_name, int loglevel)
 		case PG_TLS1_VERSION:
 			return TLS1_VERSION;
 		case PG_TLS1_1_VERSION:
-#ifdef TLS1_1_VERSION
+#if defined(TLS1_1_VERSION) && TLS1_1_VERSION <= TLS_MAX_VERSION
 			return TLS1_1_VERSION;
 #else
 			break;
 #endif
 		case PG_TLS1_2_VERSION:
-#ifdef TLS1_2_VERSION
+#if defined(TLS1_2_VERSION) &&  TLS1_2_VERSION <= TLS_MAX_VERSION
 			return TLS1_2_VERSION;
 #else
 			break;
 #endif
 		case PG_TLS1_3_VERSION:
-#ifdef TLS1_3_VERSION
+#if defined(TLS1_3_VERSION)  &&  TLS1_2_VERSION <= TLS_MAX_VERSION
 			return TLS1_3_VERSION;
 #else
 			break;
@@ -1335,11 +1335,11 @@ SSL_CTX_set_min_proto_version(SSL_CTX *ctx, int version)
 
 	if (version > TLS1_VERSION)
 		ssl_options |= SSL_OP_NO_TLSv1;
-#ifdef TLS1_1_VERSION
+#if defined(TLS1_1_VERSION) && TLS1_1_VERSION <= TLS_MAX_VERSION
 	if (version > TLS1_1_VERSION)
 		ssl_options |= SSL_OP_NO_TLSv1_1;
 #endif
-#ifdef TLS1_2_VERSION
+#if defined(TLS1_2_VERSION) && TLS1_2_VERSION <= TLS_MAX_VERSION
 	if (version > TLS1_2_VERSION)
 		ssl_options |= SSL_OP_NO_TLSv1_2;
 #endif
@@ -1356,11 +1356,11 @@ SSL_CTX_set_max_proto_version(SSL_CTX *ctx, int version)
 
 	AssertArg(version != 0);
 
-#ifdef TLS1_1_VERSION
+#if defined(TLS1_1_VERSION) && TLS1_1_VERSION <= TLS_MAX_VERSION
 	if (version < TLS1_1_VERSION)
 		ssl_options |= SSL_OP_NO_TLSv1_1;
 #endif
-#ifdef TLS1_2_VERSION
+#if defined(TLS1_2_VERSION) && TLS1_2_VERSION <= TLS_MAX_VERSION
 	if (version < TLS1_2_VERSION)
 		ssl_options |= SSL_OP_NO_TLSv1_2;
 #endif


pgpTlATx598L1.pgp
Description: OpenPGP digital signature

PostgreSQL12 and older versions of OpenSSL

[Victor Wagner]

Dear hackers,

PostgreSQL 12 documentation states, that minimum required version of
OpenSSL is 0.9.8. However, I was unable to сompile current
PGPRO_12_STABLE with OpenSSL 0.9.8j (from SLES 11sp4).


-fno-strict-aliasing -fwrapv -g -O2 -I../../../src/include  -D_GNU_SOURCE 
-I/usr/include/libxml2   -c -o be-secure-openssl.o be-secure-openssl.c
be-secure-openssl.c: In function ‘SSL_CTX_set_min_proto_version’:
be-secure-openssl.c:1340: error: ‘SSL_OP_NO_TLSv1_1’ undeclared (first use in 
this function)
be-secure-openssl.c:1340: error: (Each undeclared identifier is reported only 
once
be-secure-openssl.c:1340: error: for each function it appears in.)
be-secure-openssl.c:1344: error: ‘SSL_OP_NO_TLSv1_2’ undeclared (first use in 
this function)
be-secure-openssl.c: In function ‘SSL_CTX_set_max_proto_version’:
be-secure-openssl.c:1361: error: ‘SSL_OP_NO_TLSv1_1’ undeclared (first use in 
this function)
be-secure-openssl.c:1365: error: ‘SSL_OP_NO_TLSv1_2’ undeclared (first use in 
this function)
make: *** [be-secure-openssl.o] Error 1


Problem is that some code in src/backend/libpq/be-secure-openssl.c
assumes that if preprocessor symbols TLS1_1_VERSION and TLS1_2_VERSION
are defined in the openssl headers, corresponding versions of TLS are
supported by the library.

It is not so. Here is exempt from tls1.h header file from the openssl
0.9.8j

#define TLS1_VERSION0x0301
#define TLS1_1_VERSION  0x0302
#define TLS1_2_VERSION  0x0303
/* TLS 1.1 and 1.2 are not supported by this version of OpenSSL, so
 * TLS_MAX_VERSION indicates TLS 1.0 regardless of the above
 * definitions. (s23_clnt.c and s23_srvr.c have an OPENSSL_assert()
 * check that would catch the error if TLS_MAX_VERSION was too low.)
 */
#define TLS_MAX_VERSION TLS1_VERSION

Replacing all 

#ifdef TLS1_1_VERSION

with

#if defined(TLS1_1_VERSION) && TLS1_1_VERSION <= TLS_MAX_VERSION

and analogue for TLS1_2_VERSION fixes the problem.

Really, problem is that symbol SSL_OP_NO_TLSv1_1 (and 1_2 accordingly)
might be undefined even if TLS1_1_VERSION defined. 

Replacing 

#ifdef TLS1_1_VERSION

with 

#ifdef SSL_OP_NO_TLSv1_1 

seems to be correct solution for two of three #ifdef TLS1_1_VERSION
statements in be-secure-openssl.c, because this symbol is used inside
#ifdef block.

But there is third (first from start of file) one.
...
case PG_TLS1_1_VERSION:
#ifdef TLS1_1_VERSION
return TLS1_1_VERSION;
#else
break;
#endif
...
(line 1290). In this case check for TLS1_1_VERSION <= TLS_MAX_VERSION
seems to be more self-explanatory, than check for somewhat unrelated 
symbol SSL_OP_NO_TLSv1_1
 

-- 

Re: Perl 5.26 and windows build system

[Victor Wagner]

On Wed, 17 Oct 2018 18:45:42 +0530
Sandeep Thakkar  wrote:


> > Simple adding
> >
> > use lib ".";
> >
> > to the beginning of these script solves problem.
> >

> >
> > We observed the same issue with Strawberry Perl 5.26. We use 5.24
> > to  
> execute the build.pl.

Of course. This is (mis)feature of upstream Perl 5.26 and above and it
is not limited to Windows. I've fought with it on various modern linux
distributions (Ubuntu, Debian, Fedora), where I want to run pgbuildfarm
scripts.

I know two ways to work around this:
1. Add use lib "." to the beginning of the script (as I've mentioned in
my first letter)
2. execute command
Set PERL5LIB=%WHERE_IS_YOUR_POSTGRES_UNPACKED%\src\tools\msvc
before running perl scripts.

And I think that first should be commited into postgres.

You tell you use Strawberry Perl 5.24 to build postgres?
1. Do you run tap tests with it?
2. Do you compile and use PL/Perl with it?
3. Do you use only 64-bit builds or both 64 and 32?

Perl 5.26 and windows build system

[Victor Wagner]

Colleagues,

Since Active State stopped to distribute perl 5.22, we decided to
upgrade installer builds to most use recent version available
(5.26.1.2601 now).

But upstream perl changes policy around this version and no longer
adds current directory to the module search path.

This doesn't break work of build.pl, which allready handles module
search path itself, but breaks install.pl, mkvcbuild.pl and
vcregress.pl, which 
expect to be started from src/tools/msvc.

Simple adding 

use lib ".";

to the beginning of these script solves problem.

BTW, have anyone experienced some success using Strawberry perl instead
of Active perl both for building postgres and as PL/Perl engine?

Active State seems to abandon support for 32-bit windows and strawberry
perl license allows redistribution.
-- 

Building 64-bit postgres with GSS support on windows

[Victor Wagner]

Colleagues,

I've encountered some problems trying to enable gss support in MSVC
build of Postgres (I've experemented with REL_10_STABLE branch, but
code in question seems to be same in all supported releases, including
master).

As it is recommended in the documentation, I've downloaded MIT Kerberos
from  http://web.mit.edu/Kerberos/dist/index.html

They distribute latest version (4.1) of Kerberos as .msi installer which
installs into C:\Program Files. (32-bit version probably would install
into C:\Program Files(x86), but I've not tried it yet).

Here comes first problem.

Project->AddLibrary unconditionally quotes paths with spaces.
But some contrib modules which depends on PL language, use
Mkvcbuild::AddTransformModule for generating dependences.
It gets list of (already quoted) librariy name from dependency project
and adds them to the current project. 

I haven't investigate whether adding kerberos, xml, icu etc libraries
three times to linker command line does any good, but it works. And
double quoting filename with quotes definitely breaks things.

Fix is quite simple:

diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm
index 9817b94..3749c17 100644
--- a/src/tools/msvc/Project.pm
+++ b/src/tools/msvc/Project.pm
@@ -126,7 +126,7 @@ sub AddLibrary
 {
my ($self, $lib, $dbgsuffix) = @_;

-   if ($lib =~ m/\s/)
+   if ($lib =~ m/\s/ && !$lib =~m/^\.*\$/)
{
$lib = '' . $lib . "";
}

I suppose this would help with any 3-rd party library installed into
directory with spaces in the names, not just Kerberos.


Second problem is that names of 32-bit libraries and library directories
are hard-coded into Solution.pm

$self->{options}->{gss} . '\lib\i386\krb5_32.lib'

And for 64-bit build
'\lib\amd64\krb5_64.lib' is needed (at least for this MIT Kerberos 4.1)

There is similar platform differences with other libraries, such as
ICU, and check if ($self->{platform} eq 'Win32') is used for them.
But there is no such thing for gss libraries.

And third problem is that Solution.pm expect includes in 
$self->{options}->{gss}.'\inc\krb5' but
for Kerberos 4.1 
$self->{options}->{gss}.'\include" is needed.

I haven't dig into computer archeology and havent checked when this
change occured, just check if directore '\include' exist and use it
if so.

Attached patch fixes these problems.

Regrards, Victor.
-- 
diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm
index 9817b94..3749c17 100644
--- a/src/tools/msvc/Project.pm
+++ b/src/tools/msvc/Project.pm
@@ -126,7 +126,7 @@ sub AddLibrary
 {
 	my ($self, $lib, $dbgsuffix) = @_;
 
-	if ($lib =~ m/\s/)
+	if ($lib =~ m/\s/ && !$lib =~/^\.*\$/)
 	{
 		$lib = '' . $lib . "";
 	}
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index ad75478..2adaafd 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -591,10 +591,22 @@ sub AddProject
 	}
 	if ($self->{options}->{gss})
 	{
-		$proj->AddIncludeDir($self->{options}->{gss} . '\inc\krb5');
-		$proj->AddLibrary($self->{options}->{gss} . '\lib\i386\krb5_32.lib');
-		$proj->AddLibrary($self->{options}->{gss} . '\lib\i386\comerr32.lib');
-		$proj->AddLibrary($self->{options}->{gss} . '\lib\i386\gssapi32.lib');
+		if ( -d $self->{options}->{gss}.'\include') {
+			$proj->AddIncludeDir($self->{options}->{gss} . '\include');
+		} else {
+			$proj->AddIncludeDir($self->{options}->{gss} . '\inc\krb5');
+		}
+		my ($gss_libdir,$suffix);
+		if ($self->{platform} eq 'Win32') {
+			$gss_libdir =  $self->{options}->{gss}.'\lib\i386';
+			$suffix='32';
+		} else {
+			$gss_libdir =  $self->{options}->{gss}.'\lib\amd64';
+			$suffix='64';
+		}
+		$proj->AddLibrary("$gss_libdir\\krb5_$suffix.lib");
+		$proj->AddLibrary("$gss_libdir\\comerr$suffix.lib");
+		$proj->AddLibrary("$gss_libdir\\gssapi$suffix.lib");
 	}
 	if ($self->{options}->{iconv})
 	{

Re: Bug fix for glibc broke freebsd build in REL_11_STABLE

[Victor Wagner]

On Tue, 4 Sep 2018 17:51:30 -0700
Andres Freund  wrote:


> #endif
> in an autoconf test, and have configure complain if that
> fails. Something roughly along the lines of
> "Compiling PostgreSQL with clang, on 32bit x86, requires SSE2
> support. Use -msse2 or use gcc."

Adding CFLAGS=-msse2 to configure environment solved my problem 
both on the upgraded virtual machine with clang 6.0.0 and on old one
with clang 3.8.0

Although I now don't have 32-bit build system with clang on Linux.
-- 

Re: Bug fix for glibc broke freebsd build in REL_11_STABLE

[Victor Wagner]

В Tue, 04 Sep 2018 15:48:24 -0400
Tom Lane  пишет:


> I tried to reproduce the problem, without success, on a few different
> FreeBSD images I had laying about:
> 
> FreeBSD 11.0/x86_64, clang version 3.8.0
>   (this confirms OP's report that x86_64 is OK)
> 
> FreeBSD 10.3/ppc, gcc 4.2.1
> 
> FreeBSD 12.0-CURRENT (from around mid-May)/arm64, clang version 6.0.0
> 
> (I was testing PG HEAD, not the 11 branch, but I don't see a reason
> to think that'd make a difference.)

Alas, it does. First thing I've done after discovering this bug, it is
to look if it exists in master. And master passes this test on the same
machine where problem was discovered.


> I also looked for evidence of a bug of this sort in the clang
> bugzilla. I couldn't find anything, but it's possible that "isinf"
> isn't what I should have searched on.
> 
> Anyway, my estimation is that this is a compiler bug that's been
> repaired, and it probably isn't widespread enough to justify our
> inserting some klugy workaround.

It doesn't look so, as bug persists after I've upgraded system to
current 11.2-RELEASE with clang 6.0.0.



-- 
   Victor Wagner 

Re: Bug fix for glibc broke freebsd build in REL_11_STABLE

[Victor Wagner]

В Tue, 4 Sep 2018 11:06:56 -0700
Thomas Munro  пишет:

> On Tue, Sep 4, 2018 at 9:39 AM Andrew Gierth
>  wrote:
> >  
> > >>>>> "Andres" == Andres Freund  writes:  
> >  
> >  >> However, this commit broke float8 test on 32-bit FreeBSD 11 with
> >  >> clang 3.8.0 compiler. Regressions.diff follows:  
> >  
> >  Andres> Does this happen with a newer clang version too?  
> >
> > float8 test (and all other tests) passes for me on clang 3.9.1 on
> > fbsd11 on 32-bit ARM, and on -m32 builds on amd64.
> >
> > I also confirmed that without #define isinf(x) __builtin_isinf(x),
> > on both 32bit and 64bit fbsd isinf() compiles as a function call,
> > so the OP's proposed change would not be desirable.  
> 
> I installed FreeBSD 11.2 i386 on a virtual machine.  I couldn't
> reproduce the problem with either the base cc (clang 6.0.0) or clang38
> (clang 3.8.1) installed via pkg.

Do you reproducing problem in REL_11_STABLE? It doesn't exist in master.
Also may be it might depend on some configure options. I usually
compile postgres with as much --with-something enabled as possible

Ive put my cconfigure/build/check logs to 

https://wagner.pp.ru/~vitus/stuff/freebsd-i386-logs.tar.gz

Unfortunately there is a bit too much of them to attach to the list
message.

These logs are produced after I've upgraded my outdated system
to current 11.2 with clang 6.0.0. It haven't eliminated problem.

I can publish my KVM images both of old system (11.0 witth clang 3.8.0)
and new one (current 11.2 with clang 6.0.0)

> 
> The OP reported clang 3.8.0, so a minor version behind what I tested.
> 
> I did learn that "make check" fails in rolenames if your Unix user is
> called "user".
> 



-- 
   Victor Wagner 

Bug fix for glibc broke freebsd build in REL_11_STABLE

[Victor Wagner]

Collegues,

Few days ago commit 

 1f349aa7d9a6633e87db071390c73a39ac279ba4

Fix 8a934d67 for libc++ and make more include order resistant. 

was introduced into branch REL_11_STABLE.
It seems that it deals with GLIBC-based systems (i.e Linux and Hurd)
only, and shouldn't affect systems with non-GNU libc (such as BSD
family).

It changes code  which has a comment:

/*
 * Glibc doesn't use the builtin for clang due to a *gcc* bug in a
version
 * newer than the gcc compatibility clang claims to have. This would
cause a
 * *lot* of superfluous function calls, therefore revert when using
clang. In
 * C++ there's issues with libc++ (not libstdc++), so disable as well.
 */


However, this commit broke float8 test on 32-bit FreeBSD 11 with clang
3.8.0 compiler. Regressions.diff follows:

== pgsql.build/src/test/regress/regression.diffs
===
*** 
/usr/home/buildfarm/build-farm/REL_11_STABLE/pgsql.build/src/test/regress/expected/float8.out
Tue Sep  4 11:57:47 2018
--- 
/usr/home/buildfarm/build-farm/REL_11_STABLE/pgsql.build/src/test/regress/results/float8.out
Tue Sep  4 12:03:28 2018 *** *** 418,424  SET f1 =
FLOAT8_TBL.f1 * '-1' WHERE FLOAT8_TBL.f1 > '0.0';
  SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
! ERROR:  value out of range: overflow
  SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
  ERROR:  value out of range: overflow
  SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5;
--- 418,432 
 SET f1 = FLOAT8_TBL.f1 * '-1'
 WHERE FLOAT8_TBL.f1 > '0.0';
  SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
!  bad | ?column? 
! -+--
!  |0
!  |  -3.484e+201
!  | -1.0043e+203
!  |-Infinity
!  | -1.2345678901234
! (5 rows)
! 
  SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
  ERROR:  value out of range: overflow
  SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5; 

Problem doesn't arise on 64-bit FreeBSD and MacOS. (we have no 32-bit
MacOs at hand).

Following patch fixes this problem:

diff --git a/src/include/port.h b/src/include/port.h
index 0ce72e50e5..7daaebf568 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -350,7 +350,7 @@ extern int  isinf(double x);
  * *lot* of superfluous function calls, therefore revert when using
clang. In
  * C++ there's issues with libc++ (not libstdc++), so disable as well.
  */
-#if defined(__clang__) && !defined(__cplusplus)
+#if defined(__clang__) && !defined(__cplusplus) && defined(__linux__)
 /* needs to be separate to not confuse other compilers */
 #if __has_builtin(__builtin_isinf)
 /* need to include before, to avoid getting overwritten */

Idea of the patch is that because patch is only GLIBC-related, we
should apply this logic only if we have linux (more precisely linux or
hurd, but adding hurd would make condition overcomplicated and no one
seems to use it) system.
-- 

Re: Non-portable shell code in pg_upgrade tap tests

[Victor Wagner]

On Fri, 20 Jul 2018 10:25:47 -0400
Tom Lane  wrote:

> Victor Wagner  writes:
> > I've discovered that in the branch REL_11_STABLE there is shell
> > script src/bin/pg_upgrade/test.sh which doesn't work under Solaris
> > 10. (it uses $(command) syntax with is not compatible with original
> > Solaris /bin/sh)  

> 
> Please send a patch.  Most of us do not have access to old shells

Here it goes. Previous letter was written before fixed tests were
completed, because this old machine is slow.



> we could test this on.  (The oldest machine I have, and it's old
> enough to vote, does run that script ... I doubt very many other
> developers have anything older.)
> 
>   regards, tom lane

commit efb0bbcefeccd826ba951ac31057689e752b79d0
Author: Victor Wagner 
Date:   Fri Jul 20 16:06:50 2018 +0300

Fixed incompatibility of src/bin/pg_upgrade/test.sh with solaris /bin/sh

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 45ccd8fa66..775dd5729d 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -234,7 +234,7 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B
 # Windows hosts don't support Unix-y permissions.
 case $testhost in
 	MINGW*) ;;
-	*)	if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then
+	*)	if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
 			echo "files in PGDATA with permission != 640";
 			exit 1;
 		fi ;;
@@ -242,7 +242,7 @@ esac
 
 case $testhost in
 	MINGW*) ;;
-	*)	if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
+	*)	if [ `find ${PGDATA} -type d ! -perm 750 | wc -l` -ne 0 ]; then
 			echo "directories in PGDATA with permission != 750";
 			exit 1;
 		fi ;;

Non-portable shell code in pg_upgrade tap tests

[Victor Wagner]

Collegues,

I've discovered that in the branch REL_11_STABLE there is shell script
src/bin/pg_upgrade/test.sh which doesn't work under Solaris 10.
(it uses $(command) syntax with is not compatible with original
Solaris /bin/sh)

I was quite surprised that this problem goes unnoticed on big buildfarm,
but after some investigation found out that both Solaris machines in
that buildfarm don't configure postgres with --enable-tap-tests.

Offending code is:

# make sure all directories and files have group permissions, on Unix hosts
# Windows hosts don't support Unix-y permissions.
case $testhost in
MINGW*) ;;
*)  if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then
echo "files in PGDATA with permission != 640";
exit 1;
fi ;;
esac

case $testhost in
MINGW*) ;;
*)  if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
echo "directories in PGDATA with permission != 750";
exit 1;
fi ;;
esac


It is quite easy to replace $() syntax with backticks. Backticks are
not nestable and considered unsafe by modern shell scripting style
guides, but they do work with older shells.

--

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

В Wed, 28 Feb 2018 10:45:24 +0900
Michael Paquier <mich...@paquier.xyz> пишет:


> Replace backslashes by forward slashes in MSVC build code
> 
> This makes it possible to run some stages of these build scripts on
> non-Windows systems.  That way, we can more easily test whether file
> moves or makefile changes might break the MSVC build.

It would be nice to be able at least run perl -wc on these scripts on
non-windows platform.

Unforutnately following development seems to break this.

Now Mkvcbuild.pm depends on Win32 module and Win32API::File module,
both of which are not exist on non Win32 platforms.

Or there exist somewhere fake Win32 module which would satisfy
dependencies and do nothing?



-- 
       Victor Wagner <vi...@wagner.pp.ru>

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

On Tue, 27 Feb 2018 14:14:04 +0100
Magnus Hagander  wrote:

> 
> Thanks. I've pushed this for 9.3-9.5.
> 
> Please verify that it looks good if you can (it takes a while for the
> buildfarm to get around to it)

Works at least for 9.5, although I haven't run tap tests on this build.

---

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

On Tue, 27 Feb 2018 13:49:29 +0100
Magnus Hagander  wrote:


 
 
> Does not apply cleanly to 9.5 at least for me. Probably easy to fix,
> but I still feel we shouldn't mess around with the buildsystem in
> back branches unless we actually have to.

How interesting - somewhere between 9.3 (for which patch was made) and
9.5 path to the Makefile in windows-specific script  was changed from
Windows-style separators to unix-style and this broke context.

-- 

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

On Tue, 27 Feb 2018 13:35:33 +0100
Magnus Hagander  wrote:


> > Note that said commit (91f3ffc5249) is not limited to rearranging
> > makefile. It also changes a lot into C code itself. So it is not a
> > question of reverting commit - it is making new commit, which
> > reverts changes in just one file.  
> 
> 
> Oh, I missed that.
> 
> I think we should revert *just the changes to the Makefile*, and of
> course leave the rest of the comimt. Can you confirm if that fixes
> the problem?

It seems that it is not so easy. I've tried to revert changes in the
Makefile, and found out that some utilities (createlang, droplang,
pg_isready) now need more common object files, than they need before
this commit .

Attached patch to makefile which fixe problem for 9.3 branch.
I think it should do for 9.4 and 9.5 too.

--
 
 

diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile
old mode 100644
new mode 100755
index 996db500e9..0dba267c20
--- a/src/bin/scripts/Makefile
+++ b/src/bin/scripts/Makefile
@@ -25,17 +25,17 @@ all: $(PROGRAMS)
 %: %.o $(WIN32RES)
 	$(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
-SCRIPTS_COMMON = common.o dumputils.o kwlookup.o keywords.o
-createdb: createdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-createlang: createlang.o $(SCRIPTS_COMMON) print.o mbprint.o | submake-libpq submake-libpgport
-createuser: createuser.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-dropdb: dropdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-droplang: droplang.o $(SCRIPTS_COMMON) print.o mbprint.o | submake-libpq submake-libpgport
-dropuser: dropuser.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-clusterdb: clusterdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-vacuumdb: vacuumdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-reindexdb: reindexdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
-pg_isready: pg_isready.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+createdb: createdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+createlang: createlang.o common.o dumputils.o kwlookup.o keywords.o print.o mbprint.o | submake-libpq submake-libpgport
+createuser: createuser.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+dropdb: dropdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+droplang: droplang.o common.o dumputils.o kwlookup.o keywords.o print.o mbprint.o | submake-libpq submake-libpgport
+dropuser: dropuser.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+clusterdb: clusterdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+vacuumdb: vacuumdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+reindexdb: reindexdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+pg_isready: pg_isready.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
+
 
 dumputils.c keywords.c: % : $(top_srcdir)/src/bin/pg_dump/%
 	rm -f $@ && $(LN_S) $< .
@@ -67,5 +67,5 @@ uninstall:
 
 clean distclean maintainer-clean:
 	rm -f $(addsuffix $(X), $(PROGRAMS)) $(addsuffix .o, $(PROGRAMS))
-	rm -f $(SCRIPTS_COMMON) print.o mbprint.o $(WIN32RES)
+	rm -f common.o dumputils.o kwlookup.o keywords.o print.o mbprint.o $(WIN32RES)
 	rm -f dumputils.c print.c mbprint.c kwlookup.c keywords.c

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

On Tue, 27 Feb 2018 11:43:34 +0100
Magnus Hagander  wrote:


> I'm unsure why this was introduced in 9.5 and earlier, but not in the
> newer ones.  This smells like a possible backpatch mistake, in which
> case that part should probably be backed out of the old branches
> rather than teaching mkvcbuild about it.

Patch for Mkvcbuild.pm is actually quite simple (see attach).

Really, I have more complicated patch, which supports recursive
substitution as gmake does. It was developed to simplify inclusion of
PGXS extensions into contrib tree. (authors of modern extension often
use complex Makefile constructs).

I've not presented it to community because I hoped that current MSVC
build system would be soon replaced by cmake.

-- 


diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
old mode 100644
new mode 100755
index 2e19aa7035..2bd506bb90
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -686,6 +686,8 @@ sub mkvcbuild
 
 	$mf = Project::read_file('src\bin\scripts\Makefile');
 	$mf =~ s{\\s*[\r\n]+}{}mg;
+	$mf =~ m{SCRIPTS_COMMON\s*=\s*(.*)$}m;
+	my @common_files = split /\s+/, $1;
 	$mf =~ m{PROGRAMS\s*=\s*(.*)$}m
 	  || die 'Could not match in bin\scripts\Makefile' . "\n";
 	foreach my $prg (split /\s+/, $1)
@@ -694,6 +696,7 @@ sub mkvcbuild
 		$mf =~ m{$prg\s*:\s*(.*)$}m
 		  || die 'Could not find script define for $prg' . "\n";
 		my @files = split /\s+/, $1;
+		push @files,@common_files if (grep {$_ eq '$(SCRIPTS_COMMON)'} @files);
 		foreach my $f (@files)
 		{
 			$f =~ s/\.o$/\.c/;

Re: MSVC builld of 9.5.12 is broken?

[Victor Wagner]

On Tue, 27 Feb 2018 11:43:34 +0100
Magnus Hagander <mag...@hagander.net> wrote:

> On Tue, Feb 27, 2018 at 11:27 AM, Victor Wagner <vi...@wagner.pp.ru>
> wrote:
> 
> > Hello, hackers.
> >
> > I've tried to build last state of REL9_5_STABLE branch (commit
> > 1f19e46124eee8c6a54834) and under Win32 encountered  following
> > errors:
> >
[skip]
> 
> It's also interesting to note that this did not break in HEAD, 10 or
> 9.6. And none of those actually have the SCRIPTS_COMMON code.

It seems that it early stages of 9.6 cycle there was another approach
taken to improve readability of this Makefile - just all common code 
put into one C file. So there is no need for SCRIPTS_COMMON variable, 
because its name is longer than name of common.o which would be its sole
contents.

> I'm unsure why this was introduced in 9.5 and earlier, but not in the
> newer ones.  This smells like a possible backpatch mistake, in which
> case that part should probably be backed out of the old branches
> rather than teaching mkvcbuild about it.

Note that said commit (91f3ffc5249) is not limited to rearranging
makefile. It also changes a lot into C code itself. So it is not a
question of reverting commit - it is making new commit, which reverts
changes in just one file.



> Noah? Can you confirm if it was intentional?
 

Re: master make check fails on Solaris 10

[Victor Wagner]

On Thu, 18 Jan 2018 09:56:48 -0500
Tom Lane  wrote:

> Marina Polyakova  writes:
> > Applying your patch on commit
> > f033462d8f77c40b7d6b33c5116e50118fb4699d and using the
> > configuration command from [1], I got: checking for __int128... yes
> > checking for __int128 alignment bug... broken
> > ...
> > And make check-world passes. Victor said that he used a much
> > simpler configuration command, and I'm trying to figure out what's
> > changed..  
> 
> Weird.  Maybe the gcc bug only manifests with certain optimization
> flags?  That's not what I'd have expected from Victor's theory about

No. I've compiled test program without any optimizationf flags.
Just -m64, which tells compiler to generate 64-bit code.
(in 32-bit mode there is no __int128, so problem wouldn't manifest
inself).

From the other side, when I've tried to resolve issue with not worked
test, I've copied all gcc flags from config.log, and test program
returned 1 with exactly same flags.

Probably, I should have to regenerate configure with autoconf. instead
of applying patch to configure.

-- 

Re: master make check fails on Solaris 10

[Victor Wagner]

On Thu, 18 Jan 2018 01:47:46 -0500
Tom Lane <t...@sss.pgh.pa.us> wrote:

> Victor Wagner <vi...@wagner.pp.ru> writes:
> > Tom Lane <t...@sss.pgh.pa.us> wrote:  
> > checking for __int128 alignment bug... ok
> > As far as I understand your patch, there should be:
> > checking for __int128 alignment bug... broken  
> 
> Yes, that's what I expected to happen.

It seems that I've made some unreproducible mistake last night 
applying your patch. Marina repeapplied it later  and everything works.

I'we cherrypicked in as far back as 9.5, and with these versions it
works too.

Re: master make check fails on Solaris 10

[Victor Wagner]

On Wed, 17 Jan 2018 11:33:09 -0500
Tom Lane  wrote:

> Attached is a draft patch to incorporate Victor's slimmed-down test
> into configure.  If you have a chance, could you confirm it does
> the right thing on your Sparc machine?

It seems that what it does is not exactly a right thing.
I've applied it to commit 9c7d06d60680 in master and see following

$ ./configure CC="gcc -m64"
[skip]
checking for __int128... yes
checking for __int128 alignment bug... ok
checking alignment of PG_INT128_TYPE... 16
checking for builtin __sync char locking functions... yes
[skip]

As far as I understand your patch, there should be:

checking for __int128 alignment bug... broken

Then in the pg_config.h I see


/* The normal alignment of `PG_INT128_TYPE', in bytes. */
#define ALIGNOF_PG_INT128_TYPE 16

/* Define to the name of a signed 128-bit integer type. */
#define PG_INT128_TYPE __int128

However, make check passes. 

There are two things which puzzle me
1. Why test program doesn't detect bug.
If I cut'n'paste it from configure, compile with flags, cut'n'pasted
from config log and run, it returns 1. But configure tells that all is
ok
2. If bug exist and is not detected by configure why make check passes.

We, Marina and I would continue investigation.

Re: master make check fails on Solaris 10

[Victor Wagner]

В Wed, 17 Jan 2018 11:33:09 -0500
Tom Lane <t...@sss.pgh.pa.us> пишет:

> Attached is a draft patch to incorporate Victor's slimmed-down test
> into configure.  If you have a chance, could you confirm it does
> the right thing on your Sparc machine?
> 
Definitely. As soon as next work day begins in Moscow.

> BTW, it would be a good idea to set up a buildfarm member on that
> machine, if you care about whether that configuration continues
> to work in the future.

Really we already have buildfarm member on this machine. It is just
member of PostgresPro private buildfarm, not of big comminity
buildfarm.

So, I'll register it in the big buildfarm as soon as I figure out how
to distribute limited resources of this machine between two buildfarms.



-- 
       Victor Wagner <vi...@wagner.pp.ru>

Re: master make check fails on Solaris 10

[Victor Wagner]

On Wed, 17 Jan 2018 10:07:37 -0500
Tom Lane  wrote:

> Marina Polyakova  writes:

> Yeah, I can work with this.  What I propose to do is use a somewhat
> stripped-down version of this test as an AC_RUN_IFELSE test normally,
> but if cross-compiling, fall back to just seeing if we can link.

I'd suggest to add a configure option to switch off 128-bit support 
(--disable-int128), especially for these cross-compile cases where link
test cannot give us enough information to decide automatically.

--

Re: master make check fails on Solaris 10

[Victor Wagner]

On Wed, 17 Jan 2018 18:02:26 +0300
Marina Polyakova  wrote:


> > Attached is a possible test program.  I can confirm it passes on a
> > machine with working __int128, but I have no idea whether it will
> > detect the problem on yours.  If not, maybe you can tweak it?
> 
> Thank you! Using gcc 5.5.0 it prints that everything is ok. But, 
> investigating the regression diffs, we found out that the error
> occurs when we pass int128 as not the first argument to the function
> (perhaps its value is replaced by the value of some address):

I'm attaching stripped-down version of test program, which demonstrate
the problem and two assembler listings produced with this C source using
alignment 8 and 16. May be this stripped-down version can be used as 
base for configure test.

As it turns out, Sparc GCC passes function arguments via register ring
which is referenced as %on in the calling code and as %in in function.

And somehow it happens that alignment attribute of typedef affects
access to arguments in the function, but doesn't affect how regiser
ring is filled before call. Looks like bug in GCC.

Unfortunately, we have only one Sparc machine and started our
investigation by upgrading GCC 5.2.0 to GCC 5.5.0, so it is hard to
downgrade and test with older GCC.

--#include 
#include 


# ifndef PG_ALIGN_128
#define PG_ALIGN_128 8
# endif

/* GCC, Sunpro and XLC support aligned */
#if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
#define pg_attribute_aligned(a) __attribute__((aligned(a)))
#endif

typedef __int128 int128a
#if defined(pg_attribute_aligned)
pg_attribute_aligned(PG_ALIGN_128)
#endif
;
int128a holder;
void pass_by_val(void *buffer,int128a par) {
   holder=par;
}




int
main()
{
	long int i64 = 97656225L << 12;
	int128a q;	
	pass_by_val(main,(int128a) i64);
	q=(int128a) i64;
	printf("pass int 64 %s\n",(q==holder)?"OK":"FAILED");	
	if (q!=holder)
		return 1;
	return 0;
}


align_test8.s
Description: Binary data


align_test16.s
Description: Binary data