Re: [PATCH v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-19 Thread Ronnie Sahlberg 
I have resent v19 that does
1, remove the spurios redundant errno line
2, fixes the type
and
3, reorders the patch as mentioned previously in this thread.

This I hope will be the final version of the series we will need.

regards
ronnie sahlberg

On Wed, Jun 18, 2014 at 3:38 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 06/19/2014 12:27 AM, Ronnie Sahlberg wrote:
 It looks like we need to reorder two of the patches.
 This patch needs to be moved to later in the series and happen after
 the delete_ref conversion :

 refs.c: make delete_ref use a transaction
 refs.c: add an err argument to delete_ref_loose

 That agrees with what I have found out since my first email.  The
 failures go away starting with the later commit that you mentioned.

 I will respin a v19 with these patches reordered.

 Michael

 --
 Michael Haggerty
 mhag...@alum.mit.edu
 http://softwareswirl.blogspot.com/
--
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 v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-18 Thread Michael Haggerty 
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote:
 Add an err argument to delete_loose_ref so that we can pass a descriptive
 error string back to the caller. Pass the err argument from transaction
 commit to this function so that transaction users will have a nice error
 string if the transaction failed due to delete_loose_ref.
 
 Add a new function unlink_or_err that we can call from delete_ref_loose. This
 function is similar to unlink_or_warn except that we can pass it an err
 argument. If err is non-NULL the function will populate err instead of
 printing a warning().
 
 Simplify warn_if_unremovable.
 [...]

I'm getting test failures starting with this commit:

 Test Summary Report
 ---
 t5514-fetch-multiple.sh  (Wstat: 256 Tests: 11 
 Failed: 3)
   Failed tests:  6, 8-9
   Non-zero exit status: 1
 t6050-replace.sh (Wstat: 256 Tests: 28 
 Failed: 1)
   Failed test:  15
   Non-zero exit status: 1
 t1400-update-ref.sh  (Wstat: 256 Tests: 133 
 Failed: 4)
   Failed tests:  86-87, 130-131
   Non-zero exit status: 1
 t5540-http-push-webdav.sh(Wstat: 256 Tests: 19 
 Failed: 2)
   Failed tests:  8-9
   Non-zero exit status: 1
 t5505-remote.sh  (Wstat: 256 Tests: 76 
 Failed: 5)
   Failed tests:  11, 45-48
   Non-zero exit status: 1
 t9903-bash-prompt.sh (Wstat: 256 Tests: 51 
 Failed: 1)
   Failed test:  19
   Non-zero exit status: 1
 t9300-fast-import.sh (Wstat: 256 Tests: 170 
 Failed: 1)
   Failed test:  71
   Non-zero exit status: 1
 t6030-bisect-porcelain.sh(Wstat: 256 Tests: 55 
 Failed: 47)
   Failed tests:  2-5, 7-11, 13-14, 16-30, 32-34, 36-37, 39-44
 46-55
   Non-zero exit status: 1
 t7512-status-help.sh (Wstat: 256 Tests: 35 
 Failed: 1)
   Failed test:  27
   Non-zero exit status: 1
 t5516-fetch-push.sh  (Wstat: 256 Tests: 80 
 Failed: 3)
   Failed tests:  47-49
   Non-zero exit status: 1

Let me know if you need more information.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-18 Thread Ronnie Sahlberg 
It looks like we need to reorder two of the patches.
This patch needs to be moved to later in the series and happen after
the delete_ref conversion :

refs.c: make delete_ref use a transaction
refs.c: add an err argument to delete_ref_loose

I will respin a v19 with these patches reordered.


Thanks,
ronine sahlberg


On Wed, Jun 18, 2014 at 1:47 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote:
 Add an err argument to delete_loose_ref so that we can pass a descriptive
 error string back to the caller. Pass the err argument from transaction
 commit to this function so that transaction users will have a nice error
 string if the transaction failed due to delete_loose_ref.

 Add a new function unlink_or_err that we can call from delete_ref_loose. This
 function is similar to unlink_or_warn except that we can pass it an err
 argument. If err is non-NULL the function will populate err instead of
 printing a warning().

 Simplify warn_if_unremovable.
 [...]

 I'm getting test failures starting with this commit:

 Test Summary Report
 ---
 t5514-fetch-multiple.sh  (Wstat: 256 Tests: 11 
 Failed: 3)
   Failed tests:  6, 8-9
   Non-zero exit status: 1
 t6050-replace.sh (Wstat: 256 Tests: 28 
 Failed: 1)
   Failed test:  15
   Non-zero exit status: 1
 t1400-update-ref.sh  (Wstat: 256 Tests: 133 
 Failed: 4)
   Failed tests:  86-87, 130-131
   Non-zero exit status: 1
 t5540-http-push-webdav.sh(Wstat: 256 Tests: 19 
 Failed: 2)
   Failed tests:  8-9
   Non-zero exit status: 1
 t5505-remote.sh  (Wstat: 256 Tests: 76 
 Failed: 5)
   Failed tests:  11, 45-48
   Non-zero exit status: 1
 t9903-bash-prompt.sh (Wstat: 256 Tests: 51 
 Failed: 1)
   Failed test:  19
   Non-zero exit status: 1
 t9300-fast-import.sh (Wstat: 256 Tests: 170 
 Failed: 1)
   Failed test:  71
   Non-zero exit status: 1
 t6030-bisect-porcelain.sh(Wstat: 256 Tests: 55 
 Failed: 47)
   Failed tests:  2-5, 7-11, 13-14, 16-30, 32-34, 36-37, 39-44
 46-55
   Non-zero exit status: 1
 t7512-status-help.sh (Wstat: 256 Tests: 35 
 Failed: 1)
   Failed test:  27
   Non-zero exit status: 1
 t5516-fetch-push.sh  (Wstat: 256 Tests: 80 
 Failed: 3)
   Failed tests:  47-49
   Non-zero exit status: 1

 Let me know if you need more information.

 Michael

 --
 Michael Haggerty
 mhag...@alum.mit.edu
 http://softwareswirl.blogspot.com/
--
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 v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-18 Thread Michael Haggerty 
On 06/19/2014 12:27 AM, Ronnie Sahlberg wrote:
 It looks like we need to reorder two of the patches.
 This patch needs to be moved to later in the series and happen after
 the delete_ref conversion :
 
 refs.c: make delete_ref use a transaction
 refs.c: add an err argument to delete_ref_loose

That agrees with what I have found out since my first email.  The
failures go away starting with the later commit that you mentioned.

 I will respin a v19 with these patches reordered.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-17 Thread Ronnie Sahlberg 
Add an err argument to delete_loose_ref so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_loose_ref.

Add a new function unlink_or_err that we can call from delete_ref_loose. This
function is similar to unlink_or_warn except that we can pass it an err
argument. If err is non-NULL the function will populate err instead of
printing a warning().

Simplify warn_if_unremovable.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c| 35 +--
 wrapper.c | 14 ++
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/refs.c b/refs.c
index 4c1612a..7223542 100644
--- a/refs.c
+++ b/refs.c
@@ -2543,16 +2543,38 @@ static int repack_without_ref(const char *refname)
return repack_without_refs(refname, 1, NULL);
 }
 
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int add_err_if_unremovable(const char *op, const char *file,
+ struct strbuf *e, int rc)
+{
+   int err = errno;
+   if (rc  0  errno != ENOENT) {
+   strbuf_addf(e, unable to %s %s: %s,
+   op, file, strerror(errno));
+   errno = err;
+   return -1;
+   }
+   return 0;
+}
+
+static int unlink_or_err(const char *file, struct strbuf *err)
+{
+   if (err)
+   return add_err_if_unremovable(unlink, file, err,
+ unlink(file));
+   else
+   return unlink_or_warn(file);
+}
+
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
*err)
 {
if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
/* loose */
-   int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+   int res, i = strlen(lock-lk-filename) - 5; /* .lock */
 
lock-lk-filename[i] = 0;
-   err = unlink_or_warn(lock-lk-filename);
+   res = unlink_or_err(lock-lk-filename, err);
lock-lk-filename[i] = '.';
-   if (err  errno != ENOENT)
+   if (res)
return 1;
}
return 0;
@@ -2566,7 +2588,7 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
if (!lock)
return 1;
-   ret |= delete_ref_loose(lock, flag);
+   ret |= delete_ref_loose(lock, flag, NULL);
 
/* removing the loose one could have resurrected an earlier
 * packed one.  Also, if it was not loose we need to repack
@@ -3573,7 +3595,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
if (update-lock) {
delnames[delnum++] = update-lock-ref_name;
-   ret |= delete_ref_loose(update-lock, update-type);
+   ret |= delete_ref_loose(update-lock, update-type,
+   err);
}
}
 
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..740e193 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -429,14 +429,12 @@ int xmkstemp_mode(char *template, int mode)
 
 static int warn_if_unremovable(const char *op, const char *file, int rc)
 {
-   if (rc  0) {
-   int err = errno;
-   if (ENOENT != err) {
-   warning(unable to %s %s: %s,
-   op, file, strerror(errno));
-   errno = err;
-   }
-   }
+   int err;
+   if (rc = 0 || errno == ENOENT)
+   return rc;
+   err = errno;
+   warning(unable to %s %s: %s, op, file, strerror(errno));
+   errno = err;
return rc;
 }
 
-- 
2.0.0.438.gec92e5c

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