Re: [PATCH v6 0/6] stash: support pathspec argument

[Jeff King]

On Sun, Feb 19, 2017 at 11:03:07AM +, Thomas Gummerer wrote:

> Thanks Junio and Peff for comments on the last round.
> 
> Changes since then:
> 
> - removed mention of the "new form" of git stash create from the
>   Documentation.
> - Changed documentation for git stash without a verb, mentioning
>   stash -p now being an alias for git stash push -p and that -- can be
>   used as disambiguation for for pathspecs
> - Fixed ${1-...} which should have been ${1?...}
> - Removed unused new_style variable from create_stash, which was a
>   leftover from perious rounds.

I gave it one more read-through, and didn't see anything objectionable.
Thanks for seeing this through!

-Peff

Re: git-scm.com status report

[Jeff King]

On Sat, Feb 18, 2017 at 10:27:51PM +, pedro rijo wrote:

> I would say everyone did an amazing job, closing more than 150 old issues
> in a single week! I think the amount of issues is finally manageable (40
> issues currently).

Yes, thank you to all who have been helping. But especially you and
Samuel, who obviously spent a lot of time sifting through old issues.

> And if you agree, I would like to start looking at old PRs (some will
> probably don't make sense anymore), and will start reviewing them as soon
> as I have the time to setup the RoR app on my machine so that I can
> understand and see the changes introduced on the PRs.

Sounds good.

>  Many PRs seem to introduce small and innocent changes, but I always like
> to run the code to see :)

Yeah, many of the display-oriented changes are pretty obvious from
reading the code, but I have caught a couple of regressions just by
running the PRs and making sure the rendered result is sane.

-Peff

Re: Git bisect does not find commit introducing the bug

[Oleg Taranenko]

>>> Then you must adjust your definition of "good": All commits that do not have
>>> the feature, yet, are "good": since they do not have the feature in the
>>> first place, they cannot have the breakage that you found in the feature.
>>>
>>> That is exactly the situation in your original example! But you constructed
>>> the condition of goodness in such a simplistic way (depending on the
>>> presence of a string), that it was impossible to distinguish between "does
>>> not have the feature at all" and "has the feature, but it is broken".
>>
>> Johannes, thank you for correctly identifying the error in my logic.
>> Indeed I was using the term 'bad' also for the commit without the
>> feature. In order to find the commit introducing the bug in my example
>> a new state is needed, which would make 'git bisect' a bit more
>> complicated than the user 'most of the time' probably needs. Or do you
>> think, it would make sense to ask the user for this state (if e.g 'git
>> bisect' would be started with a new parameter)?


> If a commit doesn't have the feature, then it is by definition, not
> containing a broken feature, and you can simply use the "good" state.
> There is no need for a different state. If you can't test the commit
> because it's broken in some other way, you can use "git bisect skip"
> but that isn't what you want in this case.

Commits missing feature == 'good' commit is a very confusing one.

Looks like in real life it happens much often, then git developers can
imagine. For multi-branch/multi-feature workflow it's pretty easy not
to recognize whether it is missing or not developed yet, especially on
retrospective view where cherry-picking/squashing/merging is being
used. My experience shows most annoying bugs are generating after a
heavy merge (evil merge) with conflicts resolutions, where developer
is not involved in the knowing what happens on counterpart changes.
Then feature can be disappeared after it was worked & tested in its
own branches.

@Alex, I'm pretty interesting in fixing this weird bisect behaviour as
well, as far as I struggled on it last summer and continue struggling
so far :) If you want we can join to your efforts on fixing.

Cheers, Oleg

Re: [RFC PATCH] show decorations at the end of the line

[Linus Torvalds]

On Sun, Feb 19, 2017 at 4:46 PM, Jeff King  wrote:
>
> I think there are two potential patches:
>
>   1. Add a custom-format placeholder for the --source value.
>  This is an obvious improvement that doesn't hurt anyone.

Right.

>   2. Switch --decorate to the end by default, but _not_ --source.

.. and in fact the whole "--source" printing should not even have been
mixed up with the decorations.

So (2) is actually easy to fix: just don't mix "show_source()" with
"show_decorations()", because they are totally different things to
begin with.

That source showing should never have been in "show_decorations()" in
the first place. It just happened to be a convenient place for it.

So this attached patch is just my original patch updated to split up
"show_source()" from "show_decorations()", and show it where it used
to be.

Maybe this works for Junio's alias?

  Linus
From 97abab56fed451476f6ec676346b1e66001ef864 Mon Sep 17 00:00:00 2001
From: Linus Torvalds 
Date: Sat, 11 Feb 2017 10:03:33 -0800
Subject: [PATCH] show decorations at the end of the line

So I use "--show-decorations" all the time because I find it very useful
to see where the origin branch is, where tags are etc. In fact, my global
git config file has

[log]
decorate = auto

in it, so that I don't have to type it out all the time when I just do my
usual 'git log". It's lovely.

However, it does make one particular case uglier: with commit decorations,
the "oneline" commit format ends up being not very pretty:

[torvalds@i7 git]$ git log --oneline -10
3f07dac29 (HEAD -> master) pathspec: don't error out on  all-exclusionary pathspec patterns
ca4a562f2 pathspec magic: add '^' as alias for '!'
02555c1b2 ls-remote: add "--diff" option to show only refs that differ
6e3a7b339 (tag: v2.12.0-rc0, origin/master, origin/HEAD) Git 2.12-rc0
fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
36acf4123 Merge branch 'rs/object-id'
ecc486b1f Merge branch 'js/re-running-failed-tests'
4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
5348021c6 Merge branch 'sb/submodule-recursive-absorb'

and note how the decoration comes right after the shortened commit hash,
breaking up the alignment of the messages.

The above doesn't show it with the colorization: I also have

[color]
ui=auto

so on my terminal the decoration is also nicely colorized which makes it
much more obvious, it's not as obvious in this message.

The oneline message handling is already pretty special, this makes it even
more special by putting the decorations at the end of the line:

3f07dac29 pathspec: don't error out on all-exclusionary pathspec patterns (HEAD -> master)
ca4a562f2 pathspec magic: add '^' as alias for '!'
02555c1b2 ls-remote: add "--diff" option to show only refs that differ
6e3a7b339 Git 2.12-rc0 (tag: v2.12.0-rc0, origin/master, origin/HEAD)
fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
36acf4123 Merge branch 'rs/object-id'
ecc486b1f Merge branch 'js/re-running-failed-tests'
4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
5348021c6 Merge branch 'sb/submodule-recursive-absorb'

which looks a lot better (again, this is all particularly noticeable with
colorization).

NOTE! There's a very special case for "git log --oneline -g" that shows
the reflogs as oneliners, and this does *not* fix that special case. It's
a lot more involved and relies on the exact show_reflog_message()
implementation, so I left the format for that alone, along with a comment
about how it's not at the end of line.

Signed-off-by: Linus Torvalds 
---
 builtin/rev-list.c |  1 +
 log-tree.c | 17 ++---
 log-tree.h |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d589..8833f029a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -107,6 +107,7 @@ static void show_commit(struct commit *commit, void *data)
 			children = children->next;
 		}
 	}
+	show_source(revs, commit);
 	show_decorations(revs, commit);
 	if (revs->commit_format == CMIT_FMT_ONELINE)
 		putchar(' ');
diff --git a/log-tree.c b/log-tree.c
index 8c2415747..9ca3e8c1c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -279,12 +279,16 @@ void format_decorations_extended(struct strbuf *sb,
 	strbuf_addstr(sb, color_reset);
 }
 
+void show_source(struct rev_info *opt, struct commit *commit)
+{
+	if (opt->show_source && commit->util)
+		fprintf(opt->diffopt.file, "\t%s", (char *) commit->util);
+}
+
 void show_decorations(struct rev_info *opt, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
 
-	if (opt->show_source && commit->util)
-		fprintf(opt->diffopt.file, 

Re: [RFC PATCH] show decorations at the end of the line

[Junio C Hamano]

Jeff King  writes:

> I think there are two potential patches:
>
>   1. Add a custom-format placeholder for the --source value.
>  This is an obvious improvement that doesn't hurt anyone.
>
>   2. Switch --decorate to the end by default, but _not_ --source.
>
>  This use case _could_ be served already by using a custom format
>  with "%d". So it's really just a matter of having better-looking
>  default.

Yes, and I agree it is a better default to have "--decorate" at the
end.

I do not mind having to use a custom format myself, but I suspect
that the default for "--source" is more useful to have it at the
beginning, because "--source" annotates each and every commit, as
opposed to "--decorate" that adds annotation few and far between.

Re: [RFC PATCH] show decorations at the end of the line

[Jeff King]

On Sun, Feb 19, 2017 at 03:03:21PM -0800, Jacob Keller wrote:

> >> I just got bitten by a fallout.  I have
> >>
> >> $ git recent --help
> >> `git recent' is aliased to `log --oneline --branches --no-merges \
> >>  --source --since=3.weeks'
> >>
> >> but now the branch names are shown at the end, which defeats the
> >> whole point of the alias.
> >
> > Yes, your situation actually wants those decorations as primary
> > things, so having them at the end is indeed pointless.
> >
> > So I think we should just discard that patch of mine.
> >
> >  Linus
> 
> I would think that in general putting them at the end makes more
> sense, but we should have the ability to use them in format specifiers
> so that users are free to customize it exactly how they want. That is,
> I agree with the reasoning presented in the original patch, but think
> Junio's case can be solved by strengthening the custom formats.

I think there are two potential patches:

  1. Add a custom-format placeholder for the --source value.
 This is an obvious improvement that doesn't hurt anyone.

  2. Switch --decorate to the end by default, but _not_ --source.

 This use case _could_ be served already by using a custom format
 with "%d". So it's really just a matter of having better-looking
 default.

 It might hurt somebody's script, but for the reasons discussed
 earlier in the thread, people are unlikely to be parsing it (it's
 more likely somebody would just complain because they think the
 decoration-first behavior is prettier).

-Peff

Re: [PATCH v2] git-check-ref-format: clarify man for --normalize

[Jeff King]

On Sun, Feb 19, 2017 at 11:32:32PM +0100, Damien Regad wrote:

> Use of 'iff' may be confusing to people not familiar with this term.
> 
> Improving the --normalize option's documentation to remove the use of
> 'iff', and clearly describe what happens when the condition is not met.

Looks good to me. Thanks for following up.

-Peff

[PATCH v4 03/19] builtin/diff-tree: convert to struct object_id

[brian m. carlson]

Convert most leaf functions to struct object_id.  Change several
hardcoded numbers to uses of parse_oid_hex.  In doing so, verify that we
when we want two trees, we have exactly two trees.

Finally, in stdin_diff_commit, avoid accessing the byte after the NUL.
This will be a NUL as well, since the first NUL was a newline we
overwrote.  However, with parse_oid_hex, we no longer need to increment
the pointer directly, and can simply increment it as part of our check
for the space character.

Signed-off-by: brian m. carlson 
Signed-off-by: Jeff King 
---
 builtin/diff-tree.c | 43 +--
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 8ce00480cd..1656e092bd 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -7,46 +7,44 @@
 
 static struct rev_info log_tree_opt;
 
-static int diff_tree_commit_sha1(const unsigned char *sha1)
+static int diff_tree_commit_sha1(const struct object_id *oid)
 {
-   struct commit *commit = lookup_commit_reference(sha1);
+   struct commit *commit = lookup_commit_reference(oid->hash);
if (!commit)
return -1;
return log_tree_commit(_tree_opt, commit);
 }
 
 /* Diff one or more commits. */
-static int stdin_diff_commit(struct commit *commit, char *line, int len)
+static int stdin_diff_commit(struct commit *commit, const char *p)
 {
-   unsigned char sha1[20];
-   if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
+   struct object_id oid;
+   if (isspace(*p++) && !parse_oid_hex(p, , )) {
/* Graft the fake parents locally to the commit */
-   int pos = 41;
struct commit_list **pptr;
 
/* Free the real parent list */
free_commit_list(commit->parents);
commit->parents = NULL;
pptr = &(commit->parents);
-   while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
-   struct commit *parent = lookup_commit(sha1);
+   while (isspace(*p++) && !parse_oid_hex(p, , )) {
+   struct commit *parent = lookup_commit(oid.hash);
if (parent) {
pptr = _list_insert(parent, pptr)->next;
}
-   pos += 41;
}
}
return log_tree_commit(_tree_opt, commit);
 }
 
 /* Diff two trees. */
-static int stdin_diff_trees(struct tree *tree1, char *line, int len)
+static int stdin_diff_trees(struct tree *tree1, const char *p)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct tree *tree2;
-   if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
+   if (!isspace(*p++) || parse_oid_hex(p, , ) || *p)
return error("Need exactly two trees, separated by a space");
-   tree2 = lookup_tree(sha1);
+   tree2 = lookup_tree(oid.hash);
if (!tree2 || parse_tree(tree2))
return -1;
printf("%s %s\n", oid_to_hex(>object.oid),
@@ -60,23 +58,24 @@ static int stdin_diff_trees(struct tree *tree1, char *line, 
int len)
 static int diff_tree_stdin(char *line)
 {
int len = strlen(line);
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *obj;
+   const char *p;
 
if (!len || line[len-1] != '\n')
return -1;
line[len-1] = 0;
-   if (get_sha1_hex(line, sha1))
+   if (parse_oid_hex(line, , ))
return -1;
-   obj = parse_object(sha1);
+   obj = parse_object(oid.hash);
if (!obj)
return -1;
if (obj->type == OBJ_COMMIT)
-   return stdin_diff_commit((struct commit *)obj, line, len);
+   return stdin_diff_commit((struct commit *)obj, p);
if (obj->type == OBJ_TREE)
-   return stdin_diff_trees((struct tree *)obj, line, len);
+   return stdin_diff_trees((struct tree *)obj, p);
error("Object %s is a %s, not a commit or tree",
- sha1_to_hex(sha1), typename(obj->type));
+ oid_to_hex(), typename(obj->type));
return -1;
 }
 
@@ -141,7 +140,7 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
break;
case 1:
tree1 = opt->pending.objects[0].item;
-   diff_tree_commit_sha1(tree1->oid.hash);
+   diff_tree_commit_sha1(>oid);
break;
case 2:
tree1 = opt->pending.objects[0].item;
@@ -164,9 +163,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
   DIFF_SETUP_USE_CACHE);
while (fgets(line, sizeof(line), stdin)) {
-   unsigned char 

[PATCH v4 11/19] Convert remaining callers of resolve_refdup to object_id

[brian m. carlson]

There are a few leaf functions in various files that call
resolve_refdup.  Convert these functions to use struct object_id
internally to prepare for transitioning resolve_refdup itself.

Signed-off-by: brian m. carlson 
---
 builtin/notes.c| 18 +-
 builtin/receive-pack.c |  4 ++--
 ref-filter.c   |  4 ++--
 reflog-walk.c  | 12 ++--
 transport.c|  4 ++--
 wt-status.c|  4 ++--
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 5248a9bad8..8c569a49a0 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -693,7 +693,7 @@ static int merge_abort(struct notes_merge_options *o)
 static int merge_commit(struct notes_merge_options *o)
 {
struct strbuf msg = STRBUF_INIT;
-   unsigned char sha1[20], parent_sha1[20];
+   struct object_id oid, parent_oid;
struct notes_tree *t;
struct commit *partial;
struct pretty_print_context pretty_ctx;
@@ -705,27 +705,27 @@ static int merge_commit(struct notes_merge_options *o)
 * and target notes ref from .git/NOTES_MERGE_REF.
 */
 
-   if (get_sha1("NOTES_MERGE_PARTIAL", sha1))
+   if (get_oid("NOTES_MERGE_PARTIAL", ))
die(_("failed to read ref NOTES_MERGE_PARTIAL"));
-   else if (!(partial = lookup_commit_reference(sha1)))
+   else if (!(partial = lookup_commit_reference(oid.hash)))
die(_("could not find commit from NOTES_MERGE_PARTIAL."));
else if (parse_commit(partial))
die(_("could not parse commit from NOTES_MERGE_PARTIAL."));
 
if (partial->parents)
-   hashcpy(parent_sha1, partial->parents->item->object.oid.hash);
+   oidcpy(_oid, >parents->item->object.oid);
else
-   hashclr(parent_sha1);
+   oidclr(_oid);
 
t = xcalloc(1, sizeof(struct notes_tree));
init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
 
o->local_ref = local_ref_to_free =
-   resolve_refdup("NOTES_MERGE_REF", 0, sha1, NULL);
+   resolve_refdup("NOTES_MERGE_REF", 0, oid.hash, NULL);
if (!o->local_ref)
die(_("failed to resolve NOTES_MERGE_REF"));
 
-   if (notes_merge_commit(o, t, partial, sha1))
+   if (notes_merge_commit(o, t, partial, oid.hash))
die(_("failed to finalize notes merge"));
 
/* Reuse existing commit message in reflog message */
@@ -733,8 +733,8 @@ static int merge_commit(struct notes_merge_options *o)
format_commit_message(partial, "%s", , _ctx);
strbuf_trim();
strbuf_insert(, 0, "notes: ", 7);
-   update_ref(msg.buf, o->local_ref, sha1,
-  is_null_sha1(parent_sha1) ? NULL : parent_sha1,
+   update_ref(msg.buf, o->local_ref, oid.hash,
+  is_null_oid(_oid) ? NULL : parent_oid.hash,
   0, UPDATE_REFS_DIE_ON_ERR);
 
free_notes(t);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a0692..7966f4f4df 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1414,7 +1414,7 @@ static void execute_commands(struct command *commands,
 {
struct check_connected_options opt = CHECK_CONNECTED_INIT;
struct command *cmd;
-   unsigned char sha1[20];
+   struct object_id oid;
struct iterate_data data;
struct async muxer;
int err_fd = 0;
@@ -1471,7 +1471,7 @@ static void execute_commands(struct command *commands,
check_aliased_updates(commands);
 
free(head_name_to_free);
-   head_name = head_name_to_free = resolve_refdup("HEAD", 0, sha1, NULL);
+   head_name = head_name_to_free = resolve_refdup("HEAD", 0, oid.hash, 
NULL);
 
if (use_atomic)
execute_commands_atomic(commands, si);
diff --git a/ref-filter.c b/ref-filter.c
index 3820b21cc7..f0de30e2ef 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -961,9 +961,9 @@ static void populate_value(struct ref_array_item *ref)
ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-   unsigned char unused1[20];
+   struct object_id unused1;
ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
-unused1, NULL);
+unused1.hash, NULL);
if (!ref->symref)
ref->symref = "";
}
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af2767..f98748e2ae 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -45,11 +45,11 @@ static struct complete_reflogs *read_complete_reflog(const 
char *ref)
reflogs->ref = xstrdup(ref);
for_each_reflog_ent(ref, read_one_reflog, reflogs);
if (reflogs->nr == 0) {
-   unsigned 

[PATCH v4 10/19] builtin/merge: convert to struct object_id

[brian m. carlson]

Additionally convert several uses of the constant 40 into
GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 builtin/merge.c | 134 
 1 file changed, 66 insertions(+), 68 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a96d4fb501..099cfab447 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -244,7 +244,7 @@ static void drop_save(void)
unlink(git_path_merge_mode());
 }
 
-static int save_state(unsigned char *stash)
+static int save_state(struct object_id *stash)
 {
int len;
struct child_process cp = CHILD_PROCESS_INIT;
@@ -265,7 +265,7 @@ static int save_state(unsigned char *stash)
else if (!len)  /* no changes */
return -1;
strbuf_setlen(, buffer.len-1);
-   if (get_sha1(buffer.buf, stash))
+   if (get_oid(buffer.buf, stash))
die(_("not a valid object: %s"), buffer.buf);
return 0;
 }
@@ -305,18 +305,18 @@ static void reset_hard(unsigned const char *sha1, int 
verbose)
die(_("read-tree failed"));
 }
 
-static void restore_state(const unsigned char *head,
- const unsigned char *stash)
+static void restore_state(const struct object_id *head,
+ const struct object_id *stash)
 {
struct strbuf sb = STRBUF_INIT;
const char *args[] = { "stash", "apply", NULL, NULL };
 
-   if (is_null_sha1(stash))
+   if (is_null_oid(stash))
return;
 
-   reset_hard(head, 1);
+   reset_hard(head->hash, 1);
 
-   args[2] = sha1_to_hex(stash);
+   args[2] = oid_to_hex(stash);
 
/*
 * It is OK to ignore error here, for example when there was
@@ -376,10 +376,10 @@ static void squash_message(struct commit *commit, struct 
commit_list *remotehead
 
 static void finish(struct commit *head_commit,
   struct commit_list *remoteheads,
-  const unsigned char *new_head, const char *msg)
+  const struct object_id *new_head, const char *msg)
 {
struct strbuf reflog_message = STRBUF_INIT;
-   const unsigned char *head = head_commit->object.oid.hash;
+   const struct object_id *head = _commit->object.oid;
 
if (!msg)
strbuf_addstr(_message, getenv("GIT_REFLOG_ACTION"));
@@ -397,7 +397,7 @@ static void finish(struct commit *head_commit,
else {
const char *argv_gc_auto[] = { "gc", "--auto", NULL };
update_ref(reflog_message.buf, "HEAD",
-   new_head, head, 0,
+   new_head->hash, head->hash, 0,
UPDATE_REFS_DIE_ON_ERR);
/*
 * We ignore errors in 'gc --auto', since the
@@ -416,7 +416,7 @@ static void finish(struct commit *head_commit,
DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
opts.detect_rename = DIFF_DETECT_RENAME;
diff_setup_done();
-   diff_tree_sha1(head, new_head, "", );
+   diff_tree_sha1(head->hash, new_head->hash, "", );
diffcore_std();
diff_flush();
}
@@ -431,7 +431,7 @@ static void finish(struct commit *head_commit,
 static void merge_name(const char *remote, struct strbuf *msg)
 {
struct commit *remote_head;
-   unsigned char branch_head[20];
+   struct object_id branch_head;
struct strbuf buf = STRBUF_INIT;
struct strbuf bname = STRBUF_INIT;
const char *ptr;
@@ -441,25 +441,25 @@ static void merge_name(const char *remote, struct strbuf 
*msg)
strbuf_branchname(, remote);
remote = bname.buf;
 
-   memset(branch_head, 0, sizeof(branch_head));
+   oidclr(_head);
remote_head = get_merge_parent(remote);
if (!remote_head)
die(_("'%s' does not point to a commit"), remote);
 
-   if (dwim_ref(remote, strlen(remote), branch_head, _ref) > 0) {
+   if (dwim_ref(remote, strlen(remote), branch_head.hash, _ref) > 0) 
{
if (starts_with(found_ref, "refs/heads/")) {
strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-   sha1_to_hex(branch_head), remote);
+   oid_to_hex(_head), remote);
goto cleanup;
}
if (starts_with(found_ref, "refs/tags/")) {
strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
-   sha1_to_hex(branch_head), remote);
+   oid_to_hex(_head), remote);
goto cleanup;
}
if (starts_with(found_ref, "refs/remotes/")) {
strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of 
.\n",
-

[PATCH v4 07/19] builtin/grep: convert to struct object_id

[brian m. carlson]

Convert several functions to use struct object_id, and rename them so
that they no longer refer to SHA-1.

Signed-off-by: brian m. carlson 
---
 builtin/grep.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..0393b0fdc4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -294,17 +294,17 @@ static int grep_cmd_config(const char *var, const char 
*value, void *cb)
return st;
 }
 
-static void *lock_and_read_sha1_file(const unsigned char *sha1, enum 
object_type *type, unsigned long *size)
+static void *lock_and_read_oid_file(const struct object_id *oid, enum 
object_type *type, unsigned long *size)
 {
void *data;
 
grep_read_lock();
-   data = read_sha1_file(sha1, type, size);
+   data = read_sha1_file(oid->hash, type, size);
grep_read_unlock();
return data;
 }
 
-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
+static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 const char *filename, int tree_name_len,
 const char *path)
 {
@@ -323,7 +323,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned 
char *sha1,
 
 #ifndef NO_PTHREADS
if (num_threads) {
-   add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+   add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
strbuf_release();
return 0;
} else
@@ -332,7 +332,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned 
char *sha1,
struct grep_source gs;
int hit;
 
-   grep_source_init(, GREP_SOURCE_SHA1, pathbuf.buf, path, 
sha1);
+   grep_source_init(, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
strbuf_release();
hit = grep_source(opt, );
 
@@ -690,7 +690,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
pathspec *pathspec,
ce_skip_worktree(ce)) {
if (ce_stage(ce) || ce_intent_to_add(ce))
continue;
-   hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+   hit |= grep_oid(opt, >oid, ce->name,
 0, ce->name);
} else {
hit |= grep_file(opt, ce->name);
@@ -750,7 +750,7 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
strbuf_add(base, entry.path, te_len);
 
if (S_ISREG(entry.mode)) {
-   hit |= grep_sha1(opt, entry.oid->hash, base->buf, 
tn_len,
+   hit |= grep_oid(opt, entry.oid, base->buf, tn_len,
 check_attr ? base->buf + tn_len : 
NULL);
} else if (S_ISDIR(entry.mode)) {
enum object_type type;
@@ -758,7 +758,7 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
void *data;
unsigned long size;
 
-   data = lock_and_read_sha1_file(entry.oid->hash, , 
);
+   data = lock_and_read_oid_file(entry.oid, , );
if (!data)
die(_("unable to read tree (%s)"),
oid_to_hex(entry.oid));
@@ -787,7 +787,7 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
   struct object *obj, const char *name, const char *path)
 {
if (obj->type == OBJ_BLOB)
-   return grep_sha1(opt, obj->oid.hash, name, 0, path);
+   return grep_oid(opt, >oid, name, 0, path);
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -1152,11 +1152,11 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
/* Check revs and then paths */
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
-   unsigned char sha1[20];
+   struct object_id oid;
struct object_context oc;
/* Is it a rev? */
-   if (!get_sha1_with_context(arg, 0, sha1, )) {
-   struct object *object = parse_object_or_die(sha1, arg);
+   if (!get_sha1_with_context(arg, 0, oid.hash, )) {
+   struct object *object = parse_object_or_die(oid.hash, 
arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
add_object_array_with_path(object, arg, , oc.mode, 
oc.path);
-- 
2.11.0

[PATCH v4 13/19] reflog-walk: convert struct reflog_info to struct object_id

[brian m. carlson]

Convert struct reflog_info to use struct object_id by changing the
structure definition and applying the following semantic patch:

@@
struct reflog_info E1;
@@
- E1.osha1
+ E1.ooid.hash

@@
struct reflog_info *E1;
@@
- E1->osha1
+ E1->ooid.hash

@@
struct reflog_info E1;
@@
- E1.nsha1
+ E1.noid.hash

@@
struct reflog_info *E1;
@@
- E1->nsha1
+ E1->noid.hash

Signed-off-by: brian m. carlson 
---
 reflog-walk.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index f98748e2ae..fe5be41471 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -10,7 +10,7 @@ struct complete_reflogs {
char *ref;
const char *short_ref;
struct reflog_info {
-   unsigned char osha1[20], nsha1[20];
+   struct object_id ooid, noid;
char *email;
unsigned long timestamp;
int tz;
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned 
char *nsha1,
 
ALLOC_GROW(array->items, array->nr + 1, array->alloc);
item = array->items + array->nr;
-   hashcpy(item->osha1, osha1);
-   hashcpy(item->nsha1, nsha1);
+   hashcpy(item->ooid.hash, osha1);
+   hashcpy(item->noid.hash, nsha1);
item->email = xstrdup(email);
item->timestamp = timestamp;
item->tz = tz;
@@ -238,13 +238,13 @@ void fake_reflog_parent(struct reflog_walk_info *info, 
struct commit *commit)
do {
reflog = _reflog->reflogs->items[commit_reflog->recno];
commit_reflog->recno--;
-   logobj = parse_object(reflog->osha1);
+   logobj = parse_object(reflog->ooid.hash);
} while (commit_reflog->recno && (logobj && logobj->type != 
OBJ_COMMIT));
 
-   if (!logobj && commit_reflog->recno >= 0 && 
is_null_sha1(reflog->osha1)) {
+   if (!logobj && commit_reflog->recno >= 0 && 
is_null_sha1(reflog->ooid.hash)) {
/* a root commit, but there are still more entries to show */
reflog = _reflog->reflogs->items[commit_reflog->recno];
-   logobj = parse_object(reflog->nsha1);
+   logobj = parse_object(reflog->noid.hash);
}
 
if (!logobj || logobj->type != OBJ_COMMIT) {
-- 
2.11.0

[PATCH v4 04/19] builtin/describe: convert to struct object_id

[brian m. carlson]

Convert the functions in this file and struct commit_name  to struct
object_id.

Signed-off-by: brian m. carlson 
---
 builtin/describe.c | 50 +-
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157e..738e68f95b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -39,11 +39,11 @@ static const char *diff_index_args[] = {
 
 struct commit_name {
struct hashmap_entry entry;
-   unsigned char peeled[20];
+   struct object_id peeled;
struct tag *tag;
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
-   unsigned char sha1[20];
+   struct object_id oid;
char *path;
 };
 
@@ -54,17 +54,17 @@ static const char *prio_names[] = {
 static int commit_name_cmp(const struct commit_name *cn1,
const struct commit_name *cn2, const void *peeled)
 {
-   return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
+   return oidcmp(>peeled, peeled ? peeled : >peeled);
 }
 
-static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+static inline struct commit_name *find_commit_name(const struct object_id 
*peeled)
 {
-   return hashmap_get_from_hash(, sha1hash(peeled), peeled);
+   return hashmap_get_from_hash(, sha1hash(peeled->hash), 
peeled->hash);
 }
 
 static int replace_name(struct commit_name *e,
   int prio,
-  const unsigned char *sha1,
+  const struct object_id *oid,
   struct tag **tag)
 {
if (!e || e->prio < prio)
@@ -77,13 +77,13 @@ static int replace_name(struct commit_name *e,
struct tag *t;
 
if (!e->tag) {
-   t = lookup_tag(e->sha1);
+   t = lookup_tag(e->oid.hash);
if (!t || parse_tag(t))
return 1;
e->tag = t;
}
 
-   t = lookup_tag(sha1);
+   t = lookup_tag(oid->hash);
if (!t || parse_tag(t))
return 0;
*tag = t;
@@ -96,24 +96,24 @@ static int replace_name(struct commit_name *e,
 }
 
 static void add_to_known_names(const char *path,
-  const unsigned char *peeled,
+  const struct object_id *peeled,
   int prio,
-  const unsigned char *sha1)
+  const struct object_id *oid)
 {
struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
-   if (replace_name(e, prio, sha1, )) {
+   if (replace_name(e, prio, oid, )) {
if (!e) {
e = xmalloc(sizeof(struct commit_name));
-   hashcpy(e->peeled, peeled);
-   hashmap_entry_init(e, sha1hash(peeled));
+   oidcpy(>peeled, peeled);
+   hashmap_entry_init(e, sha1hash(peeled->hash));
hashmap_add(, e);
e->path = NULL;
}
e->tag = tag;
e->prio = prio;
e->name_checked = 0;
-   hashcpy(e->sha1, sha1);
+   oidcpy(>oid, oid);
free(e->path);
e->path = xstrdup(path);
}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const struct 
object_id *oid, int flag, voi
else
prio = 0;
 
-   add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, 
oid->hash);
+   add_to_known_names(all ? path + 5 : path + 10, , prio, oid);
return 0;
 }
 
@@ -212,7 +212,7 @@ static unsigned long finish_depth_computation(
 static void display_name(struct commit_name *n)
 {
if (n->prio == 2 && !n->tag) {
-   n->tag = lookup_tag(n->sha1);
+   n->tag = lookup_tag(n->oid.hash);
if (!n->tag || parse_tag(n->tag))
die(_("annotated tag %s not available"), n->path);
}
@@ -230,14 +230,14 @@ static void display_name(struct commit_name *n)
printf("%s", n->path);
 }
 
-static void show_suffix(int depth, const unsigned char *sha1)
+static void show_suffix(int depth, const struct object_id *oid)
 {
-   printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
+   printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
 }
 
 static void describe(const char *arg, int last_one)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *cmit, *gave_up_on = NULL;
struct commit_list *list;
struct commit_name *n;
@@ -246,20 +246,20 @@ static void describe(const char *arg, int last_one)

[PATCH v4 02/19] builtin/commit: convert to struct object_id

[brian m. carlson]

Convert most leaf functions to use struct object_id.

Signed-off-by: brian m. carlson 
---
 builtin/commit.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 2de5f6cc64..4e288bc513 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -496,7 +496,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
 static int run_status(FILE *fp, const char *index_file, const char *prefix, 
int nowarn,
  struct wt_status *s)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
 
if (s->relative_paths)
s->prefix = prefix;
@@ -509,9 +509,9 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
s->index_file = index_file;
s->fp = fp;
s->nowarn = nowarn;
-   s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+   s->is_initial = get_sha1(s->reference, oid.hash) ? 1 : 0;
if (!s->is_initial)
-   hashcpy(s->sha1_commit, sha1);
+   hashcpy(s->sha1_commit, oid.hash);
s->status_format = status_format;
s->ignore_submodule_arg = ignore_submodule_arg;
 
@@ -885,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
commitable = run_status(s->fp, index_file, prefix, 1, s);
s->use_color = saved_color_setting;
} else {
-   unsigned char sha1[20];
+   struct object_id oid;
const char *parent = "HEAD";
 
if (!active_nr && read_cache() < 0)
@@ -894,7 +894,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
if (amend)
parent = "HEAD^1";
 
-   if (get_sha1(parent, sha1)) {
+   if (get_sha1(parent, oid.hash)) {
int i, ita_nr = 0;
 
for (i = 0; i < active_nr; i++)
@@ -1332,7 +1332,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
 {
static struct wt_status s;
int fd;
-   unsigned char sha1[20];
+   struct object_id oid;
static struct option builtin_status_options[] = {
OPT__VERBOSE(, N_("be verbose")),
OPT_SET_INT('s', "short", _format,
@@ -1382,9 +1382,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
 
fd = hold_locked_index(_lock, 0);
 
-   s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+   s.is_initial = get_sha1(s.reference, oid.hash) ? 1 : 0;
if (!s.is_initial)
-   hashcpy(s.sha1_commit, sha1);
+   hashcpy(s.sha1_commit, oid.hash);
 
s.ignore_submodule_arg = ignore_submodule_arg;
s.status_format = status_format;
@@ -1418,19 +1418,19 @@ static const char *implicit_ident_advice(void)
 
 }
 
-static void print_summary(const char *prefix, const unsigned char *sha1,
+static void print_summary(const char *prefix, const struct object_id *oid,
  int initial_commit)
 {
struct rev_info rev;
struct commit *commit;
struct strbuf format = STRBUF_INIT;
-   unsigned char junk_sha1[20];
+   struct object_id junk_oid;
const char *head;
struct pretty_print_context pctx = {0};
struct strbuf author_ident = STRBUF_INIT;
struct strbuf committer_ident = STRBUF_INIT;
 
-   commit = lookup_commit(sha1);
+   commit = lookup_commit(oid->hash);
if (!commit)
die(_("couldn't look up newly created commit"));
if (parse_commit(commit))
@@ -1477,7 +1477,7 @@ static void print_summary(const char *prefix, const 
unsigned char *sha1,
rev.diffopt.break_opt = 0;
diff_setup_done();
 
-   head = resolve_ref_unsafe("HEAD", 0, junk_sha1, NULL);
+   head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
if (!strcmp(head, "HEAD"))
head = _("detached HEAD");
else
@@ -1522,8 +1522,8 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
return git_status_config(k, v, s);
 }
 
-static int run_rewrite_hook(const unsigned char *oldsha1,
-   const unsigned char *newsha1)
+static int run_rewrite_hook(const struct object_id *oldoid,
+   const struct object_id *newoid)
 {
struct child_process proc = CHILD_PROCESS_INIT;
const char *argv[3];
@@ -1544,7 +1544,7 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
code = start_command();
if (code)
return code;
-   strbuf_addf(, "%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+   strbuf_addf(, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(proc.in, sb.buf, sb.len);

[PATCH v4 18/19] builtin/merge-base: convert to struct object_id

[brian m. carlson]

Convert the remaining uses of unsigned char [20] to struct object_id.

Signed-off-by: brian m. carlson 
---
 builtin/merge-base.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index db95bc29cf..cfe2a796f8 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -36,12 +36,12 @@ static const char * const merge_base_usage[] = {
 
 static struct commit *get_commit_reference(const char *arg)
 {
-   unsigned char revkey[20];
+   struct object_id revkey;
struct commit *r;
 
-   if (get_sha1(arg, revkey))
+   if (get_oid(arg, ))
die("Not a valid object name %s", arg);
-   r = lookup_commit_reference(revkey);
+   r = lookup_commit_reference(revkey.hash);
if (!r)
die("Not a valid commit name %s", arg);
 
@@ -113,14 +113,14 @@ struct rev_collect {
unsigned int initial : 1;
 };
 
-static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
+static void add_one_commit(struct object_id *oid, struct rev_collect *revs)
 {
struct commit *commit;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return;
 
-   commit = lookup_commit(sha1);
+   commit = lookup_commit(oid->hash);
if (!commit ||
(commit->object.flags & TMP_MARK) ||
parse_commit(commit))
@@ -139,15 +139,15 @@ static int collect_one_reflog_ent(struct object_id *ooid, 
struct object_id *noid
 
if (revs->initial) {
revs->initial = 0;
-   add_one_commit(ooid->hash, revs);
+   add_one_commit(ooid, revs);
}
-   add_one_commit(noid->hash, revs);
+   add_one_commit(noid, revs);
return 0;
 }
 
 static int handle_fork_point(int argc, const char **argv)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
char *refname;
const char *commitname;
struct rev_collect revs;
@@ -155,7 +155,7 @@ static int handle_fork_point(int argc, const char **argv)
struct commit_list *bases;
int i, ret = 0;
 
-   switch (dwim_ref(argv[0], strlen(argv[0]), sha1, )) {
+   switch (dwim_ref(argv[0], strlen(argv[0]), oid.hash, )) {
case 0:
die("No such ref: '%s'", argv[0]);
case 1:
@@ -165,16 +165,16 @@ static int handle_fork_point(int argc, const char **argv)
}
 
commitname = (argc == 2) ? argv[1] : "HEAD";
-   if (get_sha1(commitname, sha1))
+   if (get_oid(commitname, ))
die("Not a valid object name: '%s'", commitname);
 
-   derived = lookup_commit_reference(sha1);
+   derived = lookup_commit_reference(oid.hash);
memset(, 0, sizeof(revs));
revs.initial = 1;
for_each_reflog_ent(refname, collect_one_reflog_ent, );
 
-   if (!revs.nr && !get_sha1(refname, sha1))
-   add_one_commit(sha1, );
+   if (!revs.nr && !get_oid(refname, ))
+   add_one_commit(, );
 
for (i = 0; i < revs.nr; i++)
revs.commit[i]->object.flags &= ~TMP_MARK;
-- 
2.11.0

[PATCH v4 00/19] object_id part 6

[brian m. carlson]

This is another series in the continuing conversion to struct object_id.

This series converts more of the builtin directory and some of the refs
code to use struct object_id. Additionally, it implements an
nth_packed_object_oid function which provides a struct object_id version
of the nth_packed_object function, and a parse_oid_hex function that
makes parsing easier.

Changes from v3:
* Move the parse_oid_hex patch earlier in the series.
* Use parse_oid_hex in builtin/diff-tree.c.
* Fix several warts with parse_oid_hex pointed out by Peff.

Changes from v2:
* Fix misnamed function in commit message.
* Improve parameter name of parse_oid_hex.
* Improve docstring of parse_oid_hex.
* Remove needless variable.
* Rebase on master.

Changes from v1:
* Implement parse_oid_hex and use it.
* Make nth_packed_object_oid take a variable into which to store the
  object ID.  This avoids concerns about unsafe casts.
* Rebase on master.

brian m. carlson (19):
  hex: introduce parse_oid_hex
  builtin/commit: convert to struct object_id
  builtin/diff-tree: convert to struct object_id
  builtin/describe: convert to struct object_id
  builtin/fast-export: convert to struct object_id
  builtin/fmt-merge-message: convert to struct object_id
  builtin/grep: convert to struct object_id
  builtin/branch: convert to struct object_id
  builtin/clone: convert to struct object_id
  builtin/merge: convert to struct object_id
  Convert remaining callers of resolve_refdup to object_id
  builtin/replace: convert to struct object_id
  reflog-walk: convert struct reflog_info to struct object_id
  refs: convert each_reflog_ent_fn to struct object_id
  refs: simplify parsing of reflog entries
  sha1_file: introduce an nth_packed_object_oid function
  Convert object iteration callbacks to struct object_id
  builtin/merge-base: convert to struct object_id
  wt-status: convert to struct object_id

 builtin/branch.c|  26 +-
 builtin/cat-file.c  |   8 +--
 builtin/clone.c |  10 ++--
 builtin/commit.c|  46 -
 builtin/count-objects.c |   4 +-
 builtin/describe.c  |  50 +-
 builtin/diff-tree.c |  43 
 builtin/fast-export.c   |  58 ++---
 builtin/fmt-merge-msg.c |  70 -
 builtin/fsck.c  |  40 +++
 builtin/grep.c  |  24 -
 builtin/merge-base.c|  30 +--
 builtin/merge.c | 134 
 builtin/notes.c |  18 +++
 builtin/pack-objects.c  |   6 +--
 builtin/prune-packed.c  |   4 +-
 builtin/prune.c |   8 +--
 builtin/receive-pack.c  |   4 +-
 builtin/reflog.c|   2 +-
 builtin/replace.c   | 112 
 cache.h |  19 ++-
 hex.c   |   8 +++
 reachable.c |  30 +--
 ref-filter.c|   4 +-
 reflog-walk.c   |  26 +-
 refs.c  |  24 -
 refs.h  |   2 +-
 refs/files-backend.c|  29 ++-
 revision.c  |  12 ++---
 sha1_file.c |  27 +++---
 sha1_name.c |   2 +-
 transport.c |   4 +-
 wt-status.c |  52 +--
 33 files changed, 484 insertions(+), 452 deletions(-)

-- 
2.11.0

[PATCH v4 16/19] sha1_file: introduce an nth_packed_object_oid function

[brian m. carlson]

There are places in the code where we would like to provide a struct
object_id *, yet read the hash directly from the pack.  Provide an
nth_packed_object_oid function that is similar to the
nth_packed_object_sha1 function.

In order to avoid a potentially invalid cast, nth_packed_object_oid
provides a variable into which to store the value, which it returns on
success; on error, it returns NULL, as nth_packed_object_sha1 does.

Signed-off-by: brian m. carlson 
---
 cache.h |  6 ++
 sha1_file.c | 17 ++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index e03a672d15..29e59cbb56 100644
--- a/cache.h
+++ b/cache.h
@@ -1608,6 +1608,12 @@ extern void check_pack_index_ptr(const struct packed_git 
*p, const void *ptr);
  * error.
  */
 extern const unsigned char *nth_packed_object_sha1(struct packed_git *, 
uint32_t n);
+/*
+ * Like nth_packed_object_sha1, but write the data into the object specified by
+ * the the first argument.  Returns the first argument on success, and NULL on
+ * error.
+ */
+extern const struct object_id *nth_packed_object_oid(struct object_id *, 
struct packed_git *, uint32_t n);
 
 /*
  * Return the offset of the nth object within the specified packfile.
diff --git a/sha1_file.c b/sha1_file.c
index ec957db5e1..777b8e8eae 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2628,6 +2628,17 @@ const unsigned char *nth_packed_object_sha1(struct 
packed_git *p,
}
 }
 
+const struct object_id *nth_packed_object_oid(struct object_id *oid,
+ struct packed_git *p,
+ uint32_t n)
+{
+   const unsigned char *hash = nth_packed_object_sha1(p, n);
+   if (!hash)
+   return NULL;
+   hashcpy(oid->hash, hash);
+   return oid;
+}
+
 void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 {
const unsigned char *ptr = vptr;
@@ -3788,13 +3799,13 @@ static int for_each_object_in_pack(struct packed_git 
*p, each_packed_object_fn c
int r = 0;
 
for (i = 0; i < p->num_objects; i++) {
-   const unsigned char *sha1 = nth_packed_object_sha1(p, i);
+   struct object_id oid;
 
-   if (!sha1)
+   if (!nth_packed_object_oid(, p, i))
return error("unable to get sha1 of object %u in %s",
 i, p->pack_name);
 
-   r = cb(sha1, p, i, data);
+   r = cb(oid.hash, p, i, data);
if (r)
break;
}
-- 
2.11.0

[PATCH v4 08/19] builtin/branch: convert to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson 
---
 builtin/branch.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 9d30f55b0b..faf472ff8f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -32,7 +32,7 @@ static const char * const builtin_branch_usage[] = {
 };
 
 static const char *head;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -117,13 +117,13 @@ static int branch_merged(int kind, const char *name,
if (kind == FILTER_REFS_BRANCHES) {
struct branch *branch = branch_get(name);
const char *upstream = branch_get_upstream(branch, NULL);
-   unsigned char sha1[20];
+   struct object_id oid;
 
if (upstream &&
(reference_name = reference_name_to_free =
 resolve_refdup(upstream, RESOLVE_REF_READING,
-   sha1, NULL)) != NULL)
-   reference_rev = lookup_commit_reference(sha1);
+   oid.hash, NULL)) != NULL)
+   reference_rev = lookup_commit_reference(oid.hash);
}
if (!reference_rev)
reference_rev = head_rev;
@@ -153,10 +153,10 @@ static int branch_merged(int kind, const char *name,
 }
 
 static int check_branch_commit(const char *branchname, const char *refname,
-  const unsigned char *sha1, struct commit 
*head_rev,
+  const struct object_id *oid, struct commit 
*head_rev,
   int kinds, int force)
 {
-   struct commit *rev = lookup_commit_reference(sha1);
+   struct commit *rev = lookup_commit_reference(oid->hash);
if (!rev) {
error(_("Couldn't look up commit object for '%s'"), refname);
return -1;
@@ -183,7 +183,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
   int quiet)
 {
struct commit *head_rev = NULL;
-   unsigned char sha1[20];
+   struct object_id oid;
char *name = NULL;
const char *fmt;
int i;
@@ -207,7 +207,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
}
 
if (!force) {
-   head_rev = lookup_commit_reference(head_sha1);
+   head_rev = lookup_commit_reference(head_oid.hash);
if (!head_rev)
die(_("Couldn't look up commit object for HEAD"));
}
@@ -235,7 +235,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
RESOLVE_REF_READING
| RESOLVE_REF_NO_RECURSE
| RESOLVE_REF_ALLOW_BAD_NAME,
-   sha1, );
+   oid.hash, );
if (!target) {
error(remote_branch
  ? _("remote-tracking branch '%s' not found.")
@@ -245,13 +245,13 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
}
 
if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
-   check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+   check_branch_commit(bname.buf, name, , head_rev, kinds,
force)) {
ret = 1;
goto next;
}
 
-   if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
+   if (delete_ref(name, is_null_oid() ? NULL : oid.hash,
   REF_NODEREF)) {
error(remote_branch
  ? _("Error deleting remote-tracking branch '%s'")
@@ -267,7 +267,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
   bname.buf,
   (flags & REF_ISBROKEN) ? "broken"
   : (flags & REF_ISSYMREF) ? target
-  : find_unique_abbrev(sha1, DEFAULT_ABBREV));
+  : find_unique_abbrev(oid.hash, DEFAULT_ABBREV));
}
delete_branch_config(bname.buf);
 
@@ -693,7 +693,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
track = git_branch_track;
 
-   head = resolve_refdup("HEAD", 0, head_sha1, NULL);
+   head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
if (!strcmp(head, "HEAD"))
-- 
2.11.0

[PATCH v4 01/19] hex: introduce parse_oid_hex

[brian m. carlson]

Introduce a function, parse_oid_hex, which parses a hexadecimal object
ID and if successful, sets a pointer to just beyond the last character.
This allows for simpler, more robust parsing without needing to
hard-code integer values throughout the codebase.

Signed-off-by: brian m. carlson 
---
 cache.h | 9 +
 hex.c   | 8 
 2 files changed, 17 insertions(+)

diff --git a/cache.h b/cache.h
index 61fc86e6d7..e03a672d15 100644
--- a/cache.h
+++ b/cache.h
@@ -1319,6 +1319,15 @@ extern char *oid_to_hex_r(char *out, const struct 
object_id *oid);
 extern char *sha1_to_hex(const unsigned char *sha1);   /* static buffer 
result! */
 extern char *oid_to_hex(const struct object_id *oid);  /* same static buffer 
as sha1_to_hex */
 
+/*
+ * Parse a 40-character hexadecimal object ID starting from hex, updating the
+ * pointer specified by end when parsing stops.  The resulting object ID is
+ * stored in oid.  Returns 0 on success.  Parsing will stop on the first NUL or
+ * other invalid character.  end is only updated on success; otherwise, it is
+ * unmodified.
+ */
+extern int parse_oid_hex(const char *hex, struct object_id *oid, const char 
**end);
+
 extern int interpret_branch_name(const char *str, int len, struct strbuf *);
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
diff --git a/hex.c b/hex.c
index 845b01a874..eab7b626ee 100644
--- a/hex.c
+++ b/hex.c
@@ -53,6 +53,14 @@ int get_oid_hex(const char *hex, struct object_id *oid)
return get_sha1_hex(hex, oid->hash);
 }
 
+int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)
+{
+   int ret = get_oid_hex(hex, oid);
+   if (!ret)
+   *end = hex + GIT_SHA1_HEXSZ;
+   return ret;
+}
+
 char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
 {
static const char hex[] = "0123456789abcdef";
-- 
2.11.0

[PATCH v4 17/19] Convert object iteration callbacks to struct object_id

[brian m. carlson]

Convert each_loose_object_fn and each_packed_object_fn to take a pointer
to struct object_id.  Update the various callbacks.  Convert several
40-based constants to use GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 builtin/cat-file.c  |  8 
 builtin/count-objects.c |  4 ++--
 builtin/fsck.c  | 24 
 builtin/pack-objects.c  |  6 +++---
 builtin/prune-packed.c  |  4 ++--
 builtin/prune.c |  8 
 cache.h |  4 ++--
 reachable.c | 30 +++---
 sha1_file.c | 12 ++--
 9 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 30383e9eb4..8b85cb8cf0 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -409,20 +409,20 @@ static int batch_object_cb(const unsigned char sha1[20], 
void *vdata)
return 0;
 }
 
-static int batch_loose_object(const unsigned char *sha1,
+static int batch_loose_object(const struct object_id *oid,
  const char *path,
  void *data)
 {
-   sha1_array_append(data, sha1);
+   sha1_array_append(data, oid->hash);
return 0;
 }
 
-static int batch_packed_object(const unsigned char *sha1,
+static int batch_packed_object(const struct object_id *oid,
   struct packed_git *pack,
   uint32_t pos,
   void *data)
 {
-   sha1_array_append(data, sha1);
+   sha1_array_append(data, oid->hash);
return 0;
 }
 
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a04b4f2ef3..acb05940fc 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -53,7 +53,7 @@ static void loose_garbage(const char *path)
report_garbage(PACKDIR_FILE_GARBAGE, path);
 }
 
-static int count_loose(const unsigned char *sha1, const char *path, void *data)
+static int count_loose(const struct object_id *oid, const char *path, void 
*data)
 {
struct stat st;
 
@@ -62,7 +62,7 @@ static int count_loose(const unsigned char *sha1, const char 
*path, void *data)
else {
loose_size += on_disk_bytes(st);
loose++;
-   if (verbose && has_sha1_pack(sha1))
+   if (verbose && has_sha1_pack(oid->hash))
packed_loose++;
}
return 0;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 9b37606858..f76e4163ab 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -491,7 +491,7 @@ static void get_default_heads(void)
}
 }
 
-static struct object *parse_loose_object(const unsigned char *sha1,
+static struct object *parse_loose_object(const struct object_id *oid,
 const char *path)
 {
struct object *obj;
@@ -500,27 +500,27 @@ static struct object *parse_loose_object(const unsigned 
char *sha1,
unsigned long size;
int eaten;
 
-   if (read_loose_object(path, sha1, , , ) < 0)
+   if (read_loose_object(path, oid->hash, , , ) < 0)
return NULL;
 
if (!contents && type != OBJ_BLOB)
die("BUG: read_loose_object streamed a non-blob");
 
-   obj = parse_object_buffer(sha1, type, size, contents, );
+   obj = parse_object_buffer(oid->hash, type, size, contents, );
 
if (!eaten)
free(contents);
return obj;
 }
 
-static int fsck_loose(const unsigned char *sha1, const char *path, void *data)
+static int fsck_loose(const struct object_id *oid, const char *path, void 
*data)
 {
-   struct object *obj = parse_loose_object(sha1, path);
+   struct object *obj = parse_loose_object(oid, path);
 
if (!obj) {
errors_found |= ERROR_OBJECT;
error("%s: object corrupt or missing: %s",
- sha1_to_hex(sha1), path);
+ oid_to_hex(oid), path);
return 0; /* keep checking other objects */
}
 
@@ -619,26 +619,26 @@ static int fsck_cache_tree(struct cache_tree *it)
return err;
 }
 
-static void mark_object_for_connectivity(const unsigned char *sha1)
+static void mark_object_for_connectivity(const struct object_id *oid)
 {
-   struct object *obj = lookup_unknown_object(sha1);
+   struct object *obj = lookup_unknown_object(oid->hash);
obj->flags |= HAS_OBJ;
 }
 
-static int mark_loose_for_connectivity(const unsigned char *sha1,
+static int mark_loose_for_connectivity(const struct object_id *oid,
   const char *path,
   void *data)
 {
-   mark_object_for_connectivity(sha1);
+   mark_object_for_connectivity(oid);
return 0;
 }
 
-static int mark_packed_for_connectivity(const unsigned char *sha1,
+static int mark_packed_for_connectivity(const struct object_id *oid,
 

[PATCH v4 06/19] builtin/fmt-merge-message: convert to struct object_id

[brian m. carlson]

Convert most of the code to use struct object_id, including struct
origin_data and struct merge_parents.  Convert several instances of
hardcoded numbers into references to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 builtin/fmt-merge-msg.c | 70 -
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index efab62fd85..6faa3c0d24 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -41,7 +41,7 @@ struct src_data {
 };
 
 struct origin_data {
-   unsigned char sha1[20];
+   struct object_id oid;
unsigned is_local_branch:1;
 };
 
@@ -59,8 +59,8 @@ static struct string_list origins = STRING_LIST_INIT_DUP;
 struct merge_parents {
int alloc, nr;
struct merge_parent {
-   unsigned char given[20];
-   unsigned char commit[20];
+   struct object_id given;
+   struct object_id commit;
unsigned char used;
} *item;
 };
@@ -70,14 +70,14 @@ struct merge_parents {
  * hundreds of heads at a time anyway.
  */
 static struct merge_parent *find_merge_parent(struct merge_parents *table,
- unsigned char *given,
- unsigned char *commit)
+ struct object_id *given,
+ struct object_id *commit)
 {
int i;
for (i = 0; i < table->nr; i++) {
-   if (given && hashcmp(table->item[i].given, given))
+   if (given && oidcmp(>item[i].given, given))
continue;
-   if (commit && hashcmp(table->item[i].commit, commit))
+   if (commit && oidcmp(>item[i].commit, commit))
continue;
return >item[i];
}
@@ -85,14 +85,14 @@ static struct merge_parent *find_merge_parent(struct 
merge_parents *table,
 }
 
 static void add_merge_parent(struct merge_parents *table,
-unsigned char *given,
-unsigned char *commit)
+struct object_id *given,
+struct object_id *commit)
 {
if (table->nr && find_merge_parent(table, given, commit))
return;
ALLOC_GROW(table->item, table->nr + 1, table->alloc);
-   hashcpy(table->item[table->nr].given, given);
-   hashcpy(table->item[table->nr].commit, commit);
+   oidcpy(>item[table->nr].given, given);
+   oidcpy(>item[table->nr].commit, commit);
table->item[table->nr].used = 0;
table->nr++;
 }
@@ -106,30 +106,30 @@ static int handle_line(char *line, struct merge_parents 
*merge_parents)
struct src_data *src_data;
struct string_list_item *item;
int pulling_head = 0;
-   unsigned char sha1[20];
+   struct object_id oid;
 
-   if (len < 43 || line[40] != '\t')
+   if (len < GIT_SHA1_HEXSZ + 3 || line[GIT_SHA1_HEXSZ] != '\t')
return 1;
 
-   if (starts_with(line + 41, "not-for-merge"))
+   if (starts_with(line + GIT_SHA1_HEXSZ + 1, "not-for-merge"))
return 0;
 
-   if (line[41] != '\t')
+   if (line[GIT_SHA1_HEXSZ + 1] != '\t')
return 2;
 
-   i = get_sha1_hex(line, sha1);
+   i = get_oid_hex(line, );
if (i)
return 3;
 
-   if (!find_merge_parent(merge_parents, sha1, NULL))
+   if (!find_merge_parent(merge_parents, , NULL))
return 0; /* subsumed by other parents */
 
origin_data = xcalloc(1, sizeof(struct origin_data));
-   hashcpy(origin_data->sha1, sha1);
+   oidcpy(_data->oid, );
 
if (line[len - 1] == '\n')
line[len - 1] = 0;
-   line += 42;
+   line += GIT_SHA1_HEXSZ + 2;
 
/*
 * At this point, line points at the beginning of comment e.g.
@@ -338,10 +338,10 @@ static void shortlog(const char *name,
struct string_list committers = STRING_LIST_INIT_DUP;
int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
struct strbuf sb = STRBUF_INIT;
-   const unsigned char *sha1 = origin_data->sha1;
+   const struct object_id *oid = _data->oid;
int limit = opts->shortlog_len;
 
-   branch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);
+   branch = deref_tag(parse_object(oid->hash), oid_to_hex(oid), 
GIT_SHA1_HEXSZ);
if (!branch || branch->type != OBJ_COMMIT)
return;
 
@@ -531,7 +531,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 }
 
 static void find_merge_parents(struct merge_parents *result,
-  struct strbuf *in, unsigned char *head)
+  struct strbuf *in, struct object_id *head)
 {
struct commit_list *parents;

[PATCH v4 19/19] wt-status: convert to struct object_id

[brian m. carlson]

Convert the remaining uses of unsigned char [20] to struct object_id.

Signed-off-by: brian m. carlson 
---
 wt-status.c | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 5fac8437b0..a8d1faf80d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1115,16 +1115,16 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 
split = strbuf_split_max(line, ' ', 3);
if (split[0] && split[1]) {
-   unsigned char sha1[20];
+   struct object_id oid;
 
/*
 * strbuf_split_max left a space. Trim it and re-add
 * it after abbreviation.
 */
strbuf_trim(split[1]);
-   if (!get_sha1(split[1]->buf, sha1)) {
+   if (!get_oid(split[1]->buf, )) {
strbuf_reset(split[1]);
-   strbuf_add_unique_abbrev(split[1], sha1,
+   strbuf_add_unique_abbrev(split[1], oid.hash,
 DEFAULT_ABBREV);
strbuf_addch(split[1], ' ');
strbuf_reset(line);
@@ -1340,7 +1340,7 @@ static void show_bisect_in_progress(struct wt_status *s,
 static char *get_branch(const struct worktree *wt, const char *path)
 {
struct strbuf sb = STRBUF_INIT;
-   unsigned char sha1[20];
+   struct object_id oid;
const char *branch_name;
 
if (strbuf_read_file(, worktree_git_path(wt, "%s", path), 0) <= 0)
@@ -1354,9 +1354,9 @@ static char *get_branch(const struct worktree *wt, const 
char *path)
strbuf_remove(, 0, branch_name - sb.buf);
else if (starts_with(sb.buf, "refs/"))
;
-   else if (!get_sha1_hex(sb.buf, sha1)) {
+   else if (!get_oid_hex(sb.buf, )) {
strbuf_reset();
-   strbuf_add_unique_abbrev(, sha1, DEFAULT_ABBREV);
+   strbuf_add_unique_abbrev(, oid.hash, DEFAULT_ABBREV);
} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
goto got_nothing;
else/* bisect */
@@ -1370,7 +1370,7 @@ static char *get_branch(const struct worktree *wt, const 
char *path)
 
 struct grab_1st_switch_cbdata {
struct strbuf buf;
-   unsigned char nsha1[20];
+   struct object_id noid;
 };
 
 static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
@@ -1387,7 +1387,7 @@ static int grab_1st_switch(struct object_id *ooid, struct 
object_id *noid,
return 0;
target += strlen(" to ");
strbuf_reset(>buf);
-   hashcpy(cb->nsha1, noid->hash);
+   oidcpy(>noid, noid);
end = strchrnul(target, '\n');
strbuf_add(>buf, target, end - target);
if (!strcmp(cb->buf.buf, "HEAD")) {
@@ -1402,7 +1402,7 @@ static void wt_status_get_detached_from(struct 
wt_status_state *state)
 {
struct grab_1st_switch_cbdata cb;
struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id oid;
char *ref = NULL;
 
strbuf_init(, 0);
@@ -1411,22 +1411,22 @@ static void wt_status_get_detached_from(struct 
wt_status_state *state)
return;
}
 
-   if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, ) == 1 &&
+   if (dwim_ref(cb.buf.buf, cb.buf.len, oid.hash, ) == 1 &&
/* sha1 is a commit? match without further lookup */
-   (!hashcmp(cb.nsha1, sha1) ||
+   (!oidcmp(, ) ||
 /* perhaps sha1 is a tag, try to dereference to a commit */
-((commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
- !hashcmp(cb.nsha1, commit->object.oid.hash {
+((commit = lookup_commit_reference_gently(oid.hash, 1)) != NULL &&
+ !oidcmp(, >object.oid {
const char *from = ref;
if (!skip_prefix(from, "refs/tags/", ))
skip_prefix(from, "refs/remotes/", );
state->detached_from = xstrdup(from);
} else
state->detached_from =
-   xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
-   hashcpy(state->detached_sha1, cb.nsha1);
-   state->detached_at = !get_sha1("HEAD", sha1) &&
-!hashcmp(sha1, state->detached_sha1);
+   xstrdup(find_unique_abbrev(cb.noid.hash, 
DEFAULT_ABBREV));
+   hashcpy(state->detached_sha1, cb.noid.hash);
+   state->detached_at = !get_oid("HEAD", ) &&
+!hashcmp(oid.hash, state->detached_sha1);
 
free(ref);
strbuf_release();
@@ -1476,22 +1476,22 @@ void wt_status_get_state(struct wt_status_state *state,
 int get_detached_from)
 {
struct stat st;
-   unsigned char sha1[20];
+   struct object_id oid;
 
if 

[PATCH v4 12/19] builtin/replace: convert to struct object_id

[brian m. carlson]

Convert various uses of unsigned char [20] to struct object_id.  Rename
replace_object_sha1 to replace_object_oid.  Finally, specify a constant
in terms of GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 builtin/replace.c | 112 +++---
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index b58c714cb8..f7716a5472 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -88,78 +88,78 @@ static int list_replace_refs(const char *pattern, const 
char *format)
 }
 
 typedef int (*each_replace_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const struct object_id *oid);
 
 static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 {
const char **p, *full_hex;
char ref[PATH_MAX];
int had_error = 0;
-   unsigned char sha1[20];
+   struct object_id oid;
 
for (p = argv; *p; p++) {
-   if (get_sha1(*p, sha1)) {
+   if (get_oid(*p, )) {
error("Failed to resolve '%s' as a valid ref.", *p);
had_error = 1;
continue;
}
-   full_hex = sha1_to_hex(sha1);
+   full_hex = oid_to_hex();
snprintf(ref, sizeof(ref), "%s%s", git_replace_ref_base, 
full_hex);
/* read_ref() may reuse the buffer */
full_hex = ref + strlen(git_replace_ref_base);
-   if (read_ref(ref, sha1)) {
+   if (read_ref(ref, oid.hash)) {
error("replace ref '%s' not found.", full_hex);
had_error = 1;
continue;
}
-   if (fn(full_hex, ref, sha1))
+   if (fn(full_hex, ref, ))
had_error = 1;
}
return had_error;
 }
 
 static int delete_replace_ref(const char *name, const char *ref,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
-   if (delete_ref(ref, sha1, 0))
+   if (delete_ref(ref, oid->hash, 0))
return 1;
printf("Deleted replace ref '%s'\n", name);
return 0;
 }
 
-static void check_ref_valid(unsigned char object[20],
-   unsigned char prev[20],
+static void check_ref_valid(struct object_id *object,
+   struct object_id *prev,
char *ref,
int ref_size,
int force)
 {
if (snprintf(ref, ref_size,
 "%s%s", git_replace_ref_base,
-sha1_to_hex(object)) > ref_size - 1)
+oid_to_hex(object)) > ref_size - 1)
die("replace ref name too long: %.*s...", 50, ref);
if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
 
-   if (read_ref(ref, prev))
-   hashclr(prev);
+   if (read_ref(ref, prev->hash))
+   oidclr(prev);
else if (!force)
die("replace ref '%s' already exists", ref);
 }
 
-static int replace_object_sha1(const char *object_ref,
-  unsigned char object[20],
+static int replace_object_oid(const char *object_ref,
+  struct object_id *object,
   const char *replace_ref,
-  unsigned char repl[20],
+  struct object_id *repl,
   int force)
 {
-   unsigned char prev[20];
+   struct object_id prev;
enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
-   obj_type = sha1_object_info(object, NULL);
-   repl_type = sha1_object_info(repl, NULL);
+   obj_type = sha1_object_info(object->hash, NULL);
+   repl_type = sha1_object_info(repl->hash, NULL);
if (!force && obj_type != repl_type)
die("Objects must be of the same type.\n"
"'%s' points to a replaced object of type '%s'\n"
@@ -167,11 +167,11 @@ static int replace_object_sha1(const char *object_ref,
object_ref, typename(obj_type),
replace_ref, typename(repl_type));
 
-   check_ref_valid(object, prev, ref, sizeof(ref), force);
+   check_ref_valid(object, , ref, sizeof(ref), force);
 
transaction = ref_transaction_begin();
if (!transaction ||
-   ref_transaction_update(transaction, ref, repl, prev,
+   ref_transaction_update(transaction, ref, repl->hash, prev.hash,
   0, NULL, ) ||

[PATCH v4 14/19] refs: convert each_reflog_ent_fn to struct object_id

[brian m. carlson]

Make each_reflog_ent_fn take two struct object_id pointers instead of
two pointers to unsigned char.  Convert the various callbacks to use
struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
fsck_handle_reflog_oid.

Signed-off-by: brian m. carlson 
---
 builtin/fsck.c   | 16 
 builtin/merge-base.c |  6 +++---
 builtin/reflog.c |  2 +-
 reflog-walk.c|  6 +++---
 refs.c   | 24 
 refs.h   |  2 +-
 refs/files-backend.c | 24 
 revision.c   | 12 ++--
 sha1_name.c  |  2 +-
 wt-status.c  |  6 +++---
 10 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1a5caccd0f..9b37606858 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -396,13 +396,13 @@ static int fsck_obj_buffer(const unsigned char *sha1, 
enum object_type type,
 
 static int default_refs;
 
-static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
+static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
unsigned long timestamp)
 {
struct object *obj;
 
-   if (!is_null_sha1(sha1)) {
-   obj = lookup_object(sha1);
+   if (!is_null_oid(oid)) {
+   obj = lookup_object(oid->hash);
if (obj && (obj->flags & HAS_OBJ)) {
if (timestamp && name_objects)
add_decoration(fsck_walk_options.object_names,
@@ -411,13 +411,13 @@ static void fsck_handle_reflog_sha1(const char *refname, 
unsigned char *sha1,
obj->used = 1;
mark_object_reachable(obj);
} else {
-   error("%s: invalid reflog entry %s", refname, 
sha1_to_hex(sha1));
+   error("%s: invalid reflog entry %s", refname, 
oid_to_hex(oid));
errors_found |= ERROR_REACHABLE;
}
}
 }
 
-static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id 
*noid,
const char *email, unsigned long timestamp, int tz,
const char *message, void *cb_data)
 {
@@ -425,10 +425,10 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
 
if (verbose)
fprintf(stderr, "Checking reflog %s->%s\n",
-   sha1_to_hex(osha1), sha1_to_hex(nsha1));
+   oid_to_hex(ooid), oid_to_hex(noid));
 
-   fsck_handle_reflog_sha1(refname, osha1, 0);
-   fsck_handle_reflog_sha1(refname, nsha1, timestamp);
+   fsck_handle_reflog_oid(refname, ooid, 0);
+   fsck_handle_reflog_oid(refname, noid, timestamp);
return 0;
 }
 
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index b572a37c26..db95bc29cf 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -131,7 +131,7 @@ static void add_one_commit(unsigned char *sha1, struct 
rev_collect *revs)
commit->object.flags |= TMP_MARK;
 }
 
-static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int collect_one_reflog_ent(struct object_id *ooid, struct object_id 
*noid,
  const char *ident, unsigned long timestamp,
  int tz, const char *message, void *cbdata)
 {
@@ -139,9 +139,9 @@ static int collect_one_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
 
if (revs->initial) {
revs->initial = 0;
-   add_one_commit(osha1, revs);
+   add_one_commit(ooid->hash, revs);
}
-   add_one_commit(nsha1, revs);
+   add_one_commit(noid->hash, revs);
return 0;
 }
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e53e..7472775778 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -615,7 +615,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
return status;
 }
 
-static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
const char *email, unsigned long timestamp, int tz,
const char *message, void *cb_data)
 {
diff --git a/reflog-walk.c b/reflog-walk.c
index fe5be41471..99679f5825 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -19,7 +19,7 @@ struct complete_reflogs {
int nr, alloc;
 };
 
-static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
+static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
const char *email, unsigned long timestamp, int tz,
const char *message, void *cb_data)
 {
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned 
char *nsha1,
 
ALLOC_GROW(array->items, array->nr + 

[PATCH v4 09/19] builtin/clone: convert to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson 
---
 builtin/clone.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edbbf9..b4c929bb8a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -681,7 +681,7 @@ static void update_head(const struct ref *our, const struct 
ref *remote,
 
 static int checkout(int submodule_progress)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
char *head;
struct lock_file *lock_file;
struct unpack_trees_options opts;
@@ -692,7 +692,7 @@ static int checkout(int submodule_progress)
if (option_no_checkout)
return 0;
 
-   head = resolve_refdup("HEAD", RESOLVE_REF_READING, sha1, NULL);
+   head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
if (!head) {
warning(_("remote HEAD refers to nonexistent ref, "
  "unable to checkout.\n"));
@@ -700,7 +700,7 @@ static int checkout(int submodule_progress)
}
if (!strcmp(head, "HEAD")) {
if (advice_detached_head)
-   detach_advice(sha1_to_hex(sha1));
+   detach_advice(oid_to_hex());
} else {
if (!starts_with(head, "refs/heads/"))
die(_("HEAD not found below refs/heads!"));
@@ -721,7 +721,7 @@ static int checkout(int submodule_progress)
opts.src_index = _index;
opts.dst_index = _index;
 
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid.hash);
parse_tree(tree);
init_tree_desc(, tree->buffer, tree->size);
if (unpack_trees(1, , ) < 0)
@@ -731,7 +731,7 @@ static int checkout(int submodule_progress)
die(_("unable to write new index file"));
 
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
-  sha1_to_hex(sha1), "1", NULL);
+  oid_to_hex(), "1", NULL);
 
if (!err && option_recursive) {
struct argv_array args = ARGV_ARRAY_INIT;
-- 
2.11.0

[PATCH v4 05/19] builtin/fast-export: convert to struct object_id

[brian m. carlson]

In addition to converting to struct object_id, write some hardcoded
buffer sizes in terms of GIT_SHA1_RAWSZ.

Signed-off-by: brian m. carlson 
---
 builtin/fast-export.c | 58 +--
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 1e815b5577..e0220630d0 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -212,7 +212,7 @@ static char *anonymize_blob(unsigned long *size)
return strbuf_detach(, NULL);
 }
 
-static void export_blob(const unsigned char *sha1)
+static void export_blob(const struct object_id *oid)
 {
unsigned long size;
enum object_type type;
@@ -223,34 +223,34 @@ static void export_blob(const unsigned char *sha1)
if (no_data)
return;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return;
 
-   object = lookup_object(sha1);
+   object = lookup_object(oid->hash);
if (object && object->flags & SHOWN)
return;
 
if (anonymize) {
buf = anonymize_blob();
-   object = (struct object *)lookup_blob(sha1);
+   object = (struct object *)lookup_blob(oid->hash);
eaten = 0;
} else {
-   buf = read_sha1_file(sha1, , );
+   buf = read_sha1_file(oid->hash, , );
if (!buf)
-   die ("Could not read blob %s", sha1_to_hex(sha1));
-   if (check_sha1_signature(sha1, buf, size, typename(type)) < 0)
-   die("sha1 mismatch in blob %s", sha1_to_hex(sha1));
-   object = parse_object_buffer(sha1, type, size, buf, );
+   die ("Could not read blob %s", oid_to_hex(oid));
+   if (check_sha1_signature(oid->hash, buf, size, typename(type)) 
< 0)
+   die("sha1 mismatch in blob %s", oid_to_hex(oid));
+   object = parse_object_buffer(oid->hash, type, size, buf, 
);
}
 
if (!object)
-   die("Could not read blob %s", sha1_to_hex(sha1));
+   die("Could not read blob %s", oid_to_hex(oid));
 
mark_next_object(object);
 
printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
if (size && fwrite(buf, size, 1, stdout) != 1)
-   die_errno ("Could not write blob '%s'", sha1_to_hex(sha1));
+   die_errno ("Could not write blob '%s'", oid_to_hex(oid));
printf("\n");
 
show_progress();
@@ -323,19 +323,19 @@ static void print_path(const char *path)
}
 }
 
-static void *generate_fake_sha1(const void *old, size_t *len)
+static void *generate_fake_oid(const void *old, size_t *len)
 {
static uint32_t counter = 1; /* avoid null sha1 */
-   unsigned char *out = xcalloc(20, 1);
-   put_be32(out + 16, counter++);
+   unsigned char *out = xcalloc(GIT_SHA1_RAWSZ, 1);
+   put_be32(out + GIT_SHA1_RAWSZ - 4, counter++);
return out;
 }
 
-static const unsigned char *anonymize_sha1(const unsigned char *sha1)
+static const unsigned char *anonymize_sha1(const struct object_id *oid)
 {
static struct hashmap sha1s;
-   size_t len = 20;
-   return anonymize_mem(, generate_fake_sha1, sha1, );
+   size_t len = GIT_SHA1_RAWSZ;
+   return anonymize_mem(, generate_fake_oid, oid, );
 }
 
 static void show_filemodify(struct diff_queue_struct *q,
@@ -383,7 +383,7 @@ static void show_filemodify(struct diff_queue_struct *q,
if (no_data || S_ISGITLINK(spec->mode))
printf("M %06o %s ", spec->mode,
   sha1_to_hex(anonymize ?
-  
anonymize_sha1(spec->oid.hash) :
+  anonymize_sha1(>oid) :
   spec->oid.hash));
else {
struct object *object = 
lookup_object(spec->oid.hash);
@@ -572,7 +572,7 @@ static void handle_commit(struct commit *commit, struct 
rev_info *rev)
/* Export the referenced blobs, and remember the marks. */
for (i = 0; i < diff_queued_diff.nr; i++)
if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
-   export_blob(diff_queued_diff.queue[i]->two->oid.hash);
+   export_blob(_queued_diff.queue[i]->two->oid);
 
refname = commit->util;
if (anonymize) {
@@ -797,14 +797,14 @@ static void get_tags_and_duplicates(struct 
rev_cmdline_info *info)
 
for (i = 0; i < info->nr; i++) {
struct rev_cmdline_entry *e = info->rev + i;
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
char *full_name;
 
if (e->flags & 

[PATCH v4 15/19] refs: simplify parsing of reflog entries

[brian m. carlson]

The current code for reflog entries uses a lot of hard-coded constants,
making it hard to read and modify.  Use parse_oid_hex and two temporary
variables to simplify the code and reduce the use of magic constants.

Signed-off-by: brian m. carlson 
---
 refs/files-backend.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index d7a5fd2a7c..fea20e99fe 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3117,12 +3117,13 @@ static int show_one_reflog_ent(struct strbuf *sb, 
each_reflog_ent_fn fn, void *c
char *email_end, *message;
unsigned long timestamp;
int tz;
+   const char *p = sb->buf;
 
/* old SP new SP name  SP time TAB msg LF */
-   if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-   get_oid_hex(sb->buf, ) || sb->buf[40] != ' ' ||
-   get_oid_hex(sb->buf + 41, ) || sb->buf[81] != ' ' ||
-   !(email_end = strchr(sb->buf + 82, '>')) ||
+   if (!sb->len || sb->buf[sb->len - 1] != '\n' ||
+   parse_oid_hex(p, , ) || *p++ != ' ' ||
+   parse_oid_hex(p, , ) || *p++ != ' ' ||
+   !(email_end = strchr(p, '>')) ||
email_end[1] != ' ' ||
!(timestamp = strtoul(email_end + 2, , 10)) ||
!message || message[0] != ' ' ||
@@ -3136,7 +3137,7 @@ static int show_one_reflog_ent(struct strbuf *sb, 
each_reflog_ent_fn fn, void *c
message += 6;
else
message += 7;
-   return fn(, , sb->buf + 82, timestamp, tz, message, cb_data);
+   return fn(, , p, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
-- 
2.11.0

Re: [RFC PATCH] show decorations at the end of the line

[Jacob Keller]

On Sun, Feb 19, 2017 at 2:33 PM, Linus Torvalds
 wrote:
> On Fri, Feb 17, 2017 at 9:27 PM, Junio C Hamano  wrote:
>>
>> I just got bitten by a fallout.  I have
>>
>> $ git recent --help
>> `git recent' is aliased to `log --oneline --branches --no-merges \
>>  --source --since=3.weeks'
>>
>> but now the branch names are shown at the end, which defeats the
>> whole point of the alias.
>
> Yes, your situation actually wants those decorations as primary
> things, so having them at the end is indeed pointless.
>
> So I think we should just discard that patch of mine.
>
>  Linus

I would think that in general putting them at the end makes more
sense, but we should have the ability to use them in format specifiers
so that users are free to customize it exactly how they want. That is,
I agree with the reasoning presented in the original patch, but think
Junio's case can be solved by strengthening the custom formats.

Thanks,
Jake

Re: [RFC PATCH] show decorations at the end of the line

[Linus Torvalds]

On Fri, Feb 17, 2017 at 9:27 PM, Junio C Hamano  wrote:
>
> I just got bitten by a fallout.  I have
>
> $ git recent --help
> `git recent' is aliased to `log --oneline --branches --no-merges \
>  --source --since=3.weeks'
>
> but now the branch names are shown at the end, which defeats the
> whole point of the alias.

Yes, your situation actually wants those decorations as primary
things, so having them at the end is indeed pointless.

So I think we should just discard that patch of mine.

 Linus

[PATCH v2] git-check-ref-format: clarify man for --normalize

[Damien Regad]

Use of 'iff' may be confusing to people not familiar with this term.

Improving the --normalize option's documentation to remove the use of
'iff', and clearly describe what happens when the condition is not met.
---
 Documentation/git-check-ref-format.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-check-ref-format.txt
b/Documentation/git-check-ref-format.txt
index 8611a99..92777ce 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -100,10 +100,10 @@ OPTIONS
 --normalize::
Normalize 'refname' by removing any leading slash (`/`)
characters and collapsing runs of adjacent slashes between
-   name components into a single slash.  Iff the normalized
+   name components into a single slash.  If the normalized
refname is valid then print it to standard output and exit
-   with a status of 0.  (`--print` is a deprecated way to spell
-   `--normalize`.)
+   with a status of 0, otherwise exit with a non-zero status.
+   (`--print` is a deprecated way to spell `--normalize`.)


 EXAMPLES
-- 
2.7.4

Re: [PATCH] git-check-ref-format: fix typo in man page

[Damien Regad]

Thanks all for the feedback.

On 2017-02-19 21:40, Philip Oakley wrote:
> For those not familiar with 'iff', then a change to the doc is worthwhile.

Exactly. Not being a native English speaker, I had never seen 'iff' used
before. Now that you guys have pointed me to its meaning I guess it
makes sense in this context.

That being said, IMHO software documentation is not a mathematics
textbook, and should be written in "plain" English, so

On 2017-02-19 03:27, Jeff King wrote:
> So maybe an overall improvement would be something like:
>
>   If the normalized refname is valid then print it to standard output
>   and exit with a status of 0. Otherwise, exit with a non-zero status.

I'll submit a revised patch shortly, following your suggestion.

Cheers
Damien

[PATCH] Documentation: Link git-ls-files to core.quotePath variable.

[Andreas Heiduk]

Add a hint for script writers where additional quoting can be configured.

Signed-off-by: Andreas Heiduk 
---
 Documentation/git-ls-files.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 446209e..19e0636 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -198,7 +198,8 @@ path. (see linkgit:git-read-tree[1] for more information on 
state)
 
 When `-z` option is not used, TAB, LF, and backslash characters
 in pathnames are represented as `\t`, `\n`, and `\\`,
-respectively.
+respectively. The path is also quoted according to the
+configuration variable `core.quotePath` (see linkgit:git-config[1]).
 
 
 Exclude Patterns
-- 
2.7.4

Re: [PATCH 3/5] name-hash: precompute hash values during preload-index

[Junio C Hamano]

Jeff Hostetler  writes:

> I looked at doing this, but I didn't think the complexity and overhead to
> forward search for peers at the current level didn't warrant the limited 
> gains.

It seems that I wasn't clear what I meant.  I didn't mean anything
complex like what you said.

Just something simple, like this on top of yours, that passes and
compares with only the previous one.  I do not know if that gives
any gain, though ;-).

 cache.h |  2 +-
 name-hash.c | 11 +--
 preload-index.c |  4 +++-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 390aa803df..bd2980f6e3 100644
--- a/cache.h
+++ b/cache.h
@@ -233,7 +233,7 @@ struct cache_entry {
 #error "CE_EXTENDED_FLAGS out of range"
 #endif
 
-void precompute_istate_hashes(struct cache_entry *ce);
+void precompute_istate_hashes(struct cache_entry *ce, struct cache_entry 
*prev);
 
 /* Forward structure decls */
 struct pathspec;
diff --git a/name-hash.c b/name-hash.c
index f95054f44c..5e09b79170 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -300,7 +300,7 @@ void free_name_hash(struct index_state *istate)
  * non-skip-worktree items (since status should not observe skipped items), but
  * because lazy_init_name_hash() hashes everything, we force it here.
  */
-void precompute_istate_hashes(struct cache_entry *ce)
+void precompute_istate_hashes(struct cache_entry *ce, struct cache_entry *prev)
 {
int namelen = ce_namelen(ce);
 
@@ -312,7 +312,14 @@ void precompute_istate_hashes(struct cache_entry *ce)
ce->precomputed_hash.root_entry = 1;
} else {
namelen--;
-   ce->precomputed_hash.dir = memihash(ce->name, namelen);
+
+   if (prev && 
+   prev->precomputed_hash.initialized &&
+   namelen <= ce_namelen(prev) &&
+   !memcmp(ce->name, prev->name, namelen))
+   ce->precomputed_hash.dir = prev->precomputed_hash.dir;
+   else
+   ce->precomputed_hash.dir = memihash(ce->name, namelen);
ce->precomputed_hash.name = memihash_continue(
ce->precomputed_hash.dir, ce->name + namelen,
ce_namelen(ce) - namelen);
diff --git a/preload-index.c b/preload-index.c
index 602737f9d0..784378ffac 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -37,6 +37,7 @@ static void *preload_thread(void *_data)
struct thread_data *p = _data;
struct index_state *index = p->index;
struct cache_entry **cep = index->cache + p->offset;
+   struct cache_entry *previous = NULL;
struct cache_def cache = CACHE_DEF_INIT;
 
nr = p->nr;
@@ -47,7 +48,8 @@ static void *preload_thread(void *_data)
struct cache_entry *ce = *cep++;
struct stat st;
 
-   precompute_istate_hashes(ce);
+   precompute_istate_hashes(ce, previous);
+   previous = ce;
 
if (ce_stage(ce))
continue;




> (I was just looking at the complexity of clear_ce_flags_1() in unpack-trees.c
> and how hard it has to look to find the end of the current directory and the
> effect that that has on the recursion and it felt like too much work for the
> potential gain.)
>
> Whereas remembering the previous one was basically free.  Granted, it only
> helps us for adjacent files in the index, so it's not perfect, but gives us 
> the
> best bang for the buck.
>
> Jeff

Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area

[Junio C Hamano]

Jeff Hostetler  writes:

>> But the other Jeff sounded like a follow-up was to follow shortly if
>> not imminent so I decided to allocate my time on other topics still
>> only on the list first while waiting to see what happens.
>
> Sorry, I was out of the office for a family emergency on Thursday
> and Friday.  Add to that the long weekend, and I won't get back around
> to this until Tuesday or Wednesday at the earliest.

The open source process makes progress at the pace of its
participants, and it is expected that some topics come fast while
others don't.

Hope things are all OK for you and your family now.

Thanks.

Re: [PATCH v2] l10n: de.po: translate 241 messages

[Phillip Sz]

Acked-by: Phillip Sz 

> Signed-off-by: Ralf Thielow 
> ---
>  po/de.po | 750 
> ++-
>  1 file changed, 409 insertions(+), 341 deletions(-)
> 
> diff --git a/po/de.po b/po/de.po
> index 2326da1fd..e9c86f548 100644
> --- a/po/de.po
> +++ b/po/de.po
> @@ -913,20 +913,20 @@ msgstr ""
>  
>  #: bisect.c:742
>  #, c-format
>  msgid ""
>  "The merge base %s is %s.\n"
>  "This means the first '%s' commit is between %s and [%s].\n"
>  msgstr ""
>  "Die Merge-Basis %s ist %s.\n"
> -"Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s]\n"
> +"Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s].\n"
>  
>  #: bisect.c:750
> -#, fuzzy, c-format
> +#, c-format
>  msgid ""
>  "Some %s revs are not ancestors of the %s rev.\n"
>  "git bisect cannot work properly in this case.\n"
>  "Maybe you mistook %s and %s revs?\n"
>  msgstr ""
>  "Manche %s Commits sind keine Vorgänger des %s Commits.\n"
>  "git bisect kann in diesem Fall nicht richtig arbeiten.\n"
>  "Vielleicht verwechselten Sie %s und %s Commits?\n"
> @@ -1343,19 +1343,19 @@ msgid "bad zlib compression level %d"
>  msgstr "ungültiger zlib Komprimierungsgrad %d"
>  
>  #: config.c:993
>  #, c-format
>  msgid "invalid mode for object creation: %s"
>  msgstr "Ungültiger Modus für Objekterstellung: %s"
>  
>  #: config.c:1149
> -#, fuzzy, c-format
> +#, c-format
>  msgid "bad pack compression level %d"
> -msgstr "Komprimierungsgrad für Paketierung"
> +msgstr "ungültiger Komprimierungsgrad (%d) für Paketierung"
>  
>  #: config.c:1339
>  msgid "unable to parse command-line config"
>  msgstr ""
>  "Konnte die über die Befehlszeile angegebene Konfiguration nicht parsen."
>  
>  #: config.c:1389
>  msgid "unknown error occurred while reading the configuration files"
> @@ -1375,19 +1375,19 @@ msgid "bad config variable '%s' in file '%s' at line 
> %d"
>  msgstr "ungültige Konfigurationsvariable '%s' in Datei '%s' bei Zeile %d"
>  
>  #: config.c:1804
>  #, c-format
>  msgid "%s has multiple values"
>  msgstr "%s hat mehrere Werte"
>  
>  #: config.c:2225 config.c:2450
> -#, fuzzy, c-format
> +#, c-format
>  msgid "fstat on %s failed"
> -msgstr "\"stash\" fehlgeschlagen"
> +msgstr "fstat auf %s fehlgeschlagen"
>  
>  #: config.c:2343
>  #, c-format
>  msgid "could not set '%s' to '%s'"
>  msgstr "Konnte '%s' nicht zu '%s' setzen."
>  
>  #: config.c:2345
>  #, c-format
> @@ -1616,19 +1616,19 @@ msgstr "Fehler beim Sammeln von Namen und 
> Informationen zum Kernel"
>  
>  #: dir.c:1981
>  msgid "Untracked cache is disabled on this system or location."
>  msgstr ""
>  "Cache für unversionierte Dateien ist auf diesem System oder\n"
>  "für dieses Verzeichnis deaktiviert."
>  
>  #: dir.c:2759
> -#, fuzzy, c-format
> +#, c-format
>  msgid "could not migrate git directory from '%s' to '%s'"
> -msgstr "Konnte Verzeichnis '%s' nicht erstellen."
> +msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' migrieren."
>  
>  #: fetch-pack.c:213
>  msgid "git fetch-pack: expected shallow list"
>  msgstr "git fetch-pack: erwartete shallow-Liste"
>  
>  #: fetch-pack.c:225
>  msgid "git fetch-pack: expected ACK/NAK, got EOF"
>  msgstr "git fetch-pack: ACK/NAK erwartet, EOF bekommen"
> @@ -1803,17 +1803,17 @@ msgstr "konnte temporäre Datei nicht erstellen"
>  #: gpg-interface.c:217
>  #, c-format
>  msgid "failed writing detached signature to '%s'"
>  msgstr "Fehler beim Schreiben der losgelösten Signatur nach '%s'"
>  
>  #: graph.c:96
>  #, c-format
>  msgid "ignore invalid color '%.*s' in log.graphColors"
> -msgstr ""
> +msgstr "Ignoriere ungültige Farbe '%.*s' in log.graphColors"
>  
>  #: grep.c:1794
>  #, c-format
>  msgid "'%s': unable to read %s"
>  msgstr "'%s': konnte %s nicht lesen"
>  
>  #: grep.c:1811 builtin/clone.c:381 builtin/diff.c:81 builtin/rm.c:133
>  #, c-format
> @@ -2320,17 +2320,17 @@ msgstr "%s: 'literal' und 'glob' sind inkompatibel"
>  #: pathspec.c:363
>  #, c-format
>  msgid "%s: '%s' is outside repository"
>  msgstr "%s: '%s' liegt außerhalb des Repositories"
>  
>  #: pathspec.c:451
>  #, c-format
>  msgid "'%s' (mnemonic: '%c')"
> -msgstr ""
> +msgstr "'%s' (Kürzel: '%c')"
>  
>  #: pathspec.c:461
>  #, c-format
>  msgid "%s: pathspec magic not supported by this command: %s"
>  msgstr ""
>  "%s: Pfadspezifikationsangabe wird von diesem Befehl nicht unterstützt: %s"
>  
>  #: pathspec.c:511
> @@ -2418,19 +2418,19 @@ msgid "%%(body) does not take arguments"
>  msgstr "%%(body) akzeptiert keine Argumente"
>  
>  #: ref-filter.c:85
>  #, c-format
>  msgid "%%(subject) does not take arguments"
>  msgstr "%%(subject) akzeptiert keine Argumente"
>  
>  #: ref-filter.c:92
> -#, fuzzy, c-format
> +#, c-format
>  msgid "%%(trailers) does not take arguments"
> -msgstr "%%(body) akzeptiert keine Argumente"
> +msgstr "%%(trailers) akzeptiert keine Argumente"
>  
>  #: ref-filter.c:111
>  #, c-format
>  msgid "positive value 

url..insteadOf vs. submodules

[Toolforger]

Hi all,

I am trying to make url..insteadOf work on the URLs inside 
.gitmodules, but it won't work (applying it to the repo itself works 
fine, to the config setting seems to be fine).


I do not want to modify .gitmodules: It is maintained upstream.

I cannot simply reconfigure submodule..url: the Configure script 
(regularly called during each compile) does

  git submodule sync
  git submodule update --init
I could tell upstream to change these commands if I can make a good 
argument; for them, it is relevant that they can change the submodule 
URL inside .gitmodule and have it "just work" for everybody downstream.


My own use case is that I want to be able to work with various 
experimental local clones even if I do not have Internet access.

I'm all ears if there's a way to do this without using insteadOf.


Here are the relevant two lines from the output of "git config -l" 
(after "git submodule init"):


url./home/jo/Projekte/perl6/bare-repos.insteadof=https://github.com
submodule.3rdparty/dynasm.url=https://github.com/MoarVM/dynasm.git


Here is what "git submodule update" does:

Cloning into '3rdparty/dyncall'...
fatal: unable to access 'https://github.com/MoarVM/dyncall.git/': Could 
not resolve host: github.com
fatal: clone of 'https://github.com/MoarVM/dyncall.git' into submodule 
path '3rdparty/dyncall' failed



Any help appreciated!

Regards,
Jo

Fast Loans

[Service Loans]

 Do you need a loan to pay up bill or to start a business? Email Us

Re: [PATCH] git-check-ref-format: fix typo in man page

[Philip Oakley]

From: "Jeff King" 

On Sun, Feb 19, 2017 at 12:20:33AM -, Philip Oakley wrote:


>  Normalize 'refname' by removing any leading slash (`/`)
>  characters and collapsing runs of adjacent slashes between
> - name components into a single slash.  Iff the normalized
> + name components into a single slash.  If the normalized
>  refname is valid then print it to standard output and exit
>  with a status of 0.  (`--print` is a deprecated way to spell
>  `--normalize`.)
> -- 

Could that be an 'iff' == 'If and only if' (which is common in 
mathematics)?

Still could be spelling error though.


When we're not sure what the intent of a change is, a good first step is
to dig up the original commit via `git blame` or similar. In this case,
it comes from a40e6fb67 (Change check_refname_format() to reject
unnormalized refnames, 2011-09-15).


Oops, blaming a bit of code feels 'obvious' but I just hadn't thought to 
blame the doc, though it does feel as though code and the docs don't always 
go hand in hand.




The commit message doesn't mention it (not that I really expected it
to), but it does tell you who the author is. And a good second step is
to cc them on the patch. :)

I suspect it _was_ intended as "iff" here. In my opinion, we probably
don't need to be so rigorous in this instance. However, I note that we
do not describe the "else" half of that "if". So maybe an overall
improvement would be something like:


I read the commit message the same, that is, only if the given ref name 
normalises to a true (properly formatted) ref will it be printed (sucess).


For those not familiar with 'iff', then a change to the doc is worthwhile.



 If the normalized refname is valid then print it to standard output
 and exit with a status of 0. Otherwise, exit with a non-zero status.

-Peff

Thanks, Philip. 

Draft of Git Rev News edition 24

[Christian Couder]

Hi,

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-24.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/221

You can also reply to this email.

In general all kinds of contribution, for example proofreading,
suggestions for articles or links, help on the issues in GitHub, and
so on, are very much appreciated.

I tried to cc everyone who appears in this edition, but maybe I missed
some people, sorry about that.

Thomas, Jakub, Markus and myself plan to publish this edition on
Wednesday February 22.

Thanks,
Christian.

Re: Git bisect does not find commit introducing the bug

[Jacob Keller]

On Sun, Feb 19, 2017 at 11:05 AM, Alex Hoffman  wrote:
>> Then you must adjust your definition of "good": All commits that do not have
>> the feature, yet, are "good": since they do not have the feature in the
>> first place, they cannot have the breakage that you found in the feature.
>>
>> That is exactly the situation in your original example! But you constructed
>> the condition of goodness in such a simplistic way (depending on the
>> presence of a string), that it was impossible to distinguish between "does
>> not have the feature at all" and "has the feature, but it is broken".
>
> Johannes, thank you for correctly identifying the error in my logic.
> Indeed I was using the term 'bad' also for the commit without the
> feature. In order to find the commit introducing the bug in my example
> a new state is needed, which would make 'git bisect' a bit more
> complicated than the user 'most of the time' probably needs. Or do you
> think, it would make sense to ask the user for this state (if e.g 'git
> bisect' would be started with a new parameter)?

If a commit doesn't have the feature, then it is by definition, not
containing a broken feature, and you can simply use the "good" state.
There is no need for a different state. If you can't test the commit
because it's broken in some other way, you can use "git bisect skip"
but that isn't what you want in this case.

Thanks,
Jake

Re: Git bisect does not find commit introducing the bug

[Alex Hoffman]

> Then you must adjust your definition of "good": All commits that do not have
> the feature, yet, are "good": since they do not have the feature in the
> first place, they cannot have the breakage that you found in the feature.
>
> That is exactly the situation in your original example! But you constructed
> the condition of goodness in such a simplistic way (depending on the
> presence of a string), that it was impossible to distinguish between "does
> not have the feature at all" and "has the feature, but it is broken".

Johannes, thank you for correctly identifying the error in my logic.
Indeed I was using the term 'bad' also for the commit without the
feature. In order to find the commit introducing the bug in my example
a new state is needed, which would make 'git bisect' a bit more
complicated than the user 'most of the time' probably needs. Or do you
think, it would make sense to ask the user for this state (if e.g 'git
bisect' would be started with a new parameter)?

Re: new git-diff switch to eliminate leading "+" and "-" characters

[Junio C Hamano]

"Vanderhoof, Tzadik"  writes:

>> From: Duy Nguyen [mailto:pclo...@gmail.com]
>> 
>> I face this "problem" every day, but most editors nowadays have block-
>> based editing that would allow you to remove a column of "+/-"
>> easily. At least it has not bothered me to think of improving it.
>
> Would a patch be welcome?

I am not enthused for at least two reasons.  

The weaker one is "it would likely to introduce a lot of noise in
the code for a feature of dubious merit".  

The other is a bit more serious.  Cutting and pasting has been a
source of lost or mangled whitespaces.  Tabs get expanded, a wrapped
long single line turns into two lines, an originally indented line
auto-indented when inserted to the receiving editor, etc., etc.,
depending on the pager that the output was passed through for the
terminal, the terminal program itself and the editor.  The "feature"
will encourage cut-and-paste, and I personally would be reluctant to
add things that encourage bad practice to the users.

As Duy said, saving the "diff" output to another file, opening in an
editor that output file and the file the patch targets to modify,
and transferring the lines while dropping unnecessray parts (i.e.
unwanted context lines and preimage lines, and possibly undesired
postimage lines, and also the leading SP/+/- designators) has no
such downside.  It obviously has an added benefit that it makes it
less likely for people to cut and paste a line and then become
unsure if they really cut from the green line or they by mistake
also pasted an adjacent red line.

So, I'd say the answer is "probably not".

[PATCH v2] l10n: de.po: translate 241 messages

[Ralf Thielow]

Translate 241 messages came from git.pot update in 673bfad09
(l10n: git.pot: v2.12.0 round 1 (239 new, 15 removed)) and a4d94835a
(l10n: git.pot: v2.12.0 round 2 (2 new)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 750 ++-
 1 file changed, 409 insertions(+), 341 deletions(-)

diff --git a/po/de.po b/po/de.po
index 2326da1fd..e9c86f548 100644
--- a/po/de.po
+++ b/po/de.po
@@ -913,20 +913,20 @@ msgstr ""
 
 #: bisect.c:742
 #, c-format
 msgid ""
 "The merge base %s is %s.\n"
 "This means the first '%s' commit is between %s and [%s].\n"
 msgstr ""
 "Die Merge-Basis %s ist %s.\n"
-"Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s]\n"
+"Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s].\n"
 
 #: bisect.c:750
-#, fuzzy, c-format
+#, c-format
 msgid ""
 "Some %s revs are not ancestors of the %s rev.\n"
 "git bisect cannot work properly in this case.\n"
 "Maybe you mistook %s and %s revs?\n"
 msgstr ""
 "Manche %s Commits sind keine Vorgänger des %s Commits.\n"
 "git bisect kann in diesem Fall nicht richtig arbeiten.\n"
 "Vielleicht verwechselten Sie %s und %s Commits?\n"
@@ -1343,19 +1343,19 @@ msgid "bad zlib compression level %d"
 msgstr "ungültiger zlib Komprimierungsgrad %d"
 
 #: config.c:993
 #, c-format
 msgid "invalid mode for object creation: %s"
 msgstr "Ungültiger Modus für Objekterstellung: %s"
 
 #: config.c:1149
-#, fuzzy, c-format
+#, c-format
 msgid "bad pack compression level %d"
-msgstr "Komprimierungsgrad für Paketierung"
+msgstr "ungültiger Komprimierungsgrad (%d) für Paketierung"
 
 #: config.c:1339
 msgid "unable to parse command-line config"
 msgstr ""
 "Konnte die über die Befehlszeile angegebene Konfiguration nicht parsen."
 
 #: config.c:1389
 msgid "unknown error occurred while reading the configuration files"
@@ -1375,19 +1375,19 @@ msgid "bad config variable '%s' in file '%s' at line %d"
 msgstr "ungültige Konfigurationsvariable '%s' in Datei '%s' bei Zeile %d"
 
 #: config.c:1804
 #, c-format
 msgid "%s has multiple values"
 msgstr "%s hat mehrere Werte"
 
 #: config.c:2225 config.c:2450
-#, fuzzy, c-format
+#, c-format
 msgid "fstat on %s failed"
-msgstr "\"stash\" fehlgeschlagen"
+msgstr "fstat auf %s fehlgeschlagen"
 
 #: config.c:2343
 #, c-format
 msgid "could not set '%s' to '%s'"
 msgstr "Konnte '%s' nicht zu '%s' setzen."
 
 #: config.c:2345
 #, c-format
@@ -1616,19 +1616,19 @@ msgstr "Fehler beim Sammeln von Namen und Informationen 
zum Kernel"
 
 #: dir.c:1981
 msgid "Untracked cache is disabled on this system or location."
 msgstr ""
 "Cache für unversionierte Dateien ist auf diesem System oder\n"
 "für dieses Verzeichnis deaktiviert."
 
 #: dir.c:2759
-#, fuzzy, c-format
+#, c-format
 msgid "could not migrate git directory from '%s' to '%s'"
-msgstr "Konnte Verzeichnis '%s' nicht erstellen."
+msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' migrieren."
 
 #: fetch-pack.c:213
 msgid "git fetch-pack: expected shallow list"
 msgstr "git fetch-pack: erwartete shallow-Liste"
 
 #: fetch-pack.c:225
 msgid "git fetch-pack: expected ACK/NAK, got EOF"
 msgstr "git fetch-pack: ACK/NAK erwartet, EOF bekommen"
@@ -1803,17 +1803,17 @@ msgstr "konnte temporäre Datei nicht erstellen"
 #: gpg-interface.c:217
 #, c-format
 msgid "failed writing detached signature to '%s'"
 msgstr "Fehler beim Schreiben der losgelösten Signatur nach '%s'"
 
 #: graph.c:96
 #, c-format
 msgid "ignore invalid color '%.*s' in log.graphColors"
-msgstr ""
+msgstr "Ignoriere ungültige Farbe '%.*s' in log.graphColors"
 
 #: grep.c:1794
 #, c-format
 msgid "'%s': unable to read %s"
 msgstr "'%s': konnte %s nicht lesen"
 
 #: grep.c:1811 builtin/clone.c:381 builtin/diff.c:81 builtin/rm.c:133
 #, c-format
@@ -2320,17 +2320,17 @@ msgstr "%s: 'literal' und 'glob' sind inkompatibel"
 #: pathspec.c:363
 #, c-format
 msgid "%s: '%s' is outside repository"
 msgstr "%s: '%s' liegt außerhalb des Repositories"
 
 #: pathspec.c:451
 #, c-format
 msgid "'%s' (mnemonic: '%c')"
-msgstr ""
+msgstr "'%s' (Kürzel: '%c')"
 
 #: pathspec.c:461
 #, c-format
 msgid "%s: pathspec magic not supported by this command: %s"
 msgstr ""
 "%s: Pfadspezifikationsangabe wird von diesem Befehl nicht unterstützt: %s"
 
 #: pathspec.c:511
@@ -2418,19 +2418,19 @@ msgid "%%(body) does not take arguments"
 msgstr "%%(body) akzeptiert keine Argumente"
 
 #: ref-filter.c:85
 #, c-format
 msgid "%%(subject) does not take arguments"
 msgstr "%%(subject) akzeptiert keine Argumente"
 
 #: ref-filter.c:92
-#, fuzzy, c-format
+#, c-format
 msgid "%%(trailers) does not take arguments"
-msgstr "%%(body) akzeptiert keine Argumente"
+msgstr "%%(trailers) akzeptiert keine Argumente"
 
 #: ref-filter.c:111
 #, c-format
 msgid "positive value expected contents:lines=%s"
 msgstr "Positiver Wert erwartet contents:lines=%s"
 
 #: ref-filter.c:113
 #, c-format
@@ -2613,32 +2613,30 @@ msgstr "  (benutzen Sie \"git branch 

Re: [PATCH] l10n: de.po: translate 241 messages

[Ralf Thielow]

Hi Phillip,

thanks for review!

Am 17. Februar 2017 um 22:56 schrieb Phillip Sz :
>>  #: remote.c:2092
>>  #, c-format
>>  msgid "Your branch is ahead of '%s' by %d commit.\n"
>>  msgid_plural "Your branch is ahead of '%s' by %d commits.\n"
>> -msgstr[0] "Ihr Branch ist vor '%s' um %d Commit.\n"
>> -msgstr[1] "Ihr Branch ist vor '%s' um %d Commits.\n"
>> +msgstr[0] "Ihr Branch ist %2$d Commit vor '%1$s'.\n"
>> +msgstr[1] "Ihr Branch ist %2$d Commits vor '%1$s'.\n"
>>
>
> Does this "%2$d" works and why not use '%s'?
>

With this syntax you can reorder the format specifiers.  In the orignal
message, the branch name comes first and then the number of commits.
Despite "%2$d" is the first specifier in the message, it tells that it is the
second in the format arguments and therefore gets replaced with the
number of commits instead of the branch name.

>>  #: sequencer.c:840
>> -#, fuzzy
>>  msgid "could not read HEAD's commit message"
>> -msgstr "Konnte Commit-Beschreibung nicht lesen: %s"
>> +msgstr "Konnte Commit-Beschreibung von HEAD nicht lesen"
>>
>
>>  #: sequencer.c:846
>> -#, fuzzy, c-format
>> +#, c-format
>>  msgid "cannot write '%s'"
>> -msgstr "kann '%s' nicht erstellen"
>> +msgstr "kann '%s' nicht schreiben"
>
> I think we should either use "kann" or "konnte".
> We have used both and maybe we should unify it? What do you think?
>

Sure.  It's a good idea to unify the translation.  I'd prefer "kann".

>>  #: sequencer.c:1341
>> -#, fuzzy
>>  msgid "please fix this using 'git rebase --edit-todo'."
>>  msgstr "Bitte beheben Sie das, indem Sie 'git rebase --edit-todo' 
>> ausführen."
>
> Maybe: "Bitte beheben Sie dieses, indem Sie 'git rebase --edit-todo'
> ausführen."
>
>>  #: git-add--interactive.perl:1074
>>  #, perl-format
>>  msgid ""
>>  "---\n"
>>  "To remove '%s' lines, make them ' ' lines (context).\n"
>>  "To remove '%s' lines, delete them.\n"
>>  "Lines starting with %s will be removed.\n"
>>  msgstr ""
>> +"---\n"
>> +"Um '%s' Zeilen zu entfernen, machen Sie aus diesen ' ' Zeilen (Kontext).\n"
>> +"Um '%s' Zeilen zu entferenen, löschen Sie diese.\n"
>> +"Zeilen, die mit %s beginnen, werden entfernt.\n"
>>
>
> "Um '%s' Zeilen zu entfernen, löschen Sie diese.\n"
>
> Anyway thanks a lot for your awesome work!
>
> Phillip

Ralf

Re: Git bisect does not find commit introducing the bug

[Johannes Sixt]

Am 19.02.2017 um 12:32 schrieb Alex Hoffman:

The assumption that there is no transition from bad to good in the
graph did not hold in my example and it does not hold when a feature
was recently introduced and gets broken relative shortly afterwards.


Then you must adjust your definition of "good": All commits that do not 
have the feature, yet, are "good": since they do not have the feature in 
the first place, they cannot have the breakage that you found in the 
feature.


That is exactly the situation in your original example! But you 
constructed the condition of goodness in such a simplistic way 
(depending on the presence of a string), that it was impossible to 
distinguish between "does not have the feature at all" and "has the 
feature, but it is broken".


-- Hannes

Re: Git bisect does not find commit introducing the bug

[Christian Couder]

On Sun, Feb 19, 2017 at 12:32 PM, Alex Hoffman  wrote:
>> At the end of the git-bisect man page (in the SEE ALSO section) there
>> is a link to 
>> https://github.com/git/git/blob/master/Documentation/git-bisect-lk2009.txt
>> which has a lot of details about how bisect works.
>
> Thanks for pointing out the SEE ALSO section. I think it makes sense
> to include/describe the entire algorithm in the man page itself,
> although I am not sure whether the graphs would be always correctly
> visually represented in the man page format.

It would possibly be very long to describe the entire algorithm, as it
can be quite complex in some cases and it is difficult to understand
without graphs. Maybe we could describe it, or some parts of it, in a
separate document and provide links at different places in the man
page.
Anyway feel free to send patches.

>> The goal is to find the first bad commit, which is a commit that has
>> only good parents.
>
> OK, bisect's mission is more exact than I thought, which is good. M

Good that you seem to agree with this goal.

>> As o1 is an ancestor of G, then o1 is considered good by the bisect 
>> algorithm.
>> If it was bad, it would means that there is a transition from bad to
>> good between o1 and G.
>> But when a good commit is an ancestor of the bad commit, git bisect
>> makes the assumption that there is no transition from bad to good in
>> the graph.
>
> The assumption that there is no transition from bad to good in the
> graph did not hold in my example and it does not hold when a feature
> was recently introduced and gets broken relative shortly afterwards.
> But I consider it is easy to change the algorithm not to assume, but
> rather to check it.

I don't think the default algorithm will change soon, but there have
been discussions for a long time about adding options to use different
algorithms.

For example people have been discussing a "--first-parent" option for
many years as well as recently. It would bisect only along the first
parents of the involved commits, and it could help find the merge
commit that introduced a bug in the mainline.

>> git bisect makes some assumptions that are true most of the time, so
>> in practice it works well most of the time.
>
> Whatever the definition of "most of the time" everyone might have, I
> think there is room for improvement.

So feel free to send patches that would implement an option with the
improvements you want.

> Below I am trying to make a small
> change to the current algorithm in order to deal with the assumption
> that sometimes does not hold (e.g in my example), by explicitly
> validating the check.
>
>> --o1--o2--o3--G--X1
>> \\
>>  x1--x2--x3--x4--X2--B--
>>   \  /
>>y1--y2--y3
>
> Step 1a. (Unchanged) keep only the commits that:
>
> a) are ancestor of the "bad" commit (including the "bad" commit 
> itself),
> b) are not ancestor of a "good" commit (excluding the "good" commits).
>
> The following graph results:
>   x1--x2--x3--x4--X2--B--
>\  /
> y1--y2--y3

I would say that the above graph is missing X1.

> Step 1b. (New) Mark all root commits of the resulting graph (i.e
> commits without parents) as unconfirmed (unconfirmed=node that has
> only bad parents). Remove all root commits that user already confirmed
> (e.g if user already marked its parent as good right before starting
> bisect run). For every unconfirmed root commit check if it has any
> good parents. In the example above check whether x1 has good parents.

I think I understand the above...

>  If the current root element has any parents and none of them is
> good, we can delete all paths from it until to the next commit that
> has a parent in the ancestors of GOOD. In the example above to delete
> the path x1-x3 and x1-y3. Also add new resulting root commits to the
> list of unconfirmed commits (commit x4).
>  Otherwise mark it as confirmed.

... but I don't understand the logic of the above. If the root element
has a bad parent, then it means that the "first bad commit" is either
the bad parent or one of its ancestors, so it is not logical to delete
it. In your example if x1 has one bad parent, this bad parent and its
ancestors should be included in the search of the first bad commit.

Otherwise the goal is not any more to find the first bad commit.

PS: I saw that you have just sent another version of the algorithm,
but I don't want to take a look at it right now. Anyway I am keeping
my above comments as they might still be useful.

> Step2. Continue the existing algorithm.
>
>
> If this improvement works (i.e you do not find any bugs in it and it
> is feasible to implement, which seems to me)

As you describe it, I don't think it is compatible with the goal of
finding the first bad commit.
Also there are many things that are feasible to implement, but it
doesn't mean that someone will soon make the effort to implement them
in a 

Re: Git bisect does not find commit introducing the bug

[Alex Hoffman]

Below is a correction of the first proposed algorithm:

>--o1--o2--o3--G --X1
>\\
> x1--x2--x3--x4--X2--B--
>  \  /
>   y1--y2--y3
>
Step 1a. (Unchanged) keep only the commits that:

a) are ancestor of the "bad" commit (including the "bad" commit itself),
b) are not ancestor of a "good" commit (excluding the "good" commits).

The following graph results:
  x1--x2--x3--x4--X2--B--
   \  /
y1--y2--y3

Step 1b. (New) Mark all commits of the resulting graph as unconfirmed
(unconfirmed=node without good ancestors).
Mark as confirmed all descendants of commits that user marked
explicitly as good (e.g if user already marked its parent/grand
parent/... as good right before starting bisect run).
Step 1c. From all unconfirmed root commits build a set SET_GOOD of
those with any good parents in the original graph (root commit =
commit without parents in the resulting graph) and one SET_BAD for
commits with only BAD parents. To build a set means to ask explicitly
the user (or the command passed in git bisect run) whether any of its
parents is good. In the example above find out whether x1 has any good
parents or no parent at all and if so add it to SET_GOOD, otherwise to
SET_BAD.
Mark as confirmed each commit in SET_GOOD and all its descendants.
For every commit in SET_BAD delete all paths from it until to the next
confirmed commit. In the example above if x1 is in SET_BAD delete the
path x1-x3 and x1-y3. If any new root commits result (commit x4), redo
step 1c with them.


Step2. Continue the existing algorithm.

If this improvement works (i.e you do not find any bugs in it and it
is feasible to implement, which seems to me) the following would be
its advantages:
1. An assumption less, as we explicitly check the assumption.
2. It might be quicker, because we delete parts of graph that cannot
contain transitions.
3. It returns more exact results.

VG

Re: Git bisect does not find commit introducing the bug

[Alex Hoffman]

> At the end of the git-bisect man page (in the SEE ALSO section) there
> is a link to 
> https://github.com/git/git/blob/master/Documentation/git-bisect-lk2009.txt
> which has a lot of details about how bisect works.
>

Thanks for pointing out the SEE ALSO section. I think it makes sense
to include/describe the entire algorithm in the man page itself,
although I am not sure whether the graphs would be always correctly
visually represented in the man page format.

> The goal is to find the first bad commit, which is a commit that has
> only good parents.

OK, bisect's mission is more exact than I thought, which is good. M

> As o1 is an ancestor of G, then o1 is considered good by the bisect algorithm.
> If it was bad, it would means that there is a transition from bad to
> good between o1 and G.
> But when a good commit is an ancestor of the bad commit, git bisect
> makes the assumption that there is no transition from bad to good in
> the graph.

The assumption that there is no transition from bad to good in the
graph did not hold in my example and it does not hold when a feature
was recently introduced and gets broken relative shortly afterwards.
But I consider it is easy to change the algorithm not to assume, but
rather to check it.

> git bisect makes some assumptions that are true most of the time, so
> in practice it works well most of the time.

Whatever the definition of "most of the time" everyone might have, I
think there is room for improvement. Below I am trying to make a small
change to the current algorithm in order to deal with the assumption
that sometimes does not hold (e.g in my example), by explicitly
validating the check.

> --o1--o2--o3--G--X1
> \\
>  x1--x2--x3--x4--X2--B--
>   \  /
>y1--y2--y3

Step 1a. (Unchanged) keep only the commits that:

a) are ancestor of the "bad" commit (including the "bad" commit itself),
b) are not ancestor of a "good" commit (excluding the "good" commits).

The following graph results:
  x1--x2--x3--x4--X2--B--
   \  /
y1--y2--y3

Step 1b. (New) Mark all root commits of the resulting graph (i.e
commits without parents) as unconfirmed (unconfirmed=node that has
only bad parents). Remove all root commits that user already confirmed
(e.g if user already marked its parent as good right before starting
bisect run). For every unconfirmed root commit check if it has any
good parents. In the example above check whether x1 has good parents.
 If the current root element has any parents and none of them is
good, we can delete all paths from it until to the next commit that
has a parent in the ancestors of GOOD. In the example above to delete
the path x1-x3 and x1-y3. Also add new resulting root commits to the
list of unconfirmed commits (commit x4).
 Otherwise mark it as confirmed.

Step2. Continue the existing algorithm.


If this improvement works (i.e you do not find any bugs in it and it
is feasible to implement, which seems to me) the following would be
its advantages:
1. An assumption less, as we explicitly check the assumption.
2. It might be quicker, because we delete parts of graph that cannot
contain transitions.
3. It returns more exact results.

VG

[PATCH v6 6/6] stash: allow pathspecs in the no verb form

[Thomas Gummerer]

Now that stash_push is used in the no verb form of stash, allow
specifying the command line for this form as well.  Always use -- to
disambiguate pathspecs from other non-option arguments.

Also make git stash -p an alias for git stash push -p.  This allows
users to use git stash -p .

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt | 11 +++
 git-stash.sh|  3 +++
 t/t3903-stash.sh| 15 +++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 3f7fa88ddc..369bfae33d 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -53,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] 
[-u|--include-untracked] [-a|--all] [-q
 
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them.  The  part is optional and gives
-   the description along with the stashed state.  For quickly making
-   a snapshot, you can omit _both_ "save" and , but giving
-   only  does not trigger this action to prevent a misspelled
-   subcommand from making an unwanted stash.
+   the description along with the stashed state.
++
+For quickly making a snapshot, you can omit "push".  In this mode,
+non-option arguments are not allowed to prevent a misspelled
+subcommand from making an unwanted stash.  The two exceptions to this
+are `stash -p` which acts as alias for `stash push -p` and pathspecs,
+which are allowed after a double hyphen `--` for disambiguation.
 +
 When pathspec is given to 'git stash push', the new stash records the
 modified states only for the files that match the pathspec.  The index
diff --git a/git-stash.sh b/git-stash.sh
index 1e55cd5fdd..18aba1346f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -666,12 +666,15 @@ apply_to_branch () {
}
 }
 
+test "$1" = "-p" && set "push" "$@"
+
 PARSE_CACHE='--not-parsed'
 # The default command is "push" if nothing but options are given
 seen_non_option=
 for opt
 do
case "$opt" in
+   --) break ;;
-*) ;;
*) seen_non_option=t; break ;;
esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 7f90a247b4..c0ae41e724 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -872,4 +872,19 @@ test_expect_success 'untracked files are left in place 
when -u is not given' '
test_path_is_file untracked
 '
 
+test_expect_success 'stash without verb with pathspec' '
+   >"foo bar" &&
+   >foo &&
+   >bar &&
+   git add foo* &&
+   git stash -- "foo b*" &&
+   test_path_is_missing "foo bar" &&
+   test_path_is_file foo &&
+   test_path_is_file bar &&
+   git stash pop &&
+   test_path_is_file "foo bar" &&
+   test_path_is_file foo &&
+   test_path_is_file bar
+'
+
 test_done
-- 
2.12.0.rc2.399.g0ca89a282

[PATCH v6 1/6] stash: introduce push verb

[Thomas Gummerer]

Introduce a new git stash push verb in addition to git stash save.  The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is given as an argument
to the -m option.

This allows us to have pathspecs at the end of the command line
arguments like other Git commands do, so that the user can say which
subset of paths to stash (and leave others behind).

Helped-by: Junio C Hamano 
Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt |  3 +++
 git-stash.sh| 46 ++---
 t/t3903-stash.sh|  9 +
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0f602ea0c8 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,6 +15,8 @@ SYNOPSIS
 'git stash' branch  []
 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] []]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+[-u|--include-untracked] [-a|--all] [-m|--message ]]
 'git stash' clear
 'git stash' create []
 'git stash' store [-m|--message ] [-q|--quiet] 
@@ -46,6 +48,7 @@ OPTIONS
 ---
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] []::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] [-m|--message ]::
 
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them.  The  part is optional and gives
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8365ebba2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,6 +9,8 @@ USAGE="list []
or: $dashless branch  []
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
   [-u|--include-untracked] [-a|--all] []]
+   or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m ]
or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -189,10 +191,11 @@ store_stash () {
return $ret
 }
 
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+   stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +219,11 @@ save_stash () {
-a|--all)
untracked=all
;;
+   -m|--message)
+   shift
+   test -z ${1+x} && usage
+   stash_msg=$1
+   ;;
--help)
show_help
;;
@@ -251,8 +259,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or 
--all at the same time")"
fi
 
-   stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +297,36 @@ save_stash () {
fi
 }
 
+save_stash () {
+   push_options=
+   while test $# != 0
+   do
+   case "$1" in
+   --)
+   shift
+   break
+   ;;
+   -*)
+   # pass all options through to push_stash
+   push_options="$push_options $1"
+   ;;
+   *)
+   break
+   ;;
+   esac
+   shift
+   done
+
+   stash_msg="$*"
+
+   if test -z "$stash_msg"
+   then
+   push_stash $push_options
+   else
+   push_stash $push_options -m "$stash_msg"
+   fi
+}
+
 have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
 }
@@ -617,6 +653,10 @@ save)
shift
save_stash "$@"
;;
+push)
+   shift
+   push_stash "$@"
+   ;;
 apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..3577115807 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial 
renames' '
test_path_is_missing file
 '
 
+test_expect_success 'push -m shows right message' '
+   >foo &&
+   git add foo &&
+   git stash push -m "test message" &&
+   echo "stash@{0}: On master: test message" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.12.0.rc2.399.g0ca89a282

[PATCH v6 3/6] stash: refactor stash_create

[Thomas Gummerer]

Refactor the internal stash_create function to use a -m flag for
specifying the message and -u flag to indicate whether untracked files
should be added to the stash.

This makes it easier to pass a pathspec argument to stash_create in the
next patch.

The user interface for git stash create stays the same.

Signed-off-by: Thomas Gummerer 
---
 git-stash.sh | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 8365ebba2a..ef5d1b45be 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,22 @@ clear_stash () {
 }
 
 create_stash () {
-   stash_msg="$1"
-   untracked="$2"
+   stash_msg=
+   untracked=
+   while test $# != 0
+   do
+   case "$1" in
+   -m|--message)
+   shift
+   stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
+   ;;
+   -u|--include-untracked)
+   shift
+   untracked=${1?"BUG: create_stash () -u requires an 
argument"}
+   ;;
+   esac
+   shift
+   done
 
git update-index -q --refresh
if no_changes
@@ -268,7 +282,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
 
-   create_stash "$stash_msg" $untracked
+   create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state 
\$stash_msg")"
@@ -667,7 +681,7 @@ clear)
;;
 create)
shift
-   create_stash "$*" && echo "$w_commit"
+   create_stash -m "$*" && echo "$w_commit"
;;
 store)
shift
-- 
2.12.0.rc2.399.g0ca89a282

[PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec

[Thomas Gummerer]

While working on a repository, it's often helpful to stash the changes
of a single or multiple files, and leave others alone.  Unfortunately
git currently offers no such option.  git stash -p can be used to work
around this, but it's often impractical when there are a lot of changes
over multiple files.

Allow 'git stash push' to take pathspec to specify which paths to stash.

Helped-by: Junio C Hamano 
Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt|  9 -
 git-stash.sh   | 48 +++--
 t/t3903-stash.sh   | 72 ++
 t/t3905-stash-include-untracked.sh | 26 ++
 4 files changed, 143 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0f602ea0c8..b7db939a06 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 [-u|--include-untracked] [-a|--all] []]
 'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] [-m|--message ]]
+[--] [...]
 'git stash' clear
 'git stash' create []
 'git stash' store [-m|--message ] [-q|--quiet] 
@@ -48,7 +49,7 @@ OPTIONS
 ---
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] []::
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] [-m|--message ]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] [-m|--message ] [--] [...]::
 
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them.  The  part is optional and gives
@@ -57,6 +58,12 @@ push [-p|--patch] [-k|--[no-]keep-index] 
[-u|--include-untracked] [-a|--all] [-q
only  does not trigger this action to prevent a misspelled
subcommand from making an unwanted stash.
 +
+When pathspec is given to 'git stash push', the new stash records the
+modified states only for the files that match the pathspec.  The index
+entries and working tree files are then rolled back to the state in
+HEAD only for these files, too, leaving files that do not match the
+pathspec intact.
++
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
 +
diff --git a/git-stash.sh b/git-stash.sh
index ef5d1b45be..b55983d1fd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -11,6 +11,7 @@ USAGE="list []
   [-u|--include-untracked] [-a|--all] []]
or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
  [-u|--include-untracked] [-a|--all] [-m ]
+ [-- ...]
or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -35,15 +36,15 @@ else
 fi
 
 no_changes () {
-   git diff-index --quiet --cached HEAD --ignore-submodules -- &&
-   git diff-files --quiet --ignore-submodules &&
+   git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
+   git diff-files --quiet --ignore-submodules -- "$@" &&
(test -z "$untracked" || test -z "$(untracked_files)")
 }
 
 untracked_files () {
excl_opt=--exclude-standard
test "$untracked" = "all" && excl_opt=
-   git ls-files -o -z $excl_opt
+   git ls-files -o -z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -71,12 +72,16 @@ create_stash () {
shift
untracked=${1?"BUG: create_stash () -u requires an 
argument"}
;;
+   --)
+   shift
+   break
+   ;;
esac
shift
done
 
git update-index -q --refresh
-   if no_changes
+   if no_changes "$@"
then
exit 0
fi
@@ -108,7 +113,7 @@ create_stash () {
# Untracked files are stored by themselves in a parentless 
commit, for
# ease of unpacking later.
u_commit=$(
-   untracked_files | (
+   untracked_files "$@" | (
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
rm -f "$TMPindex" &&
@@ -131,7 +136,7 @@ create_stash () {
git read-tree --index-output="$TMPindex" -m $i_tree &&
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
-   git diff-index --name-only -z HEAD -- 
>"$TMP-stagenames" &&
+   git diff-index --name-only -z HEAD -- "$@" 
>"$TMP-stagenames" &&
git update-index -z --add --remove --stdin 
<"$TMP-stagenames" &&
git write-tree &&
rm -f "$TMPindex"
@@ -145,7 +150,7 @@ create_stash () {
 
   

[PATCH v6 5/6] stash: use stash_push for no verb form

[Thomas Gummerer]

Now that we have stash_push, which accepts pathspec arguments, use
it instead of stash_save in git stash without any additional verbs.

Previously we allowed git stash -- -message, which is no longer allowed
after this patch.  Messages starting with a hyphen was allowed since
3c2eb80f, ("stash: simplify defaulting to "save" and reject unknown
options").  However it was never the intent to allow that, but rather it
was allowed accidentally.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt |  8 
 git-stash.sh| 16 
 t/t3903-stash.sh|  4 +---
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index b7db939a06..3f7fa88ddc 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,11 +13,11 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] []
 'git stash' ( pop | apply ) [--index] [-q|--quiet] []
 'git stash' branch  []
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
-[-u|--include-untracked] [-a|--all] []]
-'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+[-u|--include-untracked] [-a|--all] []
+'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] [-m|--message ]]
-[--] [...]
+[--] [...]]
 'git stash' clear
 'git stash' create []
 'git stash' store [-m|--message ] [-q|--quiet] 
diff --git a/git-stash.sh b/git-stash.sh
index b55983d1fd..1e55cd5fdd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,11 +7,11 @@ USAGE="list []
or: $dashless drop [-q|--quiet] []
or: $dashless ( pop | apply ) [--index] [-q|--quiet] []
or: $dashless branch  []
-   or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-  [-u|--include-untracked] [-a|--all] []]
-   or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m ]
- [-- ...]
+   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] []
+   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+  [-u|--include-untracked] [-a|--all] [-m ]
+  [-- ...]]
or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -667,7 +667,7 @@ apply_to_branch () {
 }
 
 PARSE_CACHE='--not-parsed'
-# The default command is "save" if nothing but options are given
+# The default command is "push" if nothing but options are given
 seen_non_option=
 for opt
 do
@@ -677,7 +677,7 @@ do
esac
 done
 
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
 
 # Main command set
 case "$1" in
@@ -728,7 +728,7 @@ branch)
 *)
case $# in
0)
-   save_stash &&
+   push_stash &&
say "$(gettext "(To restore them type \"git stash apply\")")"
;;
*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 4fb800eec8..7f90a247b4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -274,9 +274,7 @@ test_expect_success 'stash --invalid-option' '
git add file2 &&
test_must_fail git stash --invalid-option &&
test_must_fail git stash save --invalid-option &&
-   test bar5,bar6 = $(cat file),$(cat file2) &&
-   git stash -- -message-starting-with-dash &&
-   test bar,bar2 = $(cat file),$(cat file2)
+   test bar5,bar6 = $(cat file),$(cat file2)
 '
 
 test_expect_success 'stash an added file' '
-- 
2.12.0.rc2.399.g0ca89a282

[PATCH v6 2/6] stash: add test for the create command line arguments

[Thomas Gummerer]

Currently there is no test showing the expected behaviour of git stash
create's command line arguments.  Add a test for that to show the
current expected behaviour and to make sure future refactorings don't
break those expectations.

Signed-off-by: Thomas Gummerer 
---
 t/t3903-stash.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3577115807..ffe3549ea5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
 '
 
+test_expect_success 'create stores correct message' '
+   >foo &&
+   git add foo &&
+   STASH_ID=$(git stash create "create test message") &&
+   echo "On master: create test message" >expect &&
+   git show --pretty=%s -s ${STASH_ID} >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'create with multiple arguments for the message' '
+   >foo &&
+   git add foo &&
+   STASH_ID=$(git stash create test untracked) &&
+   echo "On master: test untracked" >expect &&
+   git show --pretty=%s -s ${STASH_ID} >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.12.0.rc2.399.g0ca89a282

[PATCH v6 0/6] stash: support pathspec argument

[Thomas Gummerer]

Thanks Junio and Peff for comments on the last round.

Changes since then:

- removed mention of the "new form" of git stash create from the
  Documentation.
- Changed documentation for git stash without a verb, mentioning
  stash -p now being an alias for git stash push -p and that -- can be
  used as disambiguation for for pathspecs
- Fixed ${1-...} which should have been ${1?...}
- Removed unused new_style variable from create_stash, which was a
  leftover from perious rounds.

Interdiff below:

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 97194576ef..369bfae33d 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -20,8 +20,6 @@ SYNOPSIS
 [--] [...]]
 'git stash' clear
 'git stash' create []
-'git stash' create [-m ] [-u|--include-untracked ]
-[-- ...]
 'git stash' store [-m|--message ] [-q|--quiet] 
 
 DESCRIPTION
@@ -55,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] 
[-u|--include-untracked] [-a|--all] [-q
 
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them.  The  part is optional and gives
-   the description along with the stashed state.  For quickly making
-   a snapshot, you can omit _both_ "save" and , but giving
-   only  does not trigger this action to prevent a misspelled
-   subcommand from making an unwanted stash.
+   the description along with the stashed state.
++
+For quickly making a snapshot, you can omit "push".  In this mode,
+non-option arguments are not allowed to prevent a misspelled
+subcommand from making an unwanted stash.  The two exceptions to this
+are `stash -p` which acts as alias for `stash push -p` and pathspecs,
+which are allowed after a double hyphen `--` for disambiguation.
 +
 When pathspec is given to 'git stash push', the new stash records the
 modified states only for the files that match the pathspec.  The index
diff --git a/git-stash.sh b/git-stash.sh
index 1446fbe2e8..18aba1346f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -61,17 +61,16 @@ clear_stash () {
 create_stash () {
stash_msg=
untracked=
-   new_style=
while test $# != 0
do
case "$1" in
-m|--message)
shift
-   stash_msg=${1-"BUG: create_stash () -m requires an 
argument"}
+   stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
;;
-u|--include-untracked)
shift
-   untracked=${1-"BUG: create_stash () -u requires an 
argument"}
+   untracked=${1?"BUG: create_stash () -u requires an 
argument"}
;;
--)
shift

Thomas Gummerer (6):
  stash: introduce push verb
  stash: add test for the create command line arguments
  stash: refactor stash_create
  stash: teach 'push' (and 'create_stash') to honor pathspec
  stash: use stash_push for no verb form
  stash: allow pathspecs in the no verb form

 Documentation/git-stash.txt|  27 ++--
 git-stash.sh   | 127 ++---
 t/t3903-stash.sh   | 118 +-
 t/t3905-stash-include-untracked.sh |  26 
 4 files changed, 267 insertions(+), 31 deletions(-)

-- 
2.12.0.rc2.399.g0ca89a282

Re: [PATCH v5 6/6] stash: allow pathspecs in the no verb form

[Thomas Gummerer]

On 02/17, Jeff King wrote:
> On Fri, Feb 17, 2017 at 10:41:41PM +, Thomas Gummerer wrote:
> 
> > Now that stash_push is used in the no verb form of stash, allow
> > specifying the command line for this form as well.  Always use -- to
> > disambiguate pathspecs from other non-option arguments.
> 
> I think that makes sense.
> 
> > Also make git stash -p an alias for git stash push -p.  This allows
> > users to use git stash -p .
> 
> And I think of all the options we discussed for handling "-p", I think
> this one makes the most sense.
> 
> It may be worth calling out in the documentation that this is how it
> works though, so people do not think that:
> 
>   git stash -k -p 
> 
> would work ("git stash -k -p" _does_ happen to work due to the old
> options-only rule, but I think we should advertise the general form as
> "-p is an alias for "push -p").

Yeah, I think adding something about this in the documentation would
be good.  I'll add a paragraph about this in the re-roll.

> -Peff

Re: [PATCH v5 3/6] stash: refactor stash_create

[Thomas Gummerer]

On 02/17, Jeff King wrote:
> On Fri, Feb 17, 2017 at 10:41:38PM +, Thomas Gummerer wrote:
> 
> > Refactor the internal stash_create function to use a -m flag for
> > specifying the message and -u flag to indicate whether untracked files
> > should be added to the stash.
> > 
> > This makes it easier to pass a pathspec argument to stash_create in the
> > next patch.
> > 
> > The user interface for git stash create stays the same.
> 
> Sounds good, but...
> 
> > diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> > index 2e9cef06e6..d93c47446a 100644
> > --- a/Documentation/git-stash.txt
> > +++ b/Documentation/git-stash.txt
> > @@ -17,6 +17,7 @@ SYNOPSIS
> >  [-u|--include-untracked] [-a|--all] []]
> >  'git stash' clear
> >  'git stash' create []
> > +'git stash' create [-m ] [-u|--include-untracked ]
> >  'git stash' store [-m|--message ] [-q|--quiet] 
> 
> Should this hunk be dropped from the manpage, then?
> 
> I think there is a similar one in the next patch that adds the
> "pathspec" argument, and should be dropped, too.

Argh yes I should have been more careful.  Thanks for catching.

> -Peff

Benefit

[Mrs Julie Leach]

You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact
( julieleac...@gmail.com ) for claims.