Re: [msysGit] [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Hi Rupert, On 2015-04-23 21:25, rupert thurner wrote: On Thursday, April 16, 2015 at 2:45:11 PM UTC+2, Johannes Schindelin wrote: However, using this code for `getppid()` would be serious overkill (not to mention an unbearable performance hit because you have to enumerate *all* processes to get that information). is the windows JobObject similar to processgroup? at least killing the parent process in a jobobject will kill all childs as well: https://msdn.microsoft.com/en-us/library/windows/desktop/ms681949%28v=vs.85%29.aspx Reading this page carefully reveals that you have to construct JobObjects explicitly. So you cannot get from a process ID to a JobObject; there is probably none. Or there is one. Or many. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Hi Junio,
On 2015-04-15 20:48, Junio C Hamano wrote:
Johannes Sixt j...@kdbg.org writes:
Windows does not have process groups. It is, therefore, the simplest
to pretend that each process is in its own process group.
While here, move the getppid() stub from its old location (between
two sync related functions) next to the two new functions.
Signed-off-by: Johannes Sixt j...@kdbg.org
---
Thanks for a quick update.
The patch should do for now, but I suspect that it may give us a
better abstraction to make the is_foreground_fd(int fd) or even
is_foreground(void) the public API that would be implemented as
int we_are_in_the_foreground(void)
{
return getpgid(0) == tcgetpgrp(fileno(stderr));
}
in POSIX and Windows can implement entirely differently.
I really like it. We already require a couple of workarounds to be able to use
`mintty` (which is a replacement terminal emulator that is more flexible than
the default Windows terminal window, but it comes at the price that most Win32
programs think they are not running interactively inside a `mintty` session). I
would really like to avoid having to finagle some really ugly code into
`getpgid()` to make that test work.
In general, I find it rewarding not only from a portability point of view, but
*especially* from a readability one if the code contains functions that are
named semantically, i.e. they describe *why* they are called, not *how* they
answer the question.
In this case, I would prefer the `is_foreground_fd(int fd)` one, but I am sure
I can make the other signature work for us, too.
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Hi kusma, On 2015-04-15 21:43, Erik Faye-Lund wrote: On Wed, Apr 15, 2015 at 8:29 PM, Johannes Sixt j...@kdbg.org wrote: Windows does not have process groups. It is, therefore, the simplest to pretend that each process is in its own process group. Windows does have some concept of process groups, but probably not quite what you want: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682083%28v=vs.85%29.aspx Yes, and we actually need that in Git for Windows anyway because shooting down a process does not kill its child processes: https://github.com/git-for-windows/msys2-runtime/commit/15f209511985092588b171703e5046eba937b47b#diff-8753cda163376cee6c80aab11eb8701fR402 However, using this code for `getppid()` would be serious overkill (not to mention an unbearable performance hit because you have to enumerate *all* processes to get that information). Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Johannes Schindelin johannes.schinde...@gmx.de writes:
On 2015-04-15 20:48, Junio C Hamano wrote:
The patch should do for now, but I suspect that it may give us a
better abstraction to make the is_foreground_fd(int fd) or even
is_foreground(void) the public API that would be implemented as
int we_are_in_the_foreground(void)
{
return getpgid(0) == tcgetpgrp(fileno(stderr));
}
in POSIX and Windows can implement entirely differently.
...
In general, I find it rewarding not only from a portability point of
view, but *especially* from a readability one if the code contains
functions that are named semantically, i.e. they describe *why* they
are called, not *how* they answer the question.
Yeah, that was the rationale behind the suggestion (i.e. how may be
different depending on the platform, and the main code flow cares
more about why and not how).
I'll queue Luke's patch with J6t's compat/ for now, but if you find
more suitable implementation for the higher-level abstraction,
please do send a follow-up patch to update it.
Two Johannes'es may need to talk between themselves to agree what
the right level of abstraction is, though. I trust two of you a lot
more than my gut feeling when it comes to POSIX vs Windows API
differences ;-)
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
On Wed, Apr 15, 2015 at 08:29:30PM +0200, Johannes Sixt wrote:
| Windows does not have process groups. It is, therefore, the simplest
| to pretend that each process is in its own process group.
|
| [...]
|
| diff --git a/compat/mingw.h b/compat/mingw.h
| index 7b523cf..a552026 100644
| @@ -118,6 +116,12 @@ static inline int sigaddset(sigset_t *set, int signum)
| #define SIG_UNBLOCK 0
| static inline int sigprocmask(int how, const sigset_t *set, sigset_t
*oldset)
| { return 0; }
| +static inline pid_t getppid(void)
| +{ return 1; }
| +static inline pid_t getpgid(pid_t pid)
| +{ return pid == 0 ? getpid() : pid; }
| +static inline pid_t tcgetpgrp(int fd)
| +{ return getpid(); }
This appears to be similar to the approach that tcsh uses too;
return the current process ID for the process group ID.
See https://github.com/tcsh-org/tcsh/blob/master/win32/ntport.h
for tcsh's implementation of getpgrp() (a variation of getpgid())
and tcgetpgrp().
regards,
Luke.
pgpyN0nrBi1n3.pgp
Description: PGP signature
[PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Windows does not have process groups. It is, therefore, the simplest
to pretend that each process is in its own process group.
While here, move the getppid() stub from its old location (between
two sync related functions) next to the two new functions.
Signed-off-by: Johannes Sixt j...@kdbg.org
---
compat/mingw.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 7b523cf..a552026 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -95,8 +95,6 @@ static inline unsigned int alarm(unsigned int seconds)
{ return 0; }
static inline int fsync(int fd)
{ return _commit(fd); }
-static inline pid_t getppid(void)
-{ return 1; }
static inline void sync(void)
{}
static inline uid_t getuid(void)
@@ -118,6 +116,12 @@ static inline int sigaddset(sigset_t *set, int signum)
#define SIG_UNBLOCK 0
static inline int sigprocmask(int how, const sigset_t *set, sigset_t *oldset)
{ return 0; }
+static inline pid_t getppid(void)
+{ return 1; }
+static inline pid_t getpgid(pid_t pid)
+{ return pid == 0 ? getpid() : pid; }
+static inline pid_t tcgetpgrp(int fd)
+{ return getpid(); }
/*
* simple adaptors
--
2.3.2.245.gb5bf9d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Johannes Sixt j...@kdbg.org writes:
Windows does not have process groups. It is, therefore, the simplest
to pretend that each process is in its own process group.
While here, move the getppid() stub from its old location (between
two sync related functions) next to the two new functions.
Signed-off-by: Johannes Sixt j...@kdbg.org
---
Thanks for a quick update.
The patch should do for now, but I suspect that it may give us a
better abstraction to make the is_foreground_fd(int fd) or even
is_foreground(void) the public API that would be implemented as
int we_are_in_the_foreground(void)
{
return getpgid(0) == tcgetpgrp(fileno(stderr));
}
in POSIX and Windows can implement entirely differently.
Thoughts?
compat/mingw.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 7b523cf..a552026 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -95,8 +95,6 @@ static inline unsigned int alarm(unsigned int seconds)
{ return 0; }
static inline int fsync(int fd)
{ return _commit(fd); }
-static inline pid_t getppid(void)
-{ return 1; }
static inline void sync(void)
{}
static inline uid_t getuid(void)
@@ -118,6 +116,12 @@ static inline int sigaddset(sigset_t *set, int signum)
#define SIG_UNBLOCK 0
static inline int sigprocmask(int how, const sigset_t *set, sigset_t *oldset)
{ return 0; }
+static inline pid_t getppid(void)
+{ return 1; }
+static inline pid_t getpgid(pid_t pid)
+{ return pid == 0 ? getpid() : pid; }
+static inline pid_t tcgetpgrp(int fd)
+{ return getpid(); }
/*
* simple adaptors
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
Am 15.04.2015 um 20:48 schrieb Junio C Hamano:
The patch should do for now, but I suspect that it may give us a
better abstraction to make the is_foreground_fd(int fd) or even
is_foreground(void) the public API that would be implemented as
int we_are_in_the_foreground(void)
{
return getpgid(0) == tcgetpgrp(fileno(stderr));
}
in POSIX and Windows can implement entirely differently.
Thoughts?
IMHO, this level of abstraction goes too far. It may be that I am not
familiar with process groups, and I find the implementation of
is_foreground_fd() in progress.c close to where it's used quite
educating and enlightening. Hiding a similar implementation miles away
in cache.h and/or compat/ would not pay off for me.
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp()
On Wed, Apr 15, 2015 at 8:29 PM, Johannes Sixt j...@kdbg.org wrote: Windows does not have process groups. It is, therefore, the simplest to pretend that each process is in its own process group. Windows does have some concept of process groups, but probably not quite what you want: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682083%28v=vs.85%29.aspx -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
