Re: GIT, libcurl and GSS-Negotiate

2014-05-16 Thread Jeff King
On Fri, May 16, 2014 at 10:34:10PM +, brian m. carlson wrote: > > The tricky part is figuring out when to return HTTP_NOAUTH ("do not try > > again, we failed") versus HTTP_REAUTH ("get credentials and try again") > > in handle_curl_result. Right now the decision is based on "did we have a > >

[PATCH v2 09/10] replace: add --edit to usage string

2014-05-16 Thread Christian Couder
Signed-off-by: Christian Couder --- builtin/replace.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/replace.c b/builtin/replace.c index 4ee3d92..1bb491d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -16,6 +16,7 @@ static const char * const git_replace_usage[] = {

[PATCH v2 10/10] Documentation: replace: describe new --edit option

2014-05-16 Thread Christian Couder
Signed-off-by: Christian Couder --- Documentation/git-replace.txt | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 0a02f70..37d872d 100644 --- a/Documentation/git-replace.txt +++ b/Documentation

[PATCH v2 02/10] replace: use OPT_CMDMODE to handle modes

2014-05-16 Thread Christian Couder
From: Jeff King By using OPT_CMDMODE, the mutual exclusion between modes is taken care of for us. It also makes it easy for us to maintain a single variable with the mode, which makes its intent more clear. We can use a single switch() to make sure we have covered all of the modes. This ends up

[PATCH v2 01/10] replace: refactor command-mode determination

2014-05-16 Thread Christian Couder
From: Jeff King The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before

[PATCH v2 08/10] replace: add tests for --edit

2014-05-16 Thread Christian Couder
Signed-off-by: Christian Couder --- t/t6050-replace.sh | 29 + 1 file changed, 29 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 719a116..7609174 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -318,6 +318,35 @@ test_expect_succes

[PATCH v2 00/10] replace: add option to edit a Git object

2014-05-16 Thread Christian Couder
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1. I added 6 more small patches to add tests, documentation and a few small improvements

[PATCH v2 05/10] replace: make sure --edit results in a different object

2014-05-16 Thread Christian Couder
It's a bad idea to create a replace ref for an object that points to the original object itself. That's why we have to check if the result from editing the original object is a different object and error out if it isn't. Signed-off-by: Christian Couder --- builtin/replace.c | 3 +++ 1 file chan

[PATCH v2 03/10] replace: factor object resolution out of replace_object

2014-05-16 Thread Christian Couder
From: Jeff King As we add new options that operate on objects before replacing them, we'll want to be able to feed raw sha1s straight into replace_object. Split replace_object into the object-resolution part and the actual replacement. Signed-off-by: Jeff King Signed-off-by: Christian Couder -

[PATCH v2 04/10] replace: add --edit option

2014-05-16 Thread Christian Couder
From: Jeff King This allows you to run: git replace --edit SHA1 to get dumped in an editor with the contents of the object for SHA1. The result is then read back in and used as a "replace" object for SHA1. The writing/reading is type-aware, so you get to edit "ls-tree" output rather than th

[PATCH v2 06/10] replace: refactor checking ref validity

2014-05-16 Thread Christian Couder
This will be useful in a following commit when we will want to check if the ref already exists before we let the user edit an object. Signed-off-by: Christian Couder --- builtin/replace.c | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/builti

[PATCH v2 07/10] replace: die early if replace ref already exists

2014-05-16 Thread Christian Couder
If a replace ref already exists for an object, it is much better for the user if we error out before we let the user edit the object, rather than after. Signed-off-by: Christian Couder --- builtin/replace.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Jeff King
On Sat, May 17, 2014 at 12:25:30AM -0500, Felipe Contreras wrote: > > I agree with the line of reasoning you laid out in your email, > > especially: > > What a shock. Please stop with these unproductive and rude comments. > > I hadn't thought of the rename idea, and it would address the concern

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Felipe Contreras
Jeff King wrote: > I agree with the line of reasoning you laid out in your email, > especially: What a shock. > > I would say that the options I see are these three, and I would rank > > the "warn every time" as less helpful to end-users: > > > > - rename contrib/remote-helpers to contrib/obso

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Felipe Contreras
James Denholm wrote: > On Fri, May 16, 2014 at 05:39:42PM -0500, Felipe Contreras wrote: > > (...) I would venture to say you have never made a package in your > > life. > > And you have, Felipe? Let us see the years of experience you surely have > in the field. As a matter of fact, yes I've writ

[PATCH] RelNotes/2.0.0.txt: Fix several grammar issues, notably a lack of hyphens, double quotes, or articles

2014-05-16 Thread Jason St. John
Signed-off-by: Jason St. John --- Documentation/RelNotes/2.0.0.txt | 75 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/Documentation/RelNotes/2.0.0.txt b/Documentation/RelNotes/2.0.0.txt index 6e628d4..e6bf9d6 100644 --- a/Documentation/R

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread James Denholm
On Fri, May 16, 2014 at 05:39:42PM -0500, Felipe Contreras wrote: > (...) I would venture to say you have never made a package in your > life. And you have, Felipe? Let us see the years of experience you surely have in the field. > The fact that you think packagers of git would simply package > g

Re: [git-users] worlds slowest git repo- what to do?

2014-05-16 Thread Duy Nguyen
On Sat, May 17, 2014 at 4:22 AM, John Fisher wrote: >> Probably known issues. But some elaboration would be nice (e.g. what >> operation is slow, how slow, some more detail >> characteristics of the repo..) in case new problems pop up. > > so far I have done add, commit, status, clone - commit an

[PATCH v2 2/2] commit: allow core.commentChar=auto for character auto selection

2014-05-16 Thread Nguyễn Thái Ngọc Duy
When core.commentChar is "auto", the comment char starts with '#' as in default but if it's already in the prepared message, find another char in a small subset. This should stop surprises because git strips some lines unexpectedly. Note that git is not smart enough to recognize '#' as the comment

[PATCH v2 1/2] config: be strict on core.commentChar

2014-05-16 Thread Nguyễn Thái Ngọc Duy
We don't support comment _strings_ (at least not yet). And multi-byte character encoding could also be misinterpreted. The test with two commas is updated because it violates this. It's added with the patch that introduces core.commentChar in eff80a9 (Allow custom "comment char" - 2013-01-16). It'

Re: [PATCH 2/2] commit: allow core.commentChar=auto for character auto selection

2014-05-16 Thread Duy Nguyen
On Fri, May 16, 2014 at 11:40 PM, Jonathan Nieder wrote: > Nguyễn Thái Ngọc Duy wrote: > >> core.commentChar starts with '#' as in default but if it's already in >> the prepared message, find another one among a small subset. This >> should stop surprises because git strips some lines unexpectedly

Re: [PATCH v8 22/44] fetch.c: clear errno before calling functions that might set it

2014-05-16 Thread Jeff King
On Fri, May 16, 2014 at 11:33:48AM -0700, Jonathan Nieder wrote: > (cc-ing peff, who may remember this STORE_REF_ERROR_DF_CONFLICT case > from long ago) No, but I have very good tools for searching the list archive. ;) > > In s_update_ref there are two calls that when they fail we return an err

Re: Returning error message from custom smart http server

2014-05-16 Thread brian m. carlson
On Tue, May 13, 2014 at 09:39:59AM +0200, "Ákos, Tajti" wrote: > Dear List, > > we implemented our own git smart http server to be able to check permissions > and other thing before pushes. It works fine, however, the error messages we > generate on the server side are not displayed by the command

Re: [PATCH v8 23/44] fetch.c: change s_update_ref to use a ref transaction

2014-05-16 Thread Jonathan Nieder
Ronnie Sahlberg wrote: > --- a/builtin/fetch.c > +++ b/builtin/fetch.c [...] > @@ -384,15 +384,16 @@ static int s_update_ref(const char *action, > snprintf(msg, sizeof(msg), "%s: %s", rla, action); > > errno = 0; > - lock = lock_any_ref_for_update(ref->name, > -

Re: [PATCH v8 24/44] fetch.c: use a single ref transaction for all ref updates

2014-05-16 Thread Jonathan Nieder
Ronnie Sahlberg wrote: > Change store_updated_refs to use a single ref transaction for all refs that > are updated during the fetch. This makes the fetch more atomic when update > failures occur. Fun. [...] > --- a/builtin/fetch.c > +++ b/builtin/fetch.c [...] > @@ -373,27 +374,13 @@ static int

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Jeff King
On Fri, May 16, 2014 at 09:52:15AM -0700, Junio C Hamano wrote: > Or am I reacting to a typo and you meant to say "I would prefer not > to instrument"? Your "shipping the warnings to end users who did > not package the software will not help" was unclear if you meant the > README that has warning

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Felipe Contreras
Junio C Hamano wrote: > Jeff King writes: > > > On Thu, May 15, 2014 at 03:56:29PM -0700, Junio C Hamano wrote: > > > >> Two announcements for their version 0.2 on the list archive are not > >> quite enough to advertise them to their users. > > > > I do not think this README nor a mention in the

Delaying 2.0 final

2014-05-16 Thread Junio C Hamano
As we seem to have a few regressions we may want to fix, I will not be cutting the 2.0 final today (https://tinyurl.com/gitCal). I queued the following near the bottom of 'pu' (these are also merged to 'next' to keep pu^{/match.next} in sync with next), and plan to cut 2.0.0-rc4 early next week.

Re: GIT, libcurl and GSS-Negotiate

2014-05-16 Thread brian m. carlson
On Mon, May 12, 2014 at 04:21:53PM -0400, Jeff King wrote: > On Sat, May 10, 2014 at 09:01:32PM +, brian m. carlson wrote: > > * Make git understand that it really needs to try again with different > > credentials in this case (how to do that is unknown). > > It should be pretty straightforw

Re: [PATCH v8 23/44] fetch.c: change s_update_ref to use a ref transaction

2014-05-16 Thread Ronnie Sahlberg
On Fri, May 16, 2014 at 12:12 PM, Jonathan Nieder wrote: > (+cc: peff for STORE_REF_ERROR_DF_CONFLICT expertise) > Ronnie Sahlberg wrote: > >> --- a/builtin/fetch.c >> +++ b/builtin/fetch.c >> @@ -375,7 +375,7 @@ static int s_update_ref(const char *action, > [...] >> + transaction = ref_transa

Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction

2014-05-16 Thread Eric Sunshine
On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano wrote: > Ronnie Sahlberg writes: > >> Allow to make multiple reflog updates to the same ref during a transaction. >> This means we only need to lock the reflog once, during the first update that >> touches the reflog, and that all further updates ca

Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction

2014-05-16 Thread Junio C Hamano
Ronnie Sahlberg writes: > Allow to make multiple reflog updates to the same ref during a transaction. > This means we only need to lock the reflog once, during the first update that > touches the reflog, and that all further updates can just write the reflog > entry since the reflog is already lo

Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL

2014-05-16 Thread Junio C Hamano
Ronnie Sahlberg writes: > When performing a reflog transaction update, only write to the reflog iff > msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform > an update that only truncates but does not write. > > Signed-off-by: Ronnie Sahlberg > --- > refs.c | 8 +--- >

Re: [git-users] worlds slowest git repo- what to do?

2014-05-16 Thread John Fisher
On 05/16/2014 03:13 AM, Duy Nguyen wrote: > On Fri, May 16, 2014 at 2:06 AM, Philip Oakley wrote: >> From: "John Fisher" >>> I assert based on one piece of evidence ( a post from a facebook dev) that >>> I now have the worlds biggest and slowest git >>> repository, and I am not a happy guy. I us

Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log

2014-05-16 Thread Junio C Hamano
Ronnie Sahlberg writes: > Add a flag that allows us to truncate the reflog before we write the update. > > Signed-off-by: Ronnie Sahlberg > --- Until we read the callers it is hard to see how such a feature is useful, though. (style) The two multi-line comments are formatted differently. > r

Re: [PATCH 03/31] refs.c: rename the transaction functions

2014-05-16 Thread Junio C Hamano
Ronnie Sahlberg writes: > Rename the transaction functions. Remove the leading ref_ from the names > and append _sha1 to the names for functions that create/delete/update sha1 > refs. > ... > - transaction = ref_transaction_begin(); > + transaction = transaction_begin();

Re: [PATCH v8 22/44] fetch.c: clear errno before calling functions that might set it

2014-05-16 Thread Ronnie Sahlberg
errno is hairy :-( You probably also want something like this : diff --git a/builtin/fetch.c b/builtin/fetch.c index 4603cb6..55e7dd8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -426,7 +426,7 @@ static int update_local_ref(struct ref *ref, _("! %-*s %-*s -> %

Re: [PATCH v8 23/44] fetch.c: change s_update_ref to use a ref transaction

2014-05-16 Thread Jonathan Nieder
(+cc: peff for STORE_REF_ERROR_DF_CONFLICT expertise) Ronnie Sahlberg wrote: > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -375,7 +375,7 @@ static int s_update_ref(const char *action, [...] > + transaction = ref_transaction_begin(); > + if (!transaction || > + ref_transactio

Re: [PATCH 10/10] git-submodule.sh: don't use the -a or -b option with the test command

2014-05-16 Thread Junio C Hamano
Jonathan Nieder writes: > Perhaps something like the following would work? > > tree-wide: convert test -a/-o to && and || > > The interaction with unary operators and operator precedence > for && and || are better known than -a and -o, and for that > reason we prefer them.

Re: [PATCH] commit: switch core.commentChar if it's found in existing commit

2014-05-16 Thread Junio C Hamano
Junio C Hamano writes: > Nguyễn Thái Ngọc Duy writes: > >> If we need to use core.commentChar and it's already in the prepared >> message, find another char among a small subset. This should stop >> surprises because git strips some lines unexpectedly. Of course if >> candicate characters happe

Re: [PATCH v2 1/2] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set

2014-05-16 Thread Junio C Hamano
Alexey Shumkin writes: > Pretty format string %<(N,[ml]trunc)>%s truncates subject to a given > length with an appropriate padding. This works for non-ASCII texts when > i18n.logOutputEncoding is UTF-8 only (independently of a printed commit > message encoding) but does not work when i18n.logOutp

Re* regression: request-pull with signed tag lacks tags/ in master

2014-05-16 Thread Junio C Hamano
Junio C Hamano writes: > "Michael S. Tsirkin" writes: > >>> My reading of the earlier parts of the series is that Linus wanted >>> us never dwim "for-upstream" to "tags/for-upstream" or any other ref >>> that happens to point at the same commit as for-upstream you have. >>> The changes done for

Re: [PATCH] commit: switch core.commentChar if it's found in existing commit

2014-05-16 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy writes: > If we need to use core.commentChar and it's already in the prepared > message, find another char among a small subset. This should stop > surprises because git strips some lines unexpectedly. Of course if > candicate characters happen to be all out, this change doe

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

2014-05-16 Thread Junio C Hamano
Jakub Narębski writes: >> Correct, but is "where does it appear" the question we are >> primarily interested in, wrt this breakage and its fix? > > That of course depends on how we want to test gitweb output. > The simplest solution, comparing with known output with perhaps > fragile / variable e

Re: [PATCH v10 00/44] Use ref transactions for all ref updates

2014-05-16 Thread Jonathan Nieder
Ronnie Sahlberg wrote: > This patch series can also be found at > https://github.com/rsahlberg/git/tree/ref-transactions I think the rerolls are coming too fast. I've been using your repository on github to check if I should not send any particular comment because you've already fixed it, so the

Re: [PATCH v8 22/44] fetch.c: clear errno before calling functions that might set it

2014-05-16 Thread Jonathan Nieder
(cc-ing peff, who may remember this STORE_REF_ERROR_DF_CONFLICT case from long ago) Ronnie Sahlberg wrote: > In s_update_ref there are two calls that when they fail we return an error > based on the errno value. In particular we want to return a specific error > if ENOTDIR happened. Both these fu

Re: [PATCH 2/2] commit: allow core.commentChar=auto for character auto selection

2014-05-16 Thread Junio C Hamano
Jonathan Nieder writes: > Nguyễn Thái Ngọc Duy wrote: > >> core.commentChar starts with '#' as in default but if it's already in >> the prepared message, find another one among a small subset. This >> should stop surprises because git strips some lines unexpectedly. > > Probably worth mentioning

Re: [PATCH 1/2] config: be strict on core.commentChar

2014-05-16 Thread Junio C Hamano
Jonathan Nieder writes: > Nguyễn Thái Ngọc Duy wrote: > >> --- a/config.c >> +++ b/config.c >> @@ -826,8 +826,12 @@ static int git_default_core_config(const char *var, >> const char *value) >> if (!strcmp(var, "core.commentchar")) { >> const char *comment; >> int r

[PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

2014-05-16 Thread Junio C Hamano
Older versions of Git before v1.7.10 did not DWIM $ git pull $URL for-linus to the tag "tags/for-linus" and the users were required to say $ git pull $URL tags/for-linus instead. Because newer versions of Git works either way, request-pull used to show tags/for-linus when asked $

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

2014-05-16 Thread Junio C Hamano
(sorry if you receive a dup; pobox.com seems to be constipated right now) Jakub Narębski writes: >> Correct, but is "where does it appear" the question we are >> primarily interested in, wrt this breakage and its fix? > > That of course depends on how we want to test gitweb output. > The simples

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Junio C Hamano
Jeff King writes: > On Thu, May 15, 2014 at 03:56:29PM -0700, Junio C Hamano wrote: > >> Two announcements for their version 0.2 on the list archive are not >> quite enough to advertise them to their users. > > I do not think this README nor a mention in the release notes will get > their attenti

Re: [PATCH] remote-helpers: point at their upstream repositories

2014-05-16 Thread Junio C Hamano
(Sorry if you receive a dup; pobox.com seems to be constipated right now). Jeff King writes: > On Thu, May 15, 2014 at 03:56:29PM -0700, Junio C Hamano wrote: > >> Two announcements for their version 0.2 on the list archive are not >> quite enough to advertise them to their users. > > I do not t

[PATCH v10 01/44] refs.c: constify the sha arguments for ref_transaction_create|delete|update

2014-05-16 Thread Ronnie Sahlberg
ref_transaction_create|delete|update has no need to modify the sha1 arguments passed to it so it should use const unsigned char* instead of unsigned char*. Some functions, such as fast_forward_to(), already have its old/new sha1 arguments as consts. This function will at some point need to use ref

[PATCH v10 11/44] refs.c: change ref_transaction_create to do error checking and return status

2014-05-16 Thread Ronnie Sahlberg
Do basic error checking in ref_transaction_create() and make it return non-zero on error. Update all callers to check the result of ref_transaction_create(). There are currently no conditions in _create that will return error but there will be in the future. Add an err argument that will be updated

[PATCH v10 37/44] refs.c: pass NULL as *flags to read_ref_full

2014-05-16 Thread Ronnie Sahlberg
We call read_ref_full with a pointer to flags from rename_ref but since we never actually use the returned flags we can just pass NULL here instead. Signed-off-by: Ronnie Sahlberg --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 98f3c85..121034

[PATCH v10 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

2014-05-16 Thread Ronnie Sahlberg
Track the status of a transaction in a new status field. Check the field for sanity, i.e. that status must be OPEN when _commit/_create/_delete or _update is called or else die(BUG:...) Signed-off-by: Ronnie Sahlberg --- refs.c | 26 +- 1 file changed, 25 insertions(+), 1

[PATCH v10 14/44] replace.c: use the ref transaction functions for updates

2014-05-16 Thread Ronnie Sahlberg
Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg --- builtin/replace.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 3da1bae..e8932cd 100644 --- a/builtin/replace.c +++ b/builtin/

[PATCH v10 22/44] fetch.c: clear errno before calling functions that might set it

2014-05-16 Thread Ronnie Sahlberg
In s_update_ref there are two calls that when they fail we return an error based on the errno value. In particular we want to return a specific error if ENOTDIR happened. Both these functions do have failure modes where they may return an error without updating errno, in which case a previous and u

[PATCH v10 03/44] refs.c: add a strbuf argument to ref_transaction_commit for error logging

2014-05-16 Thread Ronnie Sahlberg
Add a strbuf argument to _commit so that we can pass an error string back to the caller. So that we can do error logging from the caller instead of from _commit. Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR and craft any log messages from the callers themselves and f

[PATCH v10 02/44] refs.c: allow passing NULL to ref_transaction_free

2014-05-16 Thread Ronnie Sahlberg
Allow ref_transaction_free(NULL) and hence ref_transaction_rollback(NULL) as no-ops. This makes ref_transaction_rollback easier to use and more similar to plain 'free'. In particular, it lets us rollback unconditionally as part of cleanup code after setting 'transaction = NULL' if a transaction ha

[PATCH v10 34/44] refs.c: make prune_ref use a transaction to delete the ref

2014-05-16 Thread Ronnie Sahlberg
Change prune_ref to delete the ref using a ref transaction. To do this we also need to add a new flag REF_ISPRUNING that will tell the transaction that we do not want to delete this ref from the packed refs. This flag is private to refs.c and not exposed to external callers. Signed-off-by: Ronnie

[PATCH v10 20/44] refs.c: free the transaction before returning when number of updates is 0

2014-05-16 Thread Ronnie Sahlberg
We have to free the transaction before returning in the early check for 'return early if number of updates == 0' or else the following code would create a memory leak with the transaction never being freed : t = ref_transaction_begin() ref_transaction_commit(t) Signed-off-by: Ronnie Sahlberg

[PATCH v10 05/44] refs.c: make ref_update_reject_duplicates take a strbuf argument for errors

2014-05-16 Thread Ronnie Sahlberg
Make ref_update_reject_duplicates return any error that occurs through a new strbuf argument. This means that when a transaction commit fails in this function we will now be able to pass a helpful error message back to the caller. Reviewed-by: Jonathan Nieder Signed-off-by: Ronnie Sahlberg ---

[PATCH v10 28/44] refs.c: make write_ref_sha1 static

2014-05-16 Thread Ronnie Sahlberg
No external users call write_ref_sha1 any more so lets declare it static. Signed-off-by: Ronnie Sahlberg --- refs.c | 4 +++- refs.h | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 266a792..93e2cd2 100644 --- a/refs.c +++ b/refs.c @@ -251,6 +251,8 @@

[PATCH v10 12/44] refs.c: ref_transaction_delete to check for error and return status

2014-05-16 Thread Ronnie Sahlberg
Change ref_transaction_delete() to do basic error checking and return non-zero of error. Update all callers to check the return for ref_transaction_delete(). There are currently no conditions in _delete that will return error but there will be in the future. Add an err argument that will be updated

[PATCH v10 10/44] refs.c: change ref_transaction_update() to do error checking and return status

2014-05-16 Thread Ronnie Sahlberg
Update ref_transaction_update() do some basic error checking and return non-zero on error. Update all callers to check ref_transaction_update() for error. There are currently no conditions in _update that will return error but there will be in the future. Add an err argument that will be updated on

[PATCH v10 24/44] fetch.c: use a single ref transaction for all ref updates

2014-05-16 Thread Ronnie Sahlberg
Change store_updated_refs to use a single ref transaction for all refs that are updated during the fetch. This makes the fetch more atomic when update failures occur. Since ref update failures will now no longer occur in the code path for updating a single ref in s_update_ref, we no longer have as

[PATCH v10 08/44] update-ref.c: log transaction error from the update_ref

2014-05-16 Thread Ronnie Sahlberg
Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is returned to print a log message if/after the transaction fails. Reviewed-by: Jonathan Nieder Signed-off-by: Ronnie Sahlberg --- builtin/update-ref.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --

[PATCH v10 07/44] refs.c: make update_ref_write update a strbuf on failure

2014-05-16 Thread Ronnie Sahlberg
Change update_ref_write to also update an error strbuf on failure. This makes the error available to ref_transaction_commit callers if the transaction failed due to update_ref_sha1/write_ref_sha1 failures. Reviewed-by: Jonathan Nieder Signed-off-by: Ronnie Sahlberg --- refs.c | 9 ++--- 1 f

[PATCH v10 15/44] commit.c: use ref transactions for updates

2014-05-16 Thread Ronnie Sahlberg
Change commit.c to use ref transactions for all ref updates. Make sure we pass a NULL pointer to ref_transaction_update if have_old is false. Reviewed-by: Jonathan Nieder Signed-off-by: Ronnie Sahlberg --- builtin/commit.c | 23 ++- 1 file changed, 10 insertions(+), 13 delet

[PATCH v10 36/44] refs.c: pass the ref log message to _create/delete/update instead of _commit

2014-05-16 Thread Ronnie Sahlberg
Change the reference transactions so that we pass the reflog message through to the create/delete/update function instead of the commit message. This allows for individual messages for each change in a multi ref transaction. Signed-off-by: Ronnie Sahlberg --- branch.c | 4 ++-- bu

[PATCH v10 29/44] refs.c: make lock_ref_sha1 static

2014-05-16 Thread Ronnie Sahlberg
No external callers reference lock_ref_sha1 any more so lets declare it static. Signed-off-by: Ronnie Sahlberg --- refs.c | 2 +- refs.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 93e2cd2..2d9789f 100644 --- a/refs.c +++ b/refs.c @@ -2126,7 +2126

[PATCH v10 09/44] refs.c: remove the onerr argument to ref_transaction_commit

2014-05-16 Thread Ronnie Sahlberg
Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr argument any more. Remove the onerr argument from the ref_transaction_commit signature. Reviewed-by: Jonathan Nieder Signed-off-by: Ronnie Sahlberg --- builtin/update-ref.c | 3 +-- refs.c | 22 +++--

[PATCH v10 04/44] refs.c: add an err argument to repack_without_refs

2014-05-16 Thread Ronnie Sahlberg
Update repack_without_refs to take an err argument and update it if there is a failure. Pass the err variable from ref_transaction_commit to this function so that callers can print a meaningful error message if _commit fails due to a problem in repack_without_refs. Signed-off-by: Ronnie Sahlberg

[PATCH v10 25/44] receive-pack.c: use a reference transaction for updating the refs

2014-05-16 Thread Ronnie Sahlberg
Wrap all the ref updates inside a transaction to make the update atomic. Signed-off-by: Ronnie Sahlberg --- builtin/receive-pack.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c323081..5534138 100

[PATCH v10 42/44] refs.c: pass a skip list to name_conflict_fn

2014-05-16 Thread Ronnie Sahlberg
Allow passing a list of refs to skip checking to name_conflict_fn. There are some conditions where we want to allow a temporary conflict and skip checking those refs. For example if we have a transaction that 1, guarantees that m is a packed refs and there is no loose ref for m 2, the transaction w

[PATCH v10 32/44] refs.c: remove the update_ref_write function

2014-05-16 Thread Ronnie Sahlberg
Since we only call update_ref_write from a single place and we only call it with onerr==QUIET_ON_ERR we can just as well get rid of it and just call write_ref_sha1 directly. Signed-off-by: Ronnie Sahlberg --- refs.c | 35 +-- 1 file changed, 9 insertions(+), 26 de

[PATCH v10 43/44] refs.c: make rename_ref use a transaction

2014-05-16 Thread Ronnie Sahlberg
Change rename_ref to use a single transaction to perform the ref rename. Signed-off-by: Ronnie Sahlberg --- refs.c | 73 ++ 1 file changed, 20 insertions(+), 53 deletions(-) diff --git a/refs.c b/refs.c index 3967333..71e3059 10064

[PATCH v10 38/44] refs.c: pack all refs before we start to rename a ref

2014-05-16 Thread Ronnie Sahlberg
This means that most loose refs will no longer be present after the rename which triggered a test failure since it assumes the file for an unrelated ref would still be present after the rename. Signed-off-by: Ronnie Sahlberg --- refs.c| 3 +++ t/t3200-branch.sh | 2 +- 2 files change

[PATCH v10 33/44] refs.c: remove lock_ref_sha1

2014-05-16 Thread Ronnie Sahlberg
lock_ref_sha1 was only called from one place in refc.c and only provided a check that the refname was sane before adding back the initial "refs/" part of the ref path name, the initial "refs/" that this caller had already stripped off before calling lock_ref_sha1. Signed-off-by: Ronnie Sahlberg -

[PATCH v10 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-16 Thread Ronnie Sahlberg
Move the check for check_refname_format from lock_any_ref_for_update to lock_ref_sha1_basic. At some later stage we will get rid of lock_any_ref_for_update completely. This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed. But this wrapper is also called from an external c

[PATCH v10 31/44] refs.c: remove the update_ref_lock function

2014-05-16 Thread Ronnie Sahlberg
Since we now only call update_ref_lock with onerr==QUIET_ON_ERR we no longer need this function and can replace it with just calling lock_any_ref_for_update directly. Signed-off-by: Ronnie Sahlberg --- refs.c | 30 ++ 1 file changed, 6 insertions(+), 24 deletions(-)

[PATCH v10 40/44] refs.c: call lock_ref_sha1_basic directly from commit

2014-05-16 Thread Ronnie Sahlberg
Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic directly from the commit function. Signed-off-by: Ronnie Sahlberg --- refs.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index f63a356..564feb6 100644 --- a/refs.c +++

[PATCH v10 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-16 Thread Ronnie Sahlberg
Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. This flag indicates that the ref does not exist as a loose ref andf only as a packed ref. If this is the case we then change the commit code so that we skip taking out a lock file and we skip calling delete_ref_loose. Check fo

[PATCH v10 23/44] fetch.c: change s_update_ref to use a ref transaction

2014-05-16 Thread Ronnie Sahlberg
Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg --- builtin/fetch.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index a93c893..8cf70cd 100644 --- a/builtin/fetch.c +++ b/bu

[PATCH v10 26/44] fast-import.c: use a ref transaction when dumping tags

2014-05-16 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg --- fast-import.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/fast-import.c b/fast-import.c index cb3f5af..9c76c73 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1735,15 +1735,22 @@ static void dump_tags(void) {

[PATCH v10 27/44] walker.c: use ref transaction for ref updates

2014-05-16 Thread Ronnie Sahlberg
Switch to using ref transactions in walker_fetch(). As part of the refactoring to use ref transactions we also fix a potential memory leak where in the original code if write_ref_sha1() would fail we would end up returning from the function without free()ing the msg string. This changes the lockin

[PATCH v10 44/44] refs.c: remove forward declaration of write_ref_sha1

2014-05-16 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg --- refs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs.c b/refs.c index 71e3059..bf84306 100644 --- a/refs.c +++ b/refs.c @@ -260,8 +260,6 @@ struct ref_entry { }; static void read_loose_refs(const char *dirname, struct ref_dir *dir); -static int w

[PATCH v10 21/44] refs.c: ref_transaction_commit should not free the transaction

2014-05-16 Thread Ronnie Sahlberg
Change ref_transaction_commit so that it does not free the transaction. Instead require that a caller will end a transaction by calling ref_transaction_free. By having the transaction object remaining valid after _commit returns allows us to write much nicer code and still be able to call ref_tran

[PATCH v10 19/44] refs.c: change update_ref to use a transaction

2014-05-16 Thread Ronnie Sahlberg
Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg --- refs.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index a588194..78312b5 100644 --- a/refs.c +++ b/refs.c @@ -3442,11

[PATCH v10 17/44] fast-import.c: change update_branch to use ref transactions

2014-05-16 Thread Ronnie Sahlberg
Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg --- fast-import.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/fast-import.c b/fast-import.c index 6707a66..60d4538 100644 --- a/fast-import.c +++ b/fast-

[PATCH v10 35/44] refs.c: make delete_ref use a transaction

2014-05-16 Thread Ronnie Sahlberg
Change delete_ref to use a ref transaction for the deletion. At the same time since we no longer have any callers of repack_without_ref we can now delete this function. Signed-off-by: Ronnie Sahlberg --- refs.c | 31 ++- 1 file changed, 10 insertions(+), 21 deletions(

[PATCH v10 13/44] tag.c: use ref transactions when doing updates

2014-05-16 Thread Ronnie Sahlberg
Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg --- builtin/tag.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index c6e8a71..b05f9a5 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -54

[PATCH v10 16/44] sequencer.c: use ref transactions for all ref updates

2014-05-16 Thread Ronnie Sahlberg
Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg --- sequencer.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/sequencer.c b/sequencer.c index 0a80c58..b047fb8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -272

[PATCH v10 18/44] branch.c: use ref transaction for all ref updates

2014-05-16 Thread Ronnie Sahlberg
Change create_branch to use a ref transaction when creating the new branch. ref_transaction_create will check that the ref does not already exist and fail otherwise meaning that we no longer need to keep a lock on the ref during the setup_tracking. This simplifies the code since we can now do the t

[PATCH v10 06/44] refs.c: add an err argument to delete_ref_loose

2014-05-16 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 un

[PATCH v10 00/44] Use ref transactions for all ref updates

2014-05-16 Thread Ronnie Sahlberg
This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions This patch series is based on next and expands on the transaction API. It converts all ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most of the ref u

Re: [PATCH 2/2] commit: allow core.commentChar=auto for character auto selection

2014-05-16 Thread Jonathan Nieder
Nguyễn Thái Ngọc Duy wrote: > core.commentChar starts with '#' as in default but if it's already in > the prepared message, find another one among a small subset. This > should stop surprises because git strips some lines unexpectedly. Probably worth mentioning this only kicks in if someone expli

Re: [PATCH 1/2] config: be strict on core.commentChar

2014-05-16 Thread Jonathan Nieder
Nguyễn Thái Ngọc Duy wrote: > --- a/config.c > +++ b/config.c > @@ -826,8 +826,12 @@ static int git_default_core_config(const char *var, > const char *value) > if (!strcmp(var, "core.commentchar")) { > const char *comment; > int ret = git_config_string(&comment,

Re: [GUILT v3 09/31] Test suite: properly check the exit status of commands.

2014-05-16 Thread Jeff Sipek
On Fri, May 16, 2014 at 04:45:56PM +0200, Per Cederqvist wrote: > The "cmd" and "shouldfail" functions checked the exit status of the > replace_path function instead of the actual command that was running. > (The $? construct checks the exit status of the last command in a > pipeline, not the first

  1   2   >