Re: pgsql: Unify some tar functionality across different parts

On Wed, Jan 2, 2013 at 10:06 AM, Magnus Hagander <magnus@hagander.net> wrote:
>
> On Wed, Jan 2, 2013 at 4:14 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>>
>> On 01/01/2013 12:25 PM, Magnus Hagander wrote:
>>>
>>> Unify some tar functionality across different parts
>>>
>>> Move some of the tar functionality that existed mostly duplicated
>>> in both pg_dump and the walsender basebackup functionality into
>>> port/tar.c instead, so it can be used from both. It will also be
>>> used by pg_basebackup in the future, which would've caused a third
>>> copy of it around.
>>>
>>>
>>
>>
>> This seems to have broken plperl builds on Windows. Not sure what the fix
>> is.
>>
>
> It seems if plperl exists it does *not* have the uid_t and gid_t datatypes?
> port.h has this:
>
> /*
>  * Supplement to <sys/types.h>.
>  *
>  * Perl already has typedefs for uid_t and gid_t.
>  */
> #ifndef PLPERL_HAVE_UID_GID
> typedef int uid_t;
> typedef int gid_t;
> #endif
>
> So either that's not true anymore, or we're including things in the wrong
> order, I think.
>
> I'm not sure where those are supposed to come from - they're not mentioned
> anywhere in plperl. But maybe they're leaking in from the global perl
> headers?
>
> We could fix this by just changing the parameters to the tarCreateHeader()
> function to take int instead of uid_t, but that seems like the wrong fix to
> me. We should at least try to figure out why it's happening in the first
> place. What happens if you just remove that #ifndef on a win build with perl
> (sorry, don't have one around at the time - so if you have one, it would me
> much helpful if you could test it) - do we get a different error, or have we
> changed around the code and just forgotten that ifdef?

Got an env working.

So it is an ordering issue. If I remove that #ifdef, I get an error
that the perl headers are redefining uid_t and gid_t to the same thing
that they already are. So basically our problem is that the perl
headers are polluting the namespace, and our old workaround doesn't
work due to include ordering.

The problem is that this part of port/win32.h relies on it being
included first, and then the perl headers, before anybody can use
uid_t. Which means we can't use it anywhere in our own headers, which
is what causes this problem.

If it was a #define, we could just #undef it before including the perl
headers. But since it's a typedef, we can't.

I'm not really sure what the best thing is to do here. We can either
use the fact that we know that uid_t and gid_t are "int" on all our
platforms, more or less, and change the tar api to use uid_t. Or put
the tar functions in their own header and make sure we don't include
that one anywhere that we include the perl headers.

Thoughts?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/