Re: [PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread Martin Ågren 
On 9 October 2017 at 03:30, Junio C Hamano  wrote:
> On Mon, Oct 9, 2017 at 9:51 AM, brian m. carlson
>  wrote:
>> On Sun, Oct 08, 2017 at 10:32:35AM +0100, Philip Oakley wrote:
>>> From: "Martin Ågren" 
>>> > - die(_("submodule entry '%s' (%s) is a %s, not a commit"),
>>> > - cb->path, oid_to_hex(oid), typename(type));
>>> > + die(_("submodule entry '%s' (%s) is not a commit"),
>>> > + cb->path, oid_to_hex(oid));
>>> Bikeshed,
>>> maybe:
>>> "submodule entry '%s' (%s) is not a commit. It is a %s"
>>> This puts the two parts in separate sentences?
>>
>> Languages with multiple grammatical genders are going to have problems
>> with this.  In French, "a tree" is "un arbre" (masculine), but "a tag"
>> is "une étiquette" (feminine).  We don't currently have a Spanish
>> translation, but this would break there as well.
>>
>> Splitting the article out with the type name is still problematic for
>> languages where articles vary by case, like German, since the
>> translation might be reused in another place requiring a different case.
>
> While all of the above is correct, would we really need to subject typename()
> to translation? IOW, can't we just treat 'blob', 'tree', 'commit' and
> 'tag' as-is,
> as terms of art (i.e. with a specific or precise meaning within a
> given discipline
> or field and might have a different meaning in common usage)?

In another subthread, I sort of suggested "... is of type '%s', not 'commit'".
So "commit" and "%s" would appear as the file-format-level terms that they are.
I think it looks odd but I guess it might work in Swedish, FWIW.

In this particular case, we could have three specific messages plus one default
message (which at the time shouldn't ever occur). Or we have one specific
message for the "tag"-case, which seems to have been Stefan's original
motivation, and a generic message for the other cases.

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

2017-10-08 Thread Martin Ågren 
On 6 October 2017 at 11:40, Junio C Hamano  wrote:
> Simon Ruderich  writes:
>
>> Did you consider Stefan Beller's suggestion regarding a
>> (white)list of allowed versions?
>>
>> On Mon, Sep 18, 2017 at 01:06:59PM -0700, Stefan Beller wrote:
>>> Thinking about this, how about:
>>>
>>>   If not configured, we do as we want. (i.e. Git has full control over
>>>   it's decision making process, which for now is "favor v0 over v1 as
>>>   we are experimenting with v1". This strategy may change in the future
>>>   to "prefer highest version number that both client and server can speak".)
>>>
>>>   If it is configured, "use highest configured number from the given set".
>>>
>>> ?
>>
>> It would also allow the server operator to configure only a
>> specific set of versions (to handle the "version x is
>> insecure/slow"-issue raised by Stefan Beller). The current code
>> always uses the latest protocol supported by the git binary.
>
> If we do anything less trivial than "highest supported by both" (and
> I suspect we want to in the final production version), I'd prefer
> the configuration to list versions one side supports in decreasing
> order of preference (e.g. "v3 v0 v2"), and take the earliest from
> this list that both sides know how to talk, so that we can skip
> insecure versions altogether by omitting, and we can express that we
> would rather avoid talking expensive versions unless there is no
> other version that is understood by the other side.

I think I've managed to convince myself that a blacklist would be the
most future-proof approach, simply because it cannot be overloaded with
any other meanings in the future.

If an ordering needs to be possible, that would have to go into another
config item. An ordering would open up for some interesting issues, but
at least that shouldn't be any worse because the blacklist-approach has
been taken (as opposed to a whitelist-approach).

To aid with a slow roll-out, the default blacklist could be used (start
by blacklisting v1), but after that the default list should be empty. It
should not be misused for slowly rolling out any later experimental
versions.

Letting the blacklist be different server- and client-side seems useful
for driving the experiment forwards. Post-experiment, I'm not so sure,
that just seems unnecessarily complicated.

So, here's a suggestion:

* experimental.{client,server}ProtocolV1 is "0" (don't experiment) or
  "1" (experiment).

* experimental.serverProtocolV1 has default "0". Unless early feedback
  is negative, the default is changed to "1".

* experimental.clientProtocolV1 has default "0". Switch the default to
  "1" after some time.

* Big warnings that if someone finds themselves switching to "0" they
  should get in touch.

Once we feel confident, we implement protocol.blacklist and let the
default be "". The experimental.* are simply dropped, no "aliasing" or
"transitioning". That is, we activate v0 and v1. We don't respect "0" in
a blacklist (but don't forbid it either). Once we introduce v2, sure,
but until then, some will just be tempted to blacklist v0 "to get the
modern v1" -- they will have risk, but no benefits.

Martin

Re: [PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread Junio C Hamano 
On Mon, Oct 9, 2017 at 9:51 AM, brian m. carlson
 wrote:
> On Sun, Oct 08, 2017 at 10:32:35AM +0100, Philip Oakley wrote:
>> From: "Martin Ågren" 
>> > - die(_("submodule entry '%s' (%s) is a %s, not a commit"),
>> > - cb->path, oid_to_hex(oid), typename(type));
>> > + die(_("submodule entry '%s' (%s) is not a commit"),
>> > + cb->path, oid_to_hex(oid));
>> Bikeshed,
>> maybe:
>> "submodule entry '%s' (%s) is not a commit. It is a %s"
>> This puts the two parts in separate sentences?
>
> Languages with multiple grammatical genders are going to have problems
> with this.  In French, "a tree" is "un arbre" (masculine), but "a tag"
> is "une étiquette" (feminine).  We don't currently have a Spanish
> translation, but this would break there as well.
>
> Splitting the article out with the type name is still problematic for
> languages where articles vary by case, like German, since the
> translation might be reused in another place requiring a different case.

While all of the above is correct, would we really need to subject typename()
to translation? IOW, can't we just treat 'blob', 'tree', 'commit' and
'tag' as-is,
as terms of art (i.e. with a specific or precise meaning within a
given discipline
or field and might have a different meaning in common usage)?

[PATCH v2 16/24] refs: convert reflog_expire parameter to struct object_id

2017-10-08 Thread brian m. carlson 
reflog_expire already used struct object_id internally, but it did not
take it as a parameter.  Adjust the parameter (and the callers) to pass
a pointer to struct object_id instead of a pointer to unsigned char.
Remove the temporary inserted earlier as it is no longer required.

Signed-off-by: brian m. carlson 
---
 builtin/reflog.c  | 6 +++---
 refs.c| 8 
 refs.h| 6 +++---
 refs/files-backend.c  | 9 +++--
 refs/packed-backend.c | 2 +-
 refs/refs-internal.h  | 2 +-
 6 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index cd4c4847b7..ab31a3b6aa 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -589,7 +589,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
for (i = 0; i < collected.nr; i++) {
struct collected_reflog *e = collected.e[i];
set_reflog_expiry_param(, explicit_expiry, 
e->reflog);
-   status |= reflog_expire(e->reflog, e->oid.hash, flags,
+   status |= reflog_expire(e->reflog, >oid, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -607,7 +607,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
continue;
}
set_reflog_expiry_param(, explicit_expiry, ref);
-   status |= reflog_expire(ref, oid.hash, flags,
+   status |= reflog_expire(ref, , flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -683,7 +683,7 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
cb.cmd.expire_total = 0;
}
 
-   status |= reflog_expire(ref, oid.hash, flags,
+   status |= reflog_expire(ref, , flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
diff --git a/refs.c b/refs.c
index b781acc8b1..9ddf7fcf7d 100644
--- a/refs.c
+++ b/refs.c
@@ -1972,19 +1972,19 @@ int delete_reflog(const char *refname)
 }
 
 int refs_reflog_expire(struct ref_store *refs,
-  const char *refname, const unsigned char *sha1,
+  const char *refname, const struct object_id *oid,
   unsigned int flags,
   reflog_expiry_prepare_fn prepare_fn,
   reflog_expiry_should_prune_fn should_prune_fn,
   reflog_expiry_cleanup_fn cleanup_fn,
   void *policy_cb_data)
 {
-   return refs->be->reflog_expire(refs, refname, sha1, flags,
+   return refs->be->reflog_expire(refs, refname, oid, flags,
   prepare_fn, should_prune_fn,
   cleanup_fn, policy_cb_data);
 }
 
-int reflog_expire(const char *refname, const unsigned char *sha1,
+int reflog_expire(const char *refname, const struct object_id *oid,
  unsigned int flags,
  reflog_expiry_prepare_fn prepare_fn,
  reflog_expiry_should_prune_fn should_prune_fn,
@@ -1992,7 +1992,7 @@ int reflog_expire(const char *refname, const unsigned 
char *sha1,
  void *policy_cb_data)
 {
return refs_reflog_expire(get_main_ref_store(),
- refname, sha1, flags,
+ refname, oid, flags,
  prepare_fn, should_prune_fn,
  cleanup_fn, policy_cb_data);
 }
diff --git a/refs.h b/refs.h
index 3afecd2e8a..1bc241424d 100644
--- a/refs.h
+++ b/refs.h
@@ -696,20 +696,20 @@ typedef int reflog_expiry_should_prune_fn(struct 
object_id *ooid,
 typedef void reflog_expiry_cleanup_fn(void *cb_data);
 
 /*
- * Expire reflog entries for the specified reference. sha1 is the old
+ * Expire reflog entries for the specified reference. oid is the old
  * value of the reference. flags is a combination of the constants in
  * enum expire_reflog_flags. The three function pointers are described
  * above. On success, return zero.
  */
 int refs_reflog_expire(struct ref_store *refs,
   const char *refname,
-  const unsigned char *sha1,
+  const struct object_id *oid,
   unsigned int flags,
   reflog_expiry_prepare_fn prepare_fn,
   reflog_expiry_should_prune_fn should_prune_fn,

[PATCH v2 05/24] refs: update ref transactions to use struct object_id

2017-10-08 Thread brian m. carlson 
Update the ref transaction code to use struct object_id.  Remove one
NULL pointer check which was previously inserted around a dereference;
since we now pass a pointer to struct object_id directly through, the
code we're calling handles this for us.

Signed-off-by: brian m. carlson 
---
 branch.c   |  2 +-
 builtin/clone.c|  2 +-
 builtin/commit.c   |  4 ++--
 builtin/fetch.c|  4 ++--
 builtin/receive-pack.c |  4 ++--
 builtin/replace.c  |  2 +-
 builtin/tag.c  |  2 +-
 builtin/update-ref.c   |  8 
 fast-import.c  |  4 ++--
 refs.c | 44 +---
 refs.h | 24 
 refs/files-backend.c   | 12 ++--
 refs/refs-internal.h   |  4 ++--
 sequencer.c|  2 +-
 walker.c   |  2 +-
 15 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/branch.c b/branch.c
index 4377ce2fb1..45029ea142 100644
--- a/branch.c
+++ b/branch.c
@@ -305,7 +305,7 @@ void create_branch(const char *name, const char *start_name,
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_update(transaction, ref.buf,
-  oid.hash, forcing ? NULL : null_sha1,
+  , forcing ? NULL : _oid,
   0, msg, ) ||
ref_transaction_commit(transaction, ))
die("%s", err.buf);
diff --git a/builtin/clone.c b/builtin/clone.c
index 4135621aa3..665a0e2673 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -588,7 +588,7 @@ static void write_remote_refs(const struct ref *local_refs)
for (r = local_refs; r; r = r->next) {
if (!r->peer_ref)
continue;
-   if (ref_transaction_create(t, r->peer_ref->name, 
r->old_oid.hash,
+   if (ref_transaction_create(t, r->peer_ref->name, >old_oid,
   0, NULL, ))
die("%s", err.buf);
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 0f8ddb6866..d5fbf404f4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1785,9 +1785,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
 
transaction = ref_transaction_begin();
if (!transaction ||
-   ref_transaction_update(transaction, "HEAD", oid.hash,
+   ref_transaction_update(transaction, "HEAD", ,
   current_head
-  ? current_head->object.oid.hash : null_sha1,
+  ? _head->object.oid : _oid,
   0, sb.buf, ) ||
ref_transaction_commit(transaction, )) {
rollback_index_files();
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 225c734924..859be91d6c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -457,8 +457,8 @@ static int s_update_ref(const char *action,
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_update(transaction, ref->name,
-  ref->new_oid.hash,
-  check_old ? ref->old_oid.hash : NULL,
+  >new_oid,
+  check_old ? >old_oid : NULL,
   0, msg, ))
goto fail;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 29a0f3b75f..39defd4e3c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1139,7 +1139,7 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
}
if (ref_transaction_delete(transaction,
   namespaced_name,
-  old_oid ? old_oid->hash : NULL,
+  old_oid,
   0, "push", )) {
rp_error("%s", err.buf);
strbuf_release();
@@ -1156,7 +1156,7 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
 
if (ref_transaction_update(transaction,
   namespaced_name,
-  new_oid->hash, old_oid->hash,
+  new_oid, old_oid,
   0, "push",
   )) {
rp_error("%s", err.buf);
diff --git a/builtin/replace.c b/builtin/replace.c
index 2854eaa0f3..3099e55307 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -175,7 +175,7 @@ static int replace_object_oid(const char *object_ref,
 
transaction = ref_transaction_begin();
if

[PATCH v2 13/24] builtin/pack-objects: convert to struct object_id

2017-10-08 Thread brian m. carlson 
This is one of the last unconverted callers to peel_ref.  While we're
fixing that, convert the rest of the file, since it will need to be
converted at some point anyway.

Signed-off-by: brian m. carlson 
---
 builtin/pack-objects.c | 135 +
 1 file changed, 68 insertions(+), 67 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 180c17904b..3f61512242 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -151,7 +151,7 @@ static unsigned long do_compress(void **pptr, unsigned long 
size)
 }
 
 static unsigned long write_large_blob_data(struct git_istream *st, struct 
sha1file *f,
-  const unsigned char *sha1)
+  const struct object_id *oid)
 {
git_zstream stream;
unsigned char ibuf[1024 * 16];
@@ -165,7 +165,7 @@ static unsigned long write_large_blob_data(struct 
git_istream *st, struct sha1fi
int zret = Z_OK;
readlen = read_istream(st, ibuf, sizeof(ibuf));
if (readlen == -1)
-   die(_("unable to read %s"), sha1_to_hex(sha1));
+   die(_("unable to read %s"), oid_to_hex(oid));
 
stream.next_in = ibuf;
stream.avail_in = readlen;
@@ -339,7 +339,7 @@ static unsigned long write_no_reuse_object(struct sha1file 
*f, struct object_ent
sha1write(f, header, hdrlen);
}
if (st) {
-   datalen = write_large_blob_data(st, f, entry->idx.oid.hash);
+   datalen = write_large_blob_data(st, f, >idx.oid);
close_istream(st);
} else {
sha1write(f, buf, datalen);
@@ -557,13 +557,13 @@ static enum write_one_status write_one(struct sha1file *f,
 static int mark_tagged(const char *path, const struct object_id *oid, int flag,
   void *cb_data)
 {
-   unsigned char peeled[20];
+   struct object_id peeled;
struct object_entry *entry = packlist_find(_pack, oid->hash, NULL);
 
if (entry)
entry->tagged = 1;
-   if (!peel_ref(path, peeled)) {
-   entry = packlist_find(_pack, peeled, NULL);
+   if (!peel_ref(path, peeled.hash)) {
+   entry = packlist_find(_pack, peeled.hash, NULL);
if (entry)
entry->tagged = 1;
}
@@ -792,7 +792,7 @@ static void write_pack_file(void)
write_order = compute_write_order();
 
do {
-   unsigned char sha1[20];
+   struct object_id oid;
char *pack_tmp_name = NULL;
 
if (pack_to_stdout)
@@ -823,13 +823,13 @@ static void write_pack_file(void)
 * If so, rewrite it like in fast-import
 */
if (pack_to_stdout) {
-   sha1close(f, sha1, CSUM_CLOSE);
+   sha1close(f, oid.hash, CSUM_CLOSE);
} else if (nr_written == nr_remaining) {
-   sha1close(f, sha1, CSUM_FSYNC);
+   sha1close(f, oid.hash, CSUM_FSYNC);
} else {
-   int fd = sha1close(f, sha1, 0);
-   fixup_pack_header_footer(fd, sha1, pack_tmp_name,
-nr_written, sha1, offset);
+   int fd = sha1close(f, oid.hash, 0);
+   fixup_pack_header_footer(fd, oid.hash, pack_tmp_name,
+nr_written, oid.hash, offset);
close(fd);
if (write_bitmap_index) {
warning(_(no_split_warning));
@@ -863,16 +863,16 @@ static void write_pack_file(void)
strbuf_addf(, "%s-", base_name);
 
if (write_bitmap_index) {
-   bitmap_writer_set_checksum(sha1);
+   bitmap_writer_set_checksum(oid.hash);
bitmap_writer_build_type_index(written_list, 
nr_written);
}
 
finish_tmp_packfile(, pack_tmp_name,
written_list, nr_written,
-   _idx_opts, sha1);
+   _idx_opts, oid.hash);
 
if (write_bitmap_index) {
-   strbuf_addf(, "%s.bitmap", 
sha1_to_hex(sha1));
+   strbuf_addf(, "%s.bitmap", 
oid_to_hex());
 
stop_progress(_state);
 
@@ -887,7 +887,7 @@ static void write_pack_file(void)
 
strbuf_release();
free(pack_tmp_name);
-   puts(sha1_to_hex(sha1));
+   puts(oid_to_hex());

[PATCH v2 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

2017-10-08 Thread brian m. carlson 
All of the callers already pass the hash member of struct object_id, so
update them to pass a pointer to the struct directly,

This transformation was done with an update to declaration and
definition and the following semantic patch:

@@
expression E1, E2, E3, E4;
@@
- resolve_refdup(E1, E2, E3.hash, E4)
+ resolve_refdup(E1, E2, , E4)

@@
expression E1, E2, E3, E4;
@@
- resolve_refdup(E1, E2, E3->hash, E4)
+ resolve_refdup(E1, E2, E3, E4)

Signed-off-by: brian m. carlson 
---
 builtin/am.c| 2 +-
 builtin/branch.c| 6 +++---
 builtin/checkout.c  | 4 ++--
 builtin/clone.c | 2 +-
 builtin/fmt-merge-msg.c | 2 +-
 builtin/merge.c | 2 +-
 builtin/notes.c | 2 +-
 builtin/receive-pack.c  | 2 +-
 builtin/show-branch.c   | 6 +++---
 builtin/submodule--helper.c | 2 +-
 ref-filter.c| 4 ++--
 reflog-walk.c   | 4 ++--
 refs.c  | 8 
 refs.h  | 4 ++--
 refs/files-backend.c| 2 +-
 submodule.c | 2 +-
 transport.c | 2 +-
 wt-status.c | 2 +-
 18 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 32120f42df..c9bb14a6c2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2135,7 +2135,7 @@ static void am_abort(struct am_state *state)
 
am_rerere_clear();
 
-   curr_branch = resolve_refdup("HEAD", 0, curr_head.hash, NULL);
+   curr_branch = resolve_refdup("HEAD", 0, _head, NULL);
has_curr_head = curr_branch && !is_null_oid(_head);
if (!has_curr_head)
hashcpy(curr_head.hash, EMPTY_TREE_SHA1_BIN);
diff --git a/builtin/branch.c b/builtin/branch.c
index 6031b74d68..75dadd3d93 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -124,7 +124,7 @@ static int branch_merged(int kind, const char *name,
if (upstream &&
(reference_name = reference_name_to_free =
 resolve_refdup(upstream, RESOLVE_REF_READING,
-   oid.hash, NULL)) != NULL)
+   , NULL)) != NULL)
reference_rev = lookup_commit_reference();
}
if (!reference_rev)
@@ -240,7 +240,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,
-   oid.hash, );
+   , );
if (!target) {
error(remote_branch
  ? _("remote-tracking branch '%s' not found.")
@@ -613,7 +613,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
track = git_branch_track;
 
-   head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
+   head = resolve_refdup("HEAD", 0, _oid, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
if (!strcmp(head, "HEAD"))
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fd0dec401e..b803425bc4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -826,7 +826,7 @@ static int switch_branches(const struct checkout_opts *opts,
struct object_id rev;
int flag, writeout_error = 0;
memset(, 0, sizeof(old));
-   old.path = path_to_free = resolve_refdup("HEAD", 0, rev.hash, );
+   old.path = path_to_free = resolve_refdup("HEAD", 0, , );
if (old.path)
old.commit = lookup_commit_reference_gently(, 1);
if (!(flag & REF_ISSYMREF))
@@ -1125,7 +1125,7 @@ static int checkout_branch(struct checkout_opts *opts,
!opts->ignore_other_worktrees) {
struct object_id oid;
int flag;
-   char *head_ref = resolve_refdup("HEAD", 0, oid.hash, );
+   char *head_ref = resolve_refdup("HEAD", 0, , );
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
die_if_checked_out(new->path, 1);
diff --git a/builtin/clone.c b/builtin/clone.c
index 5cd1b02d53..695bdd7046 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -715,7 +715,7 @@ static int checkout(int submodule_progress)
if (option_no_checkout)
return 0;
 
-   head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
+   head = resolve_refdup("HEAD", RESOLVE_REF_READING, , NULL);
if (!head) {
warning(_("remote HEAD refers to nonexistent ref, "
  "unable to checkout.\n"));
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index e99b5ddbf9..b69f7d3be2 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@

[PATCH v2 11/24] refs: convert dwim_log to struct object_id

2017-10-08 Thread brian m. carlson 
Signed-off-by: brian m. carlson 
---
 builtin/reflog.c | 4 ++--
 reflog-walk.c| 2 +-
 refs.c   | 8 
 refs.h   | 2 +-
 sha1_name.c  | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 302fafbeef..cd4c4847b7 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -602,7 +602,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
for (; i < argc; i++) {
char *ref;
struct object_id oid;
-   if (!dwim_log(argv[i], strlen(argv[i]), oid.hash, )) {
+   if (!dwim_log(argv[i], strlen(argv[i]), , )) {
status |= error("%s points nowhere!", argv[i]);
continue;
}
@@ -668,7 +668,7 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
continue;
}
 
-   if (!dwim_log(argv[i], spec - argv[i], oid.hash, )) {
+   if (!dwim_log(argv[i], spec - argv[i], , )) {
status |= error("no reflog for '%s'", argv[i]);
continue;
}
diff --git a/reflog-walk.c b/reflog-walk.c
index 1a602a214d..65f8a6b340 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -163,7 +163,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
struct object_id oid;
char *b;
int ret = dwim_log(branch, strlen(branch),
-  oid.hash, );
+  , );
if (ret > 1)
free(b);
else if (ret == 1) {
diff --git a/refs.c b/refs.c
index 14a700ade6..40f07296e9 100644
--- a/refs.c
+++ b/refs.c
@@ -496,7 +496,7 @@ int expand_ref(const char *str, int len, struct object_id 
*oid, char **ref)
return refs_found;
 }
 
-int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
+int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 {
char *last_branch = substitute_branch_name(, );
const char **p;
@@ -505,13 +505,13 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
 
*log = NULL;
for (p = ref_rev_parse_rules; *p; p++) {
-   unsigned char hash[20];
+   struct object_id hash;
const char *ref, *it;
 
strbuf_reset();
strbuf_addf(, *p, len, str);
ref = resolve_ref_unsafe(path.buf, RESOLVE_REF_READING,
-hash, NULL);
+hash.hash, NULL);
if (!ref)
continue;
if (reflog_exists(path.buf))
@@ -522,7 +522,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, 
char **log)
continue;
if (!logs_found++) {
*log = xstrdup(it);
-   hashcpy(sha1, hash);
+   oidcpy(oid, );
}
if (!warn_ambiguous_refs)
break;
diff --git a/refs.h b/refs.h
index 832a77183c..8159b7b067 100644
--- a/refs.h
+++ b/refs.h
@@ -141,7 +141,7 @@ int refname_match(const char *abbrev_name, const char 
*full_name);
 
 int expand_ref(const char *str, int len, struct object_id *oid, char **ref);
 int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
-int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
+int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
 
 /*
  * A ref_transaction represents a collection of reference updates that
diff --git a/sha1_name.c b/sha1_name.c
index 28bad3e74b..7de12743f3 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -656,7 +656,7 @@ static int get_oid_basic(const char *str, int len, struct 
object_id *oid,
/* allow "@{...}" to mean the current branch reflog */
refs_found = dwim_ref("HEAD", 4, oid, _ref);
else if (reflog_len)
-   refs_found = dwim_log(str, len, oid->hash, _ref);
+   refs_found = dwim_log(str, len, oid, _ref);
else
refs_found = dwim_ref(str, len, oid, _ref);
 
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 14/24] refs: convert peel_ref to struct object_id

2017-10-08 Thread brian m. carlson 
Convert peel_ref (and its corresponding backend) to struct object_id.

This transformation was done with an update to the declaration,
definition, and test helper and the following semantic patch:

@@
expression E1, E2;
@@
- peel_ref(E1, E2.hash)
+ peel_ref(E1, )

@@
expression E1, E2;
@@
- peel_ref(E1, E2->hash)
+ peel_ref(E1, E2)

Signed-off-by: brian m. carlson 
---
 builtin/describe.c| 2 +-
 builtin/pack-objects.c| 4 ++--
 builtin/show-ref.c| 2 +-
 refs.c| 8 
 refs.h| 4 ++--
 refs/files-backend.c  | 8 
 refs/packed-backend.c | 4 ++--
 refs/refs-internal.h  | 2 +-
 t/helper/test-ref-store.c | 6 +++---
 upload-pack.c | 2 +-
 10 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 29075dbd0f..352f8821fd 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -181,7 +181,7 @@ static int get_name(const char *path, const struct 
object_id *oid, int flag, voi
}
 
/* Is it annotated? */
-   if (!peel_ref(path, peeled.hash)) {
+   if (!peel_ref(path, )) {
is_annotated = !!oidcmp(oid, );
} else {
oidcpy(, oid);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f61512242..dbfb2e111d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -562,7 +562,7 @@ static int mark_tagged(const char *path, const struct 
object_id *oid, int flag,
 
if (entry)
entry->tagged = 1;
-   if (!peel_ref(path, peeled.hash)) {
+   if (!peel_ref(path, )) {
entry = packlist_find(_pack, peeled.hash, NULL);
if (entry)
entry->tagged = 1;
@@ -2371,7 +2371,7 @@ static int add_ref_tag(const char *path, const struct 
object_id *oid, int flag,
struct object_id peeled;
 
if (starts_with(path, "refs/tags/") && /* is a tag? */
-   !peel_ref(path, peeled.hash)&& /* peelable? */
+   !peel_ref(path, )&& /* peelable? */
packlist_find(_pack, peeled.hash, NULL))  /* object packed? 
*/
add_tag_chain(oid);
return 0;
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index cbb8cfc7d2..41e5e71cad 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -38,7 +38,7 @@ static void show_one(const char *refname, const struct 
object_id *oid)
if (!deref_tags)
return;
 
-   if (!peel_ref(refname, peeled.hash)) {
+   if (!peel_ref(refname, )) {
hex = find_unique_abbrev(peeled.hash, abbrev);
printf("%s %s^{}\n", hex, refname);
}
diff --git a/refs.c b/refs.c
index 40f07296e9..0a3754f6da 100644
--- a/refs.c
+++ b/refs.c
@@ -1675,14 +1675,14 @@ int refs_pack_refs(struct ref_store *refs, unsigned int 
flags)
 }
 
 int refs_peel_ref(struct ref_store *refs, const char *refname,
- unsigned char *sha1)
+ struct object_id *oid)
 {
-   return refs->be->peel_ref(refs, refname, sha1);
+   return refs->be->peel_ref(refs, refname, oid);
 }
 
-int peel_ref(const char *refname, unsigned char *sha1)
+int peel_ref(const char *refname, struct object_id *oid)
 {
-   return refs_peel_ref(get_main_ref_store(), refname, sha1);
+   return refs_peel_ref(get_main_ref_store(), refname, oid);
 }
 
 int refs_create_symref(struct ref_store *refs,
diff --git a/refs.h b/refs.h
index 8159b7b067..832ade2b13 100644
--- a/refs.h
+++ b/refs.h
@@ -120,8 +120,8 @@ extern int refs_init_db(struct strbuf *err);
  * ultimately resolve to a peelable tag.
  */
 int refs_peel_ref(struct ref_store *refs, const char *refname,
- unsigned char *sha1);
-int peel_ref(const char *refname, unsigned char *sha1);
+ struct object_id *oid);
+int peel_ref(const char *refname, struct object_id *oid);
 
 /**
  * Resolve refname in the nested "gitlink" repository in the specified
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 148b98490f..a7e4b9e1e9 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -642,7 +642,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
 }
 
 static int files_peel_ref(struct ref_store *ref_store,
- const char *refname, unsigned char *sha1)
+ const char *refname, struct object_id *oid)
 {
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_READ | REF_STORE_ODB,
@@ -655,7 +655,7 @@ static int files_peel_ref(struct ref_store *ref_store,
 
if (ref_iterator_peel(current_ref_iter, ))
return -1;
-   hashcpy(sha1, peeled.hash);
+   oidcpy(oid, );
return 0;
}
 
@@ -672,10 +672,10 @@ static int files_peel_ref(struct ref_store *ref_store,
 * have REF_KNOWS_PEELED.
 */
if (flag &

[PATCH v2 08/24] refs: convert read_ref and read_ref_full to object_id

2017-10-08 Thread brian m. carlson 
All but two of the call sites already had parameters using the hash
parameter of struct object_id, so convert them to take a pointer to the
struct directly.  Also convert refs_read_refs_full, the underlying
implementation.

Signed-off-by: brian m. carlson 
---
 builtin/checkout.c |  6 +++---
 builtin/remote.c   |  2 +-
 builtin/replace.c  |  4 ++--
 builtin/show-ref.c |  2 +-
 builtin/tag.c  |  4 ++--
 builtin/update-index.c |  6 +++---
 bundle.c   |  2 +-
 fast-import.c  |  2 +-
 notes-cache.c  |  2 +-
 notes-merge.c  |  2 +-
 notes-utils.c  |  2 +-
 notes.c|  2 +-
 refs.c | 20 ++--
 refs.h |  6 +++---
 refs/files-backend.c   | 16 
 remote-testsvn.c   |  2 +-
 remote.c   |  6 +++---
 sequencer.c|  2 +-
 transport-helper.c |  5 ++---
 19 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index b803425bc4..20d2bca007 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -379,7 +379,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (write_locked_index(_index, lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
 
-   read_ref_full("HEAD", 0, rev.hash, NULL);
+   read_ref_full("HEAD", 0, , NULL);
head = lookup_commit_reference_gently(, 1);
 
errs |= post_checkout_hook(head, head, 0);
@@ -1037,7 +1037,7 @@ static int parse_branchname_arg(int argc, const char 
**argv,
setup_branch_path(new);
 
if (!check_refname_format(new->path, 0) &&
-   !read_ref(new->path, branch_rev.hash))
+   !read_ref(new->path, _rev))
oidcpy(rev, _rev);
else
new->path = NULL; /* not an existing branch */
@@ -1136,7 +1136,7 @@ static int checkout_branch(struct checkout_opts *opts,
struct object_id rev;
int flag;
 
-   if (!read_ref_full("HEAD", 0, rev.hash, ) &&
+   if (!read_ref_full("HEAD", 0, , ) &&
(flag & REF_ISSYMREF) && is_null_oid())
return switch_unborn_to_new_branch(opts);
}
diff --git a/builtin/remote.c b/builtin/remote.c
index 4f5cac96b0..0fddc64461 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -690,7 +690,7 @@ static int mv(int argc, const char **argv)
int flag = 0;
struct object_id oid;
 
-   read_ref_full(item->string, RESOLVE_REF_READING, oid.hash, 
);
+   read_ref_full(item->string, RESOLVE_REF_READING, , );
if (!(flag & REF_ISSYMREF))
continue;
if (delete_ref(NULL, item->string, NULL, REF_NODEREF))
diff --git a/builtin/replace.c b/builtin/replace.c
index 3099e55307..10078ae371 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -113,7 +113,7 @@ static int for_each_replace_name(const char **argv, 
each_replace_name_fn fn)
strbuf_addstr(, oid_to_hex());
full_hex = ref.buf + base_len;
 
-   if (read_ref(ref.buf, oid.hash)) {
+   if (read_ref(ref.buf, )) {
error("replace ref '%s' not found.", full_hex);
had_error = 1;
continue;
@@ -144,7 +144,7 @@ static void check_ref_valid(struct object_id *object,
if (check_refname_format(ref->buf, 0))
die("'%s' is not a valid ref name.", ref->buf);
 
-   if (read_ref(ref->buf, prev->hash))
+   if (read_ref(ref->buf, prev))
oidclr(prev);
else if (!force)
die("replace ref '%s' already exists", ref->buf);
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 013d241abc..cbb8cfc7d2 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -197,7 +197,7 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
struct object_id oid;
 
if ((starts_with(*pattern, "refs/") || 
!strcmp(*pattern, "HEAD")) &&
-   !read_ref(*pattern, oid.hash)) {
+   !read_ref(*pattern, )) {
show_one(*pattern, );
}
else if (!quiet)
diff --git a/builtin/tag.c b/builtin/tag.c
index 2ededc3fb1..c36d2f115c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -82,7 +82,7 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn,
for (p = argv; *p; p++) {
strbuf_reset();
strbuf_addf(, "refs/tags/%s", *p);
-   if (read_ref(ref.buf, oid.hash)) {
+   if (read_ref(ref.buf, )) {
error(_("tag '%s' not found."), *p);
had_error = 1;
continue;
@@ -517,7

[PATCH v2 17/24] sha1_file: convert index_path and index_fd to struct object_id

2017-10-08 Thread brian m. carlson 
Convert these two functions and the functions that underlie them to take
pointers to struct object_id.  This is a prerequisite to convert
resolve_gitlink_ref.  Fix a stray tab in the middle of the index_mem
call in index_pipe by converting it to a space.

Signed-off-by: brian m. carlson 
---
 sha1_file.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 5a2014811f..793fd2d194 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1660,7 +1660,7 @@ static void check_tag(const void *buf, size_t size)
die("corrupt tag");
 }
 
-static int index_mem(unsigned char *sha1, void *buf, size_t size,
+static int index_mem(struct object_id *oid, void *buf, size_t size,
 enum object_type type,
 const char *path, unsigned flags)
 {
@@ -1691,15 +1691,15 @@ static int index_mem(unsigned char *sha1, void *buf, 
size_t size,
}
 
if (write_object)
-   ret = write_sha1_file(buf, size, typename(type), sha1);
+   ret = write_sha1_file(buf, size, typename(type), oid->hash);
else
-   ret = hash_sha1_file(buf, size, typename(type), sha1);
+   ret = hash_sha1_file(buf, size, typename(type), oid->hash);
if (re_allocated)
free(buf);
return ret;
 }
 
-static int index_stream_convert_blob(unsigned char *sha1, int fd,
+static int index_stream_convert_blob(struct object_id *oid, int fd,
 const char *path, unsigned flags)
 {
int ret;
@@ -1714,22 +1714,22 @@ static int index_stream_convert_blob(unsigned char 
*sha1, int fd,
 
if (write_object)
ret = write_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
- sha1);
+ oid->hash);
else
ret = hash_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
-sha1);
+oid->hash);
strbuf_release();
return ret;
 }
 
-static int index_pipe(unsigned char *sha1, int fd, enum object_type type,
+static int index_pipe(struct object_id *oid, int fd, enum object_type type,
  const char *path, unsigned flags)
 {
struct strbuf sbuf = STRBUF_INIT;
int ret;
 
if (strbuf_read(, fd, 4096) >= 0)
-   ret = index_mem(sha1, sbuf.buf, sbuf.len, type, path, flags);
+   ret = index_mem(oid, sbuf.buf, sbuf.len, type, path, flags);
else
ret = -1;
strbuf_release();
@@ -1738,24 +1738,24 @@ static int index_pipe(unsigned char *sha1, int fd, enum 
object_type type,
 
 #define SMALL_FILE_SIZE (32*1024)
 
-static int index_core(unsigned char *sha1, int fd, size_t size,
+static int index_core(struct object_id *oid, int fd, size_t size,
  enum object_type type, const char *path,
  unsigned flags)
 {
int ret;
 
if (!size) {
-   ret = index_mem(sha1, "", size, type, path, flags);
+   ret = index_mem(oid, "", size, type, path, flags);
} else if (size <= SMALL_FILE_SIZE) {
char *buf = xmalloc(size);
if (size == read_in_full(fd, buf, size))
-   ret = index_mem(sha1, buf, size, type, path, flags);
+   ret = index_mem(oid, buf, size, type, path, flags);
else
ret = error_errno("short read");
free(buf);
} else {
void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
-   ret = index_mem(sha1, buf, size, type, path, flags);
+   ret = index_mem(oid, buf, size, type, path, flags);
munmap(buf, size);
}
return ret;
@@ -1793,12 +1793,12 @@ int index_fd(struct object_id *oid, int fd, struct stat 
*st,
 * die() for large files.
 */
if (type == OBJ_BLOB && path && would_convert_to_git_filter_fd(path))
-   ret = index_stream_convert_blob(oid->hash, fd, path, flags);
+   ret = index_stream_convert_blob(oid, fd, path, flags);
else if (!S_ISREG(st->st_mode))
-   ret = index_pipe(oid->hash, fd, type, path, flags);
+   ret = index_pipe(oid, fd, type, path, flags);
else if (st->st_size <= big_file_threshold || type != OBJ_BLOB ||
 (path && would_convert_to_git(_index, path)))
-   ret = index_core(oid->hash, fd, xsize_t(st->st_size), type, 
path,
+   ret = index_core(oid, fd, xsize_t(st->st_size), type, path,
 flags);
else
ret = index_stream(oid, fd, xsize_t(st->st_size), type, path,
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 06/24] Convert check_connected to use struct object_id

2017-10-08 Thread brian m. carlson 
Convert check_connected and the callbacks it takes to use struct
object_id.

Signed-off-by: brian m. carlson 
---
 builtin/clone.c|  4 ++--
 builtin/fetch.c|  4 ++--
 builtin/receive-pack.c | 10 +-
 connected.c| 18 +-
 connected.h|  4 ++--
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 665a0e2673..5cd1b02d53 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -615,7 +615,7 @@ static void write_followtags(const struct ref *refs, const 
char *msg)
}
 }
 
-static int iterate_ref_map(void *cb_data, unsigned char sha1[20])
+static int iterate_ref_map(void *cb_data, struct object_id *oid)
 {
struct ref **rm = cb_data;
struct ref *ref = *rm;
@@ -630,7 +630,7 @@ static int iterate_ref_map(void *cb_data, unsigned char 
sha1[20])
if (!ref)
return -1;
 
-   hashcpy(sha1, ref->old_oid.hash);
+   oidcpy(oid, >old_oid);
*rm = ref->next;
return 0;
 }
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 859be91d6c..e705237fa9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -727,7 +727,7 @@ static int update_local_ref(struct ref *ref,
}
 }
 
-static int iterate_ref_map(void *cb_data, unsigned char sha1[20])
+static int iterate_ref_map(void *cb_data, struct object_id *oid)
 {
struct ref **rm = cb_data;
struct ref *ref = *rm;
@@ -737,7 +737,7 @@ static int iterate_ref_map(void *cb_data, unsigned char 
sha1[20])
if (!ref)
return -1; /* end of the list */
*rm = ref->next;
-   hashcpy(sha1, ref->old_oid.hash);
+   oidcpy(oid, >old_oid);
return 0;
 }
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 39defd4e3c..046b600b11 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -870,7 +870,7 @@ static void refuse_unconfigured_deny_delete_current(void)
rp_error("%s", _(refuse_unconfigured_deny_delete_current_msg));
 }
 
-static int command_singleton_iterator(void *cb_data, unsigned char sha1[20]);
+static int command_singleton_iterator(void *cb_data, struct object_id *oid);
 static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 {
static struct lock_file shallow_lock;
@@ -1270,7 +1270,7 @@ static void check_aliased_updates(struct command 
*commands)
string_list_clear(_list, 0);
 }
 
-static int command_singleton_iterator(void *cb_data, unsigned char sha1[20])
+static int command_singleton_iterator(void *cb_data, struct object_id *oid)
 {
struct command **cmd_list = cb_data;
struct command *cmd = *cmd_list;
@@ -1278,7 +1278,7 @@ static int command_singleton_iterator(void *cb_data, 
unsigned char sha1[20])
if (!cmd || is_null_oid(>new_oid))
return -1; /* end of list */
*cmd_list = NULL; /* this returns only one */
-   hashcpy(sha1, cmd->new_oid.hash);
+   oidcpy(oid, >new_oid);
return 0;
 }
 
@@ -1309,7 +1309,7 @@ struct iterate_data {
struct shallow_info *si;
 };
 
-static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])
+static int iterate_receive_command_list(void *cb_data, struct object_id *oid)
 {
struct iterate_data *data = cb_data;
struct command **cmd_list = >cmds;
@@ -1320,7 +1320,7 @@ static int iterate_receive_command_list(void *cb_data, 
unsigned char sha1[20])
/* to be checked in update_shallow_ref() */
continue;
if (!is_null_oid(>new_oid) && !cmd->skip_update) {
-   hashcpy(sha1, cmd->new_oid.hash);
+   oidcpy(oid, >new_oid);
*cmd_list = cmd->next;
return 0;
}
diff --git a/connected.c b/connected.c
index f416b05051..4a47f33270 100644
--- a/connected.c
+++ b/connected.c
@@ -16,13 +16,13 @@
  *
  * Returns 0 if everything is connected, non-zero otherwise.
  */
-int check_connected(sha1_iterate_fn fn, void *cb_data,
+int check_connected(oid_iterate_fn fn, void *cb_data,
struct check_connected_options *opt)
 {
struct child_process rev_list = CHILD_PROCESS_INIT;
struct check_connected_options defaults = CHECK_CONNECTED_INIT;
-   char commit[41];
-   unsigned char sha1[20];
+   char commit[GIT_MAX_HEXSZ + 1];
+   struct object_id oid;
int err = 0;
struct packed_git *new_pack = NULL;
struct transport *transport;
@@ -32,7 +32,7 @@ int check_connected(sha1_iterate_fn fn, void *cb_data,
opt = 
transport = opt->transport;
 
-   if (fn(cb_data, sha1)) {
+   if (fn(cb_data, )) {
if (opt->err_fd)
close(opt->err_fd);
return err;
@@ -77,7 +77,7 @@ int check_connected(sha1_iterate_fn fn, void *cb_data,

[PATCH v2 15/24] refs: convert read_ref_at to struct object_id

2017-10-08 Thread brian m. carlson 
Convert the callers and internals, including struct read_ref_at_cb, of
read_ref_at to use struct object_id.

Signed-off-by: brian m. carlson 
---
 builtin/show-branch.c |  4 ++--
 refs.c| 34 +-
 refs.h|  2 +-
 sha1_name.c   |  2 +-
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 722a7f4bec..8bf42e529d 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -731,7 +731,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
/* Ah, that is a date spec... */
timestamp_t at;
at = approxidate(reflog_base);
-   read_ref_at(ref, flags, at, -1, oid.hash, NULL,
+   read_ref_at(ref, flags, at, -1, , NULL,
NULL, NULL, );
}
}
@@ -743,7 +743,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
timestamp_t timestamp;
int tz;
 
-   if (read_ref_at(ref, flags, 0, base+i, oid.hash, 
,
+   if (read_ref_at(ref, flags, 0, base + i, , ,
, , NULL)) {
reflog = i;
break;
diff --git a/refs.c b/refs.c
index 0a3754f6da..b781acc8b1 100644
--- a/refs.c
+++ b/refs.c
@@ -734,11 +734,11 @@ struct read_ref_at_cb {
timestamp_t at_time;
int cnt;
int reccnt;
-   unsigned char *sha1;
+   struct object_id *oid;
int found_it;
 
-   unsigned char osha1[20];
-   unsigned char nsha1[20];
+   struct object_id ooid;
+   struct object_id noid;
int tz;
timestamp_t date;
char **msg;
@@ -770,25 +770,25 @@ static int read_ref_at_ent(struct object_id *ooid, struct 
object_id *noid,
 * we have not yet updated cb->[n|o]sha1 so they still
 * hold the values for the previous record.
 */
-   if (!is_null_sha1(cb->osha1)) {
-   hashcpy(cb->sha1, noid->hash);
-   if (hashcmp(cb->osha1, noid->hash))
+   if (!is_null_oid(>ooid)) {
+   oidcpy(cb->oid, noid);
+   if (oidcmp(>ooid, noid))
warning("Log for ref %s has gap after %s.",
cb->refname, show_date(cb->date, 
cb->tz, DATE_MODE(RFC2822)));
}
else if (cb->date == cb->at_time)
-   hashcpy(cb->sha1, noid->hash);
-   else if (hashcmp(noid->hash, cb->sha1))
+   oidcpy(cb->oid, noid);
+   else if (oidcmp(noid, cb->oid))
warning("Log for ref %s unexpectedly ended on %s.",
cb->refname, show_date(cb->date, cb->tz,
   DATE_MODE(RFC2822)));
-   hashcpy(cb->osha1, ooid->hash);
-   hashcpy(cb->nsha1, noid->hash);
+   oidcpy(>ooid, ooid);
+   oidcpy(>noid, noid);
cb->found_it = 1;
return 1;
}
-   hashcpy(cb->osha1, ooid->hash);
-   hashcpy(cb->nsha1, noid->hash);
+   oidcpy(>ooid, ooid);
+   oidcpy(>noid, noid);
if (cb->cnt > 0)
cb->cnt--;
return 0;
@@ -808,15 +808,15 @@ static int read_ref_at_ent_oldest(struct object_id *ooid, 
struct object_id *noid
*cb->cutoff_tz = tz;
if (cb->cutoff_cnt)
*cb->cutoff_cnt = cb->reccnt;
-   hashcpy(cb->sha1, ooid->hash);
-   if (is_null_sha1(cb->sha1))
-   hashcpy(cb->sha1, noid->hash);
+   oidcpy(cb->oid, ooid);
+   if (is_null_oid(cb->oid))
+   oidcpy(cb->oid, noid);
/* We just want the first entry */
return 1;
 }
 
 int read_ref_at(const char *refname, unsigned int flags, timestamp_t at_time, 
int cnt,
-   unsigned char *sha1, char **msg,
+   struct object_id *oid, char **msg,
timestamp_t *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
struct read_ref_at_cb cb;
@@ -829,7 +829,7 @@ int read_ref_at(const char *refname, unsigned int flags, 
timestamp_t at_time, in
cb.cutoff_time = cutoff_time;
cb.cutoff_tz = cutoff_tz;
cb.cutoff_cnt = cutoff_cnt;
-   cb.sha1 = sha1;
+   cb.oid = oid;
 
for_each_reflog_ent_reverse(refname, read_ref_at_ent, );
 
diff --git a/refs.h b/refs.h
index 832ade2b13..3afecd2e8a 100644
--- a/refs.h
+++ b/refs.h
@@ -363,7 +363,7 @@ int safe_create_reflog(const char *refname, int 
force_create, struct strbuf *err
 /** Reads

[PATCH v2 12/24] pack-bitmap: convert traverse_bitmap_commit_list to object_id

2017-10-08 Thread brian m. carlson 
Convert traverse_bitmap_commit_list and the callbacks it takes to use a
pointer to struct object_id.

Signed-off-by: brian m. carlson 
---
 builtin/pack-objects.c | 8 
 builtin/rev-list.c | 4 ++--
 pack-bitmap.c  | 8 
 pack-bitmap.h  | 2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5ee2c48ffb..180c17904b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1097,20 +1097,20 @@ static int add_object_entry(const unsigned char *sha1, 
enum object_type type,
return 1;
 }
 
-static int add_object_entry_from_bitmap(const unsigned char *sha1,
+static int add_object_entry_from_bitmap(const struct object_id *oid,
enum object_type type,
int flags, uint32_t name_hash,
struct packed_git *pack, off_t offset)
 {
uint32_t index_pos;
 
-   if (have_duplicate_entry(sha1, 0, _pos))
+   if (have_duplicate_entry(oid->hash, 0, _pos))
return 0;
 
-   if (!want_object_in_pack(sha1, 0, , ))
+   if (!want_object_in_pack(oid->hash, 0, , ))
return 0;
 
-   create_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack, 
offset);
+   create_object_entry(oid->hash, type, name_hash, 0, 0, index_pos, pack, 
offset);
 
display_progress(progress_state, nr_result);
return 1;
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index c1c74d4a79..9bf8d5991c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -258,14 +258,14 @@ static int show_bisect_vars(struct rev_list_info *info, 
int reaches, int all)
 }
 
 static int show_object_fast(
-   const unsigned char *sha1,
+   const struct object_id *oid,
enum object_type type,
int exclude,
uint32_t name_hash,
struct packed_git *found_pack,
off_t found_offset)
 {
-   fprintf(stdout, "%s\n", sha1_to_hex(sha1));
+   fprintf(stdout, "%s\n", oid_to_hex(oid));
return 1;
 }
 
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 42e3d5f4f2..9270983e5f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -587,7 +587,7 @@ static void show_extended_objects(struct bitmap *objects,
continue;
 
obj = eindex->objects[i];
-   show_reach(obj->oid.hash, obj->type, 0, eindex->hashes[i], 
NULL, 0);
+   show_reach(>oid, obj->type, 0, eindex->hashes[i], NULL, 0);
}
 }
 
@@ -612,7 +612,7 @@ static void show_objects_for_type(
eword_t word = objects->words[i] & filter;
 
for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
-   const unsigned char *sha1;
+   struct object_id oid;
struct revindex_entry *entry;
uint32_t hash = 0;
 
@@ -625,12 +625,12 @@ static void show_objects_for_type(
continue;
 
entry = _git.pack->revindex[pos + offset];
-   sha1 = nth_packed_object_sha1(bitmap_git.pack, 
entry->nr);
+   nth_packed_object_oid(, bitmap_git.pack, entry->nr);
 
if (bitmap_git.hashes)
hash = get_be32(bitmap_git.hashes + entry->nr);
 
-   show_reach(sha1, object_type, 0, hash, bitmap_git.pack, 
entry->offset);
+   show_reach(, object_type, 0, hash, bitmap_git.pack, 
entry->offset);
}
 
pos += BITS_IN_EWORD;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 0adcef77b5..3742a00e14 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -27,7 +27,7 @@ enum pack_bitmap_flags {
 };
 
 typedef int (*show_reachable_fn)(
-   const unsigned char *sha1,
+   const struct object_id *oid,
enum object_type type,
int flags,
uint32_t hash,
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 20/24] worktree: convert struct worktree to object_id

2017-10-08 Thread brian m. carlson 
Convert the head_sha1 member to be head_oid instead.  This is required
to convert resolve_ref_unsafe.

Signed-off-by: brian m. carlson 
---
 builtin/worktree.c | 6 +++---
 worktree.c | 2 +-
 worktree.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index de26849f55..d9fe6f694e 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -392,7 +392,7 @@ static void show_worktree_porcelain(struct worktree *wt)
if (wt->is_bare)
printf("bare\n");
else {
-   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
+   printf("HEAD %s\n", oid_to_hex(>head_oid));
if (wt->is_detached)
printf("detached\n");
else if (wt->head_ref)
@@ -412,7 +412,7 @@ static void show_worktree(struct worktree *wt, int 
path_maxlen, int abbrev_len)
strbuf_addstr(, "(bare)");
else {
strbuf_addf(, "%-*s ", abbrev_len,
-   find_unique_abbrev(wt->head_sha1, 
DEFAULT_ABBREV));
+   find_unique_abbrev(wt->head_oid.hash, 
DEFAULT_ABBREV));
if (wt->is_detached)
strbuf_addstr(, "(detached HEAD)");
else if (wt->head_ref) {
@@ -437,7 +437,7 @@ static void measure_widths(struct worktree **wt, int 
*abbrev, int *maxlen)
 
if (path_len > *maxlen)
*maxlen = path_len;
-   sha1_len = strlen(find_unique_abbrev(wt[i]->head_sha1, 
*abbrev));
+   sha1_len = strlen(find_unique_abbrev(wt[i]->head_oid.hash, 
*abbrev));
if (sha1_len > *abbrev)
*abbrev = sha1_len;
}
diff --git a/worktree.c b/worktree.c
index 70015629dc..cb35db03fa 100644
--- a/worktree.c
+++ b/worktree.c
@@ -31,7 +31,7 @@ static void add_head_info(struct worktree *wt)
target = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
 "HEAD",
 0,
-wt->head_sha1, );
+wt->head_oid.hash, );
if (!target)
return;
 
diff --git a/worktree.h b/worktree.h
index 9276c81ae7..c28a880e18 100644
--- a/worktree.h
+++ b/worktree.h
@@ -8,7 +8,7 @@ struct worktree {
char *id;
char *head_ref; /* NULL if HEAD is broken or detached */
char *lock_reason;  /* internal use */
-   unsigned char head_sha1[20];
+   struct object_id head_oid;
int is_detached;
int is_bare;
int is_current;
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 09/24] refs: convert dwim_ref and expand_ref to struct object_id

2017-10-08 Thread brian m. carlson 
All of the callers of these functions just pass the hash member of a
struct object_id, so convert them to use a pointer to struct object_id
directly.

Signed-off-by: brian m. carlson 
---
 archive.c |  2 +-
 branch.c  |  2 +-
 builtin/fast-export.c |  2 +-
 builtin/log.c |  2 +-
 builtin/merge-base.c  |  2 +-
 builtin/merge.c   |  2 +-
 builtin/rev-parse.c   |  2 +-
 builtin/show-branch.c |  2 +-
 bundle.c  |  2 +-
 refs.c| 14 +++---
 refs.h|  4 ++--
 remote.c  |  3 +--
 sha1_name.c   |  6 +++---
 upload-pack.c |  2 +-
 wt-status.c   |  2 +-
 15 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/archive.c b/archive.c
index 1e41f4bbeb..0b7b62af0c 100644
--- a/archive.c
+++ b/archive.c
@@ -371,7 +371,7 @@ static void parse_treeish_arg(const char **argv,
const char *colon = strchrnul(name, ':');
int refnamelen = colon - name;
 
-   if (!dwim_ref(name, refnamelen, oid.hash, ))
+   if (!dwim_ref(name, refnamelen, , ))
die("no such ref: %.*s", refnamelen, name);
free(ref);
}
diff --git a/branch.c b/branch.c
index 45029ea142..62f7b0d8c2 100644
--- a/branch.c
+++ b/branch.c
@@ -264,7 +264,7 @@ void create_branch(const char *name, const char *start_name,
die(_("Not a valid object name: '%s'."), start_name);
}
 
-   switch (dwim_ref(start_name, strlen(start_name), oid.hash, _ref)) {
+   switch (dwim_ref(start_name, strlen(start_name), , _ref)) {
case 0:
/* Not branching from any existing branch */
if (explicit_tracking)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2fb60d6d48..d74c73f777 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -823,7 +823,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info 
*info)
if (e->flags & UNINTERESTING)
continue;
 
-   if (dwim_ref(e->name, strlen(e->name), oid.hash, _name) != 
1)
+   if (dwim_ref(e->name, strlen(e->name), , _name) != 1)
continue;
 
if (refspecs) {
diff --git a/builtin/log.c b/builtin/log.c
index d81a09051e..ba9d4cd786 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -975,7 +975,7 @@ static char *find_branch_name(struct rev_info *rev)
return NULL;
ref = rev->cmdline.rev[positive].name;
tip_oid = >cmdline.rev[positive].item->oid;
-   if (dwim_ref(ref, strlen(ref), branch_oid.hash, _ref) &&
+   if (dwim_ref(ref, strlen(ref), _oid, _ref) &&
skip_prefix(full_ref, "refs/heads/", ) &&
!oidcmp(tip_oid, _oid))
branch = xstrdup(v);
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 6dbd167d3b..e99f5405ce 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -156,7 +156,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]), oid.hash, )) {
+   switch (dwim_ref(argv[0], strlen(argv[0]), , )) {
case 0:
die("No such ref: '%s'", argv[0]);
case 1:
diff --git a/builtin/merge.c b/builtin/merge.c
index 99d2df965f..6071dbfe34 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -454,7 +454,7 @@ static void merge_name(const char *remote, struct strbuf 
*msg)
if (!remote_head)
die(_("'%s' does not point to a commit"), remote);
 
-   if (dwim_ref(remote, strlen(remote), branch_head.hash, _ref) > 0) 
{
+   if (dwim_ref(remote, strlen(remote), _head, _ref) > 0) {
if (starts_with(found_ref, "refs/heads/")) {
strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
oid_to_hex(_head), remote);
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index b9c13d3d9d..ad20a948f0 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -133,7 +133,7 @@ static void show_rev(int type, const struct object_id *oid, 
const char *name)
struct object_id discard;
char *full;
 
-   switch (dwim_ref(name, strlen(name), discard.hash, 
)) {
+   switch (dwim_ref(name, strlen(name), , )) {
case 0:
/*
 * Not found -- not a ref.  We could
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 0237be4975..722a7f4bec 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -720,7 +720,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
die(Q_("only %d entry can be shown at one time.",
   "only %d entries can be

[PATCH v2 10/24] builtin/reflog: convert remaining unsigned char uses to object_id

2017-10-08 Thread brian m. carlson 
Convert the remaining uses of unsigned char [20] to struct object_id.
This conversion is needed for dwim_log.

Signed-off-by: brian m. carlson 
---
 builtin/reflog.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 2067cca5b1..302fafbeef 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -42,7 +42,7 @@ struct expire_reflog_policy_cb {
 };
 
 struct collected_reflog {
-   unsigned char sha1[20];
+   struct object_id oid;
char reflog[FLEX_ARRAY];
 };
 
@@ -385,7 +385,7 @@ static int collect_reflog(const char *ref, const struct 
object_id *oid, int unus
struct collect_reflog_cb *cb = cb_data;
 
FLEX_ALLOC_STR(e, reflog, ref);
-   hashcpy(e->sha1, oid->hash);
+   oidcpy(>oid, oid);
ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
cb->e[cb->nr++] = e;
return 0;
@@ -589,7 +589,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
for (i = 0; i < collected.nr; i++) {
struct collected_reflog *e = collected.e[i];
set_reflog_expiry_param(, explicit_expiry, 
e->reflog);
-   status |= reflog_expire(e->reflog, e->sha1, flags,
+   status |= reflog_expire(e->reflog, e->oid.hash, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -601,13 +601,13 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
 
for (; i < argc; i++) {
char *ref;
-   unsigned char sha1[20];
-   if (!dwim_log(argv[i], strlen(argv[i]), sha1, )) {
+   struct object_id oid;
+   if (!dwim_log(argv[i], strlen(argv[i]), oid.hash, )) {
status |= error("%s points nowhere!", argv[i]);
continue;
}
set_reflog_expiry_param(, explicit_expiry, ref);
-   status |= reflog_expire(ref, sha1, flags,
+   status |= reflog_expire(ref, oid.hash, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -659,7 +659,7 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
 
for ( ; i < argc; i++) {
const char *spec = strstr(argv[i], "@{");
-   unsigned char sha1[20];
+   struct object_id oid;
char *ep, *ref;
int recno;
 
@@ -668,7 +668,7 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
continue;
}
 
-   if (!dwim_log(argv[i], spec - argv[i], sha1, )) {
+   if (!dwim_log(argv[i], spec - argv[i], oid.hash, )) {
status |= error("no reflog for '%s'", argv[i]);
continue;
}
@@ -683,7 +683,7 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
cb.cmd.expire_total = 0;
}
 
-   status |= reflog_expire(ref, sha1, flags,
+   status |= reflog_expire(ref, oid.hash, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 04/24] refs: convert update_ref and refs_update_ref to use struct object_id

2017-10-08 Thread brian m. carlson 
Convert update_ref, refs_update_ref, and write_pseudoref to use struct
object_id.  Update the existing callers as well.  Remove update_ref_oid,
as it is no longer needed.

Signed-off-by: brian m. carlson 
---
 bisect.c  |  5 +++--
 builtin/am.c  | 14 +++---
 builtin/checkout.c|  3 +--
 builtin/clone.c   | 14 +++---
 builtin/merge.c   | 13 ++---
 builtin/notes.c   | 10 +-
 builtin/pull.c|  2 +-
 builtin/reset.c   |  4 ++--
 builtin/update-ref.c  |  2 +-
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 refs.c| 40 
 refs.h|  5 +
 sequencer.c   |  9 +++--
 t/helper/test-ref-store.c | 10 +-
 transport-helper.c|  3 ++-
 transport.c   |  4 ++--
 17 files changed, 64 insertions(+), 78 deletions(-)

diff --git a/bisect.c b/bisect.c
index 96beeb5d13..c09f7bbbcb 100644
--- a/bisect.c
+++ b/bisect.c
@@ -685,11 +685,12 @@ static int bisect_checkout(const struct object_id 
*bisect_rev, int no_checkout)
char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
 
memcpy(bisect_rev_hex, oid_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);
-   update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev->hash, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
+   update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
 
argv_checkout[2] = bisect_rev_hex;
if (no_checkout) {
-   update_ref(NULL, "BISECT_HEAD", bisect_rev->hash, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
+   update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
+  UPDATE_REFS_DIE_ON_ERR);
} else {
int res;
res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
diff --git a/builtin/am.c b/builtin/am.c
index d7513f5375..32120f42df 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1068,8 +1068,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
if (!get_oid("HEAD", _head)) {
write_state_text(state, "abort-safety", oid_to_hex(_head));
if (!state->rebasing)
-   update_ref_oid("am", "ORIG_HEAD", _head, NULL, 0,
-   UPDATE_REFS_DIE_ON_ERR);
+   update_ref("am", "ORIG_HEAD", _head, NULL, 0,
+  UPDATE_REFS_DIE_ON_ERR);
} else {
write_state_text(state, "abort-safety", "");
if (!state->rebasing)
@@ -1686,8 +1686,8 @@ static void do_commit(const struct am_state *state)
strbuf_addf(, "%s: %.*s", reflog_msg, linelen(state->msg),
state->msg);
 
-   update_ref_oid(sb.buf, "HEAD", , old_oid, 0,
-   UPDATE_REFS_DIE_ON_ERR);
+   update_ref(sb.buf, "HEAD", , old_oid, 0,
+  UPDATE_REFS_DIE_ON_ERR);
 
if (state->rebasing) {
FILE *fp = xfopen(am_path(state, "rewritten"), "a");
@@ -2147,9 +2147,9 @@ static void am_abort(struct am_state *state)
clean_index(_head, _head);
 
if (has_orig_head)
-   update_ref_oid("am --abort", "HEAD", _head,
-   has_curr_head ? _head : NULL, 0,
-   UPDATE_REFS_DIE_ON_ERR);
+   update_ref("am --abort", "HEAD", _head,
+  has_curr_head ? _head : NULL, 0,
+  UPDATE_REFS_DIE_ON_ERR);
else if (curr_branch)
delete_ref(NULL, curr_branch, NULL, REF_NODEREF);
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3345a0d16f..fd0dec401e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -664,8 +664,7 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
/* Nothing to do. */
} else if (opts->force_detach || !new->path) {  /* No longer on any 
branch. */
-   update_ref(msg.buf, "HEAD", new->commit->object.oid.hash, NULL,
-  REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+   update_ref(msg.buf, "HEAD", >commit->object.oid, NULL, 
REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (!opts->quiet) {
if (old->path &&
advice_detached_head && !opts->force_detach)
diff --git a/builtin/clone.c b/builtin/clone.c
index dbddd98f80..4135621aa3 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -610,8 +610,8 @@ static void write_followtags(const struct ref *refs, const 
char *msg)
continue;
if (!has_object_file(>old_oid))
continue;
-   update_ref(msg, ref->name, ref->old_oid.hash,
-

[PATCH v2 24/24] refs/files-backend: convert static functions to object_id

2017-10-08 Thread brian m. carlson 
Convert several static functions to take pointers to struct object_id.
Change the relevant parameters to write_packed_entry to be const, as we
don't modify them.  Rename lock_ref_sha1_basic to lock_ref_oid_basic to
reflect its new argument.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7281f27f62..84d8e3da99 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -811,7 +811,7 @@ static struct ref_iterator *files_ref_iterator_begin(
  * return a negative value.
  */
 static int verify_lock(struct ref_store *ref_store, struct ref_lock *lock,
-  const unsigned char *old_sha1, int mustexist,
+  const struct object_id *old_oid, int mustexist,
   struct strbuf *err)
 {
assert(err);
@@ -819,7 +819,7 @@ static int verify_lock(struct ref_store *ref_store, struct 
ref_lock *lock,
if (refs_read_ref_full(ref_store, lock->ref_name,
   mustexist ? RESOLVE_REF_READING : 0,
   >old_oid, NULL)) {
-   if (old_sha1) {
+   if (old_oid) {
int save_errno = errno;
strbuf_addf(err, "can't verify ref '%s'", 
lock->ref_name);
errno = save_errno;
@@ -829,11 +829,11 @@ static int verify_lock(struct ref_store *ref_store, 
struct ref_lock *lock,
return 0;
}
}
-   if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
+   if (old_oid && oidcmp(>old_oid, old_oid)) {
strbuf_addf(err, "ref '%s' is at %s but expected %s",
lock->ref_name,
oid_to_hex(>old_oid),
-   sha1_to_hex(old_sha1));
+   oid_to_hex(old_oid));
errno = EBUSY;
return -1;
}
@@ -863,22 +863,22 @@ static int create_reflock(const char *path, void *cb)
  * Locks a ref returning the lock on success and NULL on failure.
  * On failure errno is set to something meaningful.
  */
-static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
-   const char *refname,
-   const unsigned char *old_sha1,
-   const struct string_list *extras,
-   const struct string_list *skip,
-   unsigned int flags, int *type,
-   struct strbuf *err)
+static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
+  const char *refname,
+  const struct object_id *old_oid,
+  const struct string_list *extras,
+  const struct string_list *skip,
+  unsigned int flags, int *type,
+  struct strbuf *err)
 {
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
int last_errno = 0;
-   int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
+   int mustexist = (old_oid && !is_null_oid(old_oid));
int resolve_flags = RESOLVE_REF_NO_RECURSE;
int resolved;
 
-   files_assert_main_repository(refs, "lock_ref_sha1_basic");
+   files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
 
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -944,7 +944,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct 
files_ref_store *refs,
goto error_return;
}
 
-   if (verify_lock(>base, lock, old_sha1, mustexist, err)) {
+   if (verify_lock(>base, lock, old_oid, mustexist, err)) {
last_errno = errno;
goto error_return;
}
@@ -1349,8 +1349,8 @@ static int files_rename_ref(struct ref_store *ref_store,
 
logmoved = log;
 
-   lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
-  REF_NODEREF, NULL, );
+   lock = lock_ref_oid_basic(refs, newrefname, NULL, NULL, NULL,
+ REF_NODEREF, NULL, );
if (!lock) {
error("unable to rename '%s' to '%s': %s", oldrefname, 
newrefname, err.buf);
strbuf_release();
@@ -1369,8 +1369,8 @@ static int files_rename_ref(struct ref_store *ref_store,
goto out;
 
  rollback:
-   lock = lock_ref_sha1_basic(refs, oldrefname, NULL, NULL, NULL,
-  REF_NODEREF, NULL, );
+   lock = lock_ref_oid_basic(refs, oldrefname, NULL, NULL, NULL,
+

[PATCH v2 23/24] refs: convert read_raw_ref backends to struct object_id

2017-10-08 Thread brian m. carlson 
Convert the unsigned char * parameter to struct object_id * for
files_read_raw_ref and packed_read_raw-ref.  Update the documentation.
Switch from using get_sha1_hex and a hard-coded 40 to using
parse_oid_hex.

Signed-off-by: brian m. carlson 
---
 refs.c|  8 
 refs/files-backend.c  | 13 +++--
 refs/packed-backend.c |  4 ++--
 refs/refs-internal.h  | 10 +-
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/refs.c b/refs.c
index 958234e7d6..d8d4e06438 100644
--- a/refs.c
+++ b/refs.c
@@ -1374,10 +1374,10 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
 }
 
 int refs_read_raw_ref(struct ref_store *ref_store,
- const char *refname, unsigned char *sha1,
+ const char *refname, struct object_id *oid,
  struct strbuf *referent, unsigned int *type)
 {
-   return ref_store->be->read_raw_ref(ref_store, refname, sha1, referent, 
type);
+   return ref_store->be->read_raw_ref(ref_store, refname, oid, referent, 
type);
 }
 
 /* This function needs to return a meaningful errno on failure */
@@ -1420,7 +1420,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store 
*refs,
unsigned int read_flags = 0;
 
if (refs_read_raw_ref(refs, refname,
- oid->hash, _refname, _flags)) {
+ oid, _refname, _flags)) {
*flags |= read_flags;
if (errno != ENOENT || (resolve_flags & 
RESOLVE_REF_READING))
return NULL;
@@ -1842,7 +1842,7 @@ int refs_verify_refname_available(struct ref_store *refs,
if (skip && string_list_has_string(skip, dirname.buf))
continue;
 
-   if (!refs_read_raw_ref(refs, dirname.buf, oid.hash, , 
)) {
+   if (!refs_read_raw_ref(refs, dirname.buf, , , 
)) {
strbuf_addf(err, "'%s' exists; cannot create '%s'",
dirname.buf, refname);
goto cleanup;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 417c662d5d..7281f27f62 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -261,7 +261,7 @@ static struct ref_cache *get_loose_ref_cache(struct 
files_ref_store *refs)
 }
 
 static int files_read_raw_ref(struct ref_store *ref_store,
- const char *refname, unsigned char *sha1,
+ const char *refname, struct object_id *oid,
  struct strbuf *referent, unsigned int *type)
 {
struct files_ref_store *refs =
@@ -270,6 +270,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
struct strbuf sb_path = STRBUF_INIT;
const char *path;
const char *buf;
+   const char *p;
struct stat st;
int fd;
int ret = -1;
@@ -304,7 +305,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
if (errno != ENOENT)
goto out;
if (refs_read_raw_ref(refs->packed_ref_store, refname,
- sha1, referent, type)) {
+ oid, referent, type)) {
errno = ENOENT;
goto out;
}
@@ -344,7 +345,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 * packed ref:
 */
if (refs_read_raw_ref(refs->packed_ref_store, refname,
- sha1, referent, type)) {
+ oid, referent, type)) {
errno = EISDIR;
goto out;
}
@@ -390,8 +391,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 * Please note that FETCH_HEAD has additional
 * data after the sha.
 */
-   if (get_sha1_hex(buf, sha1) ||
-   (buf[40] != '\0' && !isspace(buf[40]))) {
+   if (parse_oid_hex(buf, oid, ) ||
+   (*p != '\0' && !isspace(*p))) {
*type |= REF_ISBROKEN;
errno = EINVAL;
goto out;
@@ -545,7 +546,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
 */
 
if (files_read_raw_ref(>base, refname,
-  lock->old_oid.hash, referent, type)) {
+  >old_oid, referent, type)) {
if (errno == ENOENT) {
if (mustexist) {
/* Garden variety missing reference. */
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 25e6fc4ffe..c1960c4cfa 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -319,7 +319,7 @@ static struct ref_entry *get_packed_ref(struct 
packed_ref_store *refs,
 }
 
 static int packed_read_raw_ref(struct

[PATCH v2 21/24] refs: convert resolve_ref_unsafe to struct object_id

2017-10-08 Thread brian m. carlson 
Convert resolve_ref_unsafe to take a pointer to struct object_id by
converting one remaining caller to use struct object_id, converting the
declaration and definition, and applying the following semantic patch:

@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3.hash, E4)
+ resolve_ref_unsafe(E1, E2, , E4)

@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3->hash, E4)
+ resolve_ref_unsafe(E1, E2, E3, E4)

Signed-off-by: brian m. carlson 
---
 blame.c   |  4 ++--
 builtin/fsck.c|  2 +-
 refs.c| 28 ++--
 refs.h|  4 ++--
 refs/files-backend.c  |  8 
 sequencer.c   |  2 +-
 t/helper/test-ref-store.c |  6 +++---
 transport-helper.c|  7 +++
 worktree.c|  2 +-
 9 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/blame.c b/blame.c
index f575e9cbf4..c3060de2f8 100644
--- a/blame.c
+++ b/blame.c
@@ -166,7 +166,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
commit->date = now;
parent_tail = >parents;
 
-   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, 
NULL))
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, _oid, NULL))
die("no such ref: HEAD");
 
parent_tail = append_parent(parent_tail, _oid);
@@ -1689,7 +1689,7 @@ static struct commit *dwim_reverse_initial(struct 
rev_info *revs,
return NULL;
 
/* Do we have HEAD? */
-   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, 
NULL))
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, _oid, NULL))
return NULL;
head_commit = lookup_commit_reference_gently(_oid, 1);
if (!head_commit)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 56afe405b8..5f91116d73 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -555,7 +555,7 @@ static int fsck_head_link(void)
if (verbose)
fprintf(stderr, "Checking HEAD link\n");
 
-   head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid.hash, NULL);
+   head_points_at = resolve_ref_unsafe("HEAD", 0, _oid, NULL);
if (!head_points_at) {
errors_found |= ERROR_REFS;
return error("Invalid HEAD");
diff --git a/refs.c b/refs.c
index 2fbe9942e2..a030937f77 100644
--- a/refs.c
+++ b/refs.c
@@ -199,7 +199,7 @@ char *refs_resolve_refdup(struct ref_store *refs,
const char *result;
 
result = refs_resolve_ref_unsafe(refs, refname, resolve_flags,
-oid->hash, flags);
+oid, flags);
return xstrdup_or_null(result);
 }
 
@@ -221,7 +221,7 @@ struct ref_filter {
 int refs_read_ref_full(struct ref_store *refs, const char *refname,
   int resolve_flags, struct object_id *oid, int *flags)
 {
-   if (refs_resolve_ref_unsafe(refs, refname, resolve_flags, oid->hash, 
flags))
+   if (refs_resolve_ref_unsafe(refs, refname, resolve_flags, oid, flags))
return 0;
return -1;
 }
@@ -480,7 +480,7 @@ int expand_ref(const char *str, int len, struct object_id 
*oid, char **ref)
strbuf_reset();
strbuf_addf(, *p, len, str);
r = resolve_ref_unsafe(fullref.buf, RESOLVE_REF_READING,
-  this_result->hash, );
+  this_result, );
if (r) {
if (!refs_found++)
*ref = xstrdup(r);
@@ -511,7 +511,7 @@ int dwim_log(const char *str, int len, struct object_id 
*oid, char **log)
strbuf_reset();
strbuf_addf(, *p, len, str);
ref = resolve_ref_unsafe(path.buf, RESOLVE_REF_READING,
-hash.hash, NULL);
+, NULL);
if (!ref)
continue;
if (reflog_exists(path.buf))
@@ -1384,15 +1384,15 @@ int refs_read_raw_ref(struct ref_store *ref_store,
 const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
int resolve_flags,
-   unsigned char *sha1, int *flags)
+   struct object_id *oid, int *flags)
 {
static struct strbuf sb_refname = STRBUF_INIT;
struct object_id unused_oid;
int unused_flags;
int symref_count;
 
-   if (!sha1)
-   sha1 = unused_oid.hash;
+   if (!oid)
+   oid = _oid;
if (!flags)
flags = _flags;
 
@@ -1420,11 +1420,11 @@ const char *refs_resolve_ref_unsafe(struct ref_store 
*refs,
unsigned int read_flags = 0;
 
if

[PATCH v2 18/24] Convert remaining callers of resolve_gitlink_ref to object_id

2017-10-08 Thread brian m. carlson 
Signed-off-by: brian m. carlson 
---
 diff-lib.c | 4 ++--
 dir.c  | 8 
 read-cache.c   | 6 +++---
 unpack-trees.c | 8 
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 4e0980caa8..af4f1b7865 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -36,7 +36,7 @@ static int check_removed(const struct cache_entry *ce, struct 
stat *st)
if (has_symlink_leading_path(ce->name, ce_namelen(ce)))
return 1;
if (S_ISDIR(st->st_mode)) {
-   unsigned char sub[20];
+   struct object_id sub;
 
/*
 * If ce is already a gitlink, we can have a plain
@@ -50,7 +50,7 @@ static int check_removed(const struct cache_entry *ce, struct 
stat *st)
 * a directory --- the blob was removed!
 */
if (!S_ISGITLINK(ce->ce_mode) &&
-   resolve_gitlink_ref(ce->name, "HEAD", sub))
+   resolve_gitlink_ref(ce->name, "HEAD", sub.hash))
return 1;
}
return 0;
diff --git a/dir.c b/dir.c
index 1d17b800cf..a4198ba7fd 100644
--- a/dir.c
+++ b/dir.c
@@ -1390,8 +1390,8 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
break;
if (!(dir->flags & DIR_NO_GITLINKS)) {
-   unsigned char sha1[20];
-   if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
+   struct object_id oid;
+   if (resolve_gitlink_ref(dirname, "HEAD", oid.hash) == 0)
return path_untracked;
}
return path_recurse;
@@ -2279,10 +2279,10 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
int ret = 0, original_len = path->len, len, kept_down = 0;
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
-   unsigned char submodule_head[20];
+   struct object_id submodule_head;
 
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
-   !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
+   !resolve_gitlink_ref(path->buf, "HEAD", submodule_head.hash)) {
/* Do not descend and nuke a nested git work tree. */
if (kept_up)
*kept_up = 1;
diff --git a/read-cache.c b/read-cache.c
index 65f4fe8375..131485b3a6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -191,7 +191,7 @@ static int ce_compare_link(const struct cache_entry *ce, 
size_t expected_size)
 
 static int ce_compare_gitlink(const struct cache_entry *ce)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
 
/*
 * We don't actually require that the .git directory
@@ -201,9 +201,9 @@ static int ce_compare_gitlink(const struct cache_entry *ce)
 *
 * If so, we consider it always to match.
 */
-   if (resolve_gitlink_ref(ce->name, "HEAD", sha1) < 0)
+   if (resolve_gitlink_ref(ce->name, "HEAD", oid.hash) < 0)
return 0;
-   return hashcmp(sha1, ce->oid.hash);
+   return oidcmp(, >oid);
 }
 
 static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st)
diff --git a/unpack-trees.c b/unpack-trees.c
index 71b70ccb12..0dc76eddfe 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1541,15 +1541,15 @@ static int verify_clean_subdirectory(const struct 
cache_entry *ce,
int cnt = 0;
 
if (S_ISGITLINK(ce->ce_mode)) {
-   unsigned char sha1[20];
-   int sub_head = resolve_gitlink_ref(ce->name, "HEAD", sha1);
+   struct object_id oid;
+   int sub_head = resolve_gitlink_ref(ce->name, "HEAD", oid.hash);
/*
 * If we are not going to update the submodule, then
 * we don't care.
 */
-   if (!sub_head && !hashcmp(sha1, ce->oid.hash))
+   if (!sub_head && !oidcmp(, >oid))
return 0;
-   return verify_clean_submodule(sub_head ? NULL : 
sha1_to_hex(sha1),
+   return verify_clean_submodule(sub_head ? NULL : 
oid_to_hex(),
  ce, error_type, o);
}
 
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 19/24] refs: convert resolve_gitlink_ref to struct object_id

2017-10-08 Thread brian m. carlson 
Convert the declaration and definition of resolve_gitlink_ref to use
struct object_id and apply the following semantic patch:

@@
expression E1, E2, E3;
@@
- resolve_gitlink_ref(E1, E2, E3.hash)
+ resolve_gitlink_ref(E1, E2, )

@@
expression E1, E2, E3;
@@
- resolve_gitlink_ref(E1, E2, E3->hash)
+ resolve_gitlink_ref(E1, E2, E3)

Signed-off-by: brian m. carlson 
---
 builtin/update-index.c | 4 ++--
 combine-diff.c | 2 +-
 diff-lib.c | 2 +-
 dir.c  | 4 ++--
 read-cache.c   | 2 +-
 refs.c | 6 +++---
 refs.h | 2 +-
 sha1_file.c| 2 +-
 unpack-trees.c | 2 +-
 9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 24f4b28951..fefbe60167 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -328,7 +328,7 @@ static int process_directory(const char *path, int len, 
struct stat *st)
if (S_ISGITLINK(ce->ce_mode)) {
 
/* Do nothing to the index if there is no HEAD! */
-   if (resolve_gitlink_ref(path, "HEAD", oid.hash) < 0)
+   if (resolve_gitlink_ref(path, "HEAD", ) < 0)
return 0;
 
return add_one_path(ce, path, len, st);
@@ -354,7 +354,7 @@ static int process_directory(const char *path, int len, 
struct stat *st)
}
 
/* No match - should we add it as a gitlink? */
-   if (!resolve_gitlink_ref(path, "HEAD", oid.hash))
+   if (!resolve_gitlink_ref(path, "HEAD", ))
return add_one_path(NULL, path, len, st);
 
/* Error out. */
diff --git a/combine-diff.c b/combine-diff.c
index 9e163d5ada..82f6070977 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1014,7 +1014,7 @@ static void show_patch_diff(struct combine_diff_path 
*elem, int num_parent,
elem->mode = canon_mode(st.st_mode);
} else if (S_ISDIR(st.st_mode)) {
struct object_id oid;
-   if (resolve_gitlink_ref(elem->path, "HEAD", oid.hash) < 
0)
+   if (resolve_gitlink_ref(elem->path, "HEAD", ) < 0)
result = grab_blob(>oid, elem->mode,
   _size, NULL, NULL);
else
diff --git a/diff-lib.c b/diff-lib.c
index af4f1b7865..d2ea02f4d7 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -50,7 +50,7 @@ static int check_removed(const struct cache_entry *ce, struct 
stat *st)
 * a directory --- the blob was removed!
 */
if (!S_ISGITLINK(ce->ce_mode) &&
-   resolve_gitlink_ref(ce->name, "HEAD", sub.hash))
+   resolve_gitlink_ref(ce->name, "HEAD", ))
return 1;
}
return 0;
diff --git a/dir.c b/dir.c
index a4198ba7fd..f09a31e102 100644
--- a/dir.c
+++ b/dir.c
@@ -1391,7 +1391,7 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
break;
if (!(dir->flags & DIR_NO_GITLINKS)) {
struct object_id oid;
-   if (resolve_gitlink_ref(dirname, "HEAD", oid.hash) == 0)
+   if (resolve_gitlink_ref(dirname, "HEAD", ) == 0)
return path_untracked;
}
return path_recurse;
@@ -2282,7 +2282,7 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
struct object_id submodule_head;
 
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
-   !resolve_gitlink_ref(path->buf, "HEAD", submodule_head.hash)) {
+   !resolve_gitlink_ref(path->buf, "HEAD", _head)) {
/* Do not descend and nuke a nested git work tree. */
if (kept_up)
*kept_up = 1;
diff --git a/read-cache.c b/read-cache.c
index 131485b3a6..7766196aff 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -201,7 +201,7 @@ static int ce_compare_gitlink(const struct cache_entry *ce)
 *
 * If so, we consider it always to match.
 */
-   if (resolve_gitlink_ref(ce->name, "HEAD", oid.hash) < 0)
+   if (resolve_gitlink_ref(ce->name, "HEAD", ) < 0)
return 0;
return oidcmp(, >oid);
 }
diff --git a/refs.c b/refs.c
index 9ddf7fcf7d..2fbe9942e2 100644
--- a/refs.c
+++ b/refs.c
@@ -1476,7 +1476,7 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags,
 }
 
 int resolve_gitlink_ref(const char *submodule, const char *refname,
-   unsigned char *sha1)
+   struct object_id *oid)
 {
struct ref_store *refs;
int flags;
@@ -1486,8 +1486,8 @@ int resolve_gitlink_ref(const char *submodule, const char 
*refname,
if (!refs)
return -1;
 
-   if

[PATCH v2 22/24] refs: convert peel_object to struct object_id

2017-10-08 Thread brian m. carlson 
Signed-off-by: brian m. carlson 
---
 refs.c| 8 
 refs/files-backend.c  | 2 +-
 refs/packed-backend.c | 4 ++--
 refs/ref-cache.c  | 2 +-
 refs/refs-internal.h  | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/refs.c b/refs.c
index a030937f77..958234e7d6 100644
--- a/refs.c
+++ b/refs.c
@@ -252,12 +252,12 @@ static int filter_refs(const char *refname, const struct 
object_id *oid,
return filter->fn(refname, oid, flags, filter->cb_data);
 }
 
-enum peel_status peel_object(const unsigned char *name, unsigned char *sha1)
+enum peel_status peel_object(const struct object_id *name, struct object_id 
*oid)
 {
-   struct object *o = lookup_unknown_object(name);
+   struct object *o = lookup_unknown_object(name->hash);
 
if (o->type == OBJ_NONE) {
-   int type = sha1_object_info(name, NULL);
+   int type = sha1_object_info(name->hash, NULL);
if (type < 0 || !object_as_type(o, type, 0))
return PEEL_INVALID;
}
@@ -269,7 +269,7 @@ enum peel_status peel_object(const unsigned char *name, 
unsigned char *sha1)
if (!o)
return PEEL_INVALID;
 
-   hashcpy(sha1, o->oid.hash);
+   oidcpy(oid, >oid);
return PEEL_PEELED;
 }
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 61f3690299..417c662d5d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -675,7 +675,7 @@ static int files_peel_ref(struct ref_store *ref_store,
!refs_peel_ref(refs->packed_ref_store, refname, oid))
return 0;
 
-   return peel_object(base.hash, oid->hash);
+   return peel_object(, oid);
 }
 
 struct files_ref_iterator {
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index a450efd21e..25e6fc4ffe 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -716,8 +716,8 @@ static int write_with_updates(struct packed_ref_store *refs,
i++;
} else {
struct object_id peeled;
-   int peel_error = peel_object(update->new_oid.hash,
-peeled.hash);
+   int peel_error = peel_object(>new_oid,
+);
 
if (write_packed_entry(out, update->refname,
   update->new_oid.hash,
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 76bb723c86..e36702ed06 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -509,7 +509,7 @@ enum peel_status peel_entry(struct ref_entry *entry, int 
repeel)
if (entry->flag & REF_ISSYMREF)
return PEEL_IS_SYMREF;
 
-   status = peel_object(entry->u.value.oid.hash, 
entry->u.value.peeled.hash);
+   status = peel_object(>u.value.oid, >u.value.peeled);
if (status == PEEL_PEELED || status == PEEL_NON_TAG)
entry->flag |= REF_KNOWS_PEELED;
return status;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 0cbce76f21..cf84da33d5 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -120,11 +120,11 @@ enum peel_status {
 /*
  * Peel the named object; i.e., if the object is a tag, resolve the
  * tag recursively until a non-tag is found.  If successful, store the
- * result to sha1 and return PEEL_PEELED.  If the object is not a tag
+ * result to oid and return PEEL_PEELED.  If the object is not a tag
  * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
  * and leave sha1 unchanged.
  */
-enum peel_status peel_object(const unsigned char *name, unsigned char *sha1);
+enum peel_status peel_object(const struct object_id *name, struct object_id 
*oid);
 
 /*
  * Copy the reflog message msg to buf, which has been allocated sufficiently
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 00/24] object_id part 10

2017-10-08 Thread brian m. carlson 
This is the tenth in a series of patches to convert from unsigned char
[20] to struct object_id.  This series mostly involves changes to the
refs code.  After these changes, there are almost no references to
unsigned char in the main refs code.

The series has not been rebased on master since the last submission, but
I can do so if that's more convenient.

This series is available from the following URL:
https://github.com/bk2204/git.git object-id-part10

Changes from v1:
* Fix line wrapping in several places.
* Remove empty line.
* Update die messages to refer to "object ID" instead of "sha1".

brian m. carlson (24):
  walker: convert to struct object_id
  refs/files-backend: convert struct ref_to_prune to object_id
  refs: convert delete_ref and refs_delete_ref to struct object_id
  refs: convert update_ref and refs_update_ref to use struct object_id
  refs: update ref transactions to use struct object_id
  Convert check_connected to use struct object_id
  refs: convert resolve_refdup and refs_resolve_refdup to struct
object_id
  refs: convert read_ref and read_ref_full to object_id
  refs: convert dwim_ref and expand_ref to struct object_id
  builtin/reflog: convert remaining unsigned char uses to object_id
  refs: convert dwim_log to struct object_id
  pack-bitmap: convert traverse_bitmap_commit_list to object_id
  builtin/pack-objects: convert to struct object_id
  refs: convert peel_ref to struct object_id
  refs: convert read_ref_at to struct object_id
  refs: convert reflog_expire parameter to struct object_id
  sha1_file: convert index_path and index_fd to struct object_id
  Convert remaining callers of resolve_gitlink_ref to object_id
  refs: convert resolve_gitlink_ref to struct object_id
  worktree: convert struct worktree to object_id
  refs: convert resolve_ref_unsafe to struct object_id
  refs: convert peel_object to struct object_id
  refs: convert read_raw_ref backends to struct object_id
  refs/files-backend: convert static functions to object_id

 archive.c   |   2 +-
 bisect.c|   5 +-
 blame.c |   4 +-
 branch.c|   4 +-
 builtin/am.c|  16 ++--
 builtin/branch.c|   8 +-
 builtin/checkout.c  |  13 ++-
 builtin/clone.c |  22 ++---
 builtin/commit.c|   4 +-
 builtin/describe.c  |   2 +-
 builtin/fast-export.c   |   2 +-
 builtin/fetch.c |   8 +-
 builtin/fmt-merge-msg.c |   2 +-
 builtin/fsck.c  |   2 +-
 builtin/log.c   |   2 +-
 builtin/merge-base.c|   2 +-
 builtin/merge.c |  17 ++--
 builtin/notes.c |  12 +--
 builtin/pack-objects.c  | 139 ++-
 builtin/pull.c  |   2 +-
 builtin/receive-pack.c  |  16 ++--
 builtin/reflog.c|  18 ++--
 builtin/remote.c|   2 +-
 builtin/replace.c   |   8 +-
 builtin/reset.c |   6 +-
 builtin/rev-list.c  |   4 +-
 builtin/rev-parse.c |   2 +-
 builtin/show-branch.c   |  12 +--
 builtin/show-ref.c  |   4 +-
 builtin/submodule--helper.c |   2 +-
 builtin/tag.c   |   8 +-
 builtin/update-index.c  |  10 +-
 builtin/update-ref.c|  12 +--
 builtin/worktree.c  |   6 +-
 bundle.c|   4 +-
 combine-diff.c  |   2 +-
 connected.c |  18 ++--
 connected.h |   4 +-
 diff-lib.c  |   4 +-
 dir.c   |   8 +-
 fast-import.c   |   6 +-
 notes-cache.c   |   4 +-
 notes-merge.c   |   2 +-
 notes-utils.c   |   4 +-
 notes.c |   2 +-
 pack-bitmap.c   |   8 +-
 pack-bitmap.h   |   2 +-
 read-cache.c|   6 +-
 ref-filter.c|   4 +-
 reflog-walk.c   |   6 +-
 refs.c  | 229 +---
 refs.h  |  75 +++
 refs/files-backend.c| 118 +++
 refs/packed-backend.c   |  14 +--
 refs/ref-cache.c|   2 +-
 refs/refs-internal.h|  22 ++---
 remote-testsvn.c|   2 +-
 remote.c|   9 +-
 sequencer.c |  15 ++-
 sha1_file.c |  32 +++
 sha1_name.c |  10 +-
 submodule.c |   2 +-
 t/helper/test-ref-store.c   |  28 +++---
 transport-helper.c  |  15 ++-
 transport.c |   6 +-
 unpack-trees.c  |   8 +-
 upload-pack.c   |   4 +-
 walker.c|  24 ++---
 worktree.c  |   2 +-
 worktree.h  |   2 +-
 wt-status.c |   4 +-
 71 files changed, 533 insertions(+), 552 deletions(-)

-- 
2.14.2.920.gcf0c67979c

[PATCH v2 02/24] refs/files-backend: convert struct ref_to_prune to object_id

2017-10-08 Thread brian m. carlson 
Change the member of this struct to be a struct object_id.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fec77744b4..e3968d4f7c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -961,7 +961,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct 
files_ref_store *refs,
 
 struct ref_to_prune {
struct ref_to_prune *next;
-   unsigned char sha1[20];
+   struct object_id oid;
char name[FLEX_ARRAY];
 };
 
@@ -1029,7 +1029,7 @@ static void prune_ref(struct files_ref_store *refs, 
struct ref_to_prune *r)
 
transaction = ref_store_transaction_begin(>base, );
if (!transaction ||
-   ref_transaction_delete(transaction, r->name, r->sha1,
+   ref_transaction_delete(transaction, r->name, r->oid.hash,
   REF_ISPRUNING | REF_NODEREF, NULL, ) ||
ref_transaction_commit(transaction, )) {
ref_transaction_free(transaction);
@@ -1123,7 +1123,7 @@ static int files_pack_refs(struct ref_store *ref_store, 
unsigned int flags)
if ((flags & PACK_REFS_PRUNE)) {
struct ref_to_prune *n;
FLEX_ALLOC_STR(n, name, iter->refname);
-   hashcpy(n->sha1, iter->oid->hash);
+   oidcpy(>oid, iter->oid);
n->next = refs_to_prune;
refs_to_prune = n;
}
-- 
2.14.2.920.gcf0c67979c

[PATCH v2 03/24] refs: convert delete_ref and refs_delete_ref to struct object_id

2017-10-08 Thread brian m. carlson 
Convert delete_ref and refs_delete_ref to take a pointer to struct
object_id.  Update the documentation accordingly, including referring to
null_oid in lowercase, as it is not a #define constant.

Signed-off-by: brian m. carlson 
---
 builtin/branch.c  |  2 +-
 builtin/replace.c |  2 +-
 builtin/reset.c   |  2 +-
 builtin/tag.c |  2 +-
 builtin/update-ref.c  |  2 +-
 refs.c| 21 +++--
 refs.h| 12 ++--
 refs/files-backend.c  |  2 +-
 t/helper/test-ref-store.c |  6 +++---
 9 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 355f9ef5da..6031b74d68 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -256,7 +256,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
goto next;
}
 
-   if (delete_ref(NULL, name, is_null_oid() ? NULL : oid.hash,
+   if (delete_ref(NULL, name, is_null_oid() ? NULL : ,
   REF_NODEREF)) {
error(remote_branch
  ? _("Error deleting remote-tracking branch '%s'")
diff --git a/builtin/replace.c b/builtin/replace.c
index 3e71a77152..2854eaa0f3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -128,7 +128,7 @@ static int for_each_replace_name(const char **argv, 
each_replace_name_fn fn)
 static int delete_replace_ref(const char *name, const char *ref,
  const struct object_id *oid)
 {
-   if (delete_ref(NULL, ref, oid->hash, 0))
+   if (delete_ref(NULL, ref, oid, 0))
return 1;
printf("Deleted replace ref '%s'\n", name);
return 0;
diff --git a/builtin/reset.c b/builtin/reset.c
index 9cd89b2305..5f3632e05b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -269,7 +269,7 @@ static int reset_refs(const char *rev, const struct 
object_id *oid)
update_ref_oid(msg.buf, "ORIG_HEAD", orig, old_orig, 0,
   UPDATE_REFS_MSG_ON_ERR);
} else if (old_orig)
-   delete_ref(NULL, "ORIG_HEAD", old_orig->hash, 0);
+   delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
set_reflog_message(, "updating HEAD", rev);
update_ref_status = update_ref_oid(msg.buf, "HEAD", oid, orig, 0,
   UPDATE_REFS_MSG_ON_ERR);
diff --git a/builtin/tag.c b/builtin/tag.c
index c627794181..46ff4ca736 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -97,7 +97,7 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn,
 static int delete_tag(const char *name, const char *ref,
  const struct object_id *oid, const void *cb_data)
 {
-   if (delete_ref(NULL, ref, oid->hash, 0))
+   if (delete_ref(NULL, ref, oid, 0))
return 1;
printf(_("Deleted tag '%s' (was %s)\n"), name, 
find_unique_abbrev(oid->hash, DEFAULT_ABBREV));
return 0;
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6b90c5dead..bf0f80ebae 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -434,7 +434,7 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
 * NULL_SHA1 as "don't care" here:
 */
return delete_ref(msg, refname,
- (oldval && !is_null_oid()) ? 
oldoid.hash : NULL,
+ (oldval && !is_null_oid()) ?  : 
NULL,
  flags);
else
return update_ref(msg, refname, oid.hash, oldval ? oldoid.hash 
: NULL,
diff --git a/refs.c b/refs.c
index 6042645c40..0a5b68d6fb 100644
--- a/refs.c
+++ b/refs.c
@@ -620,25 +620,25 @@ static int write_pseudoref(const char *pseudoref, const 
unsigned char *sha1,
return ret;
 }
 
-static int delete_pseudoref(const char *pseudoref, const unsigned char 
*old_sha1)
+static int delete_pseudoref(const char *pseudoref, const struct object_id 
*old_oid)
 {
static struct lock_file lock;
const char *filename;
 
filename = git_path("%s", pseudoref);
 
-   if (old_sha1 && !is_null_sha1(old_sha1)) {
+   if (old_oid && !is_null_oid(old_oid)) {
int fd;
-   unsigned char actual_old_sha1[20];
+   struct object_id actual_old_oid;
 
fd = hold_lock_file_for_update_timeout(
, filename, LOCK_DIE_ON_ERROR,
get_files_ref_lock_timeout_ms());
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), 
filename);
-   if (read_ref(pseudoref, actual_old_sha1))
+   if (read_ref(pseudoref, actual_old_oid.hash))
die("could not read ref '%s'", pseudoref);
-   if (hashcmp(actual_old_sha1,

[PATCH v2 01/24] walker: convert to struct object_id

2017-10-08 Thread brian m. carlson 
Signed-off-by: brian m. carlson 
---
 walker.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/walker.c b/walker.c
index 274f1a4935..2d83254bc0 100644
--- a/walker.c
+++ b/walker.c
@@ -7,7 +7,7 @@
 #include "blob.h"
 #include "refs.h"
 
-static unsigned char current_commit_sha1[20];
+static struct object_id current_commit_oid;
 
 void walker_say(struct walker *walker, const char *fmt, ...)
 {
@@ -24,9 +24,9 @@ static void report_missing(const struct object *obj)
fprintf(stderr, "Cannot obtain needed %s %s\n",
obj->type ? typename(obj->type): "object",
oid_to_hex(>oid));
-   if (!is_null_sha1(current_commit_sha1))
+   if (!is_null_oid(_commit_oid))
fprintf(stderr, "while processing commit %s.\n",
-   sha1_to_hex(current_commit_sha1));
+   oid_to_hex(_commit_oid));
 }
 
 static int process(struct walker *walker, struct object *obj);
@@ -82,7 +82,7 @@ static int process_commit(struct walker *walker, struct 
commit *commit)
if (commit->object.flags & COMPLETE)
return 0;
 
-   hashcpy(current_commit_sha1, commit->object.oid.hash);
+   oidcpy(_commit_oid, >object.oid);
 
walker_say(walker, "walk %s\n", oid_to_hex(>object.oid));
 
@@ -187,14 +187,14 @@ static int loop(struct walker *walker)
return 0;
 }
 
-static int interpret_target(struct walker *walker, char *target, unsigned char 
*sha1)
+static int interpret_target(struct walker *walker, char *target, struct 
object_id *oid)
 {
-   if (!get_sha1_hex(target, sha1))
+   if (!get_oid_hex(target, oid))
return 0;
if (!check_refname_format(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
-   hashcpy(sha1, ref->old_oid.hash);
+   oidcpy(oid, >old_oid);
free(ref);
return 0;
}
@@ -259,7 +259,7 @@ int walker_fetch(struct walker *walker, int targets, char 
**target,
struct strbuf refname = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction = NULL;
-   unsigned char *sha1 = xmalloc(targets * 20);
+   struct object_id *oids = xmalloc(targets * sizeof(struct object_id));
char *msg = NULL;
int i, ret = -1;
 
@@ -279,11 +279,11 @@ int walker_fetch(struct walker *walker, int targets, char 
**target,
}
 
for (i = 0; i < targets; i++) {
-   if (interpret_target(walker, target[i], [20 * i])) {
+   if (interpret_target(walker, target[i], oids + i)) {
error("Could not interpret response from server '%s' as 
something to pull", target[i]);
goto done;
}
-   if (process(walker, lookup_unknown_object([20 * i])))
+   if (process(walker, lookup_unknown_object(oids[i].hash)))
goto done;
}
 
@@ -304,7 +304,7 @@ int walker_fetch(struct walker *walker, int targets, char 
**target,
strbuf_reset();
strbuf_addf(, "refs/%s", write_ref[i]);
if (ref_transaction_update(transaction, refname.buf,
-  [20 * i], NULL, 0,
+  oids[i].hash, NULL, 0,
   msg ? msg : "fetch (unknown)",
   )) {
error("%s", err.buf);
@@ -321,7 +321,7 @@ int walker_fetch(struct walker *walker, int targets, char 
**target,
 done:
ref_transaction_free(transaction);
free(msg);
-   free(sha1);
+   free(oids);
strbuf_release();
strbuf_release();
return ret;
-- 
2.14.2.920.gcf0c67979c

Re: [PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread brian m. carlson 
On Sun, Oct 08, 2017 at 10:32:35AM +0100, Philip Oakley wrote:
> From: "Martin Ågren" 
> > - die(_("submodule entry '%s' (%s) is a %s, not a commit"),
> > - cb->path, oid_to_hex(oid), typename(type));
> > + die(_("submodule entry '%s' (%s) is not a commit"),
> > + cb->path, oid_to_hex(oid));
> Bikeshed,
> maybe:
> "submodule entry '%s' (%s) is not a commit. It is a %s"
> This puts the two parts in separate sentences?

Languages with multiple grammatical genders are going to have problems
with this.  In French, "a tree" is "un arbre" (masculine), but "a tag"
is "une étiquette" (feminine).  We don't currently have a Spanish
translation, but this would break there as well.

Splitting the article out with the type name is still problematic for
languages where articles vary by case, like German, since the
translation might be reused in another place requiring a different case.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

GOOD DAY

2017-10-08 Thread Unistar Credit And Finance Corporation 
CONTACT US TODAY FOR BUSINESS OR PERSONAL LOAN.

Re: [PATCH v1 2/2] entry.c: check if file exists after checkout

2017-10-08 Thread Lars Schneider 

> On 06 Oct 2017, at 06:56, Jeff King  wrote:
> 
> On Fri, Oct 06, 2017 at 01:26:48PM +0900, Junio C Hamano wrote:
> 
> ...
>> -- >8 --
>> From: Lars Schneider 
>> Date: Thu, 5 Oct 2017 12:44:07 +0200
>> Subject: [PATCH] entry.c: check if file exists after checkout
>> 
>> If we are checking out a file and somebody else racily deletes our file,
>> then we would write garbage to the cache entry. Fix that by checking
>> the result of the lstat() call on that file. Print an error to the user
>> if the file does not exist.
> 
> I don't know if we wanted to capture any of the reasoning behind using
> error() here or not. Frankly, I'm not sure how to argue for it
> succinctly. :) I'm happy with letting it live on in the list archive.
> 
>> diff --git a/entry.c b/entry.c
>> index f879758c73..6d9de3a5aa 100644
>> --- a/entry.c
>> +++ b/entry.c
>> @@ -341,7 +341,9 @@ static int write_entry(struct cache_entry *ce,
>>  if (state->refresh_cache) {
>>  assert(state->istate);
>>  if (!fstat_done)
>> -lstat(ce->name, );
>> +if (lstat(ce->name, ) < 0)
>> +return error_errno("unable stat just-written 
>> file %s",
>> +   ce->name);
> 
> s/unable stat/unable to stat/, I think.
> 
> Other than that, this looks fine to me.
> 
> -Peff

Looks fine to me, too.

Thanks,
Lars

Re: [PATCH v1 1/2] entry.c: update cache entry only for existing files

2017-10-08 Thread Lars Schneider 

> On 06 Oct 2017, at 06:54, Jeff King  wrote:
> 
> On Fri, Oct 06, 2017 at 08:01:48AM +0900, Junio C Hamano wrote:
> 
>>> But
>>> I think we'd want to protect the read_blob_entry() call at the top of
>>> the case with a check for dco->state == CE_RETRY.
>> 
>> Yeah, I think that makes more sense.
>> 
>> A patch may look like this on top of these two patches, but I'd
>> prefer to see Lars's eyeballing and possibly wrapping it up in an
>> applicable patch after taking the authorship.
> 

This looks all good to me. Thank you!
A few minor style suggestions below.


> ...
> 
> The "structured" way, of course, would be to put everything under
> write_out_file into a helper function and just call it from both places
> rather than relying on a spaghetti of gotos and switch-breaks.
> 
> I'm OK with whatever structure we end up with, as long as it fixes the
> leak (and ideally the pessimization).
> 
> Anyway, here's the real patch in case anybody wants to apply it and play
> with it further.
> 
> -- >8 --
> diff --git a/entry.c b/entry.c
> index 1c7e3c11d5..d28b42d82d 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -261,6 +261,7 @@ static int write_entry(struct cache_entry *ce,
>   size_t newsize = 0;
>   struct stat st;
>   const struct submodule *sub;
> + struct delayed_checkout *dco = state->delayed_checkout;
> 
>   if (ce_mode_s_ifmt == S_IFREG) {
>   struct stream_filter *filter = get_stream_filter(ce->name,
> @@ -273,55 +274,61 @@ static int write_entry(struct cache_entry *ce,
>   }
> 
>   switch (ce_mode_s_ifmt) {
> - case S_IFREG:
>   case S_IFLNK:
>   new = read_blob_entry(ce, );
>   if (!new)
>   return error("unable to read sha1 file of %s (%s)",
>   path, oid_to_hex(>oid));
> 
> - if (ce_mode_s_ifmt == S_IFLNK && has_symlinks && !to_tempfile) {
> - ret = symlink(new, path);
> - free(new);
> - if (ret)
> - return error_errno("unable to create symlink 
> %s",
> -path);

Nit: This could go into one line now.


> - break;
> - }
> + /* fallback to handling it like a regular file if we must */
> + if (!has_symlinks || to_tempfile)
> + goto write_out_file;
> 
> + ret = symlink(new, path);
> + free(new);
> + if (ret)
> + return error_errno("unable to create symlink %s",
> +path);
> + break;
> +
> + case S_IFREG:
>   /*
>* Convert from git internal format to working tree format
>*/
> - if (ce_mode_s_ifmt == S_IFREG) {
> - struct delayed_checkout *dco = state->delayed_checkout;
> - if (dco && dco->state != CE_NO_DELAY) {
> - /* Do not send the blob in case of a retry. */
> - if (dco->state == CE_RETRY) {

Maybe we could add here something like:
/* The filer process got the blob already in case of a retry. 
Unnecessary to send it, again! */

> - new = NULL;
> - size = 0;
> - }
> - ret = async_convert_to_working_tree(
> - ce->name, new, size, , dco);

Nit: This could go into one line now.


> - if (ret && string_list_has_string(>paths, 
> ce->name)) {
> - free(new);
> - goto finish;
> - }
> - } else
> - ret = convert_to_working_tree(
> - ce->name, new, size, );

Nit: This could go into one line now.


> 
> - if (ret) {
> + if (dco && dco->state == CE_RETRY) {
> + new = NULL;
> + size = 0;
> + } else {
> + new = read_blob_entry(ce, );
> + if (!new)
> + return error ("unable to read sha1 file of %s 
> (%s)",
> +   path, oid_to_hex(>oid));
> + }
> +
> + if (dco && dco->state != CE_NO_DELAY) {
> + ret = async_convert_to_working_tree(
> + ce->name, new, 
> size, , dco);
> + if (ret && string_list_has_string(>paths, 
> ce->name)) {
>   free(new);
> - new = strbuf_detach(, );
> - size = newsize;
> + goto finish;
>

Re: Special strings in commit messages

2017-10-08 Thread Eric Wong 
Florian Weimer  wrote:
> Based on strace output, something in git rebase calls git mailsplit, and it
> probably sees the "\nFrom " string and treats it as a start of a new mail
> message, and things go downhill from there.
> 
> I will escape "\nFrom " in commit messages (probably as "\n.From " or maybe
> "\n>From ", plus escaping for "\n."/"\n>" to make the encoding reversible),
> but I wonder if there is something else I need to escape while I'm at it.

I suppose it's safe to start using mboxrd internally when
there's little danger of mixing different git versions.

Totally untested (but passes "make test"), can you try this?

-8<--
Subject: [PATCH] rebase: use mboxrd format to avoid split errors

The mboxrd format allows the use of embedded "From " lines in
commit messages without being misinterpreted by mailsplit

Reported-by: Florian Weimer 
Signed-off-by: Eric Wong 
---
 git-rebase--am.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 6e64d40d6f..14c50782e0 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -53,6 +53,7 @@ else
 
git format-patch -k --stdout --full-index --cherry-pick --right-only \
--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+   --pretty=mboxrd \
$git_format_patch_opt \
"$revisions" ${restrict_revision+^$restrict_revision} \
>"$GIT_DIR/rebased-patches"
@@ -83,6 +84,7 @@ else
fi
 
git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+   --patch-format=mboxrd \
$allow_rerere_autoupdate \
${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
ret=$?
-- 
EW

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Theodore Ts'o 
On Sun, Oct 08, 2017 at 03:44:14PM -0400, Robert P. J. Day wrote:
> >
> > find  | xargs git rm
> >
> > myself.
> 
>   that's what i would have normally used until i learned about git's
> magical globbing capabilities, and i'm going to go back to using it,
> because git's magical globbing capabilities now scare me.

Hmm, I wonder if the reason why git's magically globbing capabilities
even exist at all is for those poor benighted souls on Windows, for
which their shell (and associated utilities) doesn't have advanced
tools like "find" and "xargs"

- Ted

$850.000.00 Donation.

2017-10-08 Thread Mark J. Shapiro 
David & Maureen picked you for $850.000.00 Donation, Kindly reply for details 
and claim.

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Robert P. J. Day 
On Sun, 8 Oct 2017, Theodore Ts'o wrote:

> On Sun, Oct 08, 2017 at 10:32:40AM -0400, Paul Smith wrote:
> > Personally I don't use Git's magical globbing capabilities, and
> > use "git rm" as if it were UNIX rm.  So in your request above I'd
> > use:
> >
> >git rm $(find . -name Makefile)
> >
> > which I find simpler.
>
> I have to agree that git's magical globbing capabilities are...
> strange.  (And apologies to Robert for my earlier post; I didn't
> understand what he was complaining about.)  I don't use it either,
> although I tend to use:
>
> find  | xargs git rm
>
> myself.

  that's what i would have normally used until i learned about git's
magical globbing capabilities, and i'm going to go back to using it,
because git's magical globbing capabilities now scare me.

> One thing which is interesting is that not only is the git's magical
> globbing capabilities have somewhat unusual semantics, the how
> globbing is done in .gitignore entries are completely different.

  i know ... it would have made way more sense to try to be
consistent. oh, well, live and learn. at least now i'm aware of the
weirdness.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

[PATCH v4 1/4] p4211-line-log.sh: add log --online --raw --parents perf test

2017-10-08 Thread Derrick Stolee 
Add a new perf test for testing the performance of log while computing
OID abbreviations. Using --oneline --raw and --parents options maximizes
the number of OIDs to abbreviate while still spending some time computing
diffs.

Signed-off-by: Derrick Stolee 
---
 t/perf/p4211-line-log.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh
index b7ff68d4f..e0ed05907 100755
--- a/t/perf/p4211-line-log.sh
+++ b/t/perf/p4211-line-log.sh
@@ -31,4 +31,8 @@ test_perf 'git log -L (renames on)' '
git log -M -L 1:"$file" >/dev/null
 '
 
+test_perf 'git log --oneline --raw --parents' '
+   git log --oneline --raw --parents >/dev/null
+'
+
 test_done
-- 
2.14.1.538.g56ec8fc98.dirty

[PATCH v4 4/4] sha1_name: minimize OID comparisons during disambiguation

2017-10-08 Thread Derrick Stolee 
Minimize OID comparisons during disambiguatiosn of packfile OIDs.

Teach git to use binary search with the full OID to find the object's
position (or insertion position, if not present) in the pack-index.
The object before and immediately after (or the one at the insertion
position) give the maximum common prefix.  No subsequent linear search
is required.

Take care of which two to inspect, in case the object id exists in the
packfile.

If the input to find_unique_abbrev_r() is a partial prefix, then the
OID used for the binary search is padded with zeroes so the object will
not exist in the repo (with high probability) and the same logic
applies.

This commit completes a series of three changes to OID abbreviation
code, and the overall change can be seen using standard commands for
large repos. Below we report performance statistics for perf test 4211.6
from p4211-line-log.sh using three copies of the Linux repo:

| Packs | Loose  | HEAD~3   | HEAD | Rel%  |
|---||--|--|---|
|  1|  0 |  41.27 s |  38.93 s | -4.8% |
| 24|  0 |  98.04 s |  91.35 s | -5.7% |
| 23| 323952 | 117.78 s | 112.18 s | -4.8% |

Signed-off-by: Derrick Stolee 
---
 sha1_name.c | 70 +
 1 file changed, 66 insertions(+), 4 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 5081aeb71..49ba67955 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -478,6 +478,7 @@ struct min_abbrev_data {
unsigned int init_len;
unsigned int cur_len;
char *hex;
+   const unsigned char *hash;
 };
 
 static inline char get_hex_char_from_oid(const struct object_id *oid,
@@ -505,6 +506,65 @@ static int extend_abbrev_len(const struct object_id *oid, 
void *cb_data)
return 0;
 }
 
+static void find_abbrev_len_for_pack(struct packed_git *p,
+struct min_abbrev_data *mad)
+{
+   int match = 0;
+   uint32_t num, last, first = 0;
+   struct object_id oid;
+
+   open_pack_index(p);
+   num = p->num_objects;
+   last = num;
+   while (first < last) {
+   uint32_t mid = first + (last - first) / 2;
+   const unsigned char *current;
+   int cmp;
+
+   current = nth_packed_object_sha1(p, mid);
+   cmp = hashcmp(mad->hash, current);
+   if (!cmp) {
+   match = 1;
+   first = mid;
+   break;
+   }
+   if (cmp > 0) {
+   first = mid + 1;
+   continue;
+   }
+   last = mid;
+   }
+
+   /*
+* first is now the position in the packfile where we would insert
+* mad->hash if it does not exist (or the position of mad->hash if
+* it does exist). Hence, we consider a maximum of three objects
+* nearby for the abbreviation length.
+*/
+   mad->init_len = 0;
+   if (!match) {
+   nth_packed_object_oid(, p, first);
+   extend_abbrev_len(, mad);
+   } else if (first < num - 1) {
+   nth_packed_object_oid(, p, first + 1);
+   extend_abbrev_len(, mad);
+   }
+   if (first > 0) {
+   nth_packed_object_oid(, p, first - 1);
+   extend_abbrev_len(, mad);
+   }
+   mad->init_len = mad->cur_len;
+}
+
+static void find_abbrev_len_packed(struct min_abbrev_data *mad)
+{
+   struct packed_git *p;
+
+   prepare_packed_git();
+   for (p = packed_git; p; p = p->next)
+   find_abbrev_len_for_pack(p, mad);
+}
+
 int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 {
struct disambiguate_state ds;
@@ -536,19 +596,21 @@ int find_unique_abbrev_r(char *hex, const unsigned char 
*sha1, int len)
if (len == GIT_SHA1_HEXSZ || !len)
return GIT_SHA1_HEXSZ;
 
-   if (init_object_disambiguation(hex, len, ) < 0)
-   return -1;
-
mad.init_len = len;
mad.cur_len = len;
mad.hex = hex;
+   mad.hash = sha1;
+
+   find_abbrev_len_packed();
+
+   if (init_object_disambiguation(hex, mad.cur_len, ) < 0)
+   return -1;
 
ds.fn = extend_abbrev_len;
ds.always_call_fn = 1;
ds.cb_data = (void*)
 
find_short_object_filename();
-   find_short_packed_object();
(void)finish_object_disambiguation(, _ret);
 
hex[mad.cur_len] = 0;
-- 
2.14.1.538.g56ec8fc98.dirty

[PATCH v4 3/4] sha1_name: parse less while finding common prefix

2017-10-08 Thread Derrick Stolee 
Create get_hex_char_from_oid() to parse oids one hex character at a
time. This prevents unnecessary copying of hex characters in
extend_abbrev_len() when finding the length of a common prefix.

Signed-off-by: Derrick Stolee 
---
 sha1_name.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index f2a1ebe49..5081aeb71 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -480,13 +480,23 @@ struct min_abbrev_data {
char *hex;
 };
 
+static inline char get_hex_char_from_oid(const struct object_id *oid,
+int pos)
+{
+   static const char hex[] = "0123456789abcdef";
+
+   if ((pos & 1) == 0)
+   return hex[oid->hash[pos >> 1] >> 4];
+   else
+   return hex[oid->hash[pos >> 1] & 0xf];
+}
+
 static int extend_abbrev_len(const struct object_id *oid, void *cb_data)
 {
struct min_abbrev_data *mad = cb_data;
 
-   char *hex = oid_to_hex(oid);
unsigned int i = mad->init_len;
-   while (mad->hex[i] && mad->hex[i] == hex[i])
+   while (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))
i++;
 
if (i < GIT_MAX_RAWSZ && i >= mad->cur_len)
-- 
2.14.1.538.g56ec8fc98.dirty

[PATCH v4 0/4] Improve abbreviation disambiguation

2017-10-08 Thread Derrick Stolee 
Changes since previous version:

* Fixed an overflow error in the binary search. I sent a separate patch
  to fix this error in existing searches; that patch should be applied
  before this one.

* Removed test-list-objects and test-abbrev in favor of a new git log
  test in p4211-line-log.sh. Limited perf numbers for Linux repo are
  given in cover letter and commit 4/4.

* Silently skip packfiles that fail to open with open_pack_index()

Thanks for all the comments from Jeff, Junio, Ramsey, and Stefan!

Thanks,
 Stolee

---

When displaying object ids, we frequently want to see an abbreviation
for easier typing. That abbreviation must be unambiguous among all
object ids.

The current implementation of find_unique_abbrev() performs a loop
checking if each abbreviation length is unambiguous until finding one
that works. This causes multiple round-trips to the disk when starting
with the default abbreviation length (usually 7) but needing up to 12
characters for an unambiguous short-sha. For very large repos, this
effect is pronounced and causes issues with several commands, from
obvious consumers `status` and `log` to less obvious commands such as
`fetch` and `push`.

This patch improves performance by iterating over objects matching the
short abbreviation only once, inspecting each object id, and reporting
the minimum length of an unambiguous abbreviation.

Add a new perf test for testing the performance of log while computing
OID abbreviations. Using --oneline --raw and --parents options maximizes
the number of OIDs to abbreviate while still spending some time
computing diffs. Below we report performance statistics for perf test
4211.6 from p4211-line-log.sh using three copies of the Linux repo:

| Packs | Loose  | Base Time | New Time | Rel%  |
|---||---|--|---|
|  1|  0 |   41.27 s |  38.93 s | -4.8% |
| 24|  0 |   98.04 s |  91.35 s | -5.7% |
| 23| 323952 |  117.78 s | 112.18 s | -4.8% |

Derrick Stolee (4):
  p4211-line-log.sh: add log --online --raw --parents perf test
  sha1_name: Unroll len loop in find_unique_abbrev_r
  sha1_name: Parse less while finding common prefix
  sha1_name: Minimize OID comparisons during disambiguation

 sha1_name.c  | 129 +--
 t/perf/p4211-line-log.sh |   4 ++
 2 files changed, 118 insertions(+), 15 deletions(-)

-- 
2.14.1.538.g56ec8fc98.dirty

[PATCH v4 2/4] sha1_name: unroll len loop in find_unique_abbrev_r

2017-10-08 Thread Derrick Stolee 
Unroll the while loop inside find_unique_abbrev_r to avoid iterating
through all loose objects and packfiles multiple times when the short
name is longer than the predicted length.

Instead, inspect each object that collides with the estimated
abbreviation to find the longest common prefix.

The focus of this change is to refactor the existing method in a way
that clearly does not change the current behavior. In some cases, the
new method is slower than the previous method. Later changes will
correct all performance loss.

Signed-off-by: Derrick Stolee 
---
 sha1_name.c | 57 ++---
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 134ac9742..f2a1ebe49 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -474,10 +474,32 @@ static unsigned msb(unsigned long val)
return r;
 }
 
-int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
+struct min_abbrev_data {
+   unsigned int init_len;
+   unsigned int cur_len;
+   char *hex;
+};
+
+static int extend_abbrev_len(const struct object_id *oid, void *cb_data)
 {
-   int status, exists;
+   struct min_abbrev_data *mad = cb_data;
+
+   char *hex = oid_to_hex(oid);
+   unsigned int i = mad->init_len;
+   while (mad->hex[i] && mad->hex[i] == hex[i])
+   i++;
+
+   if (i < GIT_MAX_RAWSZ && i >= mad->cur_len)
+   mad->cur_len = i + 1;
+
+   return 0;
+}
 
+int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
+{
+   struct disambiguate_state ds;
+   struct min_abbrev_data mad;
+   struct object_id oid_ret;
if (len < 0) {
unsigned long count = approximate_object_count();
/*
@@ -503,19 +525,24 @@ int find_unique_abbrev_r(char *hex, const unsigned char 
*sha1, int len)
sha1_to_hex_r(hex, sha1);
if (len == GIT_SHA1_HEXSZ || !len)
return GIT_SHA1_HEXSZ;
-   exists = has_sha1_file(sha1);
-   while (len < GIT_SHA1_HEXSZ) {
-   struct object_id oid_ret;
-   status = get_short_oid(hex, len, _ret, GET_OID_QUIETLY);
-   if (exists
-   ? !status
-   : status == SHORT_NAME_NOT_FOUND) {
-   hex[len] = 0;
-   return len;
-   }
-   len++;
-   }
-   return len;
+
+   if (init_object_disambiguation(hex, len, ) < 0)
+   return -1;
+
+   mad.init_len = len;
+   mad.cur_len = len;
+   mad.hex = hex;
+
+   ds.fn = extend_abbrev_len;
+   ds.always_call_fn = 1;
+   ds.cb_data = (void*)
+
+   find_short_object_filename();
+   find_short_packed_object();
+   (void)finish_object_disambiguation(, _ret);
+
+   hex[mad.cur_len] = 0;
+   return mad.cur_len;
 }
 
 const char *find_unique_abbrev(const unsigned char *sha1, int len)
-- 
2.14.1.538.g56ec8fc98.dirty

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Theodore Ts'o 
On Sun, Oct 08, 2017 at 10:32:40AM -0400, Paul Smith wrote:
> Personally I don't use Git's magical globbing capabilities, and use "git
> rm" as if it were UNIX rm.  So in your request above I'd use:
> 
>git rm $(find . -name Makefile)
> 
> which I find simpler.

I have to agree that git's magical globbing capabilities
are... strange.  (And apologies to Robert for my earlier post; I
didn't understand what he was complaining about.)  I don't use it
either, although I tend to use:

find  | xargs git rm

myself.

One thing which is interesting is that not only is the git's magical
globbing capabilities have somewhat unusual semantics, the how
globbing is done in .gitignore entries are completely different.

Shrug.  I put this in the same category as "tabs are significant in
Makefile's", "whitespace is significant in python", and "the many
varied different behaviours and uses of 'git reset'".

They are all idiosyncrancies of semantics of various highly popular
tools (which being highly popular, would make changing the details
quite difficult due to backwards compatibility concerns, even if we
wanted to change them).

- Ted

[PATCH v2] cleanup: fix possible overflow errors in binary search

2017-10-08 Thread Derrick Stolee 
A common mistake when writing binary search is to allow possible
integer overflow by using the simple average:

mid = (min + max) / 2;

Instead, use the overflow-safe version:

mid = min + (max - min) / 2;

This translation is safe since the operation occurs inside a loop
conditioned on "min < max". The included changes were found using
the following git grep:

git grep '/ *2;' '*.c'

Making this cleanup will prevent future review friction when a new
binary search is contructed based on existing code.

Signed-off-by: Derrick Stolee 
---
 builtin/index-pack.c  | 4 ++--
 builtin/pack-objects.c| 2 +-
 builtin/unpack-objects.c  | 2 +-
 cache-tree.c  | 2 +-
 compat/regex/regex_internal.c | 4 ++--
 compat/regex/regexec.c| 2 +-
 packfile.c| 2 +-
 sha1-lookup.c | 4 ++--
 sha1_name.c   | 2 +-
 string-list.c | 2 +-
 utf8.c| 2 +-
 xdiff/xpatience.c | 2 +-
 12 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index f2be145e1..8ec459f52 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -633,7 +633,7 @@ static int find_ofs_delta(const off_t offset, enum 
object_type type)
int first = 0, last = nr_ofs_deltas;
 
while (first < last) {
-   int next = (first + last) / 2;
+   int next = first + (last - first) / 2;
struct ofs_delta_entry *delta = _deltas[next];
int cmp;
 
@@ -687,7 +687,7 @@ static int find_ref_delta(const unsigned char *sha1, enum 
object_type type)
int first = 0, last = nr_ref_deltas;
 
while (first < last) {
-   int next = (first + last) / 2;
+   int next = first + (last - first) / 2;
struct ref_delta_entry *delta = _deltas[next];
int cmp;
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5ee2c48ff..6e77dfd44 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1277,7 +1277,7 @@ static int done_pbase_path_pos(unsigned hash)
int lo = 0;
int hi = done_pbase_paths_num;
while (lo < hi) {
-   int mi = (hi + lo) / 2;
+   int mi = lo + (hi - lo) / 2;
if (done_pbase_paths[mi] == hash)
return mi;
if (done_pbase_paths[mi] < hash)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 689a29fac..62ea264c4 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -394,7 +394,7 @@ static void unpack_delta_entry(enum object_type type, 
unsigned long delta_size,
lo = 0;
hi = nr;
while (lo < hi) {
-   mid = (lo + hi)/2;
+   mid = lo + (hi - lo) / 2;
if (base_offset < obj_list[mid].offset) {
hi = mid;
} else if (base_offset > obj_list[mid].offset) {
diff --git a/cache-tree.c b/cache-tree.c
index 71d092ed5..d3f740127 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -49,7 +49,7 @@ static int subtree_pos(struct cache_tree *it, const char 
*path, int pathlen)
lo = 0;
hi = it->subtree_nr;
while (lo < hi) {
-   int mi = (lo + hi) / 2;
+   int mi = lo + (hi - lo) / 2;
struct cache_tree_sub *mdl = down[mi];
int cmp = subtree_name_cmp(path, pathlen,
   mdl->name, mdl->namelen);
diff --git a/compat/regex/regex_internal.c b/compat/regex/regex_internal.c
index d4121f2f4..98342b831 100644
--- a/compat/regex/regex_internal.c
+++ b/compat/regex/regex_internal.c
@@ -613,7 +613,7 @@ re_string_reconstruct (re_string_t *pstr, int idx, int 
eflags)
  int low = 0, high = pstr->valid_len, mid;
  do
{
- mid = (high + low) / 2;
+ mid = low + (high - low) / 2;
  if (pstr->offsets[mid] > offset)
high = mid;
  else if (pstr->offsets[mid] < offset)
@@ -1394,7 +1394,7 @@ re_node_set_contains (const re_node_set *set, int elem)
   right = set->nelem - 1;
   while (idx < right)
 {
-  mid = (idx + right) / 2;
+  mid = idx + (right - idx) / 2;
   if (set->elems[mid] < elem)
idx = mid + 1;
   else
diff --git a/compat/regex/regexec.c b/compat/regex/regexec.c
index 0a745d9c3..6f2b48a78 100644
--- a/compat/regex/regexec.c
+++ b/compat/regex/regexec.c
@@ -4284,7 +4284,7 @@ search_cur_bkref_entry (const re_match_context_t *mctx, 
int str_idx)
   last = right = mctx->nbkref_ents;
   for (left = 0; left < right;)
 {
-  mid = (left + right) / 2;
+  mid = left + (right - left) / 2;
   if (mctx->bkref_ents[mid].str_idx < str_idx)
left = mid +

Special strings in commit messages

2017-10-08 Thread Florian Weimer 

I have a commit which looks like this:

$ git cat-file commit 4ca76eb7b47724c2444dfea7890fa8db4edd5762
tree c845be47a0653624b1984d0dc1a0b485b527811d
parent 9eee98638ef06149e17f94afaa357e3a9e296e69
author Florian Weimer  1507481682 +0200
committer Florian Weimer  1507481682 +0200

19: glibc-fedora-nis-rh188246.patch

From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek 
Date: Mon, 1 May 2006 08:02:53 +
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May  1 2006 Jakub Jelinek  2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up
  some usages of NIS+ (#188246)


This commit causes git rebase to fail, with this error:

fatal: could not parse .git/rebase-apply/0008

At this point, .git/rebase-apply/0008 contains this:

“
From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek 
Date: Mon, 1 May 2006 08:02:53 +
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May  1 2006 Jakub Jelinek  2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up
  some usages of NIS+ (#188246)
---
 nis/nss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nis/nss b/nis/nss
index 
0ac6774a1ff29f012efaec9c4be1fcc3b83da7e8..d720e719267db5f741b67e7b98e4052e503c4333 
100644

”

Followed by the diff.  The preceding patch, .git/rebase-apply/0007, is:

“
[fweimer@oldenburg glibc-patches]$ cat .git/rebase-apply/0007
From 4ca76eb7b47724c2444dfea7890fa8db4edd5762 Mon Sep 17 00:00:00 2001
From: Florian Weimer 
Date: Sun, 8 Oct 2017 18:54:42 +0200
Subject: 19: glibc-fedora-nis-rh188246.patch


”

Based on strace output, something in git rebase calls git mailsplit, and 
it probably sees the "\nFrom " string and treats it as a start of a new 
mail message, and things go downhill from there.


I will escape "\nFrom " in commit messages (probably as "\n.From " or 
maybe "\n>From ", plus escaping for "\n."/"\n>" to make the encoding 
reversible), but I wonder if there is something else I need to escape 
while I'm at it.


Thanks,
Florian

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Paul Smith 
On Sat, 2017-10-07 at 17:55 -0400, Robert P. J. Day wrote:
> On Sat, 7 Oct 2017, Paul Smith wrote:
> > On Sat, 2017-10-07 at 15:43 -0400, Robert P. J. Day wrote:
> > > it's been a long week, so take this in the spirit in which it is
> > > intended ... i think the "git rm" command and its man page should be
> > > printed out, run through a paper shredder, then set on fire. i can't
> > > remember the last time i saw such a thoroughly badly-designed,
> > > badly-documented and non-intuitive utility.
> >
> > "git rm" works the same way that the UNIX rm command has worked, for
> > 40+ years now.  Very simple, very well designed, and very intuitive
> > (IMO).
> >
> > The major difference is the ability to handle globbing patterns,
> > which UNIX rm doesn't do.  Maybe the way this is implemented is a
> > little confusing, although I just read the man page and it seemed
> > pretty clear to me.
> 
>   um, wrong.

I don't know what part of my comment here you consider "wrong".  I've
re-read it and I believe everything I said is correct.

> > If you don't use glob patterns (or more specifically if you let the
> > shell handle glob patterns, which is how I always do it) then there
> > is really nothing bizarre about "git rm".  Maybe you could be more
> > precise in your criticism.
> 
> ok, fine, let me explain why this command is a nightmarish
> monstrosity. as i now understand, if i use an escaped wildcard
> pattern, "git rm" will *automatically* (with no further guidance from
> me, and no warning), operate recursively. so if, in the kernel source
> tree, i ran:
> 
>   $ git rm \*.c

Yes, this is what I said: "IF YOU DON'T USE GLOB PATTERNS (or more
specifically if you let the shell handle glob patterns ...) then there
is nothing bizarre about "git rm"" (emphasis added).

In this example you ARE sending glob patterns to Git, because you're
escaping them from the shell.  Hence, you might consider the behavior
bizarre, at least until you grok it fully.

If you want to avoid this, simply use normal shell globbing and don't
ask Git to do the globbing for you.  Then it behaves exactly as normal
UNIX rm, including with the '-r' option, and is very simple.

> so if i wanted git to remove, say, all files named "Makefile*" from
> everywhere in the linux kernel source tree, the (dry run) command
> would be:
> 
>   $ git rm -n Makefile\*
> 
> is that it? let's try that:
> 
>   $ git rm -n Makefile\*
>   rm 'Makefile'
>   $
> 
> oops.

Globbing in "git rm" matches on the FULL PATH, not just the file name. 
So, if you have a list of Makefiles in your repository like:

  Makefile
  foo/Makefile
  bar/Makefile

Then 'Makefile*' only matches the first one, since 'Makefile*' doesn't
match 'foo/Makefile' or 'bar/Makefile'.

If you you worry that '*Makefile' will match things you don't want to
match, you'll have to use:

  git rm -n Makefile '*/Makefile'

Personally I don't use Git's magical globbing capabilities, and use "git
rm" as if it were UNIX rm.  So in your request above I'd use:

   git rm $(find . -name Makefile)

which I find simpler.

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

2017-10-08 Thread Martin Ågren 
On 6 October 2017 at 22:27, Stefan Beller  wrote:
>> I might be naive in thinking that protocol.version could be removed or
>> redefined at our discretion just because it's marked as "experimental".
>
> Well the redefinition might very well occur, when we now say "set to v1
> to test v1 and fallback to v0 otherwise", but long term we want a white
> or black list or some other protocol selection strategy encoded in this
> configuration (we would not want to introduce yet another config to work
> around the initial "failed experiment", would we?)
>
> And hence I would be careful how we define the meaning of
> protocol.version now.

Good points. If we want to go for a more general / future-proof wording
now, then we must already now implement the config-parsing as "does this
string contain the word '1'" instead of "is this string exactly '1'". If
we claim that "34 1 5" is a valid configuration, then the implementation
should accept it. (We'd probably also want to verify that there are only
integers and spaces in the string.)

> For example we could instead now claim "protocol.version is a whitelist
> of protocol versions, order is not specified. The only guarantee we're willing
> to give is that no protocol is used that is not on the list".

If we want to be able to list more than one version, we need to define
how to signal preference from the first day, IMHO. (I know you just gave
an example; I'm simply responding with what I think makes that example
non-ideal.)

The fact that v0 is requested by lack of data and all other protocols
(whether v1 or v34) have to be requested by presence of data, is a bit
unfortunate and it is bound to bleed through into the definitions, at
least until v0 is simply ripped out of git.git. Ok, this definition
suggests that "1 0" will be the preferred variant for checking basic
robustness, while "1" will be how to ensure you have a peer which knows
v1.

> All I was trying to say initially is that "we may try (one of) 
> protocol.version,
> but fall back to whatever (currently v0)" is too broad. We'd need to redefine
> it shortly in the foreseeable future already.

Yes we would. I'll post a suggestion elsewhere in the thread.

Martin

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Robert P. J. Day 
On Sun, 8 Oct 2017, René Scharfe wrote:

> [My SMTP server still refuses to accept emails to rpj...@crashcourse.ca
>  and reports "mailbox unavailable" and "invalid DNS MX or A/ resource
>  record."  So just replying to the list.]
>
> Am 08.10.2017 um 13:56 schrieb Robert P. J. Day:
> >but as i asked in my earlier post, if i wanted to remove *all* files
> > with names of "Makefile*", why can't i use:
> >
> >$ git rm 'Makefile*'
> >
> > just as i used:
> >
> >$ git rm '*.c'
> >
> > are those not both acceptable fileglobs? why does the former clearly
> > only match the top-level Makefile, and refuse to cross directory
> > boundaries?
> >
> >$ git rm -n 'Makefile*'
> >rm 'Makefile'
> >$
>
> Try:
>
>   $ git rm -n '*Makefile'
>
> The whole path is considered.  The asterisk there matches any
> directory part -- but also any file name prefix.  Check the entry
> for "pathspec" in gitglossary(7) for more details.

  but "*Makefile" is not the wildcard pattern i'm interested in.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread René Scharfe 
[My SMTP server still refuses to accept emails to rpj...@crashcourse.ca
 and reports "mailbox unavailable" and "invalid DNS MX or A/ resource
 record."  So just replying to the list.]

Am 08.10.2017 um 13:56 schrieb Robert P. J. Day:
>but as i asked in my earlier post, if i wanted to remove *all* files
> with names of "Makefile*", why can't i use:
> 
>$ git rm 'Makefile*'
> 
> just as i used:
> 
>$ git rm '*.c'
> 
> are those not both acceptable fileglobs? why does the former clearly
> only match the top-level Makefile, and refuse to cross directory
> boundaries?
> 
>$ git rm -n 'Makefile*'
>rm 'Makefile'
>$

Try:

$ git rm -n '*Makefile'

The whole path is considered.  The asterisk there matches any
directory part -- but also any file name prefix.  Check the entry for
"pathspec" in gitglossary(7) for more details.

René

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Martin Ågren 
On 8 October 2017 at 13:56, Robert P. J. Day  wrote:
>   but as i asked in my earlier post, if i wanted to remove *all* files
> with names of "Makefile*", why can't i use:
>
>   $ git rm 'Makefile*'
>
> just as i used:
>
>   $ git rm '*.c'
>
> are those not both acceptable fileglobs? why does the former clearly
> only match the top-level Makefile, and refuse to cross directory
> boundaries?
>
>   $ git rm -n 'Makefile*'
>   rm 'Makefile'
>   $

Hmmm. The manpage says the following:

   git rm -f git-*.sh
   Because this example lets the shell expand the asterisk (i.e.
   you are listing the files explicitly), it does not remove
   subdir/git-foo.sh.

This implies that `git rm "git-*.sh"` should remove subdir/git-foo.sh.
But it doesn't, at least not in my testing. It seems that the globbing
only kicks in when the "*" comes first, as you've noted.

[PATCH] i18n: add a missing space in message

2017-10-08 Thread Jean-Noel Avila 
The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.

Signed-off-by: Jean-Noel Avila 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This is a single change discovered while doing the localization task.

Moreover, what's the status of the options in '--' in the
messages?  Must they be single-quoted or not? This may sound a bit
picky, but there has been some shifts from one form to the other in
the latest batch of strings, leading to fuzzy matchings. Settling on a
given "standard" would probably reduce translators'work.

diff --git a/sequencer.c b/sequencer.c
index 7886e2269..e258bb646 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2558,7 +2558,7 @@ static enum check_level 
get_missing_commit_check_level(void)
return CHECK_WARN;
if (!strcasecmp("error", value))
return CHECK_ERROR;
-   warning(_("unrecognized setting %s for option"
+   warning(_("unrecognized setting %s for option "
  "rebase.missingCommitsCheck. Ignoring."), value);
return CHECK_IGNORE;
 }
-- 
2.14.0

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Robert P. J. Day 
On Sun, 8 Oct 2017, Junio C Hamano wrote:

> "Robert P. J. Day"  writes:
>
> > ... so if, in the kernel source
> > tree, i ran:
> >
> >   $ git rm \*.c
> >
> > i would end up removing *all* 25,569 "*.c" files in the kernel
> > source repository.
>
> Yes, as that is exactly what the command line asks Git to do.

  ok, i truly want to understand this, so let me dig through this
carefully. i can now see (from the man page and the recent
explanations) that "git rm" will accept *escaped* fileglobs to remove
and that, further, "File globbing matches across directory
boundaries." which is why, in the linux kernel source tree, if i run
one of:

  $ git rm \*.c
  $ git rm '*.c'

the "git rm" command will internally process the fileglob and apply it
across directory boundaries. and that's why, when i try a dry run, i
can see the effect it would have on the kernel source:

  $ git rm -n '*.c' | wc -l
  25569
  $

> If you said
>
> $ git rm *.c
>
> then the shell expands the glob and all Git sees is that you want to
> remove a.c b.c d.c ...; if you said "git rm -r *.c", unless b.c is
> not a directory, these and only these files are removed.

  right, that's just regular shell fileglob processing, no surprise
there. (let's stick to just file removal for now.)

> >   however, let's say i wanted to remove, recursively, all files with a
> > *precise* (non-globbed) name, such as "Makefile". so i, naively, run:
> >
> >   $ git rm Makefile
> >
> > guess what ... the lack of globbing means i remove only the single
> > Makefile at the top of the working directory.
>
> Again, that is exactly what you asked Git to do.

  yes, now i get it -- a lack of fileglob arguments disallows
traversing directory boundaries, so one gets the "normal" behaviour.

> $ git rm $(find . -name Makefile -print)
>
> would of course one way to remove all Makefiles.  If you let POSIX
> shell glob, i.e.
>
> $ git rm */Makefile
>
> the asterisk would not expand nothing but a single level, so it may
> remove fs/Makefile, but not fs/ext4/Makefile (some shells allow
> "wildmatch" expansion so "git rm **/Makefile" may catch the latter
> with such a shell).

  sure, all regular shell fileglob processing.

> By letting Git see the glob, i.e.
>
> $ git rm Makefile \*/Makefile
>
> you would let Git to go over the paths it knows/cares about to find
> ones that match the pathspec pattern and remove them (but not
> recursively, even if you had a directory whose name is Makefile; for
> that, you would use "-r").

  right ... i can now see that '*/Makefile' would pick up all
Makefiles *below* the current directory, so you need that initial
reference to 'Makefile' to catch the top one. this just seems ...
awkward.

  but as i asked in my earlier post, if i wanted to remove *all* files
with names of "Makefile*", why can't i use:

  $ git rm 'Makefile*'

just as i used:

  $ git rm '*.c'

are those not both acceptable fileglobs? why does the former clearly
only match the top-level Makefile, and refuse to cross directory
boundaries?

  $ git rm -n 'Makefile*'
  rm 'Makefile'
  $

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Kevin Daudt 
On Sun, Oct 08, 2017 at 05:07:12AM -0400, Robert P. J. Day wrote:
> On Sun, 8 Oct 2017, Junio C Hamano wrote:
> 
> > "Robert P. J. Day"  writes:
> >
> > > ... so if, in the kernel source
> > > tree, i ran:
> > >
> > >   $ git rm \*.c
> > >
> > > i would end up removing *all* 25,569 "*.c" files in the kernel source
> > > repository.
> >
> > Yes, as that is exactly what the command line asks Git to do.
> 
>   so if i wanted git to remove, say, all files named "Makefile*" from
> everywhere in the linux kernel source tree, the (dry run) command
> would be:
> 
>   $ git rm -n Makefile\*
> 
> is that it? let's try that:
> 
>   $ git rm -n Makefile\*
>   rm 'Makefile'
>   $
> 
> oops.
> 
> rday
> 

So your question is not su much  about the recursive option (delete
mentioned directories, including their contents), but globbing
(expanding the * to any files matching the pattern).

The explanation of  mentions this:

   Files to remove. Fileglobs (e.g.  *.c) can be given to remove all
   matching files.

This indicates that git itself (not your shell alone) does file
globbing.

I think the confusing part is that most people have no clear idea of the
separation between what the shell sees and interprets, and what the
program actually gets.

When you execute:

$ git rm Makefile\*

What git actually sees is this:

Makefile*

The shell intepreted the \* to mean just '*' and not interpret it
itself, and provide that to the executed program. Git, in its turn,
would start matching any file to that pattern to see which files
matches.


If you would execute:

$ git rm 'Makefile\*'

Git would see:

Makefile\*

Which does the thing you intended. So you have to deal with 2 levels of
programs interpreting the arguments, not just one.

Whether '*.c' should match just all .c files in the current dir, or all
files ending with .c depends on whether the path seperator is matched by
* or not and is a separate discussion.

GITGLOSSARY(7) under pathspec mentions this:

globGit treats the pattern as a shell glob suitable for consumption
by fnmatch(3) with the FNM_PATHNAME flag: wildcards in the
pattern will not match a / in the pathname.

So that seems to indicate '*.c' should only match .c files in the
current dir. I'm not sure why that's not the case.

I hope this clears up what's happening a bit, and perhaps can lead to
improvements to the documentation so that it's not so surprising.

Kind regards, Kevin.

Re: [PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread Martin Ågren 
On 8 October 2017 at 11:32, Philip Oakley  wrote:
> From: "Martin Ågren" 
>
>> We currently build an error message like "entry is a %s, not a commit",
>> where the placeholder will be replaced with "blob", "tag" or "tree".
>> Apart from those three placeholder words not being translated, in some
>> languages it might be awkward or impossible to ensure a grammatically
>> correct end result.
...
>>  default:
>> - die(_("submodule entry '%s' (%s) is a %s, not a commit"),
>> - cb->path, oid_to_hex(oid), typename(type));
>> + die(_("submodule entry '%s' (%s) is not a commit"),
>> + cb->path, oid_to_hex(oid));
>
> Bikeshed,
> maybe:
> "submodule entry '%s' (%s) is not a commit. It is a %s"
> This puts the two parts in separate sentences?

I'm not doing the Swedish translation, but if I did, I would find this
just as hard to translate as the original. There are two problems here.
The first is "blob"/"tag"/"tree". "Blob" is already used in the Swedish
translation, but "tag" should be "tagg" and "tree" should be "träd"
(IMHO).

The second problem is that even if all three words were valid Swedish
words, then (IMHO) using "en %s" instead of "a %s" would be needed to
make sense with "en blob" and "en tag", but it wouldn't work with "en
tree" which should arguably be "ett tree". (But to be clear, seeing any
of "en tree" and "ett tree" makes me shiver.)

I should perhaps have been clearer that grammatical problems might arise
from the "a". (It's more or less by chance that it works in English in
the first place. Luckily there is no type "aardvark", "index" or
"other".) But I wouldn't be surprised if there's a language out there
where "a" is not a problem, but something else is.

It just occurred to me that one approach would be something like "... is
of type '%s', not 'commit'" where "commit" should not be translated and
%s would be one of "blob", "tag" and "tree". That would be sort of in
line with what `git cat-file` does, but not quite. In cat-file it seems
natural because it's about the command-line argument, here it's in an
error string and seems a bit awkward.

Martin

Re: [PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread Philip Oakley 

From: "Martin Ågren" 

We currently build an error message like "entry is a %s, not a commit",
where the placeholder will be replaced with "blob", "tag" or "tree".
Apart from those three placeholder words not being translated, in some
languages it might be awkward or impossible to ensure a grammatically
correct end result.

Shorten the error message to "entry is not a commit". We will still
error out, we will still give a hint about what is wrong, but we will
not be as explicit as before.

Alternatively, we could have different switch-cases for the different
types and pick one of three different error messages. We might still
want a `default` and maybe more tests. So go for this simpler approach
instead.

Signed-off-by: Martin Ågren 
---
I browsed the diff of the .pot and found an addition that looked a bit
translation-unfriendly. Maybe something like this?

submodule.c| 4 ++--
t/t5531-deep-submodule-push.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index 63e7094e1..3d91dbfd5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -796,8 +796,8 @@ static int check_has_commit(const struct object_id 
*oid, void *data)

 cb->result = 0;
 return 0;
 default:
- die(_("submodule entry '%s' (%s) is a %s, not a commit"),
- cb->path, oid_to_hex(oid), typename(type));
+ die(_("submodule entry '%s' (%s) is not a commit"),
+ cb->path, oid_to_hex(oid));

Bikeshed,
maybe:
"submodule entry '%s' (%s) is not a commit. It is a %s"
This puts the two parts in separate sentences?

--
Philip



 }
}

diff --git a/t/t5531-deep-submodule-push.sh 
b/t/t5531-deep-submodule-push.sh

index 39cb2c1c3..e4c98bbc5 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -305,7 +305,7 @@ test_expect_success 'submodule entry pointing at a tag 
is error' '

 git -C work commit -m "bad commit" &&
 test_when_finished "git -C work reset --hard HEAD^" &&
 test_must_fail git -C work push --recurse-submodules=on-demand ../pub.git 
master 2>err &&

- test_i18ngrep "is a tag, not a commit" err
+ test_i18ngrep "is not a commit" err
'

test_expect_success 'push fails if recurse submodules option passed as 
yes' '

--
2.15.0.rc0.37.gd35688db1

Re: "git rm" seems to do recursive removal even without "-r"

2017-10-08 Thread Robert P. J. Day 
On Sun, 8 Oct 2017, Junio C Hamano wrote:

> "Robert P. J. Day"  writes:
>
> > ... so if, in the kernel source
> > tree, i ran:
> >
> >   $ git rm \*.c
> >
> > i would end up removing *all* 25,569 "*.c" files in the kernel source
> > repository.
>
> Yes, as that is exactly what the command line asks Git to do.

  so if i wanted git to remove, say, all files named "Makefile*" from
everywhere in the linux kernel source tree, the (dry run) command
would be:

  $ git rm -n Makefile\*

is that it? let's try that:

  $ git rm -n Makefile\*
  rm 'Makefile'
  $

oops.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

[PATCH] submodule: avoid sentence-lego in translated string

2017-10-08 Thread Martin Ågren 
We currently build an error message like "entry is a %s, not a commit",
where the placeholder will be replaced with "blob", "tag" or "tree".
Apart from those three placeholder words not being translated, in some
languages it might be awkward or impossible to ensure a grammatically
correct end result.

Shorten the error message to "entry is not a commit". We will still
error out, we will still give a hint about what is wrong, but we will
not be as explicit as before.

Alternatively, we could have different switch-cases for the different
types and pick one of three different error messages. We might still
want a `default` and maybe more tests. So go for this simpler approach
instead.

Signed-off-by: Martin Ågren 
---
I browsed the diff of the .pot and found an addition that looked a bit
translation-unfriendly. Maybe something like this?

 submodule.c| 4 ++--
 t/t5531-deep-submodule-push.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index 63e7094e1..3d91dbfd5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -796,8 +796,8 @@ static int check_has_commit(const struct object_id *oid, 
void *data)
cb->result = 0;
return 0;
default:
-   die(_("submodule entry '%s' (%s) is a %s, not a commit"),
-   cb->path, oid_to_hex(oid), typename(type));
+   die(_("submodule entry '%s' (%s) is not a commit"),
+   cb->path, oid_to_hex(oid));
}
 }
 
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 39cb2c1c3..e4c98bbc5 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -305,7 +305,7 @@ test_expect_success 'submodule entry pointing at a tag is 
error' '
git -C work commit -m "bad commit" &&
test_when_finished "git -C work reset --hard HEAD^" &&
test_must_fail git -C work push --recurse-submodules=on-demand 
../pub.git master 2>err &&
-   test_i18ngrep "is a tag, not a commit" err
+   test_i18ngrep "is not a commit" err
 '
 
 test_expect_success 'push fails if recurse submodules option passed as yes' '
-- 
2.15.0.rc0.37.gd35688db1

Re: is there a truly compelling rationale for .git/info/exclude?

2017-10-08 Thread Steinar Bang 
> rpj...@crashcourse.ca:

>   but even that isn't a really compelling reason. so what's it for?

I use it to ignore stuff in my git-versioned home directory.

Every time I use a new program and it creates a config file or a config
directory, it shows up as clutter in magit in my git versioned home
directory.

I started with putting the stuff to be ignored in .gitignore, but since
I run different stuff on different machines and on different OSes,
.gitignore started to contain irrelevant stuff (ignoring a stuff from a
program that was run once and then never again, ignoring stuff on one
machine that maybe should not be ignored on a different machine), and
then I figured it was much simpler to just ignore stuff repo-locally in
.git/info/exclude

[L10N] Kickoff of translation for Git 2.15.0 round 1

2017-10-08 Thread Jiang Xin 
Hi,

Git v2.15.0-rc0 has been released, and it's time to start new round of git l10n.
This time there are 68 updated messages need to be translated since last
update.

You can get it from the usual place:

https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin