Michael Paquier <mich...@paquier.xyz> writes:
> Now, I also see that using pgwin32_setenv() instead of
> src/port/setenv.c causes cl to be confused once we update
> ecpg_regression.proj because it cannot find setenv(). Bringing the
> question, why is it necessary to have both setenv.c and
> pgwin32_setenv() on HEAD? setenv.c should be enough once you have the
> fallback implementation of putenv() available.
IIUC, what you are proposing to do is replace pgwin32_setenv with
src/port/setenv.c. I don't think that's an improvement. setenv.c
leaks memory on repeat calls, because it cannot know what
pgwin32_setenv knows about how putenv works on that platform.
It'd be okay to do it like that for the ECPG tests, perhaps,
because we don't really care about small leaks in those.
But I don't want it to happen across-the-board.
Thinking more, the real problem is that use of libpgport
goes hand-in-hand with #including port.h; it's not going
to work real well if you do one without the other.
And I don't think we want to include port.h in the ECPG
test programs, because those are trying to model the
environment that typical user applications see.
Alternatives seem to be
(1) allow just this one ECPG test to include port.h (or
probably c.h). However, there's a whole other can of worms
there, which is that I wonder if we aren't doing it wrong
on the Unix side by linking libpgport when we shouldn't.
We've not been bit by that yet, but I wonder if it isn't
just a matter of time. The MSVC build, by not linking
those libs in the first place, is really doing this the
correct way.
(2) Let pg_regress_ecpg.c pass down the environment setting.
(3) Don't try to use the environment variable for this
purpose. I'd originally tried to change test5.pgc to just
specify gssmode=disable in-line, but that only works
nicely for one of the two failing cases. The other one
is testing the case of a completely defaulted connection
target, so there's no place to add an option without
breaking the only unique aspect of that test case.
(2) is starting to seem attractive now that we've seen
the downsides of (1) and (3).
(BTW, I just noticed that regress.c is unsetenv'ing the
SSL connection environment variables, but not the GSS ones.
Seens like that needs work similar to 8279f68a1.)
regards, tom lane
