Обсуждение: Remove libpq.rc, use win32ver.rc for libpq
I was wondering why we have a separate libpq.rc for libpq and use win32ver.rc for all other components. I suspect this is also a leftover from the now-removed client-only Windows build. With a bit of tweaking we can use win32ver.rc for libpq as well and remove a bit of duplicative code. I have tested this patch with MSVC and MinGW. I've also added some comments and a documentation link to be able to understand this business a bit better. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Fri, Dec 27, 2019 at 05:25:58PM +0100, Peter Eisentraut wrote: > I was wondering why we have a separate libpq.rc for libpq and use > win32ver.rc for all other components. I suspect this is also a leftover > from the now-removed client-only Windows build. With a bit of tweaking we > can use win32ver.rc for libpq as well and remove a bit of duplicative code. > > I have tested this patch with MSVC and MinGW. The patch does not apply anymore because of two conflicts with the copyright dates, could you rebase it? Reading through it, the change looks sensible. However I have not looked at it yet in details. - FILEFLAGSMASK 0x17L + FILEFLAGSMASK VS_FFI_FILEFLAGSMASK Are you sure with the mapping here? I would have thought that VS_FF_DEBUG is not necessary when using release-quality builds, which is something that can be configured with build.pl, and that it would be better to not enforce VS_FF_PRERELEASE all the time. -- Michael
Вложения
On 2020-01-06 09:02, Michael Paquier wrote: > - FILEFLAGSMASK 0x17L > + FILEFLAGSMASK VS_FFI_FILEFLAGSMASK > Are you sure with the mapping here? I would have thought that > VS_FF_DEBUG is not necessary when using release-quality builds, which > is something that can be configured with build.pl, and that it would > be better to not enforce VS_FF_PRERELEASE all the time. Note that there is FILEFLAGSMASK and FILEFLAGS. The first is just a mask that says which bits in the second are valid. Since both libpq.rc and win32ver.rc use FILEFLAGS 0, it doesn't matter what we set FILEFLAGSMASK to. But currently libpq.rc uses 0x3fL and win32ver.rc uses 0x17L, so in order to unify this sensibly I looked for a well-recognized standard value, which led to VS_FFI_FILEFLAGSMASK. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-01-09 10:56, Peter Eisentraut wrote: > On 2020-01-06 09:02, Michael Paquier wrote: >> - FILEFLAGSMASK 0x17L >> + FILEFLAGSMASK VS_FFI_FILEFLAGSMASK >> Are you sure with the mapping here? I would have thought that >> VS_FF_DEBUG is not necessary when using release-quality builds, which >> is something that can be configured with build.pl, and that it would >> be better to not enforce VS_FF_PRERELEASE all the time. > > Note that there is FILEFLAGSMASK and FILEFLAGS. The first is just a > mask that says which bits in the second are valid. Since both libpq.rc > and win32ver.rc use FILEFLAGS 0, it doesn't matter what we set > FILEFLAGSMASK to. But currently libpq.rc uses 0x3fL and win32ver.rc > uses 0x17L, so in order to unify this sensibly I looked for a > well-recognized standard value, which led to VS_FFI_FILEFLAGSMASK. Here is a rebased patch. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
At Tue, 14 Jan 2020 22:34:10 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in > On 2020-01-09 10:56, Peter Eisentraut wrote: > > On 2020-01-06 09:02, Michael Paquier wrote: > >> - FILEFLAGSMASK 0x17L > >> + FILEFLAGSMASK VS_FFI_FILEFLAGSMASK > >> Are you sure with the mapping here? I would have thought that > >> VS_FF_DEBUG is not necessary when using release-quality builds, which > >> is something that can be configured with build.pl, and that it would > >> be better to not enforce VS_FF_PRERELEASE all the time. > > Note that there is FILEFLAGSMASK and FILEFLAGS. The first is just a > > mask that says which bits in the second are valid. Since both > > libpq.rc > > and win32ver.rc use FILEFLAGS 0, it doesn't matter what we set > > FILEFLAGSMASK to. But currently libpq.rc uses 0x3fL and win32ver.rc > > uses 0x17L, so in order to unify this sensibly I looked for a > > well-recognized standard value, which led to VS_FFI_FILEFLAGSMASK. I agree to the direction of the patch and the point above sounds sensible to me. > Here is a rebased patch. It applied on 4d8a8d0c73 cleanly and built successfully by VS2019. regares. -- Kyotaro Horiguchi NTT Open Source Software Center
On Wed, Jan 15, 2020 at 02:22:45PM +0900, Kyotaro Horiguchi wrote: > At Tue, 14 Jan 2020 22:34:10 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in >> On 2020-01-09 10:56, Peter Eisentraut wrote: >>> Note that there is FILEFLAGSMASK and FILEFLAGS. The first is just a >>> mask that says which bits in the second are valid. Since both >>> libpq.rc >>> and win32ver.rc use FILEFLAGS 0, it doesn't matter what we set >>> FILEFLAGSMASK to. But currently libpq.rc uses 0x3fL and win32ver.rc >>> uses 0x17L, so in order to unify this sensibly I looked for a >>> well-recognized standard value, which led to VS_FFI_FILEFLAGSMASK. Hmm. I agree that what you have here is sensible. I am wondering if it would be better to have VS_FF_DEBUG set dynamically in FILEFLAGS in the future though. But that's no material for this patch. >> Here is a rebased patch. > > It applied on 4d8a8d0c73 cleanly and built successfully by VS2019. I have been testing and checking the patch a bit more seriously, and the information gets generated correctly for dlls and exe files. The rest of the changes look fine to me. For src/makefiles/Makefile.win32, I don't have a MinGW environment at hand so I have not directly tested but the logic looks fine. -- Michael
Вложения
On 2020-01-15 07:44, Michael Paquier wrote: > I have been testing and checking the patch a bit more seriously, and > the information gets generated correctly for dlls and exe files. The > rest of the changes look fine to me. For src/makefiles/Makefile.win32, > I don't have a MinGW environment at hand so I have not directly > tested but the logic looks fine. I have tested MinGW. Patch committed. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services