[PATCH v8 06/44] refs.c: add an err argument ro delete_loose_ref

2014-05-15 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 failing.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 072e3e9..a150520 100644
--- a/refs.c
+++ b/refs.c
@@ -2484,17 +2484,22 @@ 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 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_warn(lock-lk-filename);
lock-lk-filename[i] = '.';
-   if (err  errno != ENOENT)
+   if (res  errno != ENOENT) {
+   if (err)
+   strbuf_addf(err,
+   failed to delete loose ref '%s',
+   lock-lk-filename);
return 1;
+   }
}
return 0;
 }
@@ -2507,7 +2512,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
@@ -3492,7 +3497,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);
}
}
 
-- 
2.0.0.rc3.477.g0f8edf7

--
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 v8 06/44] refs.c: add an err argument ro delete_loose_ref

2014-05-15 Thread Jonathan Nieder 
Ronnie Sahlberg wrote:

 [Subject: refs.c: add an err argument ro delete_loose_ref]

s/ro/to/
s/delete_loose_ref/delete_ref_loose/

 --- a/refs.c
 +++ b/refs.c
 @@ -2484,17 +2484,22 @@ 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 delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
 *err)

Should this get an onerr flag to suppress the message to stderr
or unconditionally suppress it when err != NULL?

[...]
   lock-lk-filename[i] = 0;
 - err = unlink_or_warn(lock-lk-filename);
 + res = unlink_or_warn(lock-lk-filename);

It seems like in the new error handling scheme there should be a new
variant on wrapper.c's warn_if_unremovable:

static int add_err_if_unremovable(const char *op, const char *file, 
struct strbuf *err, int rc)
{
int err = errno;
if (rc  0  err != ENOENT) {
strbuf_addf(err, unable to %s %s: %s,
op, file, strerror(errno));
errno = err;
}
return rc;
}

static int unlink_or_err(const char *file, struct strbuf *err)
{
return add_err_if_unremovable(unlink, file, err, 
unlink(file));
}

res = unlink_or_err(lock-lk-filename, err);
--
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 v8 06/44] refs.c: add an err argument ro delete_loose_ref

2014-05-15 Thread Ronnie Sahlberg 
On Thu, May 15, 2014 at 12:04 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 [Subject: refs.c: add an err argument ro delete_loose_ref]

 s/ro/to/
 s/delete_loose_ref/delete_ref_loose/

 --- a/refs.c
 +++ b/refs.c
 @@ -2484,17 +2484,22 @@ 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 delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
 *err)

 Should this get an onerr flag to suppress the message to stderr
 or unconditionally suppress it when err != NULL?


Fixed.
I added a new function unlink_or_err that will update err if non-NULL
and unse warning() otherwise.


 [...]
   lock-lk-filename[i] = 0;
 - err = unlink_or_warn(lock-lk-filename);
 + res = unlink_or_warn(lock-lk-filename);

 It seems like in the new error handling scheme there should be a new
 variant on wrapper.c's warn_if_unremovable:

 static int add_err_if_unremovable(const char *op, const char *file, 
 struct strbuf *err, int rc)
 {
 int err = errno;
 if (rc  0  err != ENOENT) {
 strbuf_addf(err, unable to %s %s: %s,
 op, file, strerror(errno));
 errno = err;
 }
 return rc;
 }

 static int unlink_or_err(const char *file, struct strbuf *err)
 {
 return add_err_if_unremovable(unlink, file, err, 
 unlink(file));
 }

 res = unlink_or_err(lock-lk-filename, err);
--
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