Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 18/01/2016 18:32, Andrew Dunstan wrote: On 01/14/2016 12:38 AM, Michael Paquier wrote: Hi all, Beginning a new thread seems more adapted regarding $subject but that's mentioned here as well: http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com Marco, as I think you do some packaging for Postgres in Cygwin, and others, does that sound acceptable? I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. cheers andrew builds fine also here on 64bit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On Tue, Jan 19, 2016 at 2:32 AM, Andrew Dunstan wrote: > > > On 01/14/2016 12:38 AM, Michael Paquier wrote: >> >> Hi all, >> >> Beginning a new thread seems more adapted regarding $subject but >> that's mentioned here as well: >> >> http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com >> >> It happens based on some investigation from Andrew that cygwin does >> not need to use the service-related code, aka register/unregister >> options or similar that are proper to WIN32 and rely instead on >> cygrunsrv with a SYSV-like init file to manage the service. Based on >> my tests with cygwin, I am able to see as well that a server started >> within a cygwin session is able to persist even after it is closed, >> which is kind of nice and I think that it is a additional reason to >> not use the service-related code paths. Hence what about the following >> patch, that makes cygwin behave like any *nix OS when using pg_ctl? >> This simplifies a bit the code. >> >> Marco, as I think you do some packaging for Postgres in Cygwin, and >> others, does that sound acceptable? >> > > > > I think we can be a bit more adventurous and remove all the Cygwin-specific > code. See attached patch, which builds fine on buildfarm cockatiel. Ah, OK. I see the difference. It builds as well for me. -#ifndef __CYGWIN__ - AddUserToTokenDacl(restrictedToken); -#endif [...] -#if defined(WIN32) || defined(__CYGWIN__) +#ifdef WIN32 setvbuf(stderr, NULL, _IONBF, 0); #endif Fine for me, those two do not seem to matter much as far as I have tested. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 01/18/2016 03:46 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: On 01/18/2016 12:38 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. Hopefully you also tested that it builds under MSVC :-) Why would I? This isn't having the slightest effect on MSVC builds. You never know ... you might have inadvertently broken some WIN32 block. If you can point out a line where that might be true I'll test it ;-) cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
Andrew Dunstan wrote: > > > On 01/18/2016 12:38 PM, Alvaro Herrera wrote: > >Andrew Dunstan wrote: > > > >>I think we can be a bit more adventurous and remove all the Cygwin-specific > >>code. See attached patch, which builds fine on buildfarm cockatiel. > >Hopefully you also tested that it builds under MSVC :-) > > Why would I? This isn't having the slightest effect on MSVC builds. You never know ... you might have inadvertently broken some WIN32 block. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 01/18/2016 12:38 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. Hopefully you also tested that it builds under MSVC :-) Why would I? This isn't having the slightest effect on MSVC builds. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
Andrew Dunstan wrote: > I think we can be a bit more adventurous and remove all the Cygwin-specific > code. See attached patch, which builds fine on buildfarm cockatiel. Hopefully you also tested that it builds under MSVC :-) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 01/14/2016 12:38 AM, Michael Paquier wrote: Hi all, Beginning a new thread seems more adapted regarding $subject but that's mentioned here as well: http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com It happens based on some investigation from Andrew that cygwin does not need to use the service-related code, aka register/unregister options or similar that are proper to WIN32 and rely instead on cygrunsrv with a SYSV-like init file to manage the service. Based on my tests with cygwin, I am able to see as well that a server started within a cygwin session is able to persist even after it is closed, which is kind of nice and I think that it is a additional reason to not use the service-related code paths. Hence what about the following patch, that makes cygwin behave like any *nix OS when using pg_ctl? This simplifies a bit the code. Marco, as I think you do some packaging for Postgres in Cygwin, and others, does that sound acceptable? I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. cheers andrew diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 919d764..98aa1a0 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -39,13 +39,6 @@ #include "getopt_long.h" #include "miscadmin.h" -#if defined(__CYGWIN__) -#include -#include -/* Cygwin defines WIN32 in windows.h, but we don't want it. */ -#undef WIN32 -#endif - /* PID can be negative for standalone backend */ typedef long pgpid_t; @@ -105,7 +98,7 @@ static char backup_file[MAXPGPATH]; static char recovery_file[MAXPGPATH]; static char promote_file[MAXPGPATH]; -#if defined(WIN32) || defined(__CYGWIN__) +#ifdef WIN32 static DWORD pgctl_start_type = SERVICE_AUTO_START; static SERVICE_STATUS status; static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0; @@ -133,7 +126,7 @@ static void do_kill(pgpid_t pid); static void print_msg(const char *msg); static void adjust_data_dir(void); -#if defined(WIN32) || defined(__CYGWIN__) +#ifdef WIN32 #if (_MSC_VER >= 1800) #include #else @@ -165,7 +158,7 @@ static void unlimit_core_size(void); #endif -#if defined(WIN32) || defined(__CYGWIN__) +#ifdef WIN32 static void write_eventlog(int level, const char *line) { @@ -207,20 +200,11 @@ write_stderr(const char *fmt,...) va_list ap; va_start(ap, fmt); -#if !defined(WIN32) && !defined(__CYGWIN__) +#ifndef WIN32 /* On Unix, we just fprintf to stderr */ vfprintf(stderr, fmt, ap); #else -/* - * On Cygwin, we don't yet have a reliable mechanism to detect when - * we're being run as a service, so fall back to the old (and broken) - * stderr test. - */ -#ifdef __CYGWIN__ -#define pgwin32_is_service() (isatty(fileno(stderr))) -#endif - /* * On Win32, we print to stderr if running on a console, or write to * eventlog if running as a service @@ -718,7 +702,7 @@ test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint) #endif /* No response, or startup still in process; wait */ -#if defined(WIN32) +#ifdef WIN32 if (do_checkpoint) { /* @@ -1342,7 +1326,7 @@ do_kill(pgpid_t pid) } } -#if defined(WIN32) || defined(__CYGWIN__) +#ifdef WIN32 #if (_MSC_VER < 1800) static bool @@ -1408,20 +1392,6 @@ pgwin32_CommandLine(bool registration) } } -#ifdef __CYGWIN__ - /* need to convert to windows path */ - { - char buf[MAXPGPATH]; - -#if CYGWIN_VERSION_DLL_MAJOR >= 1007 - cygwin_conv_path(CCP_POSIX_TO_WIN_A, cmdPath, buf, sizeof(buf)); -#else - cygwin_conv_to_full_win32_path(cmdPath, buf); -#endif - strcpy(cmdPath, buf); - } -#endif - /* if path does not end in .exe, append it */ if (strlen(cmdPath) < 4 || pg_strcasecmp(cmdPath + strlen(cmdPath) - 4, ".exe") != 0) @@ -1775,10 +1745,8 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken)) { /* - * Most Windows targets make DWORD a 32-bit unsigned long. Cygwin - * x86_64, an LP64 target, makes it a 32-bit unsigned int. In code - * built for Cygwin as well as for native Windows targets, cast DWORD - * before printing. + * Most Windows targets make DWORD a 32-bit unsigned long, but + * in case it doesn't cast DWORD before printing. */ write_stderr(_("%s: could not open process token: error code %lu\n"), progname, (unsigned long) GetLastError()); @@ -1819,10 +1787,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser return 0; } -#ifndef __CYGWIN__ - AddUserToTokenDacl(restrictedToken); -#endif - r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo); Kernel32Handle = LoadLibrary("KERNEL32.DLL"); @@ -1926,7 +1890,7 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser */
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 14/01/2016 06:38, Michael Paquier wrote: Hi all, Beginning a new thread seems more adapted regarding $subject but that's mentioned here as well: http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com It happens based on some investigation from Andrew that cygwin does not need to use the service-related code, aka register/unregister options or similar that are proper to WIN32 and rely instead on cygrunsrv with a SYSV-like init file to manage the service. Based on my tests with cygwin, I am able to see as well that a server started within a cygwin session is able to persist even after it is closed, which is kind of nice and I think that it is a additional reason to not use the service-related code paths. Hence what about the following patch, that makes cygwin behave like any *nix OS when using pg_ctl? This simplifies a bit the code. Marco, as I think you do some packaging for Postgres in Cygwin, and others, does that sound acceptable? Regards, In general cygwin to behave like any *nix OS is the preferred solution. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Removing service-related code in pg_ctl for Cygwin
Hi all, Beginning a new thread seems more adapted regarding $subject but that's mentioned here as well: http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com It happens based on some investigation from Andrew that cygwin does not need to use the service-related code, aka register/unregister options or similar that are proper to WIN32 and rely instead on cygrunsrv with a SYSV-like init file to manage the service. Based on my tests with cygwin, I am able to see as well that a server started within a cygwin session is able to persist even after it is closed, which is kind of nice and I think that it is a additional reason to not use the service-related code paths. Hence what about the following patch, that makes cygwin behave like any *nix OS when using pg_ctl? This simplifies a bit the code. Marco, as I think you do some packaging for Postgres in Cygwin, and others, does that sound acceptable? Regards, -- Michael cygwin-removal-service-master.patch Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers