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

2014-09-10 Thread Jonathan Nieder
of 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 jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- Thanks for reading. refs.c | 8 ++-- 1

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 jrnie...@gmail.com 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

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 jrnie...@gmail.com 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

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

2014-09-12 Thread Jonathan Nieder
Michael Haggerty wrote: Jonathan Nieder jrnie...@gmail.com 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

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

2014-09-16 Thread Jonathan Nieder
(+), 6 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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

2014-09-16 Thread Jonathan Nieder
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 @@

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

2014-09-16 Thread Jonathan Nieder
]) return; close_lock_file(lk); unlink_or_warn(lk-filename); lk-filename[0] = 0; With or without such changes, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More

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

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

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

2014-09-16 Thread Jonathan Nieder
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

2014-09-16 Thread Jonathan Nieder
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); +close_lock_file(lk); Doesn't need to be guarded

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

2014-09-16 Thread Jonathan Nieder
Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord

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

2014-09-16 Thread Jonathan Nieder
in hold_lock_file about? In any case, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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 09/35] lockfile.c: document the various states of lock_file objects

2014-09-16 Thread Jonathan Nieder
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 doc, which makes

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

2014-09-16 Thread Jonathan Nieder
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,6

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

2014-09-16 Thread Jonathan Nieder
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. Could

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

2014-09-16 Thread Jonathan Nieder
shouldn't be tampered with.) Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/commit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com I wonder if we should just bite the bullet and make lock_file an opaque struct

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

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

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

2014-09-16 Thread Jonathan Nieder
Michael Haggerty wrote: lockfile.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) Nice. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info

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

2014-09-16 Thread Jonathan Nieder
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)) return

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

2014-09-16 Thread Jonathan Nieder
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 assert()

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

2014-09-16 Thread Jonathan Nieder
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

2014-09-16 Thread Jonathan Nieder
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

2014-09-16 Thread Jonathan Nieder
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()

2014-09-16 Thread Jonathan Nieder
Michael Haggerty wrote: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- fast-import.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message

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

2014-09-16 Thread Jonathan Nieder
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 mhag...@alum.mit.edu This seems to involve

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

2014-09-16 Thread Jonathan Nieder
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)

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

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

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

2014-09-22 Thread Jonathan Nieder
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 would

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

2014-09-24 Thread Jonathan Nieder
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

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

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

2014-09-29 Thread Jonathan Nieder
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

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

2014-10-01 Thread Jonathan Nieder
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 .git/config:

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

2014-10-01 Thread Jonathan Nieder
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 review. It could use another set of eyes (hint

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

2014-10-01 Thread Jonathan Nieder
-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- 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 index 54d7807..69f11bd 100755 --- a/t/t7001-mv.sh +++ b/t/t7001

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

2014-10-01 Thread Jonathan Nieder
none of the callers depend on fail if the file did not exist. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Change since v21: - adjust caller to remove redundant errno check git-compat-util.h | 7 +-- refs.c| 2

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

2014-10-01 Thread Jonathan Nieder
Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Michael Haggerty mhag...@alum.mit.edu --- As before. git-compat-util.h | 9 + wrapper.c | 14 ++ 2 files changed, 23 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 611e77b..5ee140c

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

2014-10-01 Thread Jonathan Nieder
error string if the transaction failed due to delete_ref_loose. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Changes since v21: - s/delete_loose_ref/delete_ref_loose/ once in commit message (but the other one still needs fixing) refs.c

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

2014-10-01 Thread Jonathan Nieder
-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Michael Haggerty mhag...@alum.mit.edu --- As before. refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 99a9b86..39571f5 100644 --- a/refs.c +++ b/refs.c @@ -2671,7 +2671,7 @@ int rename_ref(const char

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

2014-10-01 Thread Jonathan Nieder
transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- 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

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

2014-10-01 Thread Jonathan Nieder
-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- 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

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

2014-10-01 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com

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

2014-10-01 Thread Jonathan Nieder
: m - n/n n - m/m No functional change intended yet. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Since v21: - clarified commit message - clarified comments refs.c | 44 +--- 1 file changed, 29

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

2014-10-01 Thread Jonathan Nieder
Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- 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

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

2014-10-01 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Since v21: - tweaked handling

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

2014-10-01 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Since v21: - punctuation fix

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

2014-10-01 Thread Jonathan Nieder
HEAD so we don'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 jrnie...@gmail.com Reviewed-by: Ronnie

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

2014-10-01 Thread Jonathan Nieder
and read_ref_full the same treatment for consistency. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Since v21: - clarified commit message - put output parameters last branch.c| 2 +- builtin/blame.c | 2 +- builtin/branch.c

[PATCH 15/24] branch -d: avoid repeated symref resolution

2014-10-01 Thread Jonathan Nieder
From: Jonathan Nieder jrnie...@gmail.com Date: Wed, 10 Sep 2014 18:22:48 -0700 If a repository gets in a broken state with too much symref nesting, it cannot be repaired with git branch -d: $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense $ git branch -d nonsense error: branch

[PATCH 16/24] branch -d: simplify by using RESOLVE_REF_READING flag

2014-10-01 Thread Jonathan Nieder
by relying on RESOLVE_REF_READING. No functional change intended. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Split out from the 'fix handling of badly named refs' patch. builtin/branch.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin

[PATCH 17/24] packed-ref cache: forbid dot-components in refnames

2014-10-01 Thread Jonathan Nieder
stricter with invalid refs found in a packed-refs file or during clone. read_loose_refs() already checks for and skips refnames with .components so it is not affected. Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- Noticed while reviewing

[PATCH 18/24] test: put tests for handling of bad ref names in one place

2014-10-01 Thread Jonathan Nieder
Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- New. t/t1400-update-ref.sh | 44 -- t/t1430-bad-ref-name.sh | 84 + t/t9300-fast-import.sh | 30 -- 3 files changed, 84 insertions(+), 74 deletions

[PATCH 19/24] refs.c: allow listing and deleting badly named refs

2014-10-01 Thread Jonathan Nieder
refs, but allow deleting them (unless they escape refs/ and don't match [A-Z_]*). Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Since v21: - clarified change description - handle REF_ISBROKEN case too when printing Deleted branch message

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

2014-10-01 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- As before. builtin/for-each-ref.c | 5

[PATCH 21/24] remote rm/prune: print a message when writing packed-refs fails

2014-10-01 Thread Jonathan Nieder
. 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. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Michael Haggerty mhag

[PATCH 22/24] refs.c: do not permit err == NULL

2014-10-01 Thread Jonathan Nieder
., ref_transaction_update). Some of 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 jrnie...@gmail.com --- Since v21: - dropped spurious

[PATCH 23/24] lockfile: remove unable_to_lock_error

2014-10-01 Thread Jonathan Nieder
Date: Thu, 28 Aug 2014 16:41:34 -0700 The former caller uses unable_to_lock_message now. Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- As before. cache.h| 1 - lockfile.c | 10

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

2014-10-01 Thread Jonathan Nieder
the transaction on failure. That kind of 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 jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- That's

Re: Can I fetch an arbitrary commit by sha1?

2014-10-02 Thread Jonathan Nieder
Jeff King wrote: But I think Christian is arguing that the client side should complain that $sha1 is not a remote ref, and therefore not something we can fetch. This used to be the behavior until 6e7b66e (fetch: fetch objects by their exact SHA-1 object names, 2013-01-29). The idea there is

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

2014-10-03 Thread Jonathan Nieder
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: diff --git a/refs.c b/refs.c index f124c2b..6820c93 100644 --- a/refs.c +++ b/refs.c @@ -801,14 +801,16 @@ static int names_conflict(const char *refname1, const char *refname2) struct name_conflict_cb { const char

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

2014-10-03 Thread Jonathan Nieder
that contains two renames that are both individually conflicting: m - n/n n - m/m No functional change intended yet. Change-Id: I8c68f0a47eef25759d572436ba683cb7b1ccb7eb Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Junio C Hamano wrote

Re: [PATCH] bundle: plug minor memory leak in is_tag_in_date_range()

2014-10-03 Thread Jonathan Nieder
labels. With or without such a change, Reviewed-by: Jonathan Nieder jrnie...@gmail.com diff --git i/bundle.c w/bundle.c index 4158e11..101cde0 100644 --- i/bundle.c +++ w/bundle.c @@ -209,7 +209,7 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs) { unsigned

Re: [PATCH] use skip_prefix() to avoid more magic numbers

2014-10-05 Thread Jonathan Nieder
char *line = *msg_p; + const char *name, *line = *msg_p; Likewise. With or without the changes below, Reviewed-by: Jonathan Nieder jrnie...@gmail.com diff --git i/builtin/branch.c w/builtin/branch.c index 6785097..022a88e 100644 --- i/builtin/branch.c +++ w/builtin/branch.c @@ -62,19

Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar

2014-10-13 Thread Jonathan Nieder
Jeff King wrote: For small outputs, we sometimes use: test $(some_cmd) = something we expect instead of a full test_cmp. The downside of this is that when it fails, there is no output at all from the script. There's another downside to that construct: it loses the exit status from

Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar

2014-10-13 Thread Jonathan Nieder
Jeff King wrote: On Mon, Oct 13, 2014 at 09:10:22AM -0700, Jonathan Nieder wrote: There's another downside to that construct: it loses the exit status from some_cmd. Yes, although I think in many cases it's not a big deal. For example, here we lose the exit code of count-objects

Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar

2014-10-13 Thread Jonathan Nieder
Junio C Hamano wrote: On Mon, Oct 13, 2014 at 2:31 PM, Jonathan Nieder jrnie...@gmail.com wrote: It could segfault after producing the good output, but sure, count-objects code doesn't change very often. Doesn't change very often is not the issue. Here we are not testing if it can count

Re: [PATCH] pass config slots as pointers instead of offsets

2014-10-14 Thread Jonathan Nieder
Junio C Hamano wrote: builtin/branch.c | 16 builtin/commit.c | 19 +-- builtin/log.c| 2 +- log-tree.c | 4 ++-- log-tree.h | 2 +- 5 files changed, 21 insertions(+), 22 deletions(-) Signed-off-by: Jonathan Nieder jrnie...@gmail.com

[PATCH v23 0/25] rs/ref-transaction (Use ref transactions, part 3)

2014-10-14 Thread Jonathan Nieder
of the series. Jonathan Nieder (6): mv test: recreate mod/ directory instead of relying on stale copy branch -d: avoid repeated symref resolution packed-ref cache: forbid dot-components in refnames refs.c: do not permit err == NULL lockfile: remove unable_to_lock_error ref_transaction_commit

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

2014-10-14 Thread Jonathan Nieder
From: Jonathan Nieder jrnie...@gmail.com Date: Wed, 10 Sep 2014 14:01:46 -0700 The tests for 'git mv moves a submodule' functionality often run commands like git mv sub mod/sub to move a submodule into a subdirectory. Just like plain /bin/mv, this is supposed to succeed if the mod

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

2014-10-14 Thread Jonathan Nieder
none of the callers depend on fail if the file did not exist. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- git-compat-util.h | 7 +-- refs.c| 2 +- wrapper.c | 14 ++ 3 files changed, 12 insertions

[PATCH 03/25] refs.c: lock_ref_sha1_basic is used for all refs

2014-10-14 Thread Jonathan Nieder
Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 2dcf6c6..4f2564d 100644 --- a/refs.c +++ b/refs.c @@ -2134,7 +2134,7 @@ int dwim_log(const char *str, int len, unsigned

[PATCH 04/25] wrapper.c: add a new function unlink_or_msg

2014-10-14 Thread Jonathan Nieder
Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- git-compat-util.h | 9 + wrapper.c | 14 ++ 2 files changed, 23 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index d67592f..59ecf21 100644

[PATCH 05/25] refs.c: add an err argument to delete_ref_loose

2014-10-14 Thread Jonathan Nieder
error string if the transaction failed due to delete_ref_loose. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 4f2564d..430857b 100644

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

2014-10-14 Thread Jonathan Nieder
transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- branch.c | 4 ++-- builtin/commit.c | 4 ++-- builtin/receive-pack.c | 5 +++-- builtin/replace.c | 5 +++-- builtin/tag.c | 4 ++-- builtin

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

2014-10-14 Thread Jonathan Nieder
-by: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index f5d7bd7..3c45615 100644 --- a/refs.c +++ b/refs.c @@ -2721,7 +2721,7 @@ int rename_ref(const char *oldrefname

[PATCH 09/25] refs.c: call lock_ref_sha1_basic directly from commit

2014-10-14 Thread Jonathan Nieder
Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 9c01623..b591b9c 100644 --- a/refs.c +++ b/refs.c @@ -3632,12 +3632,12 @@ int ref_transaction_commit(struct ref_transaction *transaction

[PATCH 08/25] refs.c: refuse to lock badly named refs in lock_ref_sha1_basic

2014-10-14 Thread Jonathan Nieder
-by: Ronnie Sahlberg sahlb...@google.com Reviewed-by: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 3c45615..9c01623 100644 --- a/refs.c +++ b/refs.c

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

2014-10-14 Thread Jonathan Nieder
: m - n/n n - m/m No functional change intended yet. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 50 -- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/refs.c b

[PATCH 11/25] refs.c: ref_transaction_commit: distinguish name conflicts from other errors

2014-10-14 Thread Jonathan Nieder
Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 26 -- refs.h | 9 +++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/refs.c b/refs.c index a007cf3..9d9bbeb 100644 --- a/refs.c +++ b/refs.c @@ -3637,9

[PATCH 12/25] fetch.c: change s_update_ref to use a ref transaction

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- builtin/fetch.c | 34

[PATCH 13/25] refs.c: make write_ref_sha1 static

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg sahlb...@google.com 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 sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- refs.c | 10 -- refs.h | 3

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

2014-10-14 Thread Jonathan Nieder
and read_ref_full the same treatment for consistency. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- branch.c| 2 +- builtin/blame.c | 2 +- builtin/branch.c| 9 ++--- builtin/checkout.c | 6 ++-- builtin/clone.c

[PATCH 16/25] branch -d: avoid repeated symref resolution

2014-10-14 Thread Jonathan Nieder
. Callers can 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 jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- builtin/branch.c

[PATCH 15/25] reflog test: test interaction with detached HEAD

2014-10-14 Thread Jonathan Nieder
of the reflog after detaching and reattaching HEAD so we don'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

[PATCH 18/25] packed-ref cache: forbid dot-components in refnames

2014-10-14 Thread Jonathan Nieder
for the REFNAME_DOT_COMPONENT flag. This means we'll be slightly stricter with invalid refs found in a packed-refs file or during clone. read_loose_refs() already checks for and skips refnames with .components so it is not affected. Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb

[PATCH 17/25] branch -d: simplify by using RESOLVE_REF_READING

2014-10-14 Thread Jonathan Nieder
by relying on RESOLVE_REF_READING. No functional change intended. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- builtin/branch.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c

[PATCH 19/25] test: put tests for handling of bad ref names in one place

2014-10-14 Thread Jonathan Nieder
-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- t/t1400-update-ref.sh | 44 -- t/t1430-bad-ref-name.sh | 84 + t/t9300-fast-import.sh | 30 -- 3 files changed, 84

[PATCH 20/25] refs.c: allow listing and deleting badly named refs

2014-10-14 Thread Jonathan Nieder
...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- builtin/branch.c| 9 +-- cache.h | 17 +- refs.c | 148 ++-- refs.h | 12 +++- t/t1430-bad-ref-name.sh | 125

[PATCH 22/25] remote rm/prune: print a message when writing packed-refs fails

2014-10-14 Thread Jonathan Nieder
. 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. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Michael Haggerty mhag

[PATCH 23/25] refs.c: do not permit err == NULL

2014-10-14 Thread Jonathan Nieder
., ref_transaction_update). Some of 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 jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb

[PATCH 21/25] for-each-ref: skip and warn about broken ref names

2014-10-14 Thread Jonathan Nieder
for those callers that want to and are prepared (for example by using a --format argument with %0 as a delimiter after the ref name). Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- builtin/for-each-ref.c | 5 + 1 file changed, 5 insertions

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

2014-10-14 Thread Jonathan Nieder
the transaction on failure. That kind of 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 jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- Thanks

[PATCH 24/25] lockfile: remove unable_to_lock_error

2014-10-14 Thread Jonathan Nieder
Date: Thu, 28 Aug 2014 16:41:34 -0700 The former caller uses unable_to_lock_message now. Signed-off-by: Jonathan Nieder jrnie...@gmail.com Reviewed-by: Ronnie Sahlberg sahlb...@google.com --- lockfile.c | 10 -- lockfile.h | 1 - 2 files changed, 11 deletions(-) diff --git

Re: [PATCH] builtin/merge.c: drop a parameter that is never used

2014-10-24 Thread Jonathan Nieder
Junio C Hamano wrote: Since the very beginning when we added the renormalizing parameter to this function with 7610fa57 (merge-recursive --renormalize, 2010-08-05), nobody seems to have ever referenced it. Signed-off-by: Junio C Hamano gits...@pobox.com Reviewed-by: Jonathan Nieder jrnie

Re: [PATCH] merge sequencer: turn Conflicts: hint into a comment

2014-10-27 Thread Jonathan Nieder
Jeff King wrote: For the most part, combined-diff (and --cc) will show the interesting cases anyway. But if you take a whole file from one side of the merge, then there is nothing interesting for diff to show. Do people still want to get that more complete list of potentially interesting

Re: Safe to interrupt »git gc --auto«?

2014-10-29 Thread Jonathan Nieder
Thomas Schwinge wrote: I couldn't find this answered in the documentation: if, instead of exiting right away, a »git gc --auto« actually commences its housekeeping tasks, is it safe to interrupt (C-c, SIGINT) the original git invocation at this point, or might this cause any inconsistencies,

Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly

2013-07-11 Thread Jonathan Nieder
Eric Sunshine wrote: On Thu, Jul 11, 2013 at 3:07 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: With the introduction of check-mailmap, it is now possible to check .mailmap functionality directly rather than indirectly as a side-effect of other

Re: [PATCH] .mailmap: Map email addresses to names

2013-07-12 Thread Jonathan Nieder
Stefan Beller wrote: --- a/.mailmap +++ b/.mailmap @@ -5,99 +5,146 @@ [...] Chris Shoemaker c.shoema...@cox.net -Dan Johnson computerdr...@gmail.com Dana L. How dana...@gmail.com Dana L. How h...@deathvalley.cswitch.com Daniel Barkalow barka...@iabervon.org +Dan Johnson

Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable

2013-07-12 Thread Jonathan Nieder
is to allow later settings to override earlier ones, so this change looks good. Reviewed-by: Jonathan Nieder jrnie...@gmail.com FWIW the GIT_SSL_CERT_PASSWORD_PROTECTED envvar has a similar can only enable behavior, but since it's documented, that's not as big of a problem. Do you remember why

Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone

2013-07-13 Thread Jonathan Nieder
during 1.7.8-rc0, so it might make sense to s/1.7.10/1.7.8/ here. Aside from that nit, Reviewed-by: Jonathan Nieder jrnie...@gmail.com 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

<    1   2   3   4   5   6   7   8   9   10   >