Horton

Thread

Commits

  1. Remove libpq.rc, use win32ver.rc for libpq

  1. Remove libpq.rc, use win32ver.rc for libpq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-27T16:25:58Z

    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

  2. Re: Remove libpq.rc, use win32ver.rc for libpq

    Michael Paquier <michael@paquier.xyz> — 2020-01-06T08:02:02Z

    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

  3. Re: Remove libpq.rc, use win32ver.rc for libpq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-01-09T09:56:32Z

    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

  4. Re: Remove libpq.rc, use win32ver.rc for libpq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-01-14T21:34:10Z

    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

  5. Re: Remove libpq.rc, use win32ver.rc for libpq

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-01-15T05:22:45Z

    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

  6. Re: Remove libpq.rc, use win32ver.rc for libpq

    Michael Paquier <michael@paquier.xyz> — 2020-01-15T06:44:38Z

    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

  7. Re: Remove libpq.rc, use win32ver.rc for libpq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-01-15T14:40:56Z

    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