Re: revert top most commit

Keller, Jacob E wrote: >> On Wed, 2014-08-27 at 21:14 +, Keller, Jacob E wrote: >>> I am having trouble using revert. If I attempt to >>> >>> $ git revert >>> >>> where sha1id is the same as the HEAD commit, I instead get the output of >>> what looks like git status. [...] > It's actually not

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

Matthieu Moy wrote: > Signed-off-by: Matthieu Moy [...] > --- > advice.c| 3 +-- > git-pull.sh | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) Thanks for taking it on. Reviewed-by: Jonathan Nieder [...] > It was already on my todo-list, as a friend of mine

Re: Configurable filename for what is now .gitignore

Hi, Bostjan Skufca wrote: > Git is great for tracking code development, but when deploying > mentioned code by using git itself, various configuration files must > be created additionally, which are normally .gitignored, for various > reasons (code portability, sensitive data, etc). There is curr

[PATCH v22 0/22] rs/ref-transaction-1 (Re: Transaction patch series overview)

fast-import but otherwise doesn't change the granularity of transactions. Jonathan Nieder (2): update-ref --stdin: narrow scope of err strbuf update-ref --stdin: pass transaction around explicitly Ronnie Sahlberg (20): refs.c: change ref_transaction_create to do error che

[PATCH 01/22] refs.c: change ref_transaction_create to do error checking and return status

will be in the future. Add an err argument that will be updated on failure. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/update-ref.c | 4 +++- refs.c | 18 -- refs.h | 48

[PATCH 02/22] refs.c: update ref_transaction_delete to check for error and return status

will be in the future. Add an err argument that will be updated on failure. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/update-ref.c | 5 +++-- refs.c | 16 +++- refs.h | 12 3

[PATCH 03/22] refs.c: make ref_transaction_begin take an err argument

add other types of backends in the future. For example, a hypothetical MySQL backend could fail in _begin with "Can not connect to MySQL server. No route to host". Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/update

[PATCH 05/22] tag.c: use ref transactions when doing updates

From: Ronnie Sahlberg Date: Wed, 16 Apr 2014 15:30:41 -0700 Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/tag.c | 16 ++-- 1 file changed, 10 insertions(+), 6

[PATCH 04/22] refs.c: add transaction.status and track OPEN/CLOSED

Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 9cb7908..cc63056 100644 --- a/refs.c +++ b/refs.c @@ -3387,6 +3387,21 @@ struct ref_update { }; /* + * Transaction

[PATCH 06/22] replace.c: use the ref transaction functions for updates

From: Ronnie Sahlberg Date: Wed, 16 Apr 2014 15:32:29 -0700 Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/replace.c | 14 -- 1 file changed, 8 insertions(+), 6

[PATCH 07/22] commit.c: use ref transactions for updates

From: Ronnie Sahlberg Date: Wed, 16 Apr 2014 15:34:19 -0700 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. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder

[PATCH 08/22] sequencer.c: use ref transactions for all ref updates

From: Ronnie Sahlberg Date: Wed, 16 Apr 2014 15:37:45 -0700 Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- sequencer.c | 26 ++ 1 file changed, 18 insertions

[PATCH 09/22] fast-import.c: change update_branch to use ref transactions

From: Ronnie Sahlberg Date: Wed, 16 Apr 2014 16:21:13 -0700 Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- fast-import.c | 25 +++-- 1 file changed, 15 insertions

[PATCH 10/22] branch.c: use ref transaction for all ref updates

against the ref already existing. I.e. one thread could end up overwriting a branch even if the forcing flag is false. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- branch.c | 31 +-- 1 file changed, 17 insertions

[PATCH 11/22] refs.c: change update_ref to use a transaction

From: Ronnie Sahlberg Date: Thu, 24 Apr 2014 16:36:55 -0700 Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 30 ++ 1 file changed

[PATCH 12/22] receive-pack.c: use a reference transaction for updating the refs

still refs under 'refs/heads/topic' remote: error: Cannot lock the ref 'refs/heads/topic'. To foo ! [remote rejected] HEAD -> topic (failed to update ref) Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- builtin/r

[PATCH 13/22] fast-import.c: use a ref transaction when dumping tags

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 15:23:58 -0700 Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- fast-import.c | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/fast-import.c b/fast

[PATCH 14/22] walker.c: use ref transaction for ref updates

fail for are even more rare. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- walker.c | 72 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/walker.c b/walker.c index

[PATCH 15/22] refs.c: make lock_ref_sha1 static

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 15:38:47 -0700 No external callers reference lock_ref_sha1 any more so let's declare it static. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 7 +-- refs.h | 6 -- 2 files ch

[PATCH 16/22] refs.c: remove the update_ref_lock function

Signed-off-by: Jonathan Nieder --- refs.c | 30 ++ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/refs.c b/refs.c index d4cd44b..a4445a1 100644 --- a/refs.c +++ b/refs.c @@ -3336,24 +3336,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) return

[PATCH 17/22] refs.c: remove the update_ref_write function

failures when writing to the ref. Eventually we will want _commit to start returning more detailed error conditions than the current simple success/failure. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 34

[PATCH 18/22] refs.c: remove lock_ref_sha1

already stripped off before calling lock_ref_sha1. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/refs.c b/refs.c index a6b39ec..fd67684 100644 --- a

[PATCH 19/22] refs.c: make prune_ref use a transaction to delete the ref

refs.c and not exposed to external callers. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 28 +--- refs.h | 13 +++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index

[PATCH 20/22] refs.c: make delete_ref use a transaction

of the previous 0 on success either 1 or -1 on failure. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 35 ++- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index 22eb3dd

[PATCH 21/22] update-ref --stdin: narrow scope of err strbuf

* concurrency issues, if this code starts using threads some day No functional change intended. Signed-off-by: Jonathan Nieder Reviewed-by: Michael Haggerty --- builtin/update-ref.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c

[PATCH 22/22] update-ref --stdin: pass transaction around explicitly

This makes it more obvious at a glance where the output of functions parsing the --stdin stream goes. No functional change intended. Signed-off-by: Jonathan Nieder Reviewed-by: Michael Haggerty --- Thanks for reading. builtin/update-ref.c | 27 +++ 1 file changed, 15

Re: [PATCH] parse-options: detect attempt to add a duplicate short option name

if (opts->short_name < ' ' || 0x7F <= opts->short_name) > + strbuf_addf(&errmsg, "invalid short name > (0x%02x)", > + opts->short_name); > + else if (short_o

Re: [PATCH] parse-options: detect attempt to add a duplicate short option name

On Wed, Sep 03, 2014 at 02:46:25PM -0700, Jonathan Nieder wrote: > Junio C Hamano wrote: > > > --- a/parse-options.c > > +++ b/parse-options.c > > @@ -345,12 +345,27 @@ static void check_typos(const char *arg, const struct > > option *options) > > static v

Re: [RFC PATCH v2 1/2] Makefile: add check-headers target

Hi, David Aguilar wrote: > --- /dev/null > +++ b/check-headers.sh > @@ -0,0 +1,29 @@ [...] > + "$@" -Wno-unused -I"$subdir" -c -o "$header".check -x c - <"$header" && All .c files in git are supposed to start by #include-ing git-compat-util.h, cache.h, or builtin.h to set the appropriate fea

Re: [RFC PATCH v2 2/2] headers: include dependent headers

David Aguilar wrote: > Add dependent headers so that including a header does not > require including additional headers. I agree with this goal, modulo the compat-util.h caveat. Thanks for working on it. [...] > --- a/archive.h > +++ b/archive.h > @@ -1,6 +1,7 @@ > #ifndef ARCHIVE_H > #define

Re: [RFC PATCH v2 2/2] headers: include dependent headers

Johannes Sixt wrote: > Am 07.09.2014 21:49, schrieb Jonathan Nieder: >> +enum object_type; > > Enum forward declarations are a relatively new C feature. They certainly > don't exist pre-C99. Good catch. That makes diff --git i/archive.h w/archive.h index 4a791e1..b2ca5bf

Re: [PATCH] fsck: exit with non-zero status upon error from fsck_obj()

Junio C Hamano wrote: > By the way, Jonathan, with dbedf8bf (t1450 (fsck): remove dangling > objects, 2010-09-06) you added a 'test_might_fail git fsck' to the > 1450 test that catches an object corruption. Do you remember if > there was some flakiness in this test that necessitated it, or is it

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

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)

[PATCH 01/19] mv test: recreate mod/ directory instead of relying on stale copy

d/ directory so later tests don't have to depend on the directory left behind by the earlier ones at all (making it easier to rearrange or skip some tests in the file or to tweak 'reset --hard' behavior without breaking unrelated tests). Noticed while testing a patch that fixes th

[PATCH 02/19] wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success

d not exist". Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- git-compat-util.h | 7 +-- wrapper.c | 14 ++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index b6f03b3..611e77b 100644 --- a/

[PATCH 03/19] wrapper.c: add a new function unlink_or_msg

From: Ronnie Sahlberg Date: Wed, 16 Jul 2014 11:20:36 -0700 This behaves like unlink_or_warn except that on failure it writes the message to its 'err' argument, which the caller can display in an appropriate way or ignore. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder

[PATCH 04/19] refs.c: add an err argument to delete_ref_loose

transaction failed due to delete_loose_ref. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 7235574..5609622 100644 --- a/refs.c +++ b/refs.c @@ -2548,16 +2548,16 @@ int

[PATCH 07/19] refs.c: move the check for valid refname to lock_ref_sha1_basic

d refs, at which time we will need to modify lock_ref_sha1_basic once more in order to allow locking these refs for certain use cases such as rename and delete. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) di

[PATCH 06/19] rename_ref: don't ask read_ref_full where the ref came from

From: Ronnie Sahlberg Date: Wed, 30 Apr 2014 12:41:04 -0700 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 Signed-off-by: Jonathan Nieder --- refs.c | 2 +- 1

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

-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- branch.c | 4 ++-- builtin/commit.c | 4 ++-- builtin/receive-pack.c | 5 +++-- builtin/replace.c | 5 +++-- builtin/tag.c | 4 ++-- builtin/update-ref.c | 13 +++-- fast-import.c

[PATCH 08/19] refs.c: call lock_ref_sha1_basic directly from commit

From: Ronnie Sahlberg Date: Thu, 1 May 2014 10:43:39 -0700 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 Signed-off-by: Jonathan Nieder --- refs.c | 12 ++-- 1 file changed, 6 insertions

[PATCH 10/19] refs.c: ref_transaction_commit: distinguish name conflicts from other errors

rs in that they are likely to be resolved by running "git remote prune ". "git fetch" currently inspects errno to decide whether to give that advice. Once it switches to the transaction API, it can check for UPDATE_REFS_NAME_CONFLICT instead. Signed-off-by: Ronnie Sahlberg

[PATCH 09/19] refs.c: pass a skip list to name_conflict_fn

by the transaction when it deletes m. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 41 - 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index f124c2b..b63ab2f 100644 --- a/refs.c +++ b/refs.c

[PATCH 11/19] fetch.c: change s_update_ref to use a ref transaction

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 13:49:07 -0700 Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/fetch.c | 34 -- 1 file changed, 24 insertions(+), 10

[PATCH 12/19] refs.c: make write_ref_sha1 static

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 15:36:58 -0700 No external users call write_ref_sha1 any more so lets declare it static. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 10 -- refs.h | 3 --- 2 files changed, 8 insertions(+), 5 deletions

[PATCH 13/19] refs.c: change resolve_ref_unsafe reading argument to be a flags field

capture any instances where callers have not been updated/aware of the new api so that they can be audited. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- branch.c| 2 +- builtin/blame.c | 2 +- builtin/branch.c| 9 +++--- builtin/checkout.c

[PATCH 17/19] refs.c: do not permit err == NULL

these behaviors are for historical reasons and others were accidents. Luckily no callers pass err == NULL any more. Simplify by consistently requiring the strbuf argument. Signed-off-by: Jonathan Nieder --- refs.c | 46 +++--- 1 file changed, 27 insertions

[PATCH 16/19] for-each-ref.c: improve message before aborting on broken ref

From: Ronnie Sahlberg Date: Fri, 5 Sep 2014 14:35:17 -0700 Print a warning message for any badly named refs we find in the repo. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/for-each-ref.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/builtin/for-each

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

refs and refuse to delete them unless REF_BADNAMEOK flag is passed. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/branch.c| 13 ++ builtin/update-ref.c| 3 ++- cache.h | 8 +- refs.c

[PATCH 14/19] branch -d: avoid repeated symref resolution

an still read the value of a symref (for example to print a message about it) with that flag set --- resolve_ref_unsafe will resolve one level of symrefs and stop there. Signed-off-by: Jonathan Nieder --- builtin/branch.c | 3 ++- cache.h | 1 + refs.c| 10 +++

[PATCH 18/19] lockfile: remove unable_to_lock_error

The former caller uses unable_to_lock_message now. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- cache.h| 1 - lockfile.c | 10 -- 2 files changed, 11 deletions(-) diff --git a/cache.h b/cache.h index 03a6144..995729f 100644 --- a/cache.h +++ b/cache.h @@ -558,7

[PATCH 19/19] ref_transaction_commit: bail out on failure to remove a ref

transactionality will have to wait for a later series (the plan for which is to make all destructive work happen in a single update of the packed-refs file). Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- Thanks for reading. refs.c | 8 ++-- 1 file changed, 6 insertions(+), 2

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

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 remo

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

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.

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

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

Re: [PATCH v5 01/35] unable_to_lock_die(): rename function from unable_to_lock_index_die()

; 4 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Jonathan Nieder -- 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 v5 02/35] api-lockfile: expand the documentation

Michael Haggerty wrote: > Document a couple more functions and the flags argument as used by > hold_lock_file_for_update() and hold_lock_file_for_append(). Thanks. [...] > --- a/Documentation/technical/api-lockfile.txt > +++ b/Documentation/technical/api-lockfile.txt > @@ -28,9 +28,39 @@ hold_lo

Re: [PATCH v5 03/35] rollback_lock_file(): do not clear filename redundantly

this use close_lock_file which sets fd to -1 before closing? if (!lk->filename[0]) return; close_lock_file(lk); unlink_or_warn(lk->filename); lk->filename[0] = 0; With or without such changes, Reviewed-by: Jonathan Nieder -- T

Re: [PATCH v5 04/35] rollback_lock_file(): exit early if lock is not active

Michael Haggerty wrote: > Eliminate a layer of nesting. Oh --- guess I should have been more patient. :) Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo

Re: [PATCH v5 05/35] rollback_lock_file(): set fd to -1

Michael Haggerty wrote: > --- a/lockfile.c > +++ b/lockfile.c > @@ -276,7 +276,7 @@ void rollback_lock_file(struct lock_file *lk) > return; > > if (lk->fd >= 0) > - close(lk->fd); > + close_lock_file(lk); Doesn't need to be guarded by the 'if'. -- To

Re: [PATCH v5 05/35] rollback_lock_file(): set fd to -1

Jonathan Nieder wrote: > Michael Haggerty wrote: >> --- a/lockfile.c >> +++ b/lockfile.c >> @@ -276,7 +276,7 @@ void rollback_lock_file(struct lock_file *lk) >> return; >> >> if (lk->fd >= 0) >> -close(lk->fd);

Re: [PATCH v5 06/35] lockfile: unlock file if lockfile permissions cannot be adjusted

Michael Haggerty wrote: > Signed-off-by: Michael Haggerty > --- > lockfile.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org

Re: [PATCH v5 07/35] hold_lock_file_for_append(): release lock on errors

errors (e.g., by sleeping and trying again), and if not, what is the optionally-die behavior in hold_lock_file about? In any case, Reviewed-by: Jonathan Nieder -- 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 v5 08/35] lock_file(): always add lock_file object to lock_file_list

Michael Haggerty wrote: > The ambiguity didn't have any ill effects, because lock_file objects > cannot be removed from the lock_file_list anyway. But it is > unnecessary to leave this behavior inconsistent. Nit: commit messages usually use present tense for current behavior (and imperative for

Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

Michael Haggerty wrote: > --- a/lockfile.c > +++ b/lockfile.c > @@ -4,6 +4,63 @@ > #include "cache.h" > #include "sigchain.h" > > +/* > + * File write-locks as used by Git. > + * > + * When a file at $FILENAME needs to be written, it is done as > + * follows: This overlaps a lot with the API do

Re: [PATCH v5 10/35] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

Michael Haggerty wrote: > There are a few places that use these values, so define constants for > them. Seems like a symptom of the API leaving out a useful helper (e.g., something that strips off the lock suffix and returns a memdupz'd filename). [...] > --- a/cache.h > +++ b/cache.h > @@ -570,

Re: [PATCH v5 11/35] delete_ref_loose(): don't muck around in the lock_file's filename

Michael Haggerty wrote: > It's bad manners. Especially since there could be a signal during the > call to unlink_or_warn(), in which case the signal handler will see > the wrong filename and delete the reference file, leaving the lockfile > behind. > > So make our own copy to work with. Nice. Co

Re: [PATCH v5 12/35] prepare_index(): declare return value to be (const char *)

ime and therefore shouldn't be > tampered with.) > > Signed-off-by: Michael Haggerty > --- > builtin/commit.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Jonathan Nieder I wonder if we should just bite the bullet and make lock_file an opa

Re: [PATCH v5 14/35] lock_file(): exit early if lockfile cannot be opened

Michael Haggerty wrote: > lockfile.c | 23 +++ > 1 file changed, 11 insertions(+), 12 deletions(-) Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More m

Re: [PATCH v5 15/35] remove_lock_file(): call rollback_lock_file()

Michael Haggerty wrote: > lockfile.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) Nice. Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordo

Re: [PATCH v5 16/35] commit_lock_file(): inline temporary variable

Michael Haggerty wrote: > --- a/lockfile.c > +++ b/lockfile.c > @@ -311,12 +311,14 @@ int reopen_lock_file(struct lock_file *lk) > int commit_lock_file(struct lock_file *lk) > { > char result_file[PATH_MAX]; > - size_t i; > + > if (lk->fd >= 0 && close_lock_file(lk)) >

Re: [PATCH v5 17/35] commit_lock_file(): die() if called for unlocked lockfile object

Michael Haggerty wrote: > --- a/lockfile.c > +++ b/lockfile.c > @@ -312,6 +312,9 @@ int commit_lock_file(struct lock_file *lk) > { > char result_file[PATH_MAX]; > > + if (!lk->filename[0]) > + die("BUG: attempt to commit unlocked object"); Sure, this is fine instead of an

Re: [PATCH v5 18/35] commit_lock_file(): if close fails, roll back

Michael Haggerty wrote: > If closing an open lockfile fails, then we cannot be sure of the > contents of the lockfile Is that true? It seems more like a bug in close_lock_file: if it fails, perhaps it should either set lk->fd back to fd or unlink the lockfile itself. What do other callers do on

Re: [PATCH v5 19/35] commit_lock_file(): rollback lock file on failure to rename

Michael Haggerty wrote: > If rename() fails, call rollback_lock_file() to delete the lock file > (in case it is still present) and reset the filename field to the > empty string so that the lockfile object is left in a valid state. Can you spell out more what the goal is? Is the idea to keep the

Re: [PATCH v5 20/35] api-lockfile: document edge cases

Michael Haggerty wrote: > * Document the behavior of commit_lock_file() when it fails, namely > that it rolls back the lock_file object and sets errno > appropriately. > > * Document the behavior of rollback_lock_file() when called for a > lock_file object that has already been committed or

Re: [PATCH v5 21/35] dump_marks(): remove a redundant call to rollback_lock_file()

Michael Haggerty wrote: > Signed-off-by: Michael Haggerty > --- > fast-import.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord..

Re: [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()

Michael Haggerty wrote: > After commit_lock_file() is called, then the lock_file object is > necessarily either committed or rolled back. So there is no need to > call rollback_lock_file() again in either of these cases. > > Signed-off-by: Michael Haggerty This seems to involve an unadvertised

Re: [PATCH v5 23/35] lockfile: avoid transitory invalid states

Michael Haggerty wrote: > We could probably continue to use the filename field to encode the > state by being careful to write characters 1..N-1 of the filename > first, and then overwrite the NUL at filename[0] with the first > character of the filename, but that would be awkward and error-prone.

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

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 r

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

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,

Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

Michael Haggerty wrote: > I agree with your point about overlap. I will split the documentation > into two parts with less redundancy: > > * Documentation/technical/api-lockfile.txt: How to use the API. > > * lockfile.{c,h}: Internal implementation details. > > I think the implementation details w

Re: [PATCH 2/2] use REALLOC_ARRAY for changing the allocation size of arrays

René Scharfe wrote: > --- a/khash.h > +++ b/khash.h (not really about this patch) Where did this file come from? Do we want to be able to sync with upstream to get later bugfixes (e.g., https://github.com/attractivechaos/klib/commit/000f0890)? If so, it might make sense to avoid unnecessary cha

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

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 wer

Re: `git log relative_path_to_object` does not respect the --work-tree path

Hi Roberto, Roberto Eduardo Decurnex Gorosito wrote: > When passing objects to the `git log`, by just naming them or using > the `--objects` option, relative paths are evaluated using the current > working directory instead of the current working tree path. Why should they be relative to the wor

Re: $GIT_CONFIG should either apply to all commands, or none at all

Hi, Frédéric Brière wrote[1]: > This kind of stuff caused me a lot of hair-pulling: > > $ git config core.abbrev > 32 > git log --pretty=oneline --abbrev-commit > 89be foo > > Here's the source of the discrepancy: > > $ grep abbrev $GIT_CONFIG .git/config > git.conf: abbrev=32 > .

[PATCH v22 0/24] rs/ref-transaction

Jonathan Nieder wrote: > 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 revi

[PATCH 01/24] mv test: recreate mod/ directory instead of relying on stale copy

d while testing a patch that fixes the reset --hard behavior described above. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- As before. t/t7001-mv.sh | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh ind

[PATCH 02/24] wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success

depend on "fail if the file did not exist". Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Change since v21: - adjust caller to remove redundant errno check git-compat-util.h | 7 +-- refs.c| 2 +- wrapper.c | 14 ++ 3 files c

[PATCH 03/24] wrapper.c: add a new function unlink_or_msg

From: Ronnie Sahlberg Date: Wed, 16 Jul 2014 11:20:36 -0700 This behaves like unlink_or_warn except that on failure it writes the message to its 'err' argument, which the caller can display in an appropriate way or ignore. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder

[PATCH 04/24] refs.c: add an err argument to delete_ref_loose

transaction failed due to delete_ref_loose. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Changes since v21: - s/delete_loose_ref/delete_ref_loose/ once in commit message (but the other one still needs fixing) refs.c | 11 ++- 1 file changed, 6 insertions(+), 5

[PATCH 06/24] rename_ref: don't ask read_ref_full where the ref came from

From: Ronnie Sahlberg Date: Wed, 30 Apr 2014 12:41:04 -0700 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 Signed-off-by: Jonathan Nieder Reviewed-by: Michael

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

-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Changes since v21: - cleaned up commit message - clarified ownership of msg in comment in refs.h branch.c | 4 ++-- builtin/commit.c | 4 ++-- builtin/receive-pack.c | 5 +++-- builtin/replace.c | 5

[PATCH 07/24] refs.c: refuse to lock badly named refs in lock_ref_sha1_basic

by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Change since v21: - clarified commit message refs.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 39571f5..3c2ce57 100644 --- a/refs.c +++ b/refs.c @@ -2091,6 +2091,11 @@ static struc

[PATCH 08/24] refs.c: call lock_ref_sha1_basic directly from commit

From: Ronnie Sahlberg Date: Thu, 1 May 2014 10:43:39 -0700 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 Signed-off-by: Jonathan Nieder Reviewed-by: Michael Haggerty --- As before. refs.c | 12

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

> m/m No functional change intended yet. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Since v21: - clarified commit message - clarified comments refs.c | 44 +--- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/re

[PATCH 10/24] refs.c: ref_transaction_commit: distinguish name conflicts from other errors

y: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Since v21: - clarified commit message and updated to match code - small code cleanups - clarified API doc, introduced TRANSACTION_GENERIC_ERROR so both error codes have names refs.c | 26 -- refs.h | 9 +++--

[PATCH 11/24] fetch.c: change s_update_ref to use a ref transaction

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 13:49:07 -0700 Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Since v21: - tweaked handling of ref_transaction_commit result (no functional change) builtin

[PATCH 12/24] refs.c: make write_ref_sha1 static

From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 15:36:58 -0700 No external users call write_ref_sha1 any more so let's declare it static. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Since v21: - punctuation fix in commit message - grammar tweak in doc comment refs.c

[PATCH 14/24] reflog test: test interaction with detached HEAD

x27;t have to rely on manual testing to catch such problems in the future. [jn: using 'log -g --format=%H' instead of parsing --oneline output, resetting state in each test so they can be safely reordered or skipped] Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --

[PATCH 13/24] refs.c: change resolve_ref_unsafe reading argument to be a flags field

treatment for consistency. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- Since v21: - clarified commit message - put output parameters last branch.c| 2 +- builtin/blame.c | 2 +- builtin/branch.c| 9 ++--- builtin/checkout.c | 6

<    10   11   12   13   14   15   16   17   18   19   >