Re: Does Git build things during 'make install"?

[Johannes Sixt]

Am 16.10.2017 um 07:05 schrieb Jeffrey Walton:

My script to build Git dies during cleanup. Cleanup removes the
downloaded tarball and the unpacked directory:

** Cleanup **

rm: cannot remove 'git-2.14.2/perl/blib/lib/.exists': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Fetcher.pm':
Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Utils.pm': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Ra.pm': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/GlobSpec.pm':
Permission denied
...

When I look at the permissions:

$ ls -Al git-2.14.2/perl/blib/lib/.exists
-rw-r--r--   1 root root   0 Oct 16 00:43
git-2.14.2/perl/blib/lib/.exists

The only place in my script that does anything with privileges is
'make install' because it runs with sudo.

Is Git building things in the install recipe? If so, then I don't
believe that's supposed to happen. According to the GNU coding
standards, Git should not be doing that. Cf;
https://www.gnu.org/prep/standards/html_node/Standard-Targets.html.


Yes, running "sudo make install" is a nightmare. sudo clears the path, 
and the git command is not found by the make invoked with root 
permissions. This changes the version string that gets compiled into the 
executable, which finally triggers a complete rebuild under root. Sad...


-- Hannes

[PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge

[Junio C Hamano]

The -Xours/-Xtheirs merge options were originally defined as a way
to "force" the resolution of 3way textual merge conflicts to take
one side without using your editor, hence did not even trigger in
situations where you would normally not get the <<< === >>> conflict
markers.

This was improved for binary files back in 2012 with a944af1d
("merge: teach -Xours/-Xtheirs to binary ll-merge driver",
2012-09-08).

Teach a similar trick to the codepath that deals with merging two
conflicting changes to symbolic links.

Signed-off-by: Junio C Hamano 
---

 * Looks like I queued this on 'pu' but never sent it out to the
   list for extra eyeballs.  On the tests are new, relative to what
   was sent out earlier and archived at:

   https://public-inbox.org/git/xmqqa81ichdu@gitster.mtv.corp.google.com

 merge-recursive.c| 17 +
 t/t6037-merge-ours-theirs.sh | 32 
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 1494ffdb82..ed529f2ceb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1002,10 +1002,19 @@ static int merge_file_1(struct merge_options *o,
   >oid,
   !o->call_depth);
} else if (S_ISLNK(a->mode)) {
-   oidcpy(>oid, >oid);
-
-   if (!oid_eq(>oid, >oid))
-   result->clean = 0;
+   switch (o->recursive_variant) {
+   case MERGE_RECURSIVE_NORMAL:
+   oidcpy(>oid, >oid);
+   if (!oid_eq(>oid, >oid))
+   result->clean = 0;
+   break;
+   case MERGE_RECURSIVE_OURS:
+   oidcpy(>oid, >oid);
+   break;
+   case MERGE_RECURSIVE_THEIRS:
+   oidcpy(>oid, >oid);
+   break;
+   }
} else
die("BUG: unsupported object type in the tree");
}
diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
index 3889eca4ae..0aebc6c028 100755
--- a/t/t6037-merge-ours-theirs.sh
+++ b/t/t6037-merge-ours-theirs.sh
@@ -73,4 +73,36 @@ test_expect_success 'pull passes -X to underlying merge' '
git reset --hard master && test_must_fail git pull -s recursive -X bork 
. side
 '
 
+test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' '
+   git reset --hard master &&
+   git checkout -b two master &&
+   ln -s target-zero link &&
+   git add link &&
+   git commit -m "add link pointing to zero" &&
+
+   ln -f -s target-two link &&
+   git commit -m "add link pointing to two" link &&
+
+   git checkout -b one HEAD^ &&
+   ln -f -s target-one link &&
+   git commit -m "add link pointing to one" link &&
+
+   # we expect symbolic links not to resolve automatically, of course
+   git checkout one^0 &&
+   test_must_fail git merge -s recursive two &&
+
+   # favor theirs to resolve to target-two?
+   git reset --hard &&
+   git checkout one^0 &&
+   git merge -s recursive -X theirs two &&
+   git diff --exit-code two HEAD link &&
+
+   # favor ours to resolve to target-one?
+   git reset --hard &&
+   git checkout one^0 &&
+   git merge -s recursive -X ours two &&
+   git diff --exit-code one HEAD link
+
+'
+
 test_done
-- 
2.15.0-rc1-172-gbfe4246c99

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

[Junio C Hamano]

Christian Couder  writes:

> It is error prone and tiring to use many long environment
> variables to give parameters to the 'run' script.

This topic has been sitting in the list archive without getting much
reaction from list participants.  Is anybody happy with these
patches?

Does Git build things during 'make install"?

[Jeffrey Walton]

My script to build Git dies during cleanup. Cleanup removes the
downloaded tarball and the unpacked directory:

** Cleanup **

rm: cannot remove 'git-2.14.2/perl/blib/lib/.exists': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Fetcher.pm':
Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Utils.pm': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/Ra.pm': Permission denied
rm: cannot remove 'git-2.14.2/perl/blib/lib/Git/SVN/GlobSpec.pm':
Permission denied
...

When I look at the permissions:

$ ls -Al git-2.14.2/perl/blib/lib/.exists
-rw-r--r--   1 root root   0 Oct 16 00:43
git-2.14.2/perl/blib/lib/.exists

The only place in my script that does anything with privileges is
'make install' because it runs with sudo.

Is Git building things in the install recipe? If so, then I don't
believe that's supposed to happen. According to the GNU coding
standards, Git should not be doing that. Cf;
https://www.gnu.org/prep/standards/html_node/Standard-Targets.html.

Jeff

Re: [PATCH v3 00/25] object_id part 10

[Junio C Hamano]

"brian m. carlson"  writes:

> 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.
>
> In this iteration, I chose to skip the ALLOC_ARRAY conversion to limit
> the scope of changes and because I think to be fixed properly the code
> requires converting the targets parameter to size_t, which involves
> touching more code unrelated to the series than I'd like to do right
> now.
>
> All the other issues that were brought up should be fixed.

With a hope that this might help other reviewers, here is the
interdiff between "the previous one merged with v2.15-rc1" and "this
round applied on v2.15-rc1 directly".  

The changes all looked sensible to me.  Thanks.


 builtin/checkout.c   |  3 ++-
 refs.c   | 10 +++---
 refs.h   | 28 ++--
 refs/files-backend.c |  8 
 remote.c |  3 ++-
 sequencer.c  |  9 ++---
 6 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 88e841d6bf..a8e2849f8a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -662,7 +662,8 @@ 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", >commit->object.oid, 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/refs.c b/refs.c
index 1af6ce1e1d..62a7621025 100644
--- a/refs.c
+++ b/refs.c
@@ -583,6 +583,9 @@ static int write_pseudoref(const char *pseudoref, const 
struct object_id *oid,
struct strbuf buf = STRBUF_INIT;
int ret = -1;
 
+   if (!oid)
+   return 0;
+
strbuf_addf(, "%s\n", oid_to_hex(oid));
 
filename = git_path("%s", pseudoref);
@@ -953,7 +956,7 @@ int ref_transaction_create(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
if (!new_oid || is_null_oid(new_oid))
-   die("BUG: create called without valid new_sha1");
+   die("BUG: create called without valid new_oid");
return ref_transaction_update(transaction, refname, new_oid,
  _oid, flags, msg, err);
 }
@@ -965,7 +968,7 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
if (old_oid && is_null_oid(old_oid))
-   die("BUG: delete called with old_sha1 set to zeros");
+   die("BUG: delete called with old_oid set to zeros");
return ref_transaction_update(transaction, refname,
  _oid, old_oid,
  flags, msg, err);
@@ -978,7 +981,7 @@ int ref_transaction_verify(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
if (!old_oid)
-   die("BUG: verify called with old_sha1 set to NULL");
+   die("BUG: verify called with old_oid set to NULL");
return ref_transaction_update(transaction, refname,
  NULL, old_oid,
  flags, NULL, err);
@@ -994,6 +997,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
int ret = 0;
 
if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
+   assert(refs == get_main_ref_store());
ret = write_pseudoref(refname, new_oid, old_oid, );
} else {
t = ref_store_transaction_begin(refs, );
diff --git a/refs.h b/refs.h
index d791eb2ed3..15fd419c7d 100644
--- a/refs.h
+++ b/refs.h
@@ -14,22 +14,22 @@ struct worktree;
  * at the resolved object name.  The return value, if not NULL, is a
  * pointer into either a static buffer or the input ref.
  *
- * If sha1 is non-NULL, store the referred-to object's name in it.
+ * If oid is non-NULL, store the referred-to object's name in it.
  *
  * If the reference cannot be resolved to an object, the behavior
  * depends on the RESOLVE_REF_READING flag:
  *
  * - If RESOLVE_REF_READING is set, return NULL.
  *
- * - If RESOLVE_REF_READING is not set, clear sha1 and return the name of
+ * - If RESOLVE_REF_READING is not set, clear oid and return the name of
  *   the last reference name in the chain, which will either be a non-symbolic
  *   reference or an undefined reference.  

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

[Junio C Hamano]

Ralf Thielow  writes:

> When the write opertion fails, we write that we could
> not read. Change the error message to match the operation
> and remove the full stop at the end.
>
> When ftruncate() fails, we write that we couldn't finish
> the operation on the todo file. It is more accurate to write
> that we couldn't truncate as we do in other calls of ftruncate().

Wouldn't it be more accurate to say we couldn't ftruncate, though?

>
> Signed-off-by: Ralf Thielow 
> ---
>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e258bb646..75f5356f6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>   if (fd < 0)
>   res = error_errno(_("could not open '%s'"), todo_file);
>   else if (write(fd, buf.buf, buf.len) < 0)
> - res = error_errno(_("could not read '%s'."), todo_file);
> + res = error_errno(_("could not write '%s'"), todo_file);
>   else if (ftruncate(fd, buf.len) < 0)
> - res = error_errno(_("could not finish '%s'"),
> + res = error_errno(_("could not truncate '%s'"),
>  todo_file);
>   close(fd);
>   strbuf_release();

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin  writes:

>> >> -Also respects `:remotename` to state the name of the *remote* instead of
>> >> -the ref.
>> >> +Also respects `:remotename` to state the name of the *remote* instead
>> >> +of the ref, and `:remoteref` to state the name of the *reference* as
>> >> +locally known by the remote.
>> >
>> > What does "locally known by the remote" mean in this sentence?
>> 
>> Good question.  I cannot offhand offer a better and concise
>> phrasing, but I think can explain what it wants to describe ;-).
>
> Yep, described it well.
>
> Maybe "and `:remoteref` to state the name by which the remote knows the
> *reference*"?

I dunno.  

The original and your update both seem to come from a worldview
where there is a single conceptual thing called "reference" that is
shared between our repository and the remote repository we pull from
(or push to), and the name we call it is "refs/remotes/origin/devel"
while the name they use to call it is "refs/heads/devel".  If you
subscribe to that worldview, then the updated sentence might make it
clearer than the original.

But I do not share that worldview, and I am not sure (note: I am
truly unsure---it's not like I am convinced it is a good idea but
sugarcoating my expression, at least in this case) if subscribing to
the worldview helps users' understanding.

In my view "refs/remotes/origin/devel" is a reference we use to keep
track of (or "a reference that corresponds to") the reference they
have called "refs/heads/devel" at the remote, and these are two
separate entities, so it's not like "this single thing is called
differently by us and them".

Stepping back a bit; here is how the description begins.

upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref.

So 'upstream' is like 'refs/remotes/origin/devel' in the example in
the message you are responding to.  Perhaps we can make it clear in
the description, and then add :remote* modifiers are about asking
where that remote-tracking branch comes from.  For example, instead
of these "Also respects...", something like:

For a %(upstream) that is a remote-tracking branch, the name of
the remote repository it is copied from can be obtained with
%(upstream:remotename).  Simiarly, the branch at the remote
repository whose tip is copioed to this remote-tracking branch
can be obtined with %(upstream:remoteref) as a full refname.

may reduce the chance of confusion?

[PATCH v3 13/25] pack-bitmap: convert traverse_bitmap_commit_list to object_id

[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 6e77dfd444..d2d97cc61e 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.15.0.rc0.271.g36b669edcc

[PATCH v3 19/25] Convert remaining callers of resolve_gitlink_ref to object_id

[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.15.0.rc0.271.g36b669edcc

[PATCH v3 04/25] refs: convert update_ref and refs_update_ref to use struct object_id

[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|  2 +-
 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| 39 ---
 refs.h|  5 +
 sequencer.c   |  8 
 t/helper/test-ref-store.c | 10 +-
 transport-helper.c|  3 ++-
 transport.c   |  4 ++--
 17 files changed, 65 insertions(+), 74 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 fc4f8fd2ea..2bb009ec0e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -664,7 +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,
+   update_ref(msg.buf, "HEAD", >commit->object.oid, NULL,
   REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (!opts->quiet) {
if (old->path &&
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,
-  NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+   update_ref(msg, ref->name, >old_oid, NULL, 0,
+  

[PATCH v3 09/25] refs: convert read_ref and read_ref_full to object_id

[brian m. carlson]

All but two of the call sites already have 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 | 26 +-
 refs.h |  6 +++---
 refs/files-backend.c   | 10 +-
 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 c33dbb70fb..463a337e5d 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);
@@ -1038,7 +1038,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 43c07ddeb3..8c458b9613 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;
@@ -518,7 

[PATCH v3 12/25] refs: convert dwim_log to struct object_id

[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 842b2f77dc..5008bbf6ad 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -161,7 +161,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 9bb555e7ff..ecb43a113e 100644
--- a/refs.c
+++ b/refs.c
@@ -497,7 +497,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;
@@ -506,13 +506,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))
@@ -523,7 +523,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 0d864b0ab1..9d59c414aa 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 d8ff831759..514915460f 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.15.0.rc0.271.g36b669edcc

[PATCH v3 01/25] walker: convert to struct object_id

[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.15.0.rc0.271.g36b669edcc

[PATCH v3 03/25] refs: convert delete_ref and refs_delete_ref to struct object_id

[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 b67593288c..f5237541a2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -257,7 +257,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 695cb0778e..272f0d3103 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 c590a992fb..25170d9143 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 v3 25/25] refs/files-backend: convert static functions to object_id

[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.  Update the docstring for verify lock to
account for the new parameter name, and note additionally that the
old_oid may be NULL.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index c1626490c2..b7b9e767de 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -770,13 +770,13 @@ static struct ref_iterator *files_ref_iterator_begin(
 }
 
 /*
- * Verify that the reference locked by lock has the value old_sha1.
- * Fail if the reference doesn't exist and mustexist is set. Return 0
- * on success. On error, write an error message to err, set errno, and
- * return a negative value.
+ * Verify that the reference locked by lock has the value old_oid
+ * (unless it is NULL).  Fail if the reference doesn't exist and
+ * mustexist is set. Return 0 on success. On error, write an error
+ * message to err, set errno, and 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);
@@ -784,7 +784,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;
@@ -794,11 +794,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;
}
@@ -828,22 +828,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));
@@ -909,7 +909,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;
}
@@ -1324,8 +1324,8 @@ static int files_copy_or_rename_ref(struct ref_store 
*ref_store,
 
logmoved = log;
 
-   lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
-

[PATCH v3 16/25] refs: convert read_ref_at to struct object_id

[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 f8a2d98666..d19fae5077 100644
--- a/refs.c
+++ b/refs.c
@@ -738,11 +738,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;
@@ -774,25 +774,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;
@@ -812,15 +812,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;
@@ -833,7 +833,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 89f28d482d..5f329d6618 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 v3 22/25] refs: convert resolve_ref_unsafe to struct object_id

[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, removing the
temporary NULL pointer check in expand_ref, 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| 29 ++---
 refs.h| 14 +++---
 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, 36 insertions(+), 38 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 90219d6e13..72c45a513b 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,8 +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 ? this_result->hash : NULL,
-  );
+  this_result, );
if (r) {
if (!refs_found++)
*ref = xstrdup(r);
@@ -512,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))
@@ -1393,15 +1392,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;
 
@@ -1429,7 +1428,7 @@ const char 

[PATCH v3 17/25] refs: convert reflog_expire parameter to struct object_id

[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 d19fae5077..37485283c0 100644
--- a/refs.c
+++ b/refs.c
@@ -2010,19 +2010,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,
@@ -2030,7 +2030,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 5f329d6618..50f6f0dbd7 100644
--- a/refs.h
+++ b/refs.h
@@ -703,20 +703,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 v3 21/25] worktree: convert struct worktree to object_id

[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 7b9307aa58..ed043d5f1c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -410,7 +410,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)
@@ -430,7 +430,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) {
@@ -455,7 +455,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.15.0.rc0.271.g36b669edcc

[PATCH v3 20/25] refs: convert resolve_gitlink_ref to struct object_id

[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 37485283c0..90219d6e13 100644
--- a/refs.c
+++ b/refs.c
@@ -1498,7 +1498,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;
@@ -1508,8 +1508,8 @@ int resolve_gitlink_ref(const char *submodule, const char 
*refname,
if (!refs)
return -1;
 
-   if 

[PATCH v3 02/25] refs/files-backend: convert struct ref_to_prune to object_id

[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 014dabb0bf..9db9237f1e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -926,7 +926,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];
 };
 
@@ -994,7 +994,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);
@@ -1088,7 +1088,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.15.0.rc0.271.g36b669edcc

[PATCH v3 14/25] builtin/pack-objects: convert to struct object_id

[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 d2d97cc61e..9c6b224973 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 v3 11/25] builtin/reflog: convert remaining unsigned char uses to object_id

[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.15.0.rc0.271.g36b669edcc

[PATCH v3 05/25] refs: prevent accidental NULL dereference in write_pseudoref

[brian m. carlson]

Several of the refs functions take NULL to indicate that the ref is not
to be updated.  If refs_update_ref were called with a NULL new object
ID, we could pass that NULL pointer to write_pseudoref, which would then
segfault when it dereferenced it.  Instead, simply return successfully,
since if we don't want to update the pseudoref, there's nothing to do.

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

diff --git a/refs.c b/refs.c
index edd20044c6..91c2af78b6 100644
--- a/refs.c
+++ b/refs.c
@@ -583,6 +583,9 @@ static int write_pseudoref(const char *pseudoref, const 
struct object_id *oid,
struct strbuf buf = STRBUF_INIT;
int ret = -1;
 
+   if (!oid)
+   return 0;
+
strbuf_addf(, "%s\n", oid_to_hex(oid));
 
filename = git_path("%s", pseudoref);
-- 
2.15.0.rc0.271.g36b669edcc

[PATCH v3 24/25] refs: convert read_raw_ref backends to struct object_id

[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 6546346b6a..62a7621025 100644
--- a/refs.c
+++ b/refs.c
@@ -1382,10 +1382,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 */
@@ -1428,7 +1428,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;
 
/* In reading mode, refs must eventually resolve */
@@ -1879,7 +1879,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 8bf2ef8a77..c1626490c2 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 b7652738be..74f1dea0f4 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -716,7 +716,7 @@ static struct snapshot *get_snapshot(struct 
packed_ref_store *refs)
 }
 
 static int packed_read_raw_ref(struct ref_store *ref_store,
-  const char 

[PATCH v3 00/25] object_id part 10

[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.

In this iteration, I chose to skip the ALLOC_ARRAY conversion to limit
the scope of changes and because I think to be fixed properly the code
requires converting the targets parameter to size_t, which involves
touching more code unrelated to the series than I'd like to do right
now.

All the other issues that were brought up should be fixed.

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

Changes from v2:
* Rebase on master.  This drops significant parts of one patch, as Junio
  pointed out.
* Restore sane line wrappings.
* Fix several docstrings and error messages to reflect object ID instead
  of SHA-1.
* Fix potential NULL dereference in refs_delete_ref.
* Fix potential NULL dereference in expand_ref.
* Add a patch to fix potential NULL dereference in write_pseudoref.
* Restore the missing line in the refs code.
* Fix two commit message typos.

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 (25):
  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: prevent accidental NULL dereference in write_pseudoref
  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  |  10 +-
 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  |  14 +--
 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 +-
 reflog-walk.c   |   2 +-
 refs.c  | 243 ++--
 refs.h  | 103 +--
 refs/files-backend.c| 112 ++--
 refs/packed-backend.c   |  12 +--
 refs/ref-cache.c|   2 +-
 refs/refs-internal.h|  20 ++--
 remote-testsvn.c 

[PATCH v3 08/25] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

[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  | 2 +-
 builtin/clone.c | 2 +-
 builtin/fmt-merge-msg.c | 2 +-
 builtin/merge.c | 2 +-
 builtin/notes.c | 2 +-
 builtin/show-branch.c   | 6 +++---
 builtin/submodule--helper.c | 2 +-
 refs.c  | 8 
 refs.h  | 4 ++--
 submodule.c | 2 +-
 12 files changed, 20 insertions(+), 20 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 f5237541a2..c5f88b59ef 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -125,7 +125,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)
@@ -241,7 +241,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.")
@@ -636,7 +636,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 2bb009ec0e..c33dbb70fb 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -827,7 +827,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))
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
@@ -603,7 +603,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 
/* get current branch */
current_branch = current_branch_to_free =
-   resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, 
NULL);
+   resolve_refdup("HEAD", RESOLVE_REF_READING, _oid, NULL);
if (!current_branch)
die("No current branch");
if (starts_with(current_branch, "refs/heads/"))
diff --git a/builtin/merge.c b/builtin/merge.c
index 99d4b873f0..99d2df965f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1142,7 +1142,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
 * Check if we are _not_ on a detached HEAD, i.e. if 

[PATCH v3 07/25] Convert check_connected to use struct object_id

[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 2da3c4cd5c..4d37a160d7 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 v3 15/25] refs: convert peel_ref to struct object_id

[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, comments, 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| 10 +-
 refs.h|  8 
 t/helper/test-ref-store.c |  6 +++---
 upload-pack.c |  2 +-
 7 files changed, 17 insertions(+), 17 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 9c6b224973..631de28761 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 ecb43a113e..f8a2d98666 100644
--- a/refs.c
+++ b/refs.c
@@ -1697,7 +1697,7 @@ 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)
 {
int flag;
struct object_id base;
@@ -1707,7 +1707,7 @@ int refs_peel_ref(struct ref_store *refs, const char 
*refname,
 
if (ref_iterator_peel(current_ref_iter, ))
return -1;
-   hashcpy(sha1, peeled.hash);
+   oidcpy(oid, );
return 0;
}
 
@@ -1715,12 +1715,12 @@ int refs_peel_ref(struct ref_store *refs, const char 
*refname,
   RESOLVE_REF_READING, , ))
return -1;
 
-   return peel_object(base.hash, sha1);
+   return peel_object(base.hash, oid->hash);
 }
 
-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 9d59c414aa..89f28d482d 100644
--- a/refs.h
+++ b/refs.h
@@ -114,14 +114,14 @@ extern int refs_init_db(struct strbuf *err);
 /*
  * If refname is a non-symbolic reference that refers to a tag object,
  * and the tag can be (recursively) dereferenced to a non-tag object,
- * store the SHA1 of the referred-to object to sha1 and return 0.  If
- * any of these conditions are not met, return a non-zero value.
+ * store the object ID of the referred-to object to oid and return 0.
+ * If any of these conditions are not met, return a non-zero value.
  * Symbolic references are considered unpeelable, even if they
  * 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/t/helper/test-ref-store.c 

[PATCH v3 10/25] refs: convert dwim_ref and expand_ref to struct object_id

[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.  Insert a check for NULL in expand_ref on a temporary basis;
this check can be removed when resolve_ref_unsafe is converted as well.

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| 15 ---
 refs.h|  4 ++--
 remote.c  |  2 +-
 sha1_name.c   |  6 +++---
 upload-pack.c |  2 +-
 wt-status.c   |  2 +-
 15 files changed, 25 insertions(+), 24 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 a8d7e6f7ae..74aa644cbb 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)

[PATCH v3 23/25] refs: convert peel_object to struct object_id

[brian m. carlson]

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

diff --git a/refs.c b/refs.c
index 72c45a513b..6546346b6a 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;
 }
 
@@ -1714,7 +1714,7 @@ int refs_peel_ref(struct ref_store *refs, const char 
*refname,
   RESOLVE_REF_READING, , ))
return -1;
 
-   return peel_object(base.hash, oid->hash);
+   return peel_object(, oid);
 }
 
 int peel_ref(const char *refname, struct object_id *oid)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 4ec9fcacdd..b7652738be 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -880,7 +880,7 @@ static int packed_ref_iterator_peel(struct ref_iterator 
*ref_iterator,
} else if ((iter->base.flags & (REF_ISBROKEN | REF_ISSYMREF))) {
return -1;
} else {
-   return !!peel_object(iter->oid.hash, peeled->hash);
+   return !!peel_object(>oid, peeled);
}
 }
 
@@ -1220,8 +1220,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 4f850e1b5c..043eb83748 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -493,7 +493,7 @@ static int cache_ref_iterator_advance(struct ref_iterator 
*ref_iterator)
 static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,
   struct object_id *peeled)
 {
-   return peel_object(ref_iterator->oid->hash, peeled->hash);
+   return peel_object(ref_iterator->oid, peeled);
 }
 
 static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3c4781eb87..54059c1daf 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.15.0.rc0.271.g36b669edcc

[PATCH v3 18/25] sha1_file: convert index_path and index_fd to struct object_id

[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 10c3a0083d..cd910b2103 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1664,7 +1664,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)
 {
@@ -1695,15 +1695,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;
@@ -1718,22 +1718,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();
@@ -1742,14 +1742,14 @@ 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);
ssize_t read_result = read_in_full(fd, buf, size);
@@ -1760,11 +1760,11 @@ static int index_core(unsigned char *sha1, int fd, 
size_t size,
ret = error("short read while indexing %s",
path ? path : "");
else
-   ret = index_mem(sha1, buf, size, type, path, flags);
+   ret = index_mem(oid, buf, size, type, path, flags);
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;
@@ -1802,12 +1802,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
   

[PATCH v3 06/25] refs: update ref transactions to use struct object_id

[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 | 50 --
 refs.h | 38 +++---
 refs/files-backend.c   | 12 ++--
 refs/refs-internal.h   |  4 ++--
 sequencer.c|  2 +-
 walker.c   |  2 +-
 15 files changed, 69 insertions(+), 71 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 d75b3805ea..f9c9676a3f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1788,9 +1788,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 cc48767405..2da3c4cd5c 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 = 

Re: Consider escaping special characters like 'less' does

[Joris Valette]

2017-10-15 22:06 GMT+02:00 Jeff King :
> Git's diff generation will never do such escaping by default, because it
> means creating a patch that cannot be applied to get back the original
> content.

Indeed this would be a problem. That's where my knowledge of git's
source code ends, but in that case, can't the output be discriminated
against the command that was executed?
Command that outputs an applicable patch -> don't escape
Command that outputs a diff to see changes -> escape

> There _are_ already options to create diffs that cannot be applied, like
> --textconv. So it would be possible to add a similar option for
> escaping. But I don't think we really need or want a separate option,
> when you can already do one of:
>
>   1. If your files have special binary characters that are hard to see,
>  you can use the existing textconv system to do whatever escaping
>  you like. And then the Git will diff the result of the escaping,
>  which means you get readable diffs when they change.
>
>   2. Put the raw output of git's diff through a filter that escapes. We
>  already do this most of the time by piping through less. The most
>  noticeable exception is "add --patch". There you can set up a
>  program to filter as well. There's more information in a recent
>  thread here:
>
>
> https://public-inbox.org/git/20171012184736.rglkbyryauwuv...@sigill.intra.peff.net/
>
> It doesn't seem out of the question to me to have an out-of-the-box
> default for interactive.diffFilter which does some basic escaping (we
> could even implement it inside the perl script for efficiency).

Yes I read this thread, but I was left unsatisfied because I would
like something out-of-the-box.
Your suggestion might be the best solution then: implement some
default escaping for interactive.diffFilter.

Joris

Re: Consider escaping special characters like 'less' does

[Jeff King]

On Sun, Oct 15, 2017 at 06:39:45PM +0200, Joris Valette wrote:

> > It's your MUA's fault that you get a ?, the mail didn't contain any.
> 
> Actually, the original mail contained the special BOM sequence but
> it's generally invisible. His MUA shows it with a '?', mine doesn't
> show anything, neither does Firefox on the mailing list page.
> 
> The question remains: could escaping be done?

Git's diff generation will never do such escaping by default, because it
means creating a patch that cannot be applied to get back the original
content.

There _are_ already options to create diffs that cannot be applied, like
--textconv. So it would be possible to add a similar option for
escaping. But I don't think we really need or want a separate option,
when you can already do one of:

  1. If your files have special binary characters that are hard to see,
 you can use the existing textconv system to do whatever escaping
 you like. And then the Git will diff the result of the escaping,
 which means you get readable diffs when they change.

  2. Put the raw output of git's diff through a filter that escapes. We
 already do this most of the time by piping through less. The most
 noticeable exception is "add --patch". There you can set up a
 program to filter as well. There's more information in a recent
 thread here:

   
https://public-inbox.org/git/20171012184736.rglkbyryauwuv...@sigill.intra.peff.net/

It doesn't seem out of the question to me to have an out-of-the-box
default for interactive.diffFilter which does some basic escaping (we
could even implement it inside the perl script for efficiency).

-Peff

[PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

[Ralf Thielow]

When the write opertion fails, we write that we could
not read. Change the error message to match the operation
and remove the full stop at the end.

When ftruncate() fails, we write that we couldn't finish
the operation on the todo file. It is more accurate to write
that we couldn't truncate as we do in other calls of ftruncate().

Signed-off-by: Ralf Thielow 
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index e258bb646..75f5356f6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,9 +2948,9 @@ int rearrange_squash(void)
if (fd < 0)
res = error_errno(_("could not open '%s'"), todo_file);
else if (write(fd, buf.buf, buf.len) < 0)
-   res = error_errno(_("could not read '%s'."), todo_file);
+   res = error_errno(_("could not write '%s'"), todo_file);
else if (ftruncate(fd, buf.len) < 0)
-   res = error_errno(_("could not finish '%s'"),
+   res = error_errno(_("could not truncate '%s'"),
   todo_file);
close(fd);
strbuf_release();
-- 
2.15.0.rc0.296.g7b26d72

Re: [PATCH] sequencer.c: unify error messages

[Ralf Thielow]

2017-10-13 23:12 GMT+02:00 René Scharfe :
> Am 13.10.2017 um 19:51 schrieb Ralf Thielow:
>> When ftruncate() in rearrange_squash() fails, we write
>> that we couldn't finish the operation on the todo file.
>> It is more accurate to write that we couldn't truncate
>> as we do in other calls of ftruncate().
>
> Would it make sense to factor out rewriting the to-do file to avoid
> code duplication in the first place?
>
>> While at there, remove a full stop in another error message.
>>
>> Signed-off-by: Ralf Thielow 
>> ---
>>   sequencer.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index e258bb646..b0e6459a5 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>>   if (fd < 0)
>>   res = error_errno(_("could not open '%s'"), todo_file);
>>   else if (write(fd, buf.buf, buf.len) < 0)
>> - res = error_errno(_("could not read '%s'."), 
>> todo_file);
>> + res = error_errno(_("could not read '%s'"), todo_file);
>
> That should read "write", right?
>

Sure. I'll send a new version of this patch to fix the messages. Maybe
someone else picks up the other things. Thanks.

>>   else if (ftruncate(fd, buf.len) < 0)
>> - res = error_errno(_("could not finish '%s'"),
>> + res = error_errno(_("could not truncate '%s'"),
>>  todo_file);
>
> Hmm, why call ftruncate(2) instead of opening the file with O_TRUNC?
>
>>   close(fd);
>>   strbuf_release();
>>

Re: Consider escaping special characters like 'less' does

[Joris Valette]

2017-10-15 17:46 GMT+02:00 Andreas Schwab :
> On Okt 15 2017, "Jason Pyeron"  wrote:
>
>>> -Original Message-
>>> From: Joris Valette
>>> Sent: Sunday, October 15, 2017 9:34 AM
>>> To: git@vger.kernel.org
>>> Subject: Consider escaping special characters like 'less' does
>>>
>>> The pager 'less' escapes some characters when calling 'git diff'. This
>>> is what I might get:
>>>
>>> $ git diff --cached
>>> diff --git a/some_file b/some_file
>>> new file mode 100644
>>> index 000..357323f
>>> --- /dev/null
>>> +++ b/some_file
>>> @@ -0,0 +1 @@
>>> +Hello
>>> \ No newline at end of file
>>>
>>> This example is a simple file encoded in UTF-8 *with BOM*.
>>> On the other hand, the built-in git output shows this:
>>>
>>> $ git --no-pager diff --cached
>>> diff --git a/some_file b/some_file
>>> new file mode 100644
>>> index 000..357323f
>>> --- /dev/null
>>> +++ b/some_file
>>> @@ -0,0 +1 @@
>>> +?Hello
>>> \ No newline at end of file
>>
>> It is your terminal, not git's fault that you get a ? rendered.
>
> It's your MUA's fault that you get a ?, the mail didn't contain any.

Actually, the original mail contained the special BOM sequence but
it's generally invisible. His MUA shows it with a '?', mine doesn't
show anything, neither does Firefox on the mailing list page.

The question remains: could escaping be done?

Joris

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi,

On Sat, 14 Oct 2017, Junio C Hamano wrote:

> Kevin Daudt  writes:
> 
> > On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
> >> From: J Wyman 
> >> [..] 
> >> 
> >> diff --git a/Documentation/git-for-each-ref.txt 
> >> b/Documentation/git-for-each-ref.txt
> >> index 39f50bd53eb..22767025850 100644
> >> --- a/Documentation/git-for-each-ref.txt
> >> +++ b/Documentation/git-for-each-ref.txt
> >> @@ -142,8 +142,9 @@ upstream::
> >>encountered. Append `:track,nobracket` to show tracking
> >>information without brackets (i.e "ahead N, behind M").
> >>  +
> >> -Also respects `:remotename` to state the name of the *remote* instead of
> >> -the ref.
> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> +locally known by the remote.
> >
> > What does "locally known by the remote" mean in this sentence?
> 
> Good question.  I cannot offhand offer a better and concise
> phrasing, but I think can explain what it wants to describe ;-).

Yep, described it well.

Maybe "and `:remoteref` to state the name by which the remote knows the
*reference*"?

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Fri, 13 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> (The funny "squash!" followed by a complete version of the
> >> rewritten commit message is what I do until I -- or anybody else
> >> -- comes up with a patch to implement support for "reword!".)
> 
> I have wished for something like that for a long time; it has been
> (and it still is) a bit unclear what the semantics and UI should
> look like, but still, such a feature would be quite useful.

Oh, the reword! semantics are relatively easy in my mind: it would be
called as

git commit --reword 

would take staged changes (if any), otherwise implicitly --allow-empty,
then create a commit message to be edited in this style:

reword! 





This would be presented to the user in an editor (similar to `git commit
--amend`).

Upon rebase --autosquash, it would work like a squash! but comment out all
previous commit messages, and also comment out the reword! line and the
empty line after that.

In case of multiple reword!, the last one would win, and it would
naturally integrate in any fixup!/squash! workflow.

What is *more* difficult is something else I frequently need: drop!
. That is, I want to explicitly mark a commit to be excluded in
subsequent rebase --autosquash. I *guess* the best way would be to
implement a

git revert --drop 

that would work as if you called `git revert -n  && git commit -m
'drop! '"$(git show -s --oneline )", and upon rebase --autosquash,
it would reorder like fixup!/squash!/reword!, replace the `pick` of the
previous line (if it was a `pick`) by `drop` and comment out the current
`pick drop! ` line.

The reason why the semantics are more difficult in that case is that drop!
does not mix well with fixup!/squash!/reword!.

Ciao,
Dscho

Re: Consider escaping special characters like 'less' does

[Andreas Schwab]

On Okt 15 2017, "Jason Pyeron"  wrote:

>> -Original Message-
>> From: Joris Valette
>> Sent: Sunday, October 15, 2017 9:34 AM
>> To: git@vger.kernel.org
>> Subject: Consider escaping special characters like 'less' does
>> 
>> The pager 'less' escapes some characters when calling 'git diff'. This
>> is what I might get:
>> 
>> $ git diff --cached
>> diff --git a/some_file b/some_file
>> new file mode 100644
>> index 000..357323f
>> --- /dev/null
>> +++ b/some_file
>> @@ -0,0 +1 @@
>> +Hello
>> \ No newline at end of file
>> 
>> This example is a simple file encoded in UTF-8 *with BOM*.
>> On the other hand, the built-in git output shows this:
>> 
>> $ git --no-pager diff --cached
>> diff --git a/some_file b/some_file
>> new file mode 100644
>> index 000..357323f
>> --- /dev/null
>> +++ b/some_file
>> @@ -0,0 +1 @@
>> +?Hello
>> \ No newline at end of file
>
> It is your terminal, not git's fault that you get a ? rendered.

It's your MUA's fault that you get a ?, the mail didn't contain any.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] sequencer.c: unify error messages

[Johannes Schindelin]

Hi Ralf,

On Fri, 13 Oct 2017, Ralf Thielow wrote:

> When ftruncate() in rearrange_squash() fails, we write
> that we couldn't finish the operation on the todo file.
> It is more accurate to write that we couldn't truncate
> as we do in other calls of ftruncate().
> 
> While at there, remove a full stop in another error message.

Thanks, looks good (maybe s/read/write/ as pointed out by René, though?).

Ciao,
Dscho

RE: Consider escaping special characters like 'less' does

[Jason Pyeron]

> -Original Message-
> From: Joris Valette
> Sent: Sunday, October 15, 2017 9:34 AM
> To: git@vger.kernel.org
> Subject: Consider escaping special characters like 'less' does
> 
> The pager 'less' escapes some characters when calling 'git diff'. This
> is what I might get:
> 
> $ git diff --cached
> diff --git a/some_file b/some_file
> new file mode 100644
> index 000..357323f
> --- /dev/null
> +++ b/some_file
> @@ -0,0 +1 @@
> +Hello
> \ No newline at end of file
> 
> This example is a simple file encoded in UTF-8 *with BOM*.
> On the other hand, the built-in git output shows this:
> 
> $ git --no-pager diff --cached
> diff --git a/some_file b/some_file
> new file mode 100644
> index 000..357323f
> --- /dev/null
> +++ b/some_file
> @@ -0,0 +1 @@
> +?Hello
> \ No newline at end of file

It is your terminal, not git's fault that you get a ? rendered.

$ git diff
diff --git a/test.txt b/test.txt
index af9f93a..294e397 100644
--- a/test.txt
+++ b/test.txt
@@ -1 +1 @@
-the quick brown fox jumps over the lazy dog
+the quick brown fox jumps over the lazy dog

$ git diff | hexdump.exe -C
  64 69 66 66 20 2d 2d 67  69 74 20 61 2f 74 65 73  |diff --git a/tes|
0010  74 2e 74 78 74 20 62 2f  74 65 73 74 2e 74 78 74  |t.txt b/test.txt|
0020  0a 69 6e 64 65 78 20 61  66 39 66 39 33 61 2e 2e  |.index af9f93a..|
0030  32 39 34 65 33 39 37 20  31 30 30 36 34 34 0a 2d  |294e397 100644.-|
0040  2d 2d 20 61 2f 74 65 73  74 2e 74 78 74 0a 2b 2b  |-- a/test.txt.++|
0050  2b 20 62 2f 74 65 73 74  2e 74 78 74 0a 40 40 20  |+ b/test.txt.@@ |
0060  2d 31 20 2b 31 20 40 40  0a 2d 74 68 65 20 71 75  |-1 +1 @@.-the qu|
0070  69 63 6b 20 62 72 6f 77  6e 20 66 6f 78 20 6a 75  |ick brown fox ju|
0080  6d 70 73 20 6f 76 65 72  20 74 68 65 20 6c 61 7a  |mps over the laz|

Note: 0a is newline, 2b is + ef bb bf are the specials added to the file, 74 68 
65 20 71 75 is 'the qu'

0090  79 20 64 6f 67 0a 2b ef  bb bf 74 68 65 20 71 75  |y dog.+...the qu|
00a0  69 63 6b 20 62 72 6f 77  6e 20 66 6f 78 20 6a 75  |ick brown fox ju|
00b0  6d 70 73 20 6f 76 65 72  20 74 68 65 20 6c 61 7a  |mps over the laz|
00c0  79 20 64 6f 67 0a |y dog.|
00c6

$ git diff | cat
diff --git a/test.txt b/test.txt
index af9f93a..294e397 100644
--- a/test.txt
+++ b/test.txt
@@ -1 +1 @@
-the quick brown fox jumps over the lazy dog
+the quick brown fox jumps over the lazy dog

$ git --no-pager diff
diff --git a/test.txt b/test.txt
index af9f93a..294e397 100644
--- a/test.txt
+++ b/test.txt
@@ -1 +1 @@
-the quick brown fox jumps over the lazy dog
+the quick brown fox jumps over the lazy dog

And my UTF-8 capable terminal displays it fine.

What do you get when you pipe to hexdump?

-Jason

Re: [ANNOUNCE] Git for Windows 2.14.2(3)

[Johannes Schindelin]

Hi Steve,

On Fri, 13 Oct 2017, Steve Hoelzer wrote:

> On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin
>  wrote:
> >
> > It is my pleasure to announce that Git for Windows 2.14.2(3) is available 
> > from:
> >
> > https://git-for-windows.github.io/
> >
> > Changes since Git for Windows v2.14.2(2) (October 5th 2017)
> >
> > New Features
> >
> >   * Comes with Git LFS v2.3.3.
> 
> I just ran "git update" and afterward "git version" reported
> 2.14.2(3), but "git lfs version" still said 2.3.2.
> 
> I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2.

Ah bummer. I forgot to actually update it in the VM where I build the
releases :-(

Will work on it tomorrow.

Thanks for double-checking,
Johannes

Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick

[Johannes Sixt]

Am 13.10.2017 um 12:51 schrieb Ævar Arnfjörð Bjarmason:

On Thu, Oct 05 2017, Johannes Sixt jotted:

Am 05.10.2017 um 21:33 schrieb Stefan Beller:

* Is it a good design choice to have a different command, just because the
target branch is [not] checked out?


I would not want to make it a sub-mode of cherry-pick, if that is what
you mean, because "cherry picking" is about getting something, not
giving something away.


It occurs to me that a better long-term UI design might be to make
git-{cherry-pick,pick) some sort of submodes for git-commit.


I don't quite agree. To commit an index state that is derived from a 
worktree state is such a common and important operation that it deserves 
to be its own command. Adding different modi operandi would make it 
confusing.



Right now git-commit picks the current index for committing, but "use a
patch as the source instead" could be a submode.

Right now it commits that change on top of your checked out commit, but
"other non-checked-out target commit" could be a mode instead,
i.e. exposing more of the power of the underlying git-commit-tree.


This is worth discussing, though not my preference. The picture to "pick 
cherries" has become quite common, and now that we use it for the name 
of the command, "cherry-pick", the direction of flow is quite obvious 
and strongly implied: from somewhere else to me (and not to somebody else).



[Not entirely serious]. Well if cherry-picking is taking a thing and
eating it here, maybe git-cherry-puke takes an already digested thing
and "throws" it elsewhere ?:)

It's a silly name but it's somewhat symmetric :)


One of the decisions to be made is whether to begin the new command with 
"git-cherry-" or not, because it introduces a new abiguity for command 
line completion.


-- Hannes

Consider escaping special characters like 'less' does

[Joris Valette]

The pager 'less' escapes some characters when calling 'git diff'. This
is what I might get:

$ git diff --cached
diff --git a/some_file b/some_file
new file mode 100644
index 000..357323f
--- /dev/null
+++ b/some_file
@@ -0,0 +1 @@
+Hello
\ No newline at end of file

This example is a simple file encoded in UTF-8 *with BOM*.
On the other hand, the built-in git output shows this:

$ git --no-pager diff --cached
diff --git a/some_file b/some_file
new file mode 100644
index 000..357323f
--- /dev/null
+++ b/some_file
@@ -0,0 +1 @@
+Hello
\ No newline at end of file




You can see 'less' shows  explicitly, while it is implicit and
hidden with git's built-in output.
Other characters behave the same, like ZERO WIDTH SPACE , and
I believe many others.
This is particularly annoying when there are other changes on the same
line. Here I add ' world!' but also remove the BOM:

$ git diff
diff --git a/some_file b/some_file
index 357323f..6769dd6 100644
--- a/some_file
+++ b/some_file
@@ -1 +1 @@
-Hello
\ No newline at end of file
+Hello world!
\ No newline at end of file

Compare with:

$ git --no-pager diff
diff --git a/some_file b/some_file
index 357323f..6769dd6 100644
--- a/some_file
+++ b/some_file
@@ -1 +1 @@
-Hello
\ No newline at end of file
+Hello world!
\ No newline at end of file




In the first example I can see both changes, in the second example the
BOM removal is hidden and I won't notice it because there is indeed
another genuine change on the same line.

I'll add a third example with CRLF line-terminators:

$ git diff
diff --git a/some_file b/some_file
index 357323f..dfd6895 100644
--- a/some_file
+++ b/some_file
@@ -1 +1 @@
-Hello
\ No newline at end of file
+Hello world!^M

and:

$ git --no-pager diff
diff --git a/some_file b/some_file
index 357323f..dfd6895 100644
--- a/some_file
+++ b/some_file
@@ -1 +1 @@
-Hello
\ No newline at end of file
+Hello world!




My thoughts are that git should add this feature and tend towards
less's behavior. A diff output should show as much as possible.
I would happily use 'less' all the time, but unfortunately I don't
think I can. For instance, 'git add --patch', which I use quite often,
prints through git's built-in output, and I miss less's features.

Joris Valette.

Re: Bug: git ls-remote -h / --head results differ in output

[René Scharfe]

Am 15.10.2017 um 13:08 schrieb Martin Ågren:
> On 15 October 2017 at 12:02, Thomas Rikl  wrote:
>> Example:
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
>> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]
>>   [-q | --quiet] [--exit-code] [--get-url]
>>   [--symref] [ [...]]
>>
>>  -q, --quiet   do not print remote URL
>>  --upload-pack   path of git-upload-pack on the remote host
>>  -t, --tagslimit to tags
>>  -h, --heads   limit to heads
>>  --refsdo not show peeled tags
>>  --get-url take url..insteadOf into account
>>  --exit-code   exit with exit code 2 if no matching refs are
>> found
>>  --symref  show underlying ref in addition to the object
>> pointed by it
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
>>  From https://github.com/syl20bnr/spacemacs
>> 07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
>> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
>> 2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
>> 8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
>> d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
>> 44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
>> 9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
>> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
>> tom1 ~/emacs/spacemacs/.emacs.d $ git --version
>> git version 2.14.2
>>
>> on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 09:58:47
>> CEST 2017 x86_64 GNU/Linux
> 
> The behavior you observed matches with the documentation in gitcli(7)
> and is arguably correct. A lone "-h" prints the help/usage. But I would
> agree that this can be confusing, especially when considering
> git-ls-remote(1) on its own, without any extra knowledge about what a
> lone -h should do.
> 
> So -h and --heads can only be used interchangeably if you have other
> arguments. To see this, you could, e.g., try "git ls-remote -h -h".
> 
> Some more details. This looks like ba5f28bf7 (ls-remote: use
> parse-options api, 2016-01-19). Before that, "-h" was handled internally
> in builtin/ls-files.c. After that it is handled in the general
> options-parsing machinery. See for example 5ad0d3d52 (parse-options:
> allow -h as a short option, 2015-11-17), which explicitly wants to print
> the usage-string if "-h" is given as the lone argument.
> 
> I'm not sure which is the best way forward here, or how many other
> commands could have similar pitfalls. For example, the "-h"-flag of git
> grep shouldn't be able to cause this problem.

The flag PARSE_OPT_NO_INTERNAL_HELP should be used to let git ls-remote
fully handle -h.

The same goes for git show-ref, but perhaps it's better to remove its
hidden option -h by now.

But stepping back a bit I wonder if the demure internal -h handler (that
only speaks up when nothing else is said) is a bit too subtle.
Reverting 5ad0d3d52 would make the need for PARSE_OPT_NO_INTERNAL_HELP
more obvious.

René

Re: Bug: git ls-remote -h / --head results differ in output

[Martin Ågren]

On 15 October 2017 at 12:02, Thomas Rikl  wrote:
> Example:
>
> tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8
>
> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]
>  [-q | --quiet] [--exit-code] [--get-url]
>  [--symref] [ [...]]
>
> -q, --quiet   do not print remote URL
> --upload-pack   path of git-upload-pack on the remote host
> -t, --tagslimit to tags
> -h, --heads   limit to heads
> --refsdo not show peeled tags
> --get-url take url..insteadOf into account
> --exit-code   exit with exit code 2 if no matching refs are
> found
> --symref  show underlying ref in addition to the object
> pointed by it
>
> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
> From https://github.com/syl20bnr/spacemacs
> 07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
> 2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
> 8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
> d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
> 44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
> 9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
> tom1 ~/emacs/spacemacs/.emacs.d $ git --version
> git version 2.14.2
>
> on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 09:58:47
> CEST 2017 x86_64 GNU/Linux

The behavior you observed matches with the documentation in gitcli(7)
and is arguably correct. A lone "-h" prints the help/usage. But I would
agree that this can be confusing, especially when considering
git-ls-remote(1) on its own, without any extra knowledge about what a
lone -h should do.

So -h and --heads can only be used interchangeably if you have other
arguments. To see this, you could, e.g., try "git ls-remote -h -h".

Some more details. This looks like ba5f28bf7 (ls-remote: use
parse-options api, 2016-01-19). Before that, "-h" was handled internally
in builtin/ls-files.c. After that it is handled in the general
options-parsing machinery. See for example 5ad0d3d52 (parse-options:
allow -h as a short option, 2015-11-17), which explicitly wants to print
the usage-string if "-h" is given as the lone argument.

I'm not sure which is the best way forward here, or how many other
commands could have similar pitfalls. For example, the "-h"-flag of git
grep shouldn't be able to cause this problem.

Martin

Bug: git ls-remote -h / --head results differ in output

[Thomas Rikl]

Example:

tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8

tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]
 [-q | --quiet] [--exit-code] [--get-url]
 [--symref] [ [...]]

    -q, --quiet   do not print remote URL
    --upload-pack   path of git-upload-pack on the remote host
    -t, --tags    limit to tags
    -h, --heads   limit to heads
    --refs    do not show peeled tags
    --get-url take url..insteadOf into account
    --exit-code   exit with exit code 2 if no matching refs are 
found
    --symref  show underlying ref in addition to the object 
pointed by it


tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
From https://github.com/syl20bnr/spacemacs
07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
tom1 ~/emacs/spacemacs/.emacs.d $ git --version
git version 2.14.2

on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 
09:58:47 CEST 2017 x86_64 GNU/Linux

CONTACT ME ON THIS REGARDS

[Yeger]

I request your connsent for an urgent bussiness deal for you.
 
Kindly get back to me for details.
 
Chana