Re: [PATCH v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-25 Thread Junio C Hamano
Jonathan Nieder  writes:

> The patch "fix handling of badly named refs"[1] is still undergoing
> heavy churn.
>
> I think it's worth getting that one right.

Oh, no question about it.  I was only making sure that I didn't miss
availability of updates for larger series we saw during this cycle.

--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-25 Thread Jonathan Nieder
Junio C Hamano wrote:

> I know that a review-update cycle is still going in the dark at
>
>   https://code-review.googlesource.com/#/q/topic:ref-transaction
>
> for this series.

Eh, it's at least public and doesn't flood the list with rebased
versions of the series.

Would you prefer if there were some list archived on gmane with the
automated mails from gerrit, to make it easier to look back at later?

>  Are we almost there for v22 which hopefully be the
> final before we merge it to 'next' and go incremental?

The patch "fix handling of badly named refs"[1] is still undergoing
heavy churn.

I think it's worth getting that one right.

Thanks,
Jonathan

[1] https://code-review.googlesource.com/1070
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-25 Thread Junio C Hamano
I know that a review-update cycle is still going in the dark at

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

for this series.  Are we almost there for v22 which hopefully be the
final before we merge it to 'next' and go incremental?  
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-18 Thread Jonathan Nieder
Junio C Hamano wrote:

> Does the order of changes that appear in
>
> https://code-review.googlesource.com/#/q/project:git+branch:master+topic:ref-transaction
>
> have any significance?  e.g. is a "topic" supposed to be a single
> strand of pearls on top of the "branch", and the top one is the tip,
> or something?

Alas no, though that would be a good feature.

Search results are ordered by which was last updated (for example when
someone adds a comment).

The 'Related Changes' shown on the change screen are in topological
order, with the tip at the top.

Jonathan
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-18 Thread Junio C Hamano
Jonathan Nieder  writes:

> Junio C Hamano wrote:
>
>> Jonathan: Is a current version of this patch series set up to be
>> fetched so that it can be reviewed outside Gerrit?
>
> The current tip is 06d707cb63e34fc55a18ecc47e668f3c44acae57 from
> https://code.googlesource.com/git (fetch-by-sha1 should work).  Each
> reroll gets its own refname of the form refs/changes/62/1062/
> with  increasing.  The "Download" widget in the top-right
> corner of https://code-review.googlesource.com/1062 shows the refname.

While I am showing my naiveté, how does one figure out that 1062 is
the last one in the series?

Does the order of changes that appear in

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

have any significance?  e.g. is a "topic" supposed to be a single
strand of pearls on top of the "branch", and the top one is the tip,
or something?

> There are plenty of unaddressed comments from Michael, so I'd hold off
> on picking up the latest for pu for now.

Oh, the request was not for that.  I simply cannot read the patch
with only limited context in the web browser without having a ready
access to a coherent whole, i.e. a full tree with history, to review
a change like this.
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-18 Thread Jonathan Nieder
Junio C Hamano wrote:

> Jonathan: Is a current version of this patch series set up to be
> fetched so that it can be reviewed outside Gerrit?

The current tip is 06d707cb63e34fc55a18ecc47e668f3c44acae57 from
https://code.googlesource.com/git (fetch-by-sha1 should work).  Each
reroll gets its own refname of the form refs/changes/62/1062/
with  increasing.  The "Download" widget in the top-right
corner of https://code-review.googlesource.com/1062 shows the refname.

There are plenty of unaddressed comments from Michael, so I'd hold off
on picking up the latest for pu for now.

Thanks,
Jonathan
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-18 Thread Junio C Hamano
Michael Haggerty  writes:

> On 09/13/2014 01:57 AM, Jonathan Nieder wrote:
>> Michael Haggerty wrote:
 Jonathan Nieder  writes:
>> 
> so I'll send a reroll of the series as-is in an hour or so.
>>>
>>> Jonathan: Is a current version of this patch series set up for review in
>>> Gerrit?
>> 
>> Yes.
>> (https://code-review.googlesource.com/#/q/project:git+topic:ref-transaction)
>
> I just worked through the patch series, leaving lots of comments in
> Gerrit. Overall it looks pretty good and makes a lot of very worthwhile
> progress. The only patch that gives me a bit of heartburn is
>
> [PATCH 15/19] refs.c: fix handling of badly named refs
>
> not because it is necessarily wrong, but because it has a lot of
> non-local effects that are hard to evaluate. I made a bunch of comments
> in Gerrit about that patch, too, and will wait for a response before
> having another go at it.
>
> Thanks for all your hard and detailed work, Ronnie and Jonathan!
>
> Michael

Jonathan: Is a current version of this patch series set up to be
fetched so that it can be reviewed outside Gerrit?

Running ls-remote against https://code.googlesource.com/git shows
many refs under refs/changes/* but it is unclear to me if there is a
coherent single "here is the latest and greatest, dependents rebased
on dependeds in the right order" thing that I can fetch and look at
with "log -p master..".



--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-17 Thread Michael Haggerty
On 09/13/2014 01:57 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
>>> Jonathan Nieder  writes:
> 
 so I'll send a reroll of the series as-is in an hour or so.
>>
>> Jonathan: Is a current version of this patch series set up for review in
>> Gerrit?
> 
> Yes.
> (https://code-review.googlesource.com/#/q/project:git+topic:ref-transaction)

I just worked through the patch series, leaving lots of comments in
Gerrit. Overall it looks pretty good and makes a lot of very worthwhile
progress. The only patch that gives me a bit of heartburn is

[PATCH 15/19] refs.c: fix handling of badly named refs

not because it is necessarily wrong, but because it has a lot of
non-local effects that are hard to evaluate. I made a bunch of comments
in Gerrit about that patch, too, and will wait for a response before
having another go at it.

Thanks for all your hard and detailed work, Ronnie and Jonathan!

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-13 Thread Junio C Hamano
Junio C Hamano  writes:

> I noticed that with this series merged to the version I use daily,
> detaching HEAD (i.e. "git checkout HEAD^0") breaks my HEAD reflog,
> which made me redo the integration ejecting the series out of 'pu'.
>
> Not happy, as my workflow relies fairly heavily on detached HEAD
> X-<.

Just FYI.

Bisecting the series using the attached as a test script points
"branch -d: avoid repeated symref resolution" as a possible culprit.
Perhaps these tests may want to be added to t3200 which is touched
by the said commit (or add them earlier in the series)?

-- >8 --

#!/bin/sh

test_description='reflog not nuked with co HEAD^0'
. ./test-lib.sh

check_reflog () {
while read name
do
git rev-parse --verify "$name"
done >expect &&
if test -f "$2"
then
while read object rest
do
git rev-parse --verify "$object"
done >>expect <"$2"
fi &&
while read object rest
do
git rev-parse --verify "$object"
done >actual <"$1" &&
test_cmp expect actual
}

test_expect_success setup '
test_tick &&
git commit --allow-empty -m initial &&
git branch side &&
test_tick &&
git commit --allow-empty -m second &&
git log -g --oneline >baseline &&
check_reflog baseline <<-\EOF
master
master^
EOF
'

test_expect_success 'switch to branch' '
git checkout side &&
git log -g --oneline >switched &&
check_reflog switched baseline <<-\EOF
side
EOF
'

test_expect_success 'detach to other' '
git checkout master^0 &&
git log -g --oneline >detach-1 &&
check_reflog detach-1 switched <<-\EOF
master
EOF
'

test_expect_success 'attach to self' '
git checkout master &&
git log -g --oneline >detach-2 &&
check_reflog detach-2 detach-1 <<-\EOF
master
EOF
'

test_expect_success 'detach to self' '
git checkout master^0 &&
git log -g --oneline >detach-3 &&
check_reflog detach-3 detach-2 <<-\EOF
master
EOF
'

test_expect_success 'attach to other' '
git checkout HEAD^0 &&
git checkout side &&
git log -g --oneline >detach-4 &&
check_reflog detach-4 detach-3 <<-\EOF
side
EOF
'

test_done
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Jonathan Nieder
Michael Haggerty wrote:
>> Jonathan Nieder  writes:

>>> so I'll send a reroll of the series as-is in an hour or so.
>
> Jonathan: Is a current version of this patch series set up for review in
> Gerrit?

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

Thanks,
Jonathan
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Michael Haggerty
On 09/12/2014 09:56 PM, Junio C Hamano wrote:
> Jonathan Nieder  writes:
> [...]
>> There were a few other minor changes, and I think with them the series
>> should be ready for "next".  My "send and hope that more reviewers
>> appear" strategy didn't really work,...
> 
> You sent them just a few days ago.  Expecting anybody to have a
> solid time to sit thru a 19-patch series inside that timeframe is
> not so realistic.

It's hard to tell from my glacial (tectonic?) pace, but I really do plan
to work through all of Ronnie's ref-related patches. Of course it's up
to you whether to wait for me. I really hope to get through the third
series by the end of next week.

>> so I'll send a reroll of the series as-is in an hour or so.

Jonathan: Is a current version of this patch series set up for review in
Gerrit?

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Junio C Hamano
Junio C Hamano  writes:

> Jonathan Nieder  writes:
>
>> It's "Oops, the next one (refs.c: do not permit err == NULL) is broken,
>> and this is to patch it up in advance". :)
>>
>> But it should apply on top, too.
>
> I think "in advance" makes sense for this one, too.
>
>> There were a few other minor changes, and I think with them the series
>> should be ready for "next".  My "send and hope that more reviewers
>> appear" strategy didn't really work,...
>
> You sent them just a few days ago.  Expecting anybody to have a
> solid time to sit thru a 19-patch series inside that timeframe is
> not so realistic.
>
>> so I'll send a reroll of the series as-is in an hour or so.
>
> OK.  Thanks.

I do not think I have enough time to pick a reroll up to redo the
day's integration, so please take time to make it perfect.

I noticed that with this series merged to the version I use daily,
detaching HEAD (i.e. "git checkout HEAD^0") breaks my HEAD reflog,
which made me redo the integration ejecting the series out of 'pu'.

Not happy, as my workflow relies fairly heavily on detached HEAD
X-<.

--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Junio C Hamano
Jonathan Nieder  writes:

> It's "Oops, the next one (refs.c: do not permit err == NULL) is broken,
> and this is to patch it up in advance". :)
>
> But it should apply on top, too.

I think "in advance" makes sense for this one, too.

> There were a few other minor changes, and I think with them the series
> should be ready for "next".  My "send and hope that more reviewers
> appear" strategy didn't really work,...

You sent them just a few days ago.  Expecting anybody to have a
solid time to sit thru a 19-patch series inside that timeframe is
not so realistic.

> so I'll send a reroll of the series as-is in an hour or so.

OK.  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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jonathan Nieder  writes:
> > Junio C Hamano wrote:

>>> The tag fetched and built as-is seems to break 5514 among other
>>> things ("git remote rm" segfaults).
>>
>> Yeah, I noticed that right after sending the series out. :/
>>
>> The patch below fixes it[1].
>
> Is this meant to replace anything, or is it "Oops, the previous ones
> are broken, and this is to patch it up on top"?

It's "Oops, the next one (refs.c: do not permit err == NULL) is broken,
and this is to patch it up in advance". :)

But it should apply on top, too.

There were a few other minor changes, and I think with them the series
should be ready for "next".  My "send and hope that more reviewers
appear" strategy didn't really work, so I'll send a reroll of the
series as-is in an hour or so.

Here's an interdiff as a preview.

diff --git a/builtin/branch.c b/builtin/branch.c
index 5d5bc56..4bf931e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -238,9 +238,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
RESOLVE_REF_READING
| RESOLVE_REF_NODEREF
| RESOLVE_REF_ALLOW_BAD_NAME);
-   if (!target ||
-   (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
-is_null_sha1(sha1))) {
+   if (!target) {
error(remote_branch
  ? _("remote branch '%s' not found.")
  : _("branch '%s' not found."), bname.buf);
@@ -268,8 +266,8 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
   ? _("Deleted remote branch %s (was %s).\n")
   : _("Deleted branch %s (was %s).\n"),
   bname.buf,
-  (flags & REF_ISSYMREF)
-  ? target
+  (flags & REF_ISBROKEN) ? "broken"
+  : (flags & REF_ISSYMREF) ? target
   : find_unique_abbrev(sha1, DEFAULT_ABBREV));
}
delete_branch_config(bname.buf);
diff --git a/builtin/remote.c b/builtin/remote.c
index 6eaeee7..ef1ffc3 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -750,13 +750,16 @@ static int mv(int argc, const char **argv)
 
 static int remove_branches(struct string_list *branches)
 {
+   struct strbuf err = STRBUF_INIT;
const char **branch_names;
int i, result = 0;
 
branch_names = xmalloc(branches->nr * sizeof(*branch_names));
for (i = 0; i < branches->nr; i++)
branch_names[i] = branches->items[i].string;
-   result |= repack_without_refs(branch_names, branches->nr, NULL);
+   if (repack_without_refs(branch_names, branches->nr, &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
free(branch_names);
 
for (i = 0; i < branches->nr; i++) {
@@ -1333,9 +1336,13 @@ static int prune_remote(const char *remote, int dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i < states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
-   if (!dry_run)
-   result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+   if (!dry_run) {
+   struct strbuf err = STRBUF_INIT;
+   if (repack_without_refs(delete_refs, states.stale.nr,
+   &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
+   }
free(delete_refs);
}
 
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-12 Thread Junio C Hamano
Jonathan Nieder  writes:

> Junio C Hamano wrote:
>> Jonathan Nieder  writes:
>
>>> These patches are also available from the git repository at
>>>
>>>   git://repo.or.cz/git/jrn.git tags/rs/ref-transaction
>>
>> The tag fetched and built as-is seems to break 5514 among other
>> things ("git remote rm" segfaults).
>
> Yeah, I noticed that right after sending the series out. :/
>
> The patch below fixes it[1].

Is this meant to replace anything, or is it "Oops, the previous ones
are broken, and this is to patch it up on top"?

> -- >8 --
> From: Ronnie Sahlberg 
> Date: Thu, 11 Sep 2014 08:42:57 -0700
> Subject: remote rm/prune: print a message when writing packed-refs fails
>
> Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to
> repack_without_refs, 2014-06-20), repack_without_refs forgot to
> provide an error message when commit_packed_refs fails.  Even today,
> it only provides a message for callers that pass a non-NULL err
> parameter.  Internal callers in refs.c pass non-NULL err but
> "git remote" does not.
>
> That means that "git remote rm" and "git remote prune" can fail
> without printing a message about why.  Fix them by passing in a
> non-NULL err parameter and printing the returned message.
>
> This is the last caller to a ref handling function passing err ==
> NULL.  A later patch can drop support for err == NULL, avoiding such
> problems in the future.
>
> Change-Id: Ifb8a726ef03d0aa282a25a102313064d2e8ec283
> Signed-off-by: Ronnie Sahlberg 
> Signed-off-by: Jonathan Nieder 
> ---
> [1] https://code-review.googlesource.com/1110
> https://code-review.googlesource.com/1060
>
>  builtin/remote.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 6eaeee7..ef1ffc3 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -750,13 +750,16 @@ static int mv(int argc, const char **argv)
>  
>  static int remove_branches(struct string_list *branches)
>  {
> + struct strbuf err = STRBUF_INIT;
>   const char **branch_names;
>   int i, result = 0;
>  
>   branch_names = xmalloc(branches->nr * sizeof(*branch_names));
>   for (i = 0; i < branches->nr; i++)
>   branch_names[i] = branches->items[i].string;
> - result |= repack_without_refs(branch_names, branches->nr, NULL);
> + if (repack_without_refs(branch_names, branches->nr, &err))
> + result |= error("%s", err.buf);
> + strbuf_release(&err);
>   free(branch_names);
>  
>   for (i = 0; i < branches->nr; i++) {
> @@ -1333,9 +1336,13 @@ static int prune_remote(const char *remote, int 
> dry_run)
>   delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
>   for (i = 0; i < states.stale.nr; i++)
>   delete_refs[i] = states.stale.items[i].util;
> - if (!dry_run)
> - result |= repack_without_refs(delete_refs,
> -   states.stale.nr, NULL);
> + if (!dry_run) {
> + struct strbuf err = STRBUF_INIT;
> + if (repack_without_refs(delete_refs, states.stale.nr,
> + &err))
> + result |= error("%s", err.buf);
> + strbuf_release(&err);
> + }
>   free(delete_refs);
>   }
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-11 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> These patches are also available from the git repository at
>>
>>   git://repo.or.cz/git/jrn.git tags/rs/ref-transaction
>
> The tag fetched and built as-is seems to break 5514 among other
> things ("git remote rm" segfaults).

Yeah, I noticed that right after sending the series out. :/

The patch below fixes it[1].

-- >8 --
From: Ronnie Sahlberg 
Date: Thu, 11 Sep 2014 08:42:57 -0700
Subject: remote rm/prune: print a message when writing packed-refs fails

Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to
repack_without_refs, 2014-06-20), repack_without_refs forgot to
provide an error message when commit_packed_refs fails.  Even today,
it only provides a message for callers that pass a non-NULL err
parameter.  Internal callers in refs.c pass non-NULL err but
"git remote" does not.

That means that "git remote rm" and "git remote prune" can fail
without printing a message about why.  Fix them by passing in a
non-NULL err parameter and printing the returned message.

This is the last caller to a ref handling function passing err ==
NULL.  A later patch can drop support for err == NULL, avoiding such
problems in the future.

Change-Id: Ifb8a726ef03d0aa282a25a102313064d2e8ec283
Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
[1] https://code-review.googlesource.com/1110
https://code-review.googlesource.com/1060

 builtin/remote.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 6eaeee7..ef1ffc3 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -750,13 +750,16 @@ static int mv(int argc, const char **argv)
 
 static int remove_branches(struct string_list *branches)
 {
+   struct strbuf err = STRBUF_INIT;
const char **branch_names;
int i, result = 0;
 
branch_names = xmalloc(branches->nr * sizeof(*branch_names));
for (i = 0; i < branches->nr; i++)
branch_names[i] = branches->items[i].string;
-   result |= repack_without_refs(branch_names, branches->nr, NULL);
+   if (repack_without_refs(branch_names, branches->nr, &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
free(branch_names);
 
for (i = 0; i < branches->nr; i++) {
@@ -1333,9 +1336,13 @@ static int prune_remote(const char *remote, int dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i < states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
-   if (!dry_run)
-   result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+   if (!dry_run) {
+   struct strbuf err = STRBUF_INIT;
+   if (repack_without_refs(delete_refs, states.stale.nr,
+   &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
+   }
free(delete_refs);
}
 
-- 
2.1.0.rc2.206.gedb03e5

--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-11 Thread Junio C Hamano
Jonathan Nieder  writes:

> These patches are also available from the git repository at
>
>   git://repo.or.cz/git/jrn.git tags/rs/ref-transaction

The tag fetched and built as-is seems to break 5514 among other
things ("git remote rm" segfaults).
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-11 Thread Junio C Hamano
Jonathan Nieder  writes:

> Jonathan Nieder wrote:
>
>> The next series from Ronnie's collection is available at
>> https://code-review.googlesource.com/#/q/topic:ref-transaction in case
>> someone wants a fresh series to look at.
>
> Here is the outcome of that review.  It could use another set of eyes
> (hint, hint) but should be mostly ready.  Interdiff below.  This is meant
> to replace rs/ref-transaction in 'pu' and I'd prefer to wait a little
> for more comments before merging to 'next'.

Alright.  I'd assume that all the other rs/ref-transaction* topics
that depends on rs/ref-transaction series will be rerolled on top of
this new round when ready.

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


[PATCH v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

2014-09-10 Thread Jonathan Nieder
Jonathan Nieder wrote:

> The next series from Ronnie's collection is available at
> https://code-review.googlesource.com/#/q/topic:ref-transaction in case
> someone wants a fresh series to look at.

Here is the outcome of that review.  It could use another set of eyes
(hint, hint) but should be mostly ready.  Interdiff below.  This is meant
to replace rs/ref-transaction in 'pu' and I'd prefer to wait a little
for more comments before merging to 'next'.

These patches are also available from the git repository at

  git://repo.or.cz/git/jrn.git tags/rs/ref-transaction

"Use ref transactions", part 3

Ronnie explains:

This is the third and final part of the original 48 patch
series for basic transaction support.  This version implements
some changes suggested by mhagger for the warn_if_unremovable
changes.  It also adds a new patch "fix handling of badly
named refs" that repairs the handling of badly named refs.

This includes some improvements to the transaction API (in
particular allowing different reflog messages per ref update in
a transaction), some cleanups and consistency improvements, and
preparation for an implementation of ref renaming in terms of
the transaction API.

It also improves handling of refs with invalid names.  Today "git
branch --list" and "git for-each-ref" do not provide a way to discover
refs with invalid names and "git branch -d" and "git update-ref -d"
cannot delete them.  After this series, such bad refs will be
discoverable and deletable again as in olden times.

Jonathan Nieder (5):
  mv test: recreate mod/ directory instead of relying on stale copy
  branch -d: avoid repeated symref resolution
  refs.c: do not permit err == NULL
  lockfile: remove unable_to_lock_error
  ref_transaction_commit: bail out on failure to remove a ref

Ronnie Sahlberg (14):
  wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success
  wrapper.c: add a new function unlink_or_msg
  refs.c: add an err argument to delete_ref_loose
  refs.c: pass the ref log message to _create/delete/update instead of
_commit
  rename_ref: don't ask read_ref_full where the ref came from
  refs.c: move the check for valid refname to lock_ref_sha1_basic
  refs.c: call lock_ref_sha1_basic directly from commit
  refs.c: pass a skip list to name_conflict_fn
  refs.c: ref_transaction_commit: distinguish name conflicts from other
errors
  fetch.c: change s_update_ref to use a ref transaction
  refs.c: make write_ref_sha1 static
  refs.c: change resolve_ref_unsafe reading argument to be a flags field
  refs.c: fix handling of badly named refs
  for-each-ref.c: improve message before aborting on broken ref

 branch.c|   6 +-
 builtin/blame.c |   2 +-
 builtin/branch.c|  19 ++-
 builtin/checkout.c  |   6 +-
 builtin/clone.c |   2 +-
 builtin/commit.c|   6 +-
 builtin/fetch.c |  34 +++--
 builtin/fmt-merge-msg.c |   2 +-
 builtin/for-each-ref.c  |  12 +-
 builtin/fsck.c  |   2 +-
 builtin/log.c   |   3 +-
 builtin/merge.c |   2 +-
 builtin/notes.c |   2 +-
 builtin/receive-pack.c  |   9 +-
 builtin/remote.c|   5 +-
 builtin/replace.c   |   5 +-
 builtin/show-branch.c   |   6 +-
 builtin/symbolic-ref.c  |   2 +-
 builtin/tag.c   |   4 +-
 builtin/update-ref.c|  16 ++-
 bundle.c|   2 +-
 cache.h |  31 +++--
 fast-import.c   |   8 +-
 git-compat-util.h   |  16 ++-
 http-backend.c  |   3 +-
 lockfile.c  |  10 --
 notes-merge.c   |   2 +-
 reflog-walk.c   |   5 +-
 refs.c  | 321 ++--
 refs.h  |  18 ++-
 remote.c|  10 +-
 sequencer.c |   8 +-
 t/t1400-update-ref.sh   |  16 +++
 t/t1402-check-ref-format.sh |  48 +++
 t/t3200-branch.sh   |   9 ++
 t/t7001-mv.sh   |  15 ++-
 transport-helper.c  |   4 +-
 transport.c |   5 +-
 upload-pack.c   |   2 +-
 walker.c|   5 +-
 wrapper.c   |  28 ++--
 wt-status.c |   2 +-
 42 files changed, 487 insertions(+), 226 deletions(-)
---
Changes since last appearance:

diff --git c/branch.c w/branch.c
index 76a8ec9..ba3e1c1 100644
--- c/branch.c
+++ w/branch.c
@@ -186,7 +186,7 @@ int validate_new_branchname(const char *name, struct strbuf 
*ref,
const char *head;
unsigned char sha1[20];
 
-   head = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
+   head = resolve_ref_unsafe("HEAD", sha1, NULL, 0);
if (!is_bare_repository() && head && !strcmp(head, ref->buf))
die(_("Cannot force update the current branch."));
}
diff --gi