Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-11-07 Thread Tom Lane
Albe Laurenz writes: > Tom Lane wrote: >> I'm inclined to give this up as a bad job and go back to the >> previous state. We have a solution that works and doesn't >> produce warnings; third-party authors who don't want to use it >> are on their own. > I think you are

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-11-07 Thread Albe Laurenz
Tom Lane wrote: >> Albe Laurenz writes: >>> Anyway, I have prepared a patch along the lines you suggest. >> >> Pushed, we'll see if the buildfarm likes this iteration any better. > > And the answer is "not very much". The Windows builds aren't actually > failing, but

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-11-05 Thread Tom Lane
I wrote: > Albe Laurenz writes: > Anyway, I have prepared a patch along the lines you suggest. > Pushed, we'll see if the buildfarm likes this iteration any better. And the answer is "not very much". The Windows builds aren't actually failing, but they are producing

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-11-04 Thread Tom Lane
Albe Laurenz writes: >> Anyway, I have prepared a patch along the lines you suggest. Pushed, we'll see if the buildfarm likes this iteration any better. > It occurred to me that the documentation still suggests that you should > add a declaration to a C function; I have

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-25 Thread Albe Laurenz
I wrote: > Anyway, I have prepared a patch along the lines you suggest. It occurred to me that the documentation still suggests that you should add a declaration to a C function; I have fixed that too. I'll add the patch to the next commitfest. Yours, Laurenz Albe

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-24 Thread Albe Laurenz
Tom Lane wrote: > I poked at this a little bit. AFAICT, the only actual cross-file > references are in contrib/ltree/, which has quite a few. Maybe we > could hold our noses and attach PGDLLEXPORT to the declarations in > ltree.h. > > hstore's HSTORE_POLLUTE macro would also need

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-18 Thread Tom Lane
Albe Laurenz writes: > The buildfarm log at > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thrips=2016-10-12%2018%3A37%3A26 > shows the build failing (among others) for contrib/tablefunc > (close to the bottom end of the log). That's a build of 9.6. > Now when

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-18 Thread Albe Laurenz
Tom Lane wrote: > No, it's cross *file* references within a single contrib module that > would be likely to need extern declarations in a header file. That's > not especially weird IMO. I'm not sure how many cases there actually > are though. I went through the contrib moules and tried to look

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-18 Thread Tom Lane
I wrote: > No, it's cross *file* references within a single contrib module that > would be likely to need extern declarations in a header file. That's > not especially weird IMO. I'm not sure how many cases there actually > are though. I poked at this a little bit. AFAICT, the only actual

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-18 Thread Tom Lane
Craig Ringer writes: > On 18 October 2016 at 04:11, Tom Lane wrote: >> As for the core problem, I wonder why we aren't recommending that >> third-party modules be built using the same infrastructure contrib >> uses, rather than people ginning up their

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-18 Thread Tom Lane
Craig Ringer writes: > On 18 October 2016 at 04:19, Andres Freund wrote: >> On 2016-10-17 16:16:37 -0400, Robert Haas wrote: >>> I wouldn't think that cross-file references would be especially >>> common. Functions that take PG_FUNCTION_ARGS and return

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Pavel Stehule
2016-10-18 5:48 GMT+02:00 Craig Ringer : > On 18 October 2016 at 04:19, Andres Freund wrote: > > On 2016-10-17 16:16:37 -0400, Robert Haas wrote: > >> I wouldn't think that cross-file references would be especially > >> common. Functions that take

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Craig Ringer
On 18 October 2016 at 04:11, Tom Lane wrote: > As for the core problem, I wonder why we aren't recommending that > third-party modules be built using the same infrastructure contrib > uses, rather than people ginning up their own infrastructure and > then finding out the hard

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Craig Ringer
On 18 October 2016 at 04:19, Andres Freund wrote: > On 2016-10-17 16:16:37 -0400, Robert Haas wrote: >> I wouldn't think that cross-file references would be especially >> common. Functions that take PG_FUNCTION_ARGS and return Datum aren't >> a lot of fun to call from C. But

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Yury Zhuravlev
Robert Haas wrote: Yeah, I don't know. For my money, decorating the function definitions in place seems easier than having to maintain a separate export list, especially if it can be hidden under the carpet using the existing stupid macro tricks. But I am not a Windows expert. I suppose we

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Yury Zhuravlev
17 октября 2016 г. 23:42:30 MSK, Tom Lane wrote: [ wanders away wondering what cmake does with this... ] CMake can export all symbols using only one setting - WINDOWS_EXPORT_ALL_SYMBOLS for shared libraries and special for Postgres I made "export all" for executable files. You can try this

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Robert Haas
On Mon, Oct 17, 2016 at 4:42 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Oct 17, 2016 at 4:11 PM, Tom Lane wrote: >>> As for the core problem, I wonder why we aren't recommending that >>> third-party modules be built using

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Tom Lane
Robert Haas writes: > On Mon, Oct 17, 2016 at 4:11 PM, Tom Lane wrote: >> As for the core problem, I wonder why we aren't recommending that >> third-party modules be built using the same infrastructure contrib >> uses, rather than people ginning up

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Andres Freund
On 2016-10-17 16:16:37 -0400, Robert Haas wrote: > I wouldn't think that cross-file references would be especially > common. Functions that take PG_FUNCTION_ARGS and return Datum aren't > a lot of fun to call from C. But maybe I'm wrong. There's a fair number of DirectFunctionCall$Ns over the

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Robert Haas
On Mon, Oct 17, 2016 at 4:11 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Oct 14, 2016 at 10:14 AM, Albe Laurenz >> wrote: >>> Actually I would say that the correct solution is to remove the function >>> declarations

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Tom Lane
Robert Haas writes: > On Fri, Oct 14, 2016 at 10:14 AM, Albe Laurenz > wrote: >> Actually I would say that the correct solution is to remove the function >> declarations from all the header files in contrib, since from commit e7128e8d >> on the

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Robert Haas
On Fri, Oct 14, 2016 at 10:14 AM, Albe Laurenz wrote: > Tom Lane wrote: >>> Well, the buildfarm doesn't like that even a little bit. It seems that >>> the MSVC compiler does not like seeing both "extern Datum foo(...)" and >>> "extern PGDLLEXPORT Datum foo(...)", so

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-17 Thread Albe Laurenz
I wrote: > But I'd understand if you think that this is too much code churn for too > little > benefit, even if it could be considered a clean-up. > > In that case, I'd argue that in the sample in doc/src/sgml/xfunc.sgml > the function definitions should be changed to read > > PGDLLEXPORT

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-14 Thread Albe Laurenz
Tom Lane wrote: >> Well, the buildfarm doesn't like that even a little bit. It seems that >> the MSVC compiler does not like seeing both "extern Datum foo(...)" and >> "extern PGDLLEXPORT Datum foo(...)", so anything that had an extern in >> a .h file is failing. There is also quite a bit of

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-13 Thread Craig Ringer
On 13 Oct. 2016 05:28, "Tom Lane" wrote: > > I wrote: > > Albe Laurenz writes: > >> Tom Lane wrote: > >>> I'm okay with adding PGDLLEXPORT to the extern, but we should update > >>> that comment to note that it's not necessary with any of our standard

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
I wrote: > Well, the buildfarm doesn't like that even a little bit. It seems that > the MSVC compiler does not like seeing both "extern Datum foo(...)" and > "extern PGDLLEXPORT Datum foo(...)", so anything that had an extern in > a .h file is failing. There is also quite a bit of

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
I wrote: > Albe Laurenz writes: >> Tom Lane wrote: >>> I'm okay with adding PGDLLEXPORT to the extern, but we should update >>> that comment to note that it's not necessary with any of our standard >>> Windows build processes. (For that matter, the comment fails to

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
Albe Laurenz writes: > Tom Lane wrote: >> I'm okay with adding PGDLLEXPORT to the extern, but we should update >> that comment to note that it's not necessary with any of our standard >> Windows build processes. (For that matter, the comment fails to explain >> why this

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Albe Laurenz
Tom Lane wrote: > I'm okay with adding PGDLLEXPORT to the extern, but we should update > that comment to note that it's not necessary with any of our standard > Windows build processes. (For that matter, the comment fails to explain > why this macro is providing an extern for the base function at

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
Albe Laurenz writes: > Tom Lane wrote: >> Except that we don't. There aren't PGDLLEXPORT markings for any >> functions exported from contrib modules, and we don't use dlltool >> on them either. By your argument, none of contrib would work on >> Windows builds at all,

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Albe Laurenz
Tom Lane wrote: >> PostgreSQL itself seems to use export files that explicitly declare the >> exported symbols, so it gets away without these decorations. > > Except that we don't. There aren't PGDLLEXPORT markings for any > functions exported from contrib modules, and we don't use dlltool > on

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
Albe Laurenz writes: > Tom Lane wrote: >> The lack of complaints about this suggest that it's not actually necessary >> to do so. So what this makes me wonder is whether we can't drop the >> DLLEXPORT on the finfo function too. I'd rather reduce the number of >>

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Albe Laurenz
Tom Lane wrote: > Albe Laurenz writes: >> Currently, PG_FUNCTION_INFO_V1 is defined as [...] > >> Is there a good reason why the "funcname" declaration is not decorated >> with PGDLLEXPORT? > > The lack of complaints about this suggest that it's not actually necessary >

Re: [HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Tom Lane
Albe Laurenz writes: > Currently, PG_FUNCTION_INFO_V1 is defined as > /* >* Macro to build an info function associated with the given function name. >* Win32 loadable functions usually link with 'dlltool --export-all', but > it >* doesn't hurt to add

[HACKERS] Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

2016-10-12 Thread Albe Laurenz
Currently, PG_FUNCTION_INFO_V1 is defined as /* * Macro to build an info function associated with the given function name. * Win32 loadable functions usually link with 'dlltool --export-all', but it * doesn't hurt to add PGDLLIMPORT in case they don't. */ #define