Re: [PATCH] gc: notice gc processes run by other users

2014-01-02 Thread Kyle 

On Dec 31, 2013, at 04:07, Kyle J. McKay wrote:

Since 64a99eb4 git gc refuses to run without the --force option if
another gc process on the same repository is already running.

However, if the repository is shared and user A runs git gc on the
repository and while that gc is still running user B runs git gc on
the same repository the gc process run by user A will not be noticed
and the gc run by user B will go ahead and run.

The problem is that the kill(pid, 0) test fails with an EPERM error
since user B is not allowed to signal processes owned by user A
(unless user B is root).

Update the test to recognize an EPERM error as meaning the process
exists and another gc should not be run (unless --force is given).


Oops, sorry, forgot sign off, here's my sign off:

Signed-off-by: Kyle J. McKay mack...@gmail.com


---

I suggest this be included in maint as others may also have expected  
the

shared repository, different user gc scenario to be caught by the new
code when in fact it's not without this patch.

builtin/gc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c14190f8..25f2237c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -222,7 +222,7 @@ static const char *lock_repo_for_gc(int force,  
pid_t* ret_pid)

time(NULL) - st.st_mtime = 12 * 3600 
fscanf(fp, %PRIuMAX %127c, pid, locking_host) == 2 

/* be gentle to concurrent gc on remote hosts */
-   (strcmp(locking_host, my_host) || !kill(pid, 0));
+			(strcmp(locking_host, my_host) || !kill(pid, 0) || errno ==  
EPERM);

if (fp != NULL)
fclose(fp);
if (should_exit) {
--
1.8.5.2



--
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

[PATCH] gc: notice gc processes run by other users

2013-12-31 Thread Kyle J. McKay 
Since 64a99eb4 git gc refuses to run without the --force option if
another gc process on the same repository is already running.

However, if the repository is shared and user A runs git gc on the
repository and while that gc is still running user B runs git gc on
the same repository the gc process run by user A will not be noticed
and the gc run by user B will go ahead and run.

The problem is that the kill(pid, 0) test fails with an EPERM error
since user B is not allowed to signal processes owned by user A
(unless user B is root).

Update the test to recognize an EPERM error as meaning the process
exists and another gc should not be run (unless --force is given).
---

I suggest this be included in maint as others may also have expected the
shared repository, different user gc scenario to be caught by the new
code when in fact it's not without this patch.

 builtin/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c14190f8..25f2237c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -222,7 +222,7 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
time(NULL) - st.st_mtime = 12 * 3600 
fscanf(fp, %PRIuMAX %127c, pid, locking_host) == 2 

/* be gentle to concurrent gc on remote hosts */
-   (strcmp(locking_host, my_host) || !kill(pid, 0));
+   (strcmp(locking_host, my_host) || !kill(pid, 0) || 
errno == EPERM);
if (fp != NULL)
fclose(fp);
if (should_exit) {
-- 
1.8.5.2

--
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] gc: notice gc processes run by other users

2013-12-31 Thread Duy Nguyen 
On Tue, Dec 31, 2013 at 7:07 PM, Kyle J. McKay mack...@gmail.com wrote:
 Since 64a99eb4 git gc refuses to run without the --force option if
 another gc process on the same repository is already running.

 However, if the repository is shared and user A runs git gc on the
 repository and while that gc is still running user B runs git gc on
 the same repository the gc process run by user A will not be noticed
 and the gc run by user B will go ahead and run.

 The problem is that the kill(pid, 0) test fails with an EPERM error
 since user B is not allowed to signal processes owned by user A
 (unless user B is root).

 Update the test to recognize an EPERM error as meaning the process
 exists and another gc should not be run (unless --force is given).

Ack. Looking at kill(2) the other errors are EINVAL and ESRCH, which
are fine to ignore.

 ---

 I suggest this be included in maint as others may also have expected the
 shared repository, different user gc scenario to be caught by the new
 code when in fact it's not without this patch.

  builtin/gc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/builtin/gc.c b/builtin/gc.c
 index c14190f8..25f2237c 100644
 --- a/builtin/gc.c
 +++ b/builtin/gc.c
 @@ -222,7 +222,7 @@ static const char *lock_repo_for_gc(int force, pid_t* 
 ret_pid)
 time(NULL) - st.st_mtime = 12 * 3600 
 fscanf(fp, %PRIuMAX %127c, pid, locking_host) == 
 2 
 /* be gentle to concurrent gc on remote hosts */
 -   (strcmp(locking_host, my_host) || !kill(pid, 0));
 +   (strcmp(locking_host, my_host) || !kill(pid, 0) || 
 errno == EPERM);
 if (fp != NULL)
 fclose(fp);
 if (should_exit) {
 --
 1.8.5.2




-- 
Duy
--
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] gc: notice gc processes run by other users

2013-12-31 Thread Thiago Farina 
On Tue, Dec 31, 2013 at 10:07 AM, Kyle J. McKay mack...@gmail.com wrote:
 Since 64a99eb4 git gc refuses to run without the --force option if
 another gc process on the same repository is already running.

 However, if the repository is shared and user A runs git gc on the
 repository and while that gc is still running user B runs git gc on
 the same repository the gc process run by user A will not be noticed
 and the gc run by user B will go ahead and run.

 The problem is that the kill(pid, 0) test fails with an EPERM error
 since user B is not allowed to signal processes owned by user A
 (unless user B is root).

 Update the test to recognize an EPERM error as meaning the process
 exists and another gc should not be run (unless --force is given).

Looks like you are missing your Signed-off-by: line.

--
Thiago Farina
--
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