Compiling C++ extensions on MSVC using scripts in src/tools

Hi all,

In the stuff I work on in a daily basis there are a couple of
extensions written in C++, compiling them with MSVC on Windows using
slightly-different scripts available in src/tools after copying them
directly in contrib/. However, the build scripts available in
src/tools/msvc are not able to detect files suffixed as cpp or
similar. Attached is a two-line patch that enables their detection. I
believe this would be useful for packagers on Windows.

Patch is added to the next commit fest.
Regards,
--
Michael

On 11/25/2014 11:46 PM, Michael Paquier wrote:
> Hi all,
>
> In the stuff I work on in a daily basis there are a couple of
> extensions written in C++, compiling them with MSVC on Windows using
> slightly-different scripts available in src/tools after copying them
> directly in contrib/. However, the build scripts available in
> src/tools/msvc are not able to detect files suffixed as cpp or
> similar. Attached is a two-line patch that enables their detection. I
> believe this would be useful for packagers on Windows.
>

+ unless ($fileNameWithPath =~ /^(.*)\\([^\\]+)\.[r]?[cyl](pp)?$/);

This doesn't seem to me to be terribly well expressed (I know it's not your fault, quite possibly it's mine.) Perhaps we should replace

[r]?[cyl](pp)?

with

(c|cpp|y|l|rc)

which I think is what's intended, and seems much less obscure to me.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> This doesn't seem to me to be terribly well expressed (I know it's not your fault, quite possibly it's mine.) Perhaps we should replace
> [r]?[cyl](pp)?
> with
> (c|cpp|y|l|rc)

+1 ... the original coding is illegible already, not to mention wrong
since it will match stuff it shouldn't.

regards, tom lane

On Thu, Nov 27, 2014 at 1:40 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> This doesn't seem to me to be terribly well expressed (I know it's not your fault, quite possibly it's mine.) Perhaps we should replace
>> [r]?[cyl](pp)?
>> with
>> (c|cpp|y|l|rc)
>
> +1 ... the original coding is illegible already, not to mention wrong
> since it will match stuff it shouldn't.
Yes even the older code could find matches with ry or rl. Except that,
lpp and ypp could be present as well. OK, there are low chances to be
present in a Postgres extension (I don't have such extensions myself),
still they could. So I think that this expression should be written
like that instead:
(c|cpp|l|lpp|y|ypp|rc).
Updated patch is attached.
--
Michael

On 11/27/2014 06:39 AM, Michael Paquier wrote:
> On Thu, Nov 27, 2014 at 1:40 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> This doesn't seem to me to be terribly well expressed (I know it's not your fault, quite possibly it's mine.) Perhaps we should replace
>>> [r]?[cyl](pp)?
>>> with
>>> (c|cpp|y|l|rc)
>>
>> +1 ... the original coding is illegible already, not to mention wrong
>> since it will match stuff it shouldn't.
> Yes even the older code could find matches with ry or rl. Except that,
> lpp and ypp could be present as well. OK, there are low chances to be
> present in a Postgres extension (I don't have such extensions myself),
> still they could. So I think that this expression should be written
> like that instead:
> (c|cpp|l|lpp|y|ypp|rc).
> Updated patch is attached.

If .lp and .ypp files are supposed to be Bison and Flex files with C++
code in them, it wouldn't work anyway, because the rules elsewhere in
the MSVC scripts just check for /\.y$/) to decide whether to run bison
on it.

I committed Andrew's suggestion:

/^(.*)\\([^\\]+)\.(c|cpp|y|l|rc)$/

- Heikki

On Thu, Dec 18, 2014 at 4:55 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> If .lp and .ypp files are supposed to be Bison and Flex files with C++ code
> in them, it wouldn't work anyway, because the rules elsewhere in the MSVC
> scripts just check for /\.y$/) to decide whether to run bison on it.
>
> I committed Andrew's suggestion:
> /^(.*)\\([^\\]+)\.(c|cpp|y|l|rc)$/
Thanks, that's enough for my stuff.
--
Michael