Re: pg_upgrade failure on Windows Server

On Thu, Mar 19, 2015 at 12:25 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> wrote:

> On Thu, Mar 19, 2015 at 6:42 AM, Asif Naeem wrote:
> > I do agree, pg_ctl refactoring is making current patch more complicated,
> > eventually we can come up with robust patch that include pg_ctl but
> result
> > patch could be big and touching lot of areas, I doubt that if such
> > complicated and big patch going to make its way into the repository. It
> > seems appropriate to do incremental work, we can refactor pg_ctl changes
> as
> > next effort. PFA update patch, it removes pg_ctl related code changes.
> > Please do let me know if I missed something. Thanks.
>
> After looking closely at those things, I agree with you: having some
> code duplicated in two places instead of five is still a win... Now, I
> think having this declaration of write_stderr in restricted_token.c is
> confusing:
> +#define write_stderr(fmt, ...) fprintf(stderr, fmt, __VA_ARGS__)
> We could have bad surprises in the future if there is some work to
> link pg_ctl with the stuff of restricted_token.c. Hence, could you
> replace that with plain calls to fprintf(stderr, ...)? That's as well
> what the other files of src/common are doing, so it would make the new
> code more consistent with the rest.
>
> [nitpicking]
> --- a/src/test/regress/pg_regress.c
> +++ b/src/test/regress/pg_regress.c
> @@ -30,6 +30,7 @@
> #endif
>
> #include "common/username.h"
> +#include "common/restricted_token.h"
>
> Alphabetical order of headers.
> [/nitpicking]
>
> Except those two small things, well I guess that's it for this patch.
>

Thank you so much. PFA v5 updated patch with above 2 changes.

> The stuff for write_stderr may need a TODO item, but I think that even
> with that we are going to finish by fixing the call to isatty that
> looks indeed strange...
>

+1

>
> For the backpatching, the patches sent previously here (=>
>
> http://www.postgresql.org/message-id/CAEB4t-OHNE95=n5U4ySsYkWipQsWeQuTBSJkaYJ63_1VzkzkhA@mail.gmail.com
> )
> are fine IMO. They simply consist of a copy of what is done in
> initdb.c. Now, perhaps we had better apply the patch duplicating the
> logic to all branches, including HEAD, first, see what the buildfarm
> says, and then finish wrapping up the refactoring patch.
>

PFA patch for older branches, v1 patch was not applying cleaning other than
94. Thanks.

> Regards,
> --
> Michael
>