Horton

Thread

Commits

  1. Remove mingwcompat.c

  2. Fix compilation warning of pg_basebackup with MinGW

  1. Compiler warnings with MinGW

    Michael Paquier <michael@paquier.xyz> — 2019-07-19T05:08:30Z

    Hi all,

Just browsing through the logs of the buildfarm, I have noticed that
some buildfarm animals complain with warnings (jacana uses MinGW):
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=jacana&dt=2019-07-19%2001%3A45%3A28&stg=make

There are two of them:
c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/backend/port/win32/mingwcompat.c:60:1:
warning: 'RegisterWaitForSingleObject' redeclared without dllimport
attribute: previous dllimport ignored [-Wattributes]

c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/bin/pg_basebackup/pg_basebackup.c:1448:8:
warning: variable 'filemode' set but not used
[-Wunused-but-set-variable]
Jul 18 21:59:49     int   filemode;

The first one has been discussed already some time ago and is a cause
of 811be893, but nothing got actually fixed (protagonists in CC):
https://www.postgresql.org/message-id/CABUevEyeZfUvaYMuNop3NyRvvRh2Up2tStK8SXVAPDERf8p9eg@mail.gmail.com

The second one is rather obvious to fix, because we don't care about
the file mode on Windows, so the attached should do the work.  I am
actually surprised that the Visual Studio compilers don't complain
about that, but let's fix it.

Thoughts?
--
Michael

  2. Re: Compiler warnings with MinGW

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-07-19T12:41:28Z

    On 7/19/19 1:08 AM, Michael Paquier wrote:
> Hi all,
>
> Just browsing through the logs of the buildfarm, I have noticed that
> some buildfarm animals complain with warnings (jacana uses MinGW):
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=jacana&dt=2019-07-19%2001%3A45%3A28&stg=make
>
> There are two of them:
> c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/backend/port/win32/mingwcompat.c:60:1:
> warning: 'RegisterWaitForSingleObject' redeclared without dllimport
> attribute: previous dllimport ignored [-Wattributes]
>
> c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/bin/pg_basebackup/pg_basebackup.c:1448:8:
> warning: variable 'filemode' set but not used
> [-Wunused-but-set-variable]
> Jul 18 21:59:49     int   filemode;
>
> The first one has been discussed already some time ago and is a cause
> of 811be893, but nothing got actually fixed (protagonists in CC):
> https://www.postgresql.org/message-id/CABUevEyeZfUvaYMuNop3NyRvvRh2Up2tStK8SXVAPDERf8p9eg@mail.gmail.com


To answer Magnus' question in that thread, the Mingw headers on jacana
declare the function with WINBASEAPI which in turn is defined as
DECLSPEC_IMPORT, as long as _KERNEL32_ isn't defined, and we don't do
that (and I don't think anything else does either).


So the fix Peter proposed looks like it should be correct.



>
> The second one is rather obvious to fix, because we don't care about
> the file mode on Windows, so the attached should do the work.  I am
> actually surprised that the Visual Studio compilers don't complain
> about that, but let's fix it.
>
> Thoughts?


+1.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: Compiler warnings with MinGW

    Michael Paquier <michael@paquier.xyz> — 2019-07-20T09:19:34Z

    On Fri, Jul 19, 2019 at 08:41:28AM -0400, Andrew Dunstan wrote:
> On 7/19/19 1:08 AM, Michael Paquier wrote:
>> The second one is rather obvious to fix, because we don't care about
>> the file mode on Windows, so the attached should do the work.  I am
>> actually surprised that the Visual Studio compilers don't complain
>> about that, but let's fix it.
>>
>> Thoughts?
> 
> +1.

Just wanted to double-check something.  We usually don't bother
back-patching warning fixes like this one, right?
--
Michael

  4. Re: Compiler warnings with MinGW

    Michael Paquier <michael@paquier.xyz> — 2019-07-21T13:47:31Z

    On Sat, Jul 20, 2019 at 06:19:34PM +0900, Michael Paquier wrote:
> Just wanted to double-check something.  We usually don't bother
> back-patching warning fixes like this one, right?

I have double-checked the thing, and applied it only on HEAD as we
have that for some time (since 9.1 actually and 00cdd83 has improved
the original situation here).
--
Michael

  5. Re: Compiler warnings with MinGW

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-09-06T22:11:25Z

    On 2019-07-19 14:41, Andrew Dunstan wrote:
> On 7/19/19 1:08 AM, Michael Paquier wrote:
>> Just browsing through the logs of the buildfarm, I have noticed that
>> some buildfarm animals complain with warnings (jacana uses MinGW):
>> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=jacana&dt=2019-07-19%2001%3A45%3A28&stg=make
>>
>> There are two of them:
>> c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/backend/port/win32/mingwcompat.c:60:1:
>> warning: 'RegisterWaitForSingleObject' redeclared without dllimport
>> attribute: previous dllimport ignored [-Wattributes]
>>
>> The first one has been discussed already some time ago and is a cause
>> of 811be893, but nothing got actually fixed (protagonists in CC):
>> https://www.postgresql.org/message-id/CABUevEyeZfUvaYMuNop3NyRvvRh2Up2tStK8SXVAPDERf8p9eg@mail.gmail.com
> 
> To answer Magnus' question in that thread, the Mingw headers on jacana
> declare the function with WINBASEAPI which in turn is defined as
> DECLSPEC_IMPORT, as long as _KERNEL32_ isn't defined, and we don't do
> that (and I don't think anything else does either).
> 
> So the fix Peter proposed looks like it should be correct.

I'm not sure exactly what the upstream of mingw is these days, but I
think the original issue that led to 811be893 has long been fixed [0],
and the other stuff in mingwcompat.c is also no longer relevant [1].  I
think mingwcompat.c could be removed altogether.  I'm not sure to what
extent we need to support 5+ year old mingw versions.

[0]:
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/9d937a7f4f766f903c9433044f77bfa97a0bc1d8/
[1]:
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/88ab6fbdd0a185702a1fce4db935e303030e082f/

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: Compiler warnings with MinGW

    Michael Paquier <michael@paquier.xyz> — 2019-09-07T02:58:14Z

    On Sat, Sep 07, 2019 at 12:11:25AM +0200, Peter Eisentraut wrote:
> I'm not sure exactly what the upstream of mingw is these days, but I
> think the original issue that led to 811be893 has long been fixed [0],
> and the other stuff in mingwcompat.c is also no longer relevant [1].  I
> think mingwcompat.c could be removed altogether.  I'm not sure to what
> extent we need to support 5+ year old mingw versions.

On HEAD I would not be against removing that as this leads to a
cleanup of our code.  For MSVC, we only support VS 2013~ on HEAD, so
saying that we don't support MinGW older than what was proposed 5
years ago sounds sensible.
--
Michael

  7. Re: Compiler warnings with MinGW

    Magnus Hagander <magnus@hagander.net> — 2019-09-09T12:24:22Z

    On Sat, Sep 7, 2019 at 4:58 AM Michael Paquier <michael@paquier.xyz> wrote:

> On Sat, Sep 07, 2019 at 12:11:25AM +0200, Peter Eisentraut wrote:
> > I'm not sure exactly what the upstream of mingw is these days, but I
> > think the original issue that led to 811be893 has long been fixed [0],
> > and the other stuff in mingwcompat.c is also no longer relevant [1].  I
> > think mingwcompat.c could be removed altogether.  I'm not sure to what
> > extent we need to support 5+ year old mingw versions.
>
> On HEAD I would not be against removing that as this leads to a
> cleanup of our code.  For MSVC, we only support VS 2013~ on HEAD, so
> saying that we don't support MinGW older than what was proposed 5
> years ago sounds sensible.
>

+1, definitely.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  8. Re: Compiler warnings with MinGW

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-09-17T10:00:39Z

    On 2019-09-09 14:24, Magnus Hagander wrote:
> On Sat, Sep 7, 2019 at 4:58 AM Michael Paquier <michael@paquier.xyz
> <mailto:michael@paquier.xyz>> wrote:
> 
>     On Sat, Sep 07, 2019 at 12:11:25AM +0200, Peter Eisentraut wrote:
>     > I'm not sure exactly what the upstream of mingw is these days, but I
>     > think the original issue that led to 811be893 has long been fixed [0],
>     > and the other stuff in mingwcompat.c is also no longer relevant
>     [1].  I
>     > think mingwcompat.c could be removed altogether.  I'm not sure to what
>     > extent we need to support 5+ year old mingw versions.
> 
>     On HEAD I would not be against removing that as this leads to a
>     cleanup of our code.  For MSVC, we only support VS 2013~ on HEAD, so
>     saying that we don't support MinGW older than what was proposed 5
>     years ago sounds sensible.
> 
> +1, definitely. 

committed

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: Compiler warnings with MinGW

    Michael Paquier <michael@paquier.xyz> — 2019-09-18T01:30:31Z

    On Tue, Sep 17, 2019 at 12:00:39PM +0200, Peter Eisentraut wrote:
> committed

Thanks, Peter.
--
Michael