Обсуждение: Compiler warnings with MinGW
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
Вложения
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
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
Вложения
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
Вложения
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
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
Вложения
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.
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
On Tue, Sep 17, 2019 at 12:00:39PM +0200, Peter Eisentraut wrote: > committed Thanks, Peter. -- Michael