Review of VS 2010 support patches

From: Craig Ringer <craig(at)postnewspapers(dot)com(dot)au>
To: PG Hackers <pgsql-hackers(at)postgresql(dot)org>, Brar Piening <brar(at)gmx(dot)de>
Subject: Review of VS 2010 support patches
Date: 2011-07-05 12:25:03
Message-ID: 4E13029F.8080201@postnewspapers.com.au
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all

I've got through a review of the VS 2010 support patches. Between work
being busy and some interesting issues getting my 64-bit build
environment set up it took longer than anticipated. Sorry.

It looks good so far. I haven't had any reply to my email to Brar, so
there are a few details (like whether x64 builds were tested and how x64
required libraries were built) I could use, but what I've got done so
far seems fine.

Details follow.

PATCH FORMATTING
==================

The patch (VS2010v7.patch) seems to mix significant changes with
whitespace fixes etc. These should be separated for clarity and ease of
future bisect testing etc. Any "perltidy" run should be done as a
separate patch, too. This is easy if you are using git, because you can
just commit each to your local tree then use git format-patch to produce
nice patches. If you have a local tree with a more complicated history,
you can use git rebase to tidy up the history before you format-patch.

The patches apply cleanly to git master as of
21f1e15aafb13ab2430e831a3da7d4d4f525d1ce .

pgflex.pl and pgbison.pl
=====================

pgflex.pl and pgbison.pl are a big improvement over the horrid batch
files, but are perhaps too little a translation. There's no need for the
big if(string) then (otherstring) stuff; it can be done much more
cleanly by storing a simple hash of paths to options and doing a file
extension substitution to generate the output filenames. The hash only
needs to be populated for files that get processed with non-default
options, so for pgflex all you need is:

%LEX_OPTS = { 'src\backend\parser\scan.c' -> '-CF' };

I can send adjusted versions of pgflex.pl and pgbison.pl that

DOCUMENTATION
===============

I didn't notice any documentation updates to reflect the fact that
Visual Studio 2010 is now supported. It'd be a good idea to change
install-windows-full.html (or the source of it, anyway) to mention VS
2010 support.

TEST RESULTS (x86)
=================

I used a buildenv.pl and config.pl that's known to build under VS 2008
and pass "vcregress check" with an unpatched copy of git master. I built
with everything except uuid and tcl enabled; I'll see if I can add them
later.

The patches applied cleanly, and didn't break VS 2008 builds, which
continued to work fine after a "clean dist" and "build". "vcregress
check" still passes.

Builds done with VS 2010 using the patches worked fine, and passed
"vcregress check" tests.

I should have plcheck and contribcheck results as soon as I've got
things rebuilt with uuid and tcl.

TEST RESULTS (x64)
=================

I'm still working on 64-bit tests. I've finally found out how to get
64-bit compilation working under Visual Studio 2008 Express Edition (or,
rather, Microsoft Windows SDK for Windows 7 and .NET Framework 3.5 SP1)
so I'll be testing that shortly.

I'm not sure if I'll be able to get 64-bit copies of all the optional
libraries built, so it may be a more minimal build. It'll include at
least zlib, plperl and plpython 64-bit support, though. Information from
Briar about whether he built for 64-bit and if so how he got his
libraries built would help.

--
Craig Ringer

POST Newspapers
276 Onslow Rd, Shenton Park
Ph: 08 9381 3088 Fax: 08 9388 2258
ABN: 50 008 917 717
http://www.postnewspapers.com.au/

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-07-05 13:02:15 Re: Crash dumps
Previous Message Hitoshi Harada 2011-07-05 11:43:46 Re: Parameterized aggregate subquery (was: Pull up aggregate subquery)