Re: [PATCH v22.5 09/24] refs.c: pass a list of names to skip to is_refname_available

[Junio C Hamano]

Jonathan Nieder jrnie...@gmail.com writes:

 Will take a look over there (I really hate having to context switch
 only to review this series, though).

 Here's a copy to save a trip.

The patch itself looks fine, but Gerrit really needs a way to convey
these changes make a single topic and here is the tip somehow.  It
could be that there is and it is not well advertised, but the end
result is I visit the URL,

  https://code-review.googlesource.com/#/q/project:git+topic:ref-transaction

pick and view a few patches at random to view the changes in pretty
side-by-side diff output (which by itself is fine), check Download
to see the fetch URL, when it is followed it often gives me only an
early half of the topic, get confused and scratch my head, give up
and abandon and the whole time doing so ends up being wasted.
--
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 v22.5 09/24] refs.c: pass a list of names to skip to is_refname_available

[Jonathan Nieder]

From: Ronnie Sahlberg sahlb...@google.com
Date: Thu, 1 May 2014 11:16:07 -0700

commit 23acf975d74825789112a3a7ba97dbbdc76904f4 upstream.

Change is_refname_available to take a list of strings to exclude when
checking for conflicts instead of just one single name. We can already
exclude a single name for the sake of renames. This generalizes that support.

ref_transaction_commit already tracks a set of refs that are being deleted
in an array.  This array is then used to exclude refs from being written to
the packed-refs file.  At some stage we will want to change this array to a
struct string_list and then we can pass it to is_refname_available via the
call to lock_ref_sha1_basic.  That will allow us to perform transactions
that perform multiple renames as long as there are no conflicts within the
starting or ending state.

For example, that would allow a single transaction that contains two
renames that are both individually conflicting:

   m - n/n
   n - m/m

No functional change intended yet.

Change-Id: I8c68f0a47eef25759d572436ba683cb7b1ccb7eb
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:

 Thanks for the heads up.  (I hadn't realized the ref-transaction-1 was
 part of 'master' --- yay!)  Rebased and reworked:
 https://code-review.googlesource.com/1027

 Heh, I was sort of expecting that heavy-weight contributors that
 throw many and/or large patch series would at least be paying
 attention to What's cooking reports.

That's fair.  I don't pay enough attention to the 'graduated' section.

 Will take a look over there (I really hate having to context switch
 only to review this series, though).

Here's a copy to save a trip.

 refs.c | 50 --
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/refs.c b/refs.c
index 83b2c77..05e42a7 100644
--- a/refs.c
+++ b/refs.c
@@ -785,13 +785,13 @@ static void prime_ref_dir(struct ref_dir *dir)
}
 }
 
-static int entry_matches(struct ref_entry *entry, const char *refname)
+static int entry_matches(struct ref_entry *entry, const struct string_list 
*list)
 {
-   return refname  !strcmp(entry-name, refname);
+   return list  string_list_has_string(list, entry-name);
 }
 
 struct nonmatching_ref_data {
-   const char *skip;
+   const struct string_list *skip;
struct ref_entry *found;
 };
 
@@ -815,16 +815,19 @@ static void report_refname_conflict(struct ref_entry 
*entry,
 /*
  * Return true iff a reference named refname could be created without
  * conflicting with the name of an existing reference in dir.  If
- * oldrefname is non-NULL, ignore potential conflicts with oldrefname
- * (e.g., because oldrefname is scheduled for deletion in the same
+ * skip is non-NULL, ignore potential conflicts with refs in skip
+ * (e.g., because they are scheduled for deletion in the same
  * operation).
  *
  * Two reference names conflict if one of them exactly matches the
  * leading components of the other; e.g., foo/bar conflicts with
  * both foo and with foo/bar/baz but not with foo/bar or
  * foo/barbados.
+ *
+ * skip must be sorted.
  */
-static int is_refname_available(const char *refname, const char *oldrefname,
+static int is_refname_available(const char *refname,
+   const struct string_list *skip,
struct ref_dir *dir)
 {
const char *slash;
@@ -838,12 +841,12 @@ static int is_refname_available(const char *refname, 
const char *oldrefname,
 * looking for a conflict with a leaf entry.
 *
 * If we find one, we still must make sure it is
-* not oldrefname.
+* not in skip.
 */
pos = search_ref_dir(dir, refname, slash - refname);
if (pos = 0) {
struct ref_entry *entry = dir-entries[pos];
-   if (entry_matches(entry, oldrefname))
+   if (entry_matches(entry, skip))
return 1;
report_refname_conflict(entry, refname);
return 0;
@@ -876,13 +879,13 @@ static int is_refname_available(const char *refname, 
const char *oldrefname,
/*
 * We found a directory named refname. It is a
 * problem iff it contains any ref that is not
-* oldrefname.
+* in skip.
 */
struct ref_entry *entry = dir-entries[pos];
struct ref_dir *dir = get_ref_dir(entry);
struct nonmatching_ref_data data;
 
-   data.skip = oldrefname;
+   data.skip = skip;
sort_ref_dir(dir);
if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, 
data))
return