Re: I suggest a new feature: One copy from files

[Samuel Lijin]

Looping the listserv back in, didn't realize this had gone off thread.

I'm not sure what you mean by "update only the last change, and not
record the old changes".

update-index is, like most Git commands, per repo.

On Mon, Apr 24, 2017 at 12:08 AM, Rm Beer  wrote:
> Yes, but i say about of have a update only the last change, and not
> record the old changes. Interesting command the 'update-index', this
> is a permanent config for each dir/files?
>
> 2017-04-24 1:59 GMT-03:00 Samuel Lijin :
>> Ah - I see what you're asking for now, basically the ability to tell Git to
>> ignore changes to a file once you've already started tracking it, right?
>>
>> If I'm not mistaken, git update-index --skip-worktree will do this for you.
>> (I'm on my phone, so I don't have easy access to documentation right now.)
>>
>> Hope this helps.
>>
>> On Apr 23, 2017 11:51 PM, "Rm Beer"  wrote:
>>>
>>> in a repository have two behaviors. (How understand i)
>>>
>>> 1) A file can check and updated in the repository, take any change
>>> from the file like a record logs.
>>> 2) If the file have in .gitignore . Git never see it.
>>>
>>> I suggest add a new behaviors:
>>>
>>> 3) A file can check and updated in the repository. But never add in
>>> the record logs, only have one copy in the repository. (Maybe add
>>> filters in .gitonecopy or something)
>>>
>>>
>>> 2017-04-24 1:02 GMT-03:00 Samuel Lijin :
>>> > ...what?
>>> >
>>> > I'm sorry, I have absolutely no idea what you're asking. You're going
>>> > to have to be a lot more specific with your description of the desired
>>> > behavior because as is, I have no idea what purpose your .gitonecopy
>>> > or .gitonelog would serve. I also have no idea what this has to do
>>> > with the binary files ignored by .gitignore.
>>> >
>>> > It would probably make sense for you to describe a use case for this
>>> > as well, to help us understand why you want what you're asking for.
>>> >
>>> > On Sun, Apr 23, 2017 at 10:47 PM, Rm Beer  wrote:
>>> >> I have a several directories with binary files datas that is discard
>>> >> by .gitignore.
>>> >>
>>> >> I recommend make a new .gitonecopy or .gitonelog or something that
>>> >> take the directories with only 1 copy of last updated and not take
>>> >> history of files in the repository.
>>> >> Maybe anyone found other best method for apply this idea.

Re: [PATCH v6 3/8] convert: Split start_multi_file_filter into two separate functions

[Junio C Hamano]

Ben Peart  writes:

> Subject: [PATCH v6 3/8] convert: Split start_multi_file_filter into two 
> separate functions

Two minor nits, because the capital after ":" looks ugly in shortlog
output, and having () there makes it easier to notice that a
function name is being discussed.  I.e.

convert: split start_multi_file_filter() into two separate functions

> To enable future reuse of the filter..process infrastructure,
> split start_multi_file_filter into two separate parts.
>
> start_multi_file_filter will now only contain the generic logic to
> manage the creation and tracking of the child process in a hashmap.
>
> start_multi_file_filter_fn is a protocol specific initialization
> function that will negotiate the multi-file-filter interface version
> and capabilities.

The above fails to describe a lot more important/significant change
this patch makes.  Two mistaken check "errno == EPIPE" have been
removed as a preliminary bugfix.  I think the bugfix deserves to be
split into a separate patch by itself and hoisted much earlier in
the series.  It alone is a very good change, with or without the
remainder of the changes in this patch.

Thanks.

> Signed-off-by: Ben Peart 
> ---
>  convert.c | 62 --
>  1 file changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index 793c29ebfd..36401fe087 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -565,35 +565,14 @@ static void stop_multi_file_filter(struct child_process 
> *process)
>   finish_command(process);
>  }
>  
> -static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, 
> const char *cmd)
> +static int start_multi_file_filter_fn(struct cmd2process *entry)
>  {
>   int err;
> - struct cmd2process *entry;
> - struct child_process *process;
> - const char *argv[] = { cmd, NULL };
>   struct string_list cap_list = STRING_LIST_INIT_NODUP;
>   char *cap_buf;
>   const char *cap_name;
> -
> - entry = xmalloc(sizeof(*entry));
> - entry->cmd = cmd;
> - entry->supported_capabilities = 0;
> - process = >process;
> -
> - child_process_init(process);
> - process->argv = argv;
> - process->use_shell = 1;
> - process->in = -1;
> - process->out = -1;
> - process->clean_on_exit = 1;
> - process->clean_on_exit_handler = stop_multi_file_filter;
> -
> - if (start_command(process)) {
> - error("cannot fork to run external filter '%s'", cmd);
> - return NULL;
> - }
> -
> - hashmap_entry_init(entry, strhash(cmd));
> + struct child_process *process = >process;
> + const char *cmd = entry->cmd;
>  
>   sigchain_push(SIGPIPE, SIG_IGN);
>  
> @@ -642,7 +621,38 @@ static struct cmd2process 
> *start_multi_file_filter(struct hashmap *hashmap, cons
>  done:
>   sigchain_pop(SIGPIPE);
>  
> - if (err || errno == EPIPE) {
> + return err;
> +}
> +
> +static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, 
> const char *cmd)
> +{
> + int err;
> + struct cmd2process *entry;
> + struct child_process *process;
> + const char *argv[] = { cmd, NULL };
> +
> + entry = xmalloc(sizeof(*entry));
> + entry->cmd = cmd;
> + entry->supported_capabilities = 0;
> + process = >process;
> +
> + child_process_init(process);
> + process->argv = argv;
> + process->use_shell = 1;
> + process->in = -1;
> + process->out = -1;
> + process->clean_on_exit = 1;
> + process->clean_on_exit_handler = stop_multi_file_filter;
> +
> + if (start_command(process)) {
> + error("cannot fork to run external filter '%s'", cmd);
> + return NULL;
> + }
> +
> + hashmap_entry_init(entry, strhash(cmd));
> +
> + err = start_multi_file_filter_fn(entry);
> + if (err) {
>   error("initialization for external filter '%s' failed", cmd);
>   kill_multi_file_filter(hashmap, entry);
>   return NULL;
> @@ -733,7 +743,7 @@ static int apply_multi_file_filter(const char *path, 
> const char *src, size_t len
>  done:
>   sigchain_pop(SIGPIPE);
>  
> - if (err || errno == EPIPE) {
> + if (err) {
>   if (!strcmp(filter_status.buf, "error")) {
>   /* The filter signaled a problem with the file. */
>   } else if (!strcmp(filter_status.buf, "abort")) {

Re: [PATCH v6 1/8] pkt-line: add packet_read_line_gently()

[Junio C Hamano]

Ben Peart  writes:

> Add packet_read_line_gently() to enable reading a line without dying on
> EOF.
>
> Signed-off-by: Ben Peart 
> ---
>  pkt-line.c | 14 +-
>  pkt-line.h | 10 ++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index d4b6bfe076..bfdb177b34 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -315,7 +315,7 @@ static char *packet_read_line_generic(int fd,
> PACKET_READ_CHOMP_NEWLINE);
>   if (dst_len)
>   *dst_len = len;
> - return len ? packet_buffer : NULL;
> + return (len > 0) ? packet_buffer : NULL;
>  }

The log does not seem to explain what this change is about.  

Is this supposed to be a preliminary bugfix where this helper used
to return a non-NULL buffer when underlying packet_read() signaled
an error by returning a negative len?  If so, this should probably
be a separate patch early in the series.

Should len==0 be considered an error?  Especially given that
PACKET_READ_CHOMP_NEWLINE is in use, I would expect that len==0
should be treated similarly to positive length, i.e. the otherside
gave us an empty line.

>  char *packet_read_line(int fd, int *len_p)
> @@ -323,6 +323,18 @@ char *packet_read_line(int fd, int *len_p)
>   return packet_read_line_generic(fd, NULL, NULL, len_p);
>  }
>  
> +int packet_read_line_gently(int fd, int *dst_len, char** dst_line)

ERROR: "foo** bar" should be "foo **bar"
#29: FILE: pkt-line.c:326:
+int packet_read_line_gently(int fd, int *dst_len, char** dst_line)

> +{
> + int len = packet_read(fd, NULL, NULL,
> +   packet_buffer, sizeof(packet_buffer),
> +   
> PACKET_READ_CHOMP_NEWLINE|PACKET_READ_GENTLE_ON_EOF);
> + if (dst_len)
> + *dst_len = len;
> + if (dst_line)
> + *dst_line = (len > 0) ? packet_buffer : NULL;

I have the same doubt as above for len == 0 case.

> + return len;
> +}
> +
>  char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)
>  {
>   return packet_read_line_generic(-1, src, src_len, dst_len);
> diff --git a/pkt-line.h b/pkt-line.h
> index 18eac64830..ad30db101a 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -74,6 +74,16 @@ int packet_read(int fd, char **src_buffer, size_t 
> *src_len, char
>  char *packet_read_line(int fd, int *size);
>  
>  /*
> + * Convenience wrapper for packet_read that sets the 
> PACKET_READ_GENTLE_ON_EOF
> + * and CHOMP_NEWLINE options. The return value specifies the number of bytes
> + * read into the buffer or -1 on truncated input. If the *dst_line parameter
> + * is not NULL it will return NULL for a flush packet and otherwise points to
> + * a static buffer (that may be overwritten by subsequent calls). If the size
> + * parameter is not NULL, the length of the packet is written to it.
> + */
> +int packet_read_line_gently(int fd, int *size, char** dst_line);
> +
> +/*
>   * Same as packet_read_line, but read from a buf rather than a descriptor;
>   * see packet_read for details on how src_* is used.
>   */

Re: I suggest a new feature: One copy from files

[Samuel Lijin]

...what?

I'm sorry, I have absolutely no idea what you're asking. You're going
to have to be a lot more specific with your description of the desired
behavior because as is, I have no idea what purpose your .gitonecopy
or .gitonelog would serve. I also have no idea what this has to do
with the binary files ignored by .gitignore.

It would probably make sense for you to describe a use case for this
as well, to help us understand why you want what you're asking for.

On Sun, Apr 23, 2017 at 10:47 PM, Rm Beer  wrote:
> I have a several directories with binary files datas that is discard
> by .gitignore.
>
> I recommend make a new .gitonecopy or .gitonelog or something that
> take the directories with only 1 copy of last updated and not take
> history of files in the repository.
> Maybe anyone found other best method for apply this idea.

Re: [PATCH] test-lib: abort when can't remove trash directory

[Junio C Hamano]

Junio C Hamano  writes:

>> That looks fine, assuming the answer to the "is the cwd important"
>> question is "no".
>
> And I do think the answer would be "yes", unfortunately.  There are
> systems that do not even allow a file to be removed while it is
> open, so...

In addition to "some platforms may not want cwd removed", this
directory would be where test_at_end_hook_ will be running in, so
we'd better be consistent with the current behaviour.

Second try that hopefully is much less damaging

 t/test-lib.sh | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cb0766b9ee..4e8f511870 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -760,12 +760,15 @@ test_done () {
say "1..$test_count$skip_all"
fi
 
-   test -d "$remove_trash" ||
-   error "Tests passed but trash directory already removed before 
test cleanup; aborting"
+   if test -z "$debug"
+   then
+   test -d "$TRASH_DIRECTORY" ||
+   error "Tests passed but trash directory already removed 
before test cleanup; aborting"
 
-   cd "$(dirname "$remove_trash")" &&
-   rm -rf "$(basename "$remove_trash")" ||
-   error "Tests passed but test cleanup failed; aborting"
+   cd "$(dirname "$TRASH_DIRECTORY")" &&
+   rm -fr "$TRASH_DIRECTORY" ||
+   error "Tests passed but test cleanup failed; aborting"
+   fi
 
test_at_end_hook_
 

I suggest a new feature: One copy from files

[Rm Beer]

I have a several directories with binary files datas that is discard
by .gitignore.

I recommend make a new .gitonecopy or .gitonelog or something that
take the directories with only 1 copy of last updated and not take
history of files in the repository.
Maybe anyone found other best method for apply this idea.

Re: [PATCH v4 0/9] Introduce timestamp_t for timestamps

[Junio C Hamano]

Johannes Schindelin  writes:

> Changes since v3:
>
> - fixed the fix in archive-zip.c that tried to report a too large
>   timestamp (and would have reported the uninitialized time_t instead)
>
> - adjusted the so-far forgotten each_reflog() function (that was
>   introduced after v1, in 80f2a6097c4 (t/helper: add test-ref-store to
>   test ref-store functions, 2017-03-26)) to use timestamp_t and PRItime,
>   too
>
> - removed the date_overflows() check from time_to_tm(), as it calls
>   gm_time_t() which already performs that check
>
> - the date_overflows() check in show_ident_date() was removed, as we do
>   not know at that point yet whether we use the system functions to
>   render the date or not (and there would not be a problem in the latter
>   case)

Assuming that the list consensus is to go with a separate
timestamp_t (for that added Cc for those whose comments I saw in an
earlier round), the patches looked mostly good (I didn't read with
fine toothed comb the largest one 6/8 to see if there were
inadvertent or missed conversions from ulong to timestamp_t,
though), modulo a few minor "huh?" comments I sent separately.

Will queue; thanks.

Re: [PATCH v4 8/9] Use uintmax_t for timestamps

[Junio C Hamano]

Johannes Schindelin  writes:

> Previously, we used `unsigned long` for timestamps. This was only a good
> choice on Linux, where we know implicitly that `unsigned long` is what is
> used for `time_t`.
>
> However, we want to use a different data type for timestamps for two
> reasons:
>
> - there is nothing that says that `unsigned long` should be the same data
>   type as `time_t`, and indeed, on 64-bit Windows for example, it is not:
>   `unsigned long` is 32-bit but `time_t` is 64-bit.
>
> - even on 32-bit Linux, where `unsigned long` (and thereby `time_t`) is
>   32-bit, we *want* to be able to encode timestamps in Git that are
>   currently absurdly far in the future, *even if* the system library is
>   not able to format those timestamps into date strings.
>
> So let's just switch to the maximal integer type available, which should
> be at least 64-bit for all practical purposes these days. It certainly
> cannot be worse than `unsigned long`, so...

Should we at least clamp in date_overflows() so that large values
representable with timestamp_t that will become unrepresentable when
we start allowing negative timestamps would be rejected?  That way
we won't have to hear complaints from the people who used timestamps
far in the future that we regressed the implementation for them by
halving the possible timestamp range.



> Signed-off-by: Johannes Schindelin 
> ---
>  git-compat-util.h | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 72c12173a14..c678ca94b8f 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -319,10 +319,14 @@ extern char *gitdirname(char *);
>  #define PRIo32 "o"
>  #endif
>  
> -typedef unsigned long timestamp_t;
> -#define PRItime "lu"
> -#define parse_timestamp strtoul
> +typedef uintmax_t timestamp_t;
> +#define PRItime PRIuMAX
> +#define parse_timestamp strtoumax
> +#ifdef ULLONG_MAX
> +#define TIME_MAX ULLONG_MAX
> +#else
>  #define TIME_MAX ULONG_MAX
> +#endif
>  
>  #ifndef PATH_SEP
>  #define PATH_SEP ':'

[PATCH] rebase -i: add config to abbreviate command name

[Liam Beguin]

Add the 'rebase.abbrevCmd' boolean config option to allow
the user to abbreviate the default command name while editing
the 'git-rebase-todo' file.

Signed-off-by: Liam Beguin 
---
Notes:

 *  This allows the lines to remain aligned when using single
letter commands.

 Documentation/config.txt | 3 +++
 Documentation/git-rebase.txt | 3 +++
 git-rebase--interactive.sh   | 8 ++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d5155..59b64832aeb4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2614,6 +2614,9 @@ rebase.instructionFormat::
the instruction list during an interactive rebase.  The format will 
automatically
have the long commit hash prepended to the format.
 
+rebase.abbrevCmd::
+   If set to true, abbreviate command name in interactive mode.
+
 receive.advertiseAtomic::
By default, git-receive-pack will advertise the atomic push
capability to its clients. If you don't want to advertise this
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e688315..0c423d903625 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -222,6 +222,9 @@ rebase.missingCommitsCheck::
 rebase.instructionFormat::
Custom commit list format to use during an `--interactive` rebase.
 
+rebase.abbrevCmd::
+   If set to true, abbreviate command name in interactive mode.
+
 OPTIONS
 ---
 --onto ::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c9c0165b5ab..9f3e82b79615 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1210,6 +1210,10 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
 fi
+
+rebasecmd=pick
+test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
+
 format=$(git config --get rebase.instructionFormat)
 # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to 
parse
 git rev-list $merges_option --format="%m%H ${format:-%s}" \
@@ -1228,7 +1232,7 @@ do
 
if test t != "$preserve_merges"
then
-   printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+   printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
else
if test -z "$rebase_root"
then
@@ -1246,7 +1250,7 @@ do
if test f = "$preserve"
then
touch "$rewritten"/$sha1
-   printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+   printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" 
>>"$todo"
fi
fi
 done
-- 
2.9.3

Re: [PATCH v4 4/9] Specify explicitly where we parse timestamps

[Junio C Hamano]

Johannes Schindelin  writes:

> Currently, Git's source code represents all timestamps as `unsigned
> long`. In preparation for using a more appropriate data type, let's
> introduce a symbol `parse_timestamp` (currently being defined to
> `strtoul`) where appropriate, so that we can later easily switch to,
> say, use `strtoull()` instead.

Apparently a very good first step in this series.

Re: [PATCH v4 7/9] Abort if the system time cannot handle one of our timestamps

[Junio C Hamano]

Johannes Schindelin  writes:

> We are about to switch to a new data type for time stamps that is
> definitely not smaller or equal, but larger or equal to time_t.
>
> So before using the system functions to process or format timestamps,
> let's make extra certain that they can handle what we feed them.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  date.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/date.c b/date.c
> index 92ab31aa441..75f6335cd09 100644
> --- a/date.c
> +++ b/date.c
> @@ -46,7 +46,10 @@ static time_t gm_time_t(timestamp_t time, int tz)
>   minutes = tz < 0 ? -tz : tz;
>   minutes = (minutes / 100)*60 + (minutes % 100);
>   minutes = tz < 0 ? -minutes : minutes;
> - return time + minutes * 60;
> +
> + if (date_overflows(time + minutes * 60))
> + die("Timestamp too large for this system: %"PRItime, time);
> + return (time_t)time + minutes * 60;
>  }

All the other calls to date_overflows() take a variable that holds
timestamp_t and presumably they are checking for integer wraparound
when the values are computed, but this one is not.  Perhaps we want
to make it a bit more careful here?  I wonder if something like this 
is a good approach:

#define date_overflows(time) date_overflows_add(time, 0)

int date_overflows_add(timestamp_t base, timestamp_t minutes)
{
timestamp_t t;
if (unsigned_add_overflows(base, minutes))
return 1;
t = base + minutes;
if ((uintmax_t) t >= TIME_MAX)
return 1;
... what you have in date_overflows() ...
}

Re: [PATCH] test-lib: abort when can't remove trash directory

[Junio C Hamano]

Jeff King  writes:

> On Sun, Apr 23, 2017 at 05:14:54PM -0700, Junio C Hamano wrote:
>
>> OK.  I am wondering why we do not do 
>> 
>>  rm -fr "$TRASH_DIRECTORY"
>> 
>> and do this instead:
>> 
>>  cd "$(dirname "$remove_trash")" &&
>>  rm -rf "$(basename "$remove_trash")"
>> 
>> in the original.  It feels somewhat unnatural.
>
> I assumed the "cd" was there because some systems may be unhappy
> removing a directory which is our current working directory. That might
> just be superstition, though.

Ahh, OK, that makes sense.  I forgot about that.

> The use of "basename" in the second does seem superfluous, since the
> trash directory should be the full path (I suspect it wasn't in the
> early days, though).
>
>> So perhaps we can simplify and make it more robust by doing this?
>> [...]
>> +if test -z "$debug"
>> +then
>> +test -d "$TRASH_DIRECTORY" ||
>> +error "Tests passed but trash directory already removed 
>> before test cleanup; aborting"
>> +
>> +rm -fr "$TRASH_DIRECTORY" ||
>> +error "Tests passed but test cleanup failed; aborting"
>> +fi
>
> That looks fine, assuming the answer to the "is the cwd important"
> question is "no".

And I do think the answer would be "yes", unfortunately.  There are
systems that do not even allow a file to be removed while it is
open, so...

Re: [PATCH] doc: git-pull.txt use US spelling, fix minor typo

[Junio C Hamano]

René Genz  writes:

> ---

Missing sign-off.  Otherwise all good changes.  Thanks.

> Instead of using two command lines I could have replaced the comma with a 
> semicolon.
>
> I do not mind, if this patch is squashed into the other minor typo fixes of 
> mine:
> 3c228f462d02e76956062b8d8572158cbcdbbc7b
>
>
>  Documentation/git-pull.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 4470e4b..942af8e 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -67,7 +67,7 @@ with uncommitted changes is discouraged: while possible, it 
> leaves you
>  in a state that may be hard to back out of in the case of a conflict.
>  
>  If any of the remote changes overlap with local uncommitted changes,
> -the merge will be automatically cancelled and the work tree untouched.
> +the merge will be automatically canceled and the work tree untouched.
>  It is generally best to get any local changes in working order before
>  pulling or stash them away with linkgit:git-stash[1].
>  
> @@ -210,7 +210,8 @@ EXAMPLES
>current branch:
>  +
>  
> -$ git pull, git pull origin
> +$ git pull
> +$ git pull origin
>  
>  +
>  Normally the branch merged in is the HEAD of the remote repository,

Re: [PATCH] doc: update SubmittingPatches

[Junio C Hamano]

René Genz  writes:

 (updates for minor irritations skipped as I do not have strong
 opinions against them).

> @@ -261,7 +261,7 @@ smaller project it is a good discipline to follow it.
>  The sign-off is a simple line at the end of the explanation for
>  the patch, which certifies that you wrote it or otherwise have
>  the right to pass it on as a open-source patch.  The rules are
> -pretty simple: if you can certify the below:
> +pretty simple: if you can certify the below D-C-O:

OK.  There is another instance of D-C-O away from here and new
people who read the document would not know without this addition
what it refers to.  This is a good addition.

>  Developer's Certificate of Origin 1.1
>  
> @@ -376,6 +376,25 @@ from the list and queue it to 'pu', in order to make it 
> easier for
>  people play with it without having to pick up and apply the patch to
>  their trees themselves.


An oversimplified one is not necessarily welcome.  Can you split
this patch into "fixes" (all of the above) and the remainder?

>  
> +
> +An oversimplified summary of the commands to run:

> +* clone repo
> +  $ git clone https://github.com/git/git
> +
> +* change files in your local repo copy
> +
> +* commit your changes
> +  $ git commit -a
> +
> +* create '.patch' file for the latest commit
> +  $ git format-patch HEAD^
> +
> +* install 'git-email' package and configure it, f.e.
> +
> https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/
> +  send an email to yourself using your MUA in order to find out the value
> +  for the "--smtp-domain" option; look at the 'Received' header option
> +  $ git send-email --annotate --smtp-domain=LONGSTRING 
> --to=git@vger.kernel.org --cc=MAINTAINER --smtp-debug=1 NAME.patch
> +
>  
>  Know the status of your patch after submission

Re: [PATCH] fix minor typing mistakes

[Junio C Hamano]

René Genz  writes:

> Thanks-to: Stefan Beller 
> ---

It is nice to give credit to where credit is due (it is more common
to say Helped-by: around here, though), but don't forget to sign off
your patch yourself ;-)  IOW

Helped-by: Stefan Beller 
Signed-off-by: René Genz 

>  Documentation/git-commit.txt| 4 ++--
>  Documentation/gitremote-helpers.txt | 2 +-
>  ci/run-windows-build.sh | 2 +-
>  diff.c  | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)

This "a the" and "the a" seem to be quite a popular typo.  Thanks
for catching them.

> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index ed0f5b9..afb06ad 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -95,7 +95,7 @@ OPTIONS
>  
>  --reset-author::
>   When used with -C/-c/--amend options, or when committing after a
> - a conflicting cherry-pick, declare that the authorship of the
> + conflicting cherry-pick, declare that the authorship of the
>   resulting commit now belongs to the committer. This also renews
>   the author timestamp.
>  
> @@ -112,7 +112,7 @@ OPTIONS
>   `--dry-run`.
>  
>  --long::
> - When doing a dry-run, give the output in a the long-format.
> + When doing a dry-run, give the output in the long-format.
>   Implies `--dry-run`.
>  
>  -z::
> diff --git a/Documentation/gitremote-helpers.txt 
> b/Documentation/gitremote-helpers.txt
> index e4b785e..4a584f3 100644
> --- a/Documentation/gitremote-helpers.txt
> +++ b/Documentation/gitremote-helpers.txt
> @@ -463,7 +463,7 @@ set by Git if the remote helper has the 'option' 
> capability.
>   GPG sign pushes.
>  
>  'option push-option ::
> - Transmit  as a push option. As the a push option
> + Transmit  as a push option. As the push option
>   must not contain LF or NUL characters, the string is not encoded.
>  
>  SEE ALSO
> diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
> index 4e3a50b..9f89d54 100755
> --- a/ci/run-windows-build.sh
> +++ b/ci/run-windows-build.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  #
> -# Script to trigger the a Git for Windows build and test run.
> +# Script to trigger the Git for Windows build and test run.
>  # Set the $GFW_CI_TOKEN as environment variable.
>  # Pass the branch (only branches on https://github.com/git/git are
>  # supported) and a commit hash.
> diff --git a/diff.c b/diff.c
> index 11eef1c..74283d9 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -911,7 +911,7 @@ static int fn_out_diff_words_write_helper(FILE *fp,
>  /*
>   * '--color-words' algorithm can be described as:
>   *
> - *   1. collect a the minus/plus lines of a diff hunk, divided into
> + *   1. collect the minus/plus lines of a diff hunk, divided into
>   *  minus-lines and plus-lines;
>   *
>   *   2. break both minus-lines and plus-lines into words and

Re: [GSoC][RFC/PATCH v2] submodule: port subcommand foreach from shell to C

[Junio C Hamano]

> +static void foreach_submodule(int argc, const char **argv, const char *path,
> +   const unsigned char *sha1, const char *prefix,
> +   int quiet, int recursive)
> +{

I think that a reader would expect that a function whose name is
foreach_something() to iterate over some collection and do something
on each of them, but this is not.  It is "do something on one thing"
part in a loop that exists elsewhere.

Do not call such a helper "foreach_".

Would it make more sense to use the "struct object_id" (instead of
uchar[40]) interface for new functions like this?

> + const char *toplevel = xgetcwd();
> + const struct submodule *sub;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf sub_sha1 = STRBUF_INIT;
> + struct strbuf file = STRBUF_INIT;
> + char* displaypath = NULL;
> + int i;
> +
> + /* Only loads from .gitmodules, no overlay with .git/config */
> + gitmodules_config();
> +
> + if (prefix && get_super_prefix()) {
> + die("BUG: cannot have prefix and superprefix");
> + } else if (prefix) {
> + displaypath = xstrdup(relative_path(path, prefix,  ));

An extra SP after the last comma?

> + } else if (get_super_prefix()) {
> + strbuf_addf(, "%s/%s", get_super_prefix(), path);
> + displaypath = strbuf_detach(, NULL);
> + } else {
> + displaypath = xstrdup(path);
> + }
> +
> + sub = submodule_from_path(null_sha1, path);
> +
> + if (!sub)
> + die(_("No url found for submodule path '%s' in .gitmodules"),
> +   displaypath);
> + strbuf_add_unique_abbrev(_sha1, sha1 , 40);

Why add-unique-abbrev if we do not want to abbreviate at all (with
hardcoded constant 40)?  IOW, shouldn't this be

strbuf_addstr(_sha1, sha1_to_hex(sha1));

> + argv_array_pushf(_array, "name=%s", sub->name);
> + argv_array_pushf(_array, "path=%s", displaypath);
> + argv_array_pushf(_array, "sm_path=%s", displaypath);

Why are we sending a value that will always be the same in two
variables?  "git submodule --help" seems to list only name, path,
sha1 and toplevel variables, and not sm_path.  Is it a documentation
bug, or are we clobbering a variable that the end user may be using
for other purposes in her foreach script?

> + argv_array_pushf(_array, "sha1=%s", sub_sha1.buf);
> + argv_array_pushf(_array, "toplevel=%s", toplevel);
> +
> + cp.use_shell = 1;
> + cp.dir = path;
> +
> + for (i = 0; i < argc; i++)
> + argv_array_push(, argv[i]);
> +
> + strbuf_addstr(, path);
> + strbuf_addstr(, "/.git");
> +
> + if (!quiet && !access_or_warn(file.buf, R_OK, 0))
> + printf(_("Entering '%s'\n"), displaypath);
> +
> + if (!access_or_warn(file.buf, R_OK, 0))
> + run_command();
> +
> + if(recursive) {

Missing SP after "if".

More importantly, if the earlier access-or-warn failed and we didn't
do the run-command for the path, does it even make sense to recurse
into it?

The original scripted version seems to refrain from recursing into
it if the command fails in the submodule in the first place.  This
code discards the exit status from run_command(), which looks wrong.

> + struct child_process cpr = CHILD_PROCESS_INIT;
> +
> + cpr.use_shell = 1;
> + cpr.dir = path;
> +
> + argv_array_pushf(, "git");
> + argv_array_pushf(, "--super-prefix");
> + argv_array_push(, displaypath);
> + argv_array_pushf(, "submodule--helper");
> +
> + if (quiet)
> + argv_array_pushf(, "--quiet");
> +
> + argv_array_pushf(, "foreach");
> + argv_array_pushf(, "--recursive");
> +
> + for (i = 0; i < argc; i++)
> + argv_array_push(, argv[i]);
> +
> + run_command();

Similarly, the exit status of this invocation is discarded, which
probably is wrong.  The original seems to pay attention to the
failure from the command invoked via "foreach --recursive" and stops
interation when it sees a failure.

> +static int module_foreach(int argc, const char **argv, const char *prefix)
> +{
> + struct pathspec pathspec;
> + struct module_list list = MODULE_LIST_INIT;
> + int quiet = 0;
> + int recursive = 0;
> + int i;
> +
> + struct option module_foreach_options[] = {
> + OPT__QUIET(, N_("Suppress output of entering each 
> submodule command")),
> + OPT_BOOL(0, "recursive", ,
> +  N_("Recurse into nested submodules")),
> + OPT_END()
> + };
> +
> + const char *const git_submodule_helper_usage[] = {
> + N_("git submodule--helper [--quiet] foreach [--recursive] 
> "),
> + NULL
> + };
> +
> + argc = parse_options(argc, argv, prefix, 

Re: [PATCH] Documentation/git-checkout: make doc. of checkout clearer

[Junio C Hamano]

Christoph Michelbach  writes:

>> Care to send in a patch to update the documentation?

Thanks.  The attached patch text appears linewrapped, but I think
I'll be able to salvage, so forgetting about the formatting, here
are some comments.


> From: Christoph Michelbach 
> Date: Sat, 22 Apr 2017 18:49:57 +0200
> Subject: [PATCH] Doc./git-checkout: correct doc. of checkout
> ...
>
> The previous documentation states that the named paths are
> updated both in the index and the working tree. This is not
> correct as those paths can point to folders which are not
> updated to what they are in the given tree-ish. Rather,
> the files pointed to by the pathspecs are copied from the

s/pointed to by/that match/

> tree-ish to the index and working tree One difference being

Missing full-stop after "tree".

> that one can name paths which are not present in the working
> tree ...

That one is not a difference, I would think.  If your current index
and working tree lack F, you give pathspec that match F, and the
tree-ish has that path F, that named path _is_ updated both in the
index and in the working tree.

> ... and another being that only files which are already
> present in the given tree-ish are affected.

This one indeed was missing piece of information.

> Signed-off-by: Christoph Michelbach 
> ---
>  Documentation/git-checkout.txt | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-
> checkout.txt
> index 8e2c066..f74f237 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -81,13 +81,15 @@ Omitting  detaches HEAD at the tip of the
> current branch.
>  'git checkout' [-p|--patch] [] [--] ...::
>  
>   When  or `--patch` are given, 'git checkout' does *not*
> + switch branches.  It copies the files matching the pathspecs in
> +  (i.e. a commit, tag, or tree) to the index and

 is misspelled here.  Drop (i.e. ...) as (1) it is not
correct (a tag may not point at a tree-ish) and (2) "checkout" is
not a place to learn what a tree-ish is (glossary is).

> + subsequently to the working directory, overwriting changes

Do we need to say "subsequently" when we are aiming to be more
intelligible by not describing the order of execution?  The end
result is that the blobs named by the pathspecs from the tree-ish
are checked out to the index and to the working tree at the same
time.

> +...  Note that because the index is updated, the
> + changes introduced by this command are automatically staged.

This is redundant and unnecessary, I would say.  If you absolutely
need to say this, at least drop "automatically".  There is nothing
automatic about it.  The user is asking to checkout the named blobs
out of the tree-ish to the index and to the working tree, and Git is
merely doing as it was told.

In addition the description is fuzzy; what are "changes introduced
by this command" relative to?  If you did "checkout HEAD path" after
editing path in the working tree, is "reverting my edit" the
"changes introduced by this command"?  If you did "checkout HEAD
path" after editing path, "git add path" and then editing path
further, what are the "changes introduced by this command"?

Re: [PATCH] test-lib: abort when can't remove trash directory

[Jeff King]

On Sun, Apr 23, 2017 at 05:14:54PM -0700, Junio C Hamano wrote:

> OK.  I am wondering why we do not do 
> 
>   rm -fr "$TRASH_DIRECTORY"
> 
> and do this instead:
> 
>   cd "$(dirname "$remove_trash")" &&
>   rm -rf "$(basename "$remove_trash")"
> 
> in the original.  It feels somewhat unnatural.

I assumed the "cd" was there because some systems may be unhappy
removing a directory which is our current working directory. That might
just be superstition, though.

The use of "basename" in the second does seem superfluous, since the
trash directory should be the full path (I suspect it wasn't in the
early days, though).

> So perhaps we can simplify and make it more robust by doing this?
> [...]
> + if test -z "$debug"
> + then
> + test -d "$TRASH_DIRECTORY" ||
> + error "Tests passed but trash directory already removed 
> before test cleanup; aborting"
> +
> + rm -fr "$TRASH_DIRECTORY" ||
> + error "Tests passed but test cleanup failed; aborting"
> + fi

That looks fine, assuming the answer to the "is the cwd important"
question is "no".

-Peff

Re: [PATCH] git_fopen: fix a sparse 'not declared' warning

[Junio C Hamano]

Ramsay Jones  writes:

> Commit 8f4f6e53d2 ("config.mak.uname: set FREAD_READS_DIRECTORIES for
> Linux and FreeBSD", 20-04-2017) caused sparse to issue a 'not declared,
> should it be static?' warning on Linux. This is a result of the method
> employed by 'compat/fopen.c' to suppress the (possible) redefinition of
> the (system) fopen macro, which also removes the extern declaration of
> the git_fopen function.
>
> In order to suppress the warning, introduce a new macro to suppress the
> definition (or possibly the re-definition) of the fopen symbol as a
> macro override. This new macro (SUPPRESS_FOPEN_REDEFINITION) is only
> defined in 'compat/fopen.c', just prior to the inclusion of the
> 'git-compat-util.h' header file.
>
> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Duy,
>
> Could you (or Junio) please add this to your 'nd/fopen-errors'
> branch, either as a separate patch or squash it into commit
> 8f4f6e53d2 ("config.mak.uname: set FREAD_READS_DIRECTORIES for
> Linux and FreeBSD", 20-04-2017). I think it would be better as a
> separate commit, but I will leave you to decide on that. ;-)

Yeah, I think it makes sense to have this as a separate patch,
applying even before the other series.

Thanks.

Re: [BUG] test suite broken with GETTEXT_POISON=YesPlease

[Junio C Hamano]

Michael J Gruber  writes:

> Ævar Arnfjörð Bjarmason venit, vidit, dixit 20.04.2017 23:58:
>> As a refresh of everyone's memory (because mine needed it). This is a
>> feature I added back in 2011 when the i18n support was initially
>> added.
>> 
>> There was concern at the time that we would inadvertently mark
>> plumbing messages for translation, particularly something in a shared
>> code path, and this was a way to hopefully smoke out those issues with
>> the test suite.
>> 
>> However compiling with it breaks a couple of dozen tests, I stopped
>> digging when I saw some broke back in 2014.
>> 
>> What should be done about this? I think if we're going to keep them
>> they need to be run regularly by something like Travis (Lars CC'd),
>> however empirical evidence suggests that not running them is just fine
>> too, so should we just remove support for this test mode?
>> 
>> I don't care, but I can come up with the patch either way, but would
>> only be motivated to write the one-time fix for it if some CI system
>> is actually running them regularly, otherwise they'll just be subject
>> to bitrotting again.
>
> I use that switch when I change something that involves l10n, but
> usually I run specific tests only. To be honest: I have to make sure not
> to get confused by (nor forget one of) the build flag GETTEXT_POISON and
> the environment variable GIT_GETTEXT_POISON. I'm not sure I always
> tested what I meant to test...

To be quite honest, I have always felt that we are just as likely
inadvertently use test_i18ncmp when we should use test_cmp (and vice
versa) as we would mark plumbing messages with _() by mistake with
this approach, and even with constant monitoring by something like
Travis, GETTEXT_POISON may be able to catch mistakes only some of
the time (i.e. when we do not make mistakes in writing our tests).
Without constant monitoring, I agree that the mechanism does not
work well to catch our mistakes.

Re: [PATCH] tests: fix tests broken under GETTEXT_POISON=YesPlease

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> The GETTEXT_POISON=YesPlease compile-time testing option added in my
> bb946bba76 ("i18n: add GETTEXT_POISON to simulate unfriendly
> translator", 2011-02-22) has been slowly bitrotting as strings have
> been marked for translation, and new tests have been added without
> running it.
>
> I brought this up on the list ("[BUG] test suite broken with
> GETTEXT_POISON=YesPlease", [1]) asking whether this mode was useful at
> all anymore. At least one person occasionally uses it, and Lars
> Schneider offered to change one of the the Travis builds to run in
> this mode, so fix up the failing ones.
>
> My test setup runs most of the tests, with the notable exception of
> skipping all the p4 tests, so it's possible that there's still some
> lurking regressions I haven't fixed.
>
> 1. 

To be honest, I feel quite uneasy about this patch.  It is no
brainer to take fixes like the one to 1309 where we used grep on a
localizable string to use test_i18ngrep instead---they are obviously
good changes.

But changes that skip tests with !GETTEXT_POISON look suspicious.

> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index 8937e25e49..2003ec7907 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -122,7 +122,7 @@ test_expect_success 'push cannot create a badly named 
> ref' '
>   ! grep -e "broken\.\.\.ref" output
>  '
>  
> -test_expect_failure 'push --mirror can delete badly named ref' '
> +test_expect_failure !GETTEXT_POISON 'push --mirror can delete badly named 
> ref' '
>   top=$(pwd) &&
>   git init src &&
>   git init dest &&

This test affects only src and dest repositories that does not seem
to be looked at by later tests, so skipping it is presumably safe.

I am guessing that the reason why this is skipped is because the
error message is looked at with "! grep ", not "grep", but by
skipping the entire test, we are not checking that "push" succeeds
(which should happen regardless of the locale).

It also is VERY curious that the test before this one (whose tail is
visible in the pre-context) does not need to be skipped.  Is that
because we can expect "broken...ref", which is litrally a part of a
refname, would be emitted intact in any locale (including the
poisoned one)?  If that is the case it is curous why this one needs
to be skipped.

> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index 5778c0afe1..a428ae6703 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -236,7 +236,7 @@ test_expect_success 'git branch --format option' '
>   Refname is refs/heads/ref-to-remote
>   EOF
>   git branch --format="Refname is %(refname)" >actual &&
> - test_cmp expect actual
> + test_i18ncmp expect actual
>  '

This is a strange change.  Filling the placeholder in a format
string "Refname is %(refname)" should be affeced by i18n???

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 33d392ba11..e07d6d8cd2 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -366,7 +366,7 @@ test_expect_success 'verbose flag is heeded, even after --
> ...
> -test_expect_success 'multi-squash only fires up editor once' '
> +test_expect_success !GETTEXT_POISON 'multi-squash only fires up editor once' 
> '
>   base=$(git rev-parse HEAD~4) &&
>   set_fake_editor &&
>   FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 squash 2 squash 3 squash 4" \

This is also curious.  Is this because the poison locale does not substitute
anything passed to format template and literal strings like ONCE and
the instructions do not reach the edited file?

 (skipping many changes, not because I find nothing to comment on them)

> diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh
> index 37143ea0ac..2ed479b712 100755
> --- a/t/t5316-pack-delta-depth.sh
> +++ b/t/t5316-pack-delta-depth.sh
> @@ -82,12 +82,16 @@ test_expect_success 'packing produces a long delta' '
>   # Use --window=0 to make sure we are seeing reused deltas,
>   # not computing a new long chain.
>   pack=$(git pack-objects --all --window=0  - test 9 = "$(max_chain pack-$pack.pack)"
> + echo 9 >expect &&
> + max_chain pack-$pack.pack >actual &&
> + test_i18ncmp expect actual
>  '

This is also curious.  Why do we needto protect comparision with a
line whose contents is '9' from poison locale?  If the last one were
test_cmp I think this is a good change, by the way.

>  test_expect_success '--depth limits depth' '
>   pack=$(git pack-objects --all --depth=5  - test 5 = "$(max_chain pack-$pack.pack)"
> + echo 5 >expect &&
> + max_chain pack-$pack.pack >actual &&
> + test_i18ncmp expect actual
>  '

Likewise.

Re: [PATCH 00/15] Handle fopen() errors

[Junio C Hamano]

Jeff King  writes:

> On Fri, Apr 21, 2017 at 07:27:20PM +0700, Duy Nguyen wrote:
>
>> On Fri, Apr 21, 2017 at 6:52 PM, Junio C Hamano  wrote:
>> > Yes, but (1) we'd need to be careful about --quiet
>> 
>> Yeah. It's a real pain point for making changes like this. At some
>> point we should just have a global (maybe multi-level) quiet flag.
>
> I don't think it's too bad here. Isn't it just:
>
>   FILE *fh = quiet ? fopen(x, y) : fopen_or_warn(x, y);
>
> It is a little annoying to have to repeat "x", though (which is
> sometimes a git_path() invocation).

Sure, but you could do

fopen_or_warn(quiet, x, y)

if it is a problem ;-)

Re: What's cooking in git.git (Apr 2017, #04; Wed, 19)

[Junio C Hamano]

Johannes Schindelin  writes:

> Part of the reason is that you push out all of the branches in one go,
> typically at the very end of your work day. The idea of Continuous
> Integration is a little orthogonal to that style, suggesting to build &
> test whenever new changes come into the integration branch.
>
> As a consequence, my original setup was a little overloaded: the VM sat
> idle most of the time, and when you pushed, it was overloaded.

I do not see pushing out all them in one go is making the problem
worse for you, though.

As of this writing, master..pu counts 60+ first-parent merges.
Instead of pushing out the final one at the end of the day, I could
push out after every merge.  Behind the scenes, because some topics
are extended or tweaked while I read the list discussion, the number
of merges I am doing during a day is about twice or more than that
before I reach the final version for the day.  

Many issues can be noticed locally even before the patches hit a
topic, before the topic gets merged to 'pu', or before the tentative
'pu' is pushed out, and breakage at each of these points can be
locally corrected without bothering external test setups.  I've been
assuming that pushing out all in one go at the end will help
reducing the load at external test setups.

Re: [PATCH] test-lib: abort when can't remove trash directory

[Junio C Hamano]

Jeff King  writes:

>> -test -d "$remove_trash" &&
>> +test -d "$remove_trash" ||
>> +error "Tests passed but trash directory already removed before 
>> test cleanup; aborting"
>
> I think I found out why this "test -d" was here in the first place:
>
>   $ ./t-basic.sh --debug
>   [...]
>   # passed all 77 test(s)
>   1..77
>   error: Tests passed but trash directory already removed before test 
> cleanup; aborting
>
> When --debug is in use, we do not set $remove_trash. The original was
> relying on 'test -d ""' to return false.
>
> I think this whole removal block should probably be moved inside a
> conditional like:
>
>   if test -n "$remove_trash"
>   then
>  ...
>   fi

Thanks for digging.  Yes, checking for non-empty string is
definitely better.

> I also wonder if we should come up with a better name than
> $remove_trash. A script which unknowingly overwrites that variable would
> be disastrous.
>
> Perhaps we should drop it entirely and just do:
>
>   if test -z "$debug"
>   then
>  test -d "$TRASH_DIRECTORY" ||
>  error "Tests passed but..."
>   [and so forth...]
>   fi

OK.  I am wondering why we do not do 

rm -fr "$TRASH_DIRECTORY"

and do this instead:

cd "$(dirname "$remove_trash")" &&
rm -rf "$(basename "$remove_trash")"

in the original.  It feels somewhat unnatural.

... goes and looks ...

Of course, back when abc5d372 ("Enable parallel tests", 2008-08-08)
was writen, we didn't even have TRASH_DIRECTORY variable; because
the way the test-lib.sh ensured that the trash directory is prestine
was to first do a 'rm -fr "$test"' before the first test_create_repo,
the above makes sort of matches how that initial removal is done.

So perhaps we can simplify and make it more robust by doing this?

 t/test-lib.sh | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cde7fc7fcf..f1ab8f33d9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -760,9 +760,14 @@ test_done () {
say "1..$test_count$skip_all"
fi
 
-   test -d "$remove_trash" &&
-   cd "$(dirname "$remove_trash")" &&
-   rm -rf "$(basename "$remove_trash")"
+   if test -z "$debug"
+   then
+   test -d "$TRASH_DIRECTORY" ||
+   error "Tests passed but trash directory already removed 
before test cleanup; aborting"
+
+   rm -fr "$TRASH_DIRECTORY" ||
+   error "Tests passed but test cleanup failed; aborting"
+   fi
 
test_at_end_hook_
 

[PATCH 50/53] diff-lib: convert do_diff_cache to struct object_id

[brian m. carlson]

This is needed to convert parse_tree_indirect.

Signed-off-by: brian m. carlson 
---
 builtin/am.c|  2 +-
 builtin/blame.c |  6 +++---
 builtin/reset.c |  2 +-
 diff-lib.c  | 12 ++--
 diff.h  |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 46828646c..210ecc554 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1143,7 +1143,7 @@ static int index_has_changes(struct strbuf *sb)
DIFF_OPT_SET(, EXIT_WITH_STATUS);
if (!sb)
DIFF_OPT_SET(, QUICK);
-   do_diff_cache(head.hash, );
+   do_diff_cache(, );
diffcore_std();
for (i = 0; sb && i < diff_queued_diff.nr; i++) {
if (i)
diff --git a/builtin/blame.c b/builtin/blame.c
index 58bb274d0..e920314a7 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -563,7 +563,7 @@ static struct origin *find_origin(struct scoreboard *sb,
diff_setup_done(_opts);
 
if (is_null_oid(>commit->object.oid))
-   do_diff_cache(parent->tree->object.oid.hash, _opts);
+   do_diff_cache(>tree->object.oid, _opts);
else
diff_tree_sha1(parent->tree->object.oid.hash,
   origin->commit->tree->object.oid.hash,
@@ -633,7 +633,7 @@ static struct origin *find_rename(struct scoreboard *sb,
diff_setup_done(_opts);
 
if (is_null_oid(>commit->object.oid))
-   do_diff_cache(parent->tree->object.oid.hash, _opts);
+   do_diff_cache(>tree->object.oid, _opts);
else
diff_tree_sha1(parent->tree->object.oid.hash,
   origin->commit->tree->object.oid.hash,
@@ -1272,7 +1272,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
DIFF_OPT_SET(_opts, FIND_COPIES_HARDER);
 
if (is_null_oid(>commit->object.oid))
-   do_diff_cache(parent->tree->object.oid.hash, _opts);
+   do_diff_cache(>tree->object.oid, _opts);
else
diff_tree_sha1(parent->tree->object.oid.hash,
   target->commit->tree->object.oid.hash,
diff --git a/builtin/reset.c b/builtin/reset.c
index 0be52fa36..3415dac5d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -154,7 +154,7 @@ static int read_from_tree(const struct pathspec *pathspec,
opt.format_callback = update_index_from_diff;
opt.format_callback_data = _to_add;
 
-   if (do_diff_cache(tree_oid->hash, ))
+   if (do_diff_cache(tree_oid, ))
return 1;
diffcore_std();
diff_flush();
diff --git a/diff-lib.c b/diff-lib.c
index 52447466b..ee9df0f84 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -478,7 +478,7 @@ static int oneway_diff(const struct cache_entry * const 
*src,
 }
 
 static int diff_cache(struct rev_info *revs,
- const unsigned char *tree_sha1,
+ const struct object_id *tree_oid,
  const char *tree_name,
  int cached)
 {
@@ -486,10 +486,10 @@ static int diff_cache(struct rev_info *revs,
struct tree_desc t;
struct unpack_trees_options opts;
 
-   tree = parse_tree_indirect(tree_sha1);
+   tree = parse_tree_indirect(tree_oid->hash);
if (!tree)
return error("bad tree object %s",
-tree_name ? tree_name : sha1_to_hex(tree_sha1));
+tree_name ? tree_name : oid_to_hex(tree_oid));
memset(, 0, sizeof(opts));
opts.head_idx = 1;
opts.index_only = cached;
@@ -512,7 +512,7 @@ int run_diff_index(struct rev_info *revs, int cached)
struct object_array_entry *ent;
 
ent = revs->pending.objects;
-   if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
+   if (diff_cache(revs, >item->oid, ent->name, cached))
exit(128);
 
diff_set_mnemonic_prefix(>diffopt, "c/", cached ? "i/" : "w/");
@@ -522,7 +522,7 @@ int run_diff_index(struct rev_info *revs, int cached)
return 0;
 }
 
-int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
+int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
 {
struct rev_info revs;
 
@@ -530,7 +530,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct 
diff_options *opt)
copy_pathspec(_data, >pathspec);
revs.diffopt = *opt;
 
-   if (diff_cache(, tree_sha1, NULL, 1))
+   if (diff_cache(, tree_oid, NULL, 1))
exit(128);
return 0;
 }
diff --git a/diff.h b/diff.h
index 5be1ee77a..d75e6d15e 100644
--- a/diff.h
+++ b/diff.h
@@ -354,7 +354,7 @@ extern const char *diff_aligned_abbrev(const struct 
object_id *sha1, int);
 extern int run_diff_files(struct rev_info *revs, unsigned int option);
 extern int run_diff_index(struct rev_info *revs, int 

[PATCH 44/53] sha1_name: convert internals of peel_onion to object_id

[brian m. carlson]

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

diff --git a/sha1_name.c b/sha1_name.c
index b7e09ac13..72e72ab9a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -798,7 +798,7 @@ struct object *peel_to_type(const char *name, int namelen,
 static int peel_onion(const char *name, int len, unsigned char *sha1,
  unsigned lookup_flags)
 {
-   unsigned char outer[20];
+   struct object_id outer;
const char *sp;
unsigned int expected_type = 0;
struct object *o;
@@ -846,10 +846,10 @@ static int peel_onion(const char *name, int len, unsigned 
char *sha1,
else if (expected_type == OBJ_TREE)
lookup_flags |= GET_SHA1_TREEISH;
 
-   if (get_sha1_1(name, sp - name - 2, outer, lookup_flags))
+   if (get_sha1_1(name, sp - name - 2, outer.hash, lookup_flags))
return -1;
 
-   o = parse_object(outer);
+   o = parse_object(outer.hash);
if (!o)
return -1;
if (!expected_type) {

[PATCH 52/53] tree: convert parse_tree_indirect to struct object_id

[brian m. carlson]

Convert parse_tree_indirect to take a pointer to struct object_id.
Update all the callers.  This transformation was achieved using the
following semantic patch and manual updates to the declaration and
definition.  Update builtin/checkout.c manually as well, since it uses a
ternary expression not handled by the semantic patch.

@@
expression E1;
@@
- parse_tree_indirect(E1.hash)
+ parse_tree_indirect()

@@
expression E1;
@@
- parse_tree_indirect(E1->hash)
+ parse_tree_indirect(E1)

Signed-off-by: brian m. carlson 
---
 archive.c   | 4 ++--
 builtin/am.c| 6 +++---
 builtin/checkout.c  | 8 
 builtin/clone.c | 2 +-
 builtin/commit.c| 2 +-
 builtin/ls-files.c  | 2 +-
 builtin/ls-tree.c   | 2 +-
 builtin/merge.c | 6 +++---
 builtin/read-tree.c | 2 +-
 builtin/reset.c | 4 ++--
 diff-lib.c  | 2 +-
 merge.c | 4 ++--
 sequencer.c | 2 +-
 t/helper/test-match-trees.c | 4 ++--
 tree.c  | 4 ++--
 tree.h  | 2 +-
 16 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/archive.c b/archive.c
index 54701e8bb..b15a922da 100644
--- a/archive.c
+++ b/archive.c
@@ -369,7 +369,7 @@ static void parse_treeish_arg(const char **argv,
archive_time = time(NULL);
}
 
-   tree = parse_tree_indirect(oid.hash);
+   tree = parse_tree_indirect();
if (tree == NULL)
die("not a tree object");
 
@@ -383,7 +383,7 @@ static void parse_treeish_arg(const char **argv,
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
 
-   tree = parse_tree_indirect(tree_oid.hash);
+   tree = parse_tree_indirect(_oid);
}
ar_args->tree = tree;
ar_args->commit_sha1 = commit_sha1;
diff --git a/builtin/am.c b/builtin/am.c
index 210ecc554..c21059e6e 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2046,11 +2046,11 @@ static int clean_index(const struct object_id *head, 
const struct object_id *rem
struct tree *head_tree, *remote_tree, *index_tree;
struct object_id index;
 
-   head_tree = parse_tree_indirect(head->hash);
+   head_tree = parse_tree_indirect(head);
if (!head_tree)
return error(_("Could not parse object '%s'."), 
oid_to_hex(head));
 
-   remote_tree = parse_tree_indirect(remote->hash);
+   remote_tree = parse_tree_indirect(remote);
if (!remote_tree)
return error(_("Could not parse object '%s'."), 
oid_to_hex(remote));
 
@@ -2062,7 +2062,7 @@ static int clean_index(const struct object_id *head, 
const struct object_id *rem
if (write_cache_as_tree(index.hash, 0, NULL))
return -1;
 
-   index_tree = parse_tree_indirect(index.hash);
+   index_tree = parse_tree_indirect();
if (!index_tree)
return error(_("Could not parse object '%s'."), 
oid_to_hex());
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7f1eeea94..13365fb62 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -527,10 +527,10 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
setup_standard_excludes(topts.dir);
}
tree = parse_tree_indirect(old->commit ?
-  old->commit->object.oid.hash :
-  EMPTY_TREE_SHA1_BIN);
+  >commit->object.oid :
+  _tree_oid);
init_tree_desc([0], tree->buffer, tree->size);
-   tree = parse_tree_indirect(new->commit->object.oid.hash);
+   tree = parse_tree_indirect(>commit->object.oid);
init_tree_desc([1], tree->buffer, tree->size);
 
ret = unpack_trees(2, trees, );
@@ -1050,7 +1050,7 @@ static int parse_branchname_arg(int argc, const char 
**argv,
new->commit = lookup_commit_reference_gently(rev, 1);
if (!new->commit) {
/* not a commit */
-   *source_tree = parse_tree_indirect(rev->hash);
+   *source_tree = parse_tree_indirect(rev);
} else {
parse_commit_or_die(new->commit);
*source_tree = new->commit->tree;
diff --git a/builtin/clone.c b/builtin/clone.c
index 646f28792..da2d3c1ae 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -739,7 +739,7 @@ static int checkout(int submodule_progress)
opts.src_index = _index;
opts.dst_index = _index;
 
-   tree = parse_tree_indirect(oid.hash);
+   tree = parse_tree_indirect();
parse_tree(tree);
init_tree_desc(, tree->buffer, tree->size);
if (unpack_trees(1, , ) < 0)
diff --git a/builtin/commit.c b/builtin/commit.c
index 

[PATCH 53/53] object: convert parse_object* to take struct object_id

[brian m. carlson]

Make parse_object, parse_object_or_die, and parse_object_buffer take a
pointer to struct object_id.  Remove the temporary variables inserted
earlier, since they are no longer necessary.  Transform all of the
callers using the following semantic patch:

@@
expression E1;
@@
- parse_object(E1.hash)
+ parse_object()

@@
expression E1;
@@
- parse_object(E1->hash)
+ parse_object(E1)

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

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

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

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

Signed-off-by: brian m. carlson 
---
 builtin/diff-tree.c  |  2 +-
 builtin/diff.c   |  2 +-
 builtin/fast-export.c|  4 ++--
 builtin/fmt-merge-msg.c  |  4 ++--
 builtin/fsck.c   |  8 
 builtin/grep.c   |  2 +-
 builtin/index-pack.c |  3 ++-
 builtin/log.c|  2 +-
 builtin/name-rev.c   |  6 +++---
 builtin/prune.c  |  3 ++-
 builtin/receive-pack.c   |  6 +++---
 builtin/reflog.c |  2 +-
 builtin/rev-list.c   |  2 +-
 builtin/unpack-objects.c |  3 ++-
 bundle.c | 10 ++
 commit.c |  4 ++--
 fetch-pack.c | 14 +++---
 fsck.c   |  2 +-
 http-backend.c   |  2 +-
 http-push.c  |  4 ++--
 log-tree.c   |  6 +++---
 merge-recursive.c|  2 +-
 object.c | 44 +++-
 object.h |  8 
 pack-bitmap.c|  4 ++--
 pretty.c |  2 +-
 reachable.c  |  4 ++--
 ref-filter.c |  4 ++--
 reflog-walk.c|  4 ++--
 refs/files-backend.c |  2 +-
 remote.c |  4 ++--
 revision.c   | 12 ++--
 server-info.c|  2 +-
 sha1_name.c  | 14 +++---
 tag.c|  4 ++--
 tree.c   |  4 ++--
 upload-pack.c|  8 
 walker.c |  2 +-
 38 files changed, 107 insertions(+), 108 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 95b8d1ba7..5ea1c1431 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -67,7 +67,7 @@ static int diff_tree_stdin(char *line)
line[len-1] = 0;
if (parse_oid_hex(line, , ))
return -1;
-   obj = parse_object(oid.hash);
+   obj = parse_object();
if (!obj)
return -1;
if (obj->type == OBJ_COMMIT)
diff --git a/builtin/diff.c b/builtin/diff.c
index 895f92897..8c03ddaf5 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -395,7 +395,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
const char *name = entry->name;
int flags = (obj->flags & UNINTERESTING);
if (!obj->parsed)
-   obj = parse_object(obj->oid.hash);
+   obj = parse_object(>oid);
obj = deref_tag(obj, NULL, 0);
if (!obj)
die(_("invalid object '%s' given."), name);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index ae36b14db..969550531 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -240,7 +240,7 @@ static void export_blob(const struct object_id *oid)
die ("Could not read blob %s", oid_to_hex(oid));
if (check_sha1_signature(oid->hash, buf, size, typename(type)) 
< 0)
die("sha1 mismatch in blob %s", oid_to_hex(oid));
-   object = parse_object_buffer(oid->hash, type, size, buf, 
);
+   object = parse_object_buffer(oid, type, size, buf, );
}
 
if (!object)
@@ -777,7 +777,7 @@ static struct commit *get_commit(struct rev_cmdline_entry 
*e, char *full_name)
 
/* handle nested tags */
while (tag && tag->object.type == OBJ_TAG) {
-   parse_object(tag->object.oid.hash);
+   parse_object(>object.oid);
string_list_append(_refs, full_name)->util = tag;
tag = (struct tag *)tag->tagged;
}
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 91dd753dd..70137b0b7 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -341,7 +341,7 @@ static void shortlog(const char *name,
const struct object_id *oid = _data->oid;
int limit = opts->shortlog_len;
 
-   branch = deref_tag(parse_object(oid->hash), oid_to_hex(oid), 
GIT_SHA1_HEXSZ);
+   branch = deref_tag(parse_object(oid), oid_to_hex(oid), GIT_SHA1_HEXSZ);
if (!branch || 

[PATCH 48/53] merge: convert checkout_fast_forward to struct object_id

[brian m. carlson]

Converting checkout_fast_forward is required to convert
parse_tree_indirect.

Signed-off-by: brian m. carlson 
---
 builtin/merge.c | 4 ++--
 builtin/pull.c  | 4 ++--
 cache.h | 4 ++--
 merge.c | 8 
 sequencer.c | 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index f11b5f3de..5ea7f7da9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1372,8 +1372,8 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
goto done;
}
 
-   if (checkout_fast_forward(head_commit->object.oid.hash,
- commit->object.oid.hash,
+   if (checkout_fast_forward(_commit->object.oid,
+ >object.oid,
  overwrite_ignore)) {
ret = 1;
goto done;
diff --git a/builtin/pull.c b/builtin/pull.c
index 21e114ec0..f539c7f78 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -523,7 +523,7 @@ static int pull_into_void(const struct object_id 
*merge_head,
 * index/worktree changes that the user already made on the unborn
 * branch.
 */
-   if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head->hash, 0))
+   if (checkout_fast_forward(_tree_oid, merge_head, 0))
return 1;
 
if (update_ref("initial pull", "HEAD", merge_head->hash, 
curr_head->hash, 0, UPDATE_REFS_DIE_ON_ERR))
@@ -839,7 +839,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
"fast-forwarding your working tree from\n"
"commit %s."), oid_to_hex(_head));
 
-   if (checkout_fast_forward(orig_head.hash, curr_head.hash, 0))
+   if (checkout_fast_forward(_head, _head, 0))
die(_("Cannot fast-forward your working tree.\n"
"After making sure that you saved anything 
precious from\n"
"$ git diff %s\n"
diff --git a/cache.h b/cache.h
index ba27595d5..552c44ef0 100644
--- a/cache.h
+++ b/cache.h
@@ -2188,8 +2188,8 @@ struct commit_list;
 int try_merge_command(const char *strategy, size_t xopts_nr,
const char **xopts, struct commit_list *common,
const char *head_arg, struct commit_list *remotes);
-int checkout_fast_forward(const unsigned char *from,
- const unsigned char *to,
+int checkout_fast_forward(const struct object_id *from,
+ const struct object_id *to,
  int overwrite_ignore);
 
 
diff --git a/merge.c b/merge.c
index 04ee5fc91..b0cffe16f 100644
--- a/merge.c
+++ b/merge.c
@@ -44,8 +44,8 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
return ret;
 }
 
-int checkout_fast_forward(const unsigned char *head,
- const unsigned char *remote,
+int checkout_fast_forward(const struct object_id *head,
+ const struct object_id *remote,
  int overwrite_ignore)
 {
struct tree *trees[MAX_UNPACK_TREES];
@@ -79,10 +79,10 @@ int checkout_fast_forward(const unsigned char *head,
opts.fn = twoway_merge;
setup_unpack_trees_porcelain(, "merge");
 
-   trees[nr_trees] = parse_tree_indirect(head);
+   trees[nr_trees] = parse_tree_indirect(head->hash);
if (!trees[nr_trees++])
return -1;
-   trees[nr_trees] = parse_tree_indirect(remote);
+   trees[nr_trees] = parse_tree_indirect(remote->hash);
if (!trees[nr_trees++])
return -1;
for (i = 0; i < nr_trees; i++) {
diff --git a/sequencer.c b/sequencer.c
index c673bb930..dfa44afa7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -382,7 +382,7 @@ static int fast_forward_to(const struct object_id *to, 
const struct object_id *f
struct strbuf err = STRBUF_INIT;
 
read_cache();
-   if (checkout_fast_forward(from->hash, to->hash, 1))
+   if (checkout_fast_forward(from, to, 1))
return -1; /* the callee should have complained already */
 
strbuf_addf(, _("%s: fast-forward"), _(action_name(opts)));

[PATCH 42/53] revision: convert remaining parse_object callers to object_id

[brian m. carlson]

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

diff --git a/revision.c b/revision.c
index f82c56e1f..80f74bb7b 100644
--- a/revision.c
+++ b/revision.c
@@ -177,23 +177,23 @@ void add_pending_object(struct rev_info *revs,
 
 void add_head_to_pending(struct rev_info *revs)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *obj;
-   if (get_sha1("HEAD", sha1))
+   if (get_oid("HEAD", ))
return;
-   obj = parse_object(sha1);
+   obj = parse_object(oid.hash);
if (!obj)
return;
add_pending_object(revs, obj, "HEAD");
 }
 
 static struct object *get_reference(struct rev_info *revs, const char *name,
-   const unsigned char *sha1,
+   const struct object_id *oid,
unsigned int flags)
 {
struct object *object;
 
-   object = parse_object(sha1);
+   object = parse_object(oid->hash);
if (!object) {
if (revs->ignore_missing)
return object;
@@ -206,7 +206,7 @@ static struct object *get_reference(struct rev_info *revs, 
const char *name,
 void add_pending_oid(struct rev_info *revs, const char *name,
  const struct object_id *oid, unsigned int flags)
 {
-   struct object *object = get_reference(revs, name, oid->hash, flags);
+   struct object *object = get_reference(revs, name, oid, flags);
add_pending_object(revs, object, name);
 }
 
@@ -1157,7 +1157,7 @@ static int handle_one_ref(const char *path, const struct 
object_id *oid,
if (ref_excluded(cb->all_revs->ref_excludes, path))
return 0;
 
-   object = get_reference(cb->all_revs, path, oid->hash, cb->all_flags);
+   object = get_reference(cb->all_revs, path, oid, cb->all_flags);
add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags);
add_pending_oid(cb->all_revs, path, oid, cb->all_flags);
return 0;
@@ -1292,7 +1292,7 @@ void add_index_objects_to_pending(struct rev_info *revs, 
unsigned flags)
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
int exclude_parent)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *it;
struct commit *commit;
struct commit_list *parents;
@@ -1303,17 +1303,17 @@ static int add_parents_only(struct rev_info *revs, 
const char *arg_, int flags,
flags ^= UNINTERESTING | BOTTOM;
arg++;
}
-   if (get_sha1_committish(arg, sha1))
+   if (get_sha1_committish(arg, oid.hash))
return 0;
while (1) {
-   it = get_reference(revs, arg, sha1, 0);
+   it = get_reference(revs, arg, , 0);
if (!it && revs->ignore_missing)
return 0;
if (it->type != OBJ_TAG)
break;
if (!((struct tag*)it)->tagged)
return 0;
-   hashcpy(sha1, ((struct tag*)it)->tagged->oid.hash);
+   oidcpy(, &((struct tag*)it)->tagged->oid);
}
if (it->type != OBJ_COMMIT)
return 0;
@@ -1434,7 +1434,7 @@ int handle_revision_arg(const char *arg_, struct rev_info 
*revs, int flags, unsi
struct object_context oc;
char *dotdot;
struct object *object;
-   unsigned char sha1[20];
+   struct object_id oid;
int local_flags;
const char *arg = arg_;
int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
@@ -1444,7 +1444,7 @@ int handle_revision_arg(const char *arg_, struct rev_info 
*revs, int flags, unsi
 
dotdot = strstr(arg, "..");
if (dotdot) {
-   unsigned char from_sha1[20];
+   struct object_id from_oid;
const char *next = dotdot + 2;
const char *this = arg;
int symmetric = *next == '.';
@@ -1470,8 +1470,8 @@ int handle_revision_arg(const char *arg_, struct rev_info 
*revs, int flags, unsi
return -1;
}
}
-   if (!get_sha1_committish(this, from_sha1) &&
-   !get_sha1_committish(next, sha1)) {
+   if (!get_sha1_committish(this, from_oid.hash) &&
+   !get_sha1_committish(next, oid.hash)) {
struct object *a_obj, *b_obj;
 
if (!cant_be_filename) {
@@ -1479,8 +1479,8 @@ int handle_revision_arg(const char *arg_, struct rev_info 
*revs, int flags, unsi
verify_non_filename(revs->prefix, arg);
}
 
-   a_obj = parse_object(from_sha1);
-

[PATCH 49/53] builtin/ls-tree: convert to struct object_id

[brian m. carlson]

This is a prerequisite to convert do_diff_cache, which is required to
convert parse_tree_indirect.

Signed-off-by: brian m. carlson 
---
 builtin/ls-tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index d7ebeb4ce..5baac3ef2 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -119,7 +119,7 @@ static int show_tree(const unsigned char *sha1, struct 
strbuf *base,
 
 int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct tree *tree;
int i, full_tree = 0;
const struct option ls_tree_options[] = {
@@ -164,7 +164,7 @@ int cmd_ls_tree(int argc, const char **argv, const char 
*prefix)
 
if (argc < 1)
usage_with_options(ls_tree_usage, ls_tree_options);
-   if (get_sha1(argv[0], sha1))
+   if (get_oid(argv[0], ))
die("Not a valid object name %s", argv[0]);
 
/*
@@ -180,7 +180,7 @@ int cmd_ls_tree(int argc, const char **argv, const char 
*prefix)
for (i = 0; i < pathspec.nr; i++)
pathspec.items[i].nowildcard_len = pathspec.items[i].len;
pathspec.has_wildcard = 0;
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid.hash);
if (!tree)
die("not a tree object");
return !!read_tree_recursive(tree, "", 0, 0, , show_tree, 
NULL);

[PATCH 45/53] builtin/read-tree: convert to struct object_id

[brian m. carlson]

This is a caller of parse_tree_indirect, which must be converted in
order to convert parse_object.

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

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8..92eff23e4 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -23,13 +23,13 @@ static int read_empty;
 static struct tree *trees[MAX_UNPACK_TREES];
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 
-static int list_tree(unsigned char *sha1)
+static int list_tree(struct object_id *oid)
 {
struct tree *tree;
 
if (nr_trees >= MAX_UNPACK_TREES)
die("I cannot read more than %d trees", MAX_UNPACK_TREES);
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid->hash);
if (!tree)
return -1;
trees[nr_trees++] = tree;
@@ -121,7 +121,7 @@ static struct lock_file lock_file;
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
int i, stage = 0;
-   unsigned char sha1[20];
+   struct object_id oid;
struct tree_desc t[MAX_UNPACK_TREES];
struct unpack_trees_options opts;
int prefix_set = 0;
@@ -204,9 +204,9 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
 
-   if (get_sha1(arg, sha1))
+   if (get_oid(arg, ))
die("Not a valid object name %s", arg);
-   if (list_tree(sha1) < 0)
+   if (list_tree() < 0)
die("failed to unpack tree object %s", arg);
stage++;
}

[PATCH 41/53] revision: rename add_pending_sha1 to add_pending_oid

[brian m. carlson]

Rename this function and convert it to take a pointer to struct
object_id.

This is a prerequisite for converting get_reference, which is needed to
convert parse_object.

Signed-off-by: brian m. carlson 
---
 builtin/am.c   | 2 +-
 builtin/checkout.c | 4 ++--
 revision.c | 8 
 revision.h | 6 +++---
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e12da7b3f..46828646c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1622,7 +1622,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
init_revisions(_info, NULL);
rev_info.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;
diff_opt_parse(_info.diffopt, _filter_str, 1, 
rev_info.prefix);
-   add_pending_sha1(_info, "HEAD", our_tree.hash, 0);
+   add_pending_oid(_info, "HEAD", _tree, 0);
diff_setup_done(_info.diffopt);
run_diff_index(_info, 1);
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index afa99fb8a..7f1eeea94 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -721,7 +721,7 @@ static int add_pending_uninteresting_ref(const char 
*refname,
 const struct object_id *oid,
 int flags, void *cb_data)
 {
-   add_pending_sha1(cb_data, refname, oid->hash, UNINTERESTING);
+   add_pending_oid(cb_data, refname, oid, UNINTERESTING);
return 0;
 }
 
@@ -807,7 +807,7 @@ static void orphaned_commit_warning(struct commit *old, 
struct commit *new)
add_pending_object(, object, oid_to_hex(>oid));
 
for_each_ref(add_pending_uninteresting_ref, );
-   add_pending_sha1(, "HEAD", new->object.oid.hash, UNINTERESTING);
+   add_pending_oid(, "HEAD", >object.oid, UNINTERESTING);
 
refs = revs.pending;
revs.leak_pending = 1;
diff --git a/revision.c b/revision.c
index c2091b6de..f82c56e1f 100644
--- a/revision.c
+++ b/revision.c
@@ -203,10 +203,10 @@ static struct object *get_reference(struct rev_info 
*revs, const char *name,
return object;
 }
 
-void add_pending_sha1(struct rev_info *revs, const char *name,
- const unsigned char *sha1, unsigned int flags)
+void add_pending_oid(struct rev_info *revs, const char *name,
+ const struct object_id *oid, unsigned int flags)
 {
-   struct object *object = get_reference(revs, name, sha1, flags);
+   struct object *object = get_reference(revs, name, oid->hash, flags);
add_pending_object(revs, object, name);
 }
 
@@ -1159,7 +1159,7 @@ static int handle_one_ref(const char *path, const struct 
object_id *oid,
 
object = get_reference(cb->all_revs, path, oid->hash, cb->all_flags);
add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags);
-   add_pending_sha1(cb->all_revs, path, oid->hash, cb->all_flags);
+   add_pending_oid(cb->all_revs, path, oid, cb->all_flags);
return 0;
 }
 
diff --git a/revision.h b/revision.h
index 14886ec92..728425a02 100644
--- a/revision.h
+++ b/revision.h
@@ -263,9 +263,9 @@ extern void show_object_with_name(FILE *, struct object *, 
const char *);
 
 extern void add_pending_object(struct rev_info *revs,
   struct object *obj, const char *name);
-extern void add_pending_sha1(struct rev_info *revs,
-const char *name, const unsigned char *sha1,
-unsigned int flags);
+extern void add_pending_oid(struct rev_info *revs,
+   const char *name, const struct object_id *oid,
+   unsigned int flags);
 
 extern void add_head_to_pending(struct rev_info *);
 extern void add_reflogs_to_pending(struct rev_info *, unsigned int flags);

[PATCH 39/53] refs/files-backend: convert many internals to struct object_id

[brian m. carlson]

Convert many of the internals of the files backend to use struct
object_id.  Avoid converting public APIs to limit the scope of the
changes.

Convert one use of get_sha1_hex to parse_oid_hex, and rely on the fact
that a strbuf will be NUL-terminated and that parse_oid_hex will fail on
truncated input to avoid the need to check for an explicit length.

This is a requirement to convert parse_object later on.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index ccabf761e..171a157f7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -166,8 +166,8 @@ static struct ref_entry *create_dir_entry(struct 
files_ref_store *ref_store,
  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
 static int files_log_ref_write(struct files_ref_store *refs,
-  const char *refname, const unsigned char 
*old_sha1,
-  const unsigned char *new_sha1, const char *msg,
+  const char *refname, const struct object_id 
*old_oid,
+  const struct object_id *new_oid, const char *msg,
   int flags, struct strbuf *err);
 
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
@@ -201,7 +201,7 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 }
 
 static struct ref_entry *create_ref_entry(const char *refname,
- const unsigned char *sha1, int flag,
+ const struct object_id *oid, int flag,
  int check_name)
 {
struct ref_entry *ref;
@@ -210,7 +210,7 @@ static struct ref_entry *create_ref_entry(const char 
*refname,
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
die("Reference has invalid format: '%s'", refname);
FLEX_ALLOC_STR(ref, name, refname);
-   hashcpy(ref->u.value.oid.hash, sha1);
+   oidcpy(>u.value.oid, oid);
oidclr(>u.value.peeled);
ref->flag = flag;
return ref;
@@ -1049,27 +1049,18 @@ static const char PACKED_REFS_HEADER[] =
  * Return a pointer to the refname within the line (null-terminated),
  * or NULL if there was a problem.
  */
-static const char *parse_ref_line(struct strbuf *line, unsigned char *sha1)
+static const char *parse_ref_line(struct strbuf *line, struct object_id *oid)
 {
const char *ref;
 
-   /*
-* 42: the answer to everything.
-*
-* In this case, it happens to be the answer to
-*  40 (length of sha1 hex representation)
-*  +1 (space in between hex and name)
-*  +1 (newline at the end of the line)
-*/
-   if (line->len <= 42)
+   if (!line->len)
return NULL;
 
-   if (get_sha1_hex(line->buf, sha1) < 0)
+   if (parse_oid_hex(line->buf, oid, ) < 0)
return NULL;
-   if (!isspace(line->buf[40]))
+   if (!isspace(*ref++))
return NULL;
 
-   ref = line->buf + 41;
if (isspace(*ref))
return NULL;
 
@@ -1114,7 +1105,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
 
while (strbuf_getwholeline(, f, '\n') != EOF) {
-   unsigned char sha1[20];
+   struct object_id oid;
const char *refname;
const char *traits;
 
@@ -1127,17 +1118,17 @@ static void read_packed_refs(FILE *f, struct ref_dir 
*dir)
continue;
}
 
-   refname = parse_ref_line(, sha1);
+   refname = parse_ref_line(, );
if (refname) {
int flag = REF_ISPACKED;
 
if (check_refname_format(refname, 
REFNAME_ALLOW_ONELEVEL)) {
if (!refname_is_safe(refname))
die("packed refname is dangerous: %s", 
refname);
-   hashclr(sha1);
+   oidclr();
flag |= REF_BAD_NAME | REF_ISBROKEN;
}
-   last = create_ref_entry(refname, sha1, flag, 0);
+   last = create_ref_entry(refname, , flag, 0);
if (peeled == PEELED_FULLY ||
(peeled == PEELED_TAGS && starts_with(refname, 
"refs/tags/")))
last->flag |= REF_KNOWS_PEELED;
@@ -1148,8 +1139,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
line.buf[0] == '^' &&
line.len == 

[PATCH 35/53] Convert the verify_pack callback to struct object_id

[brian m. carlson]

Make the verify_pack_callback take a pointer to struct object_id.  Use a
struct object_id to hold the pack checksum, even though it is not
strictly an object ID.  Doing so ensures resilience against future hash
size changes, and allows us to remove hard-coded assumptions about how
big the buffer needs to be.

Also, use a union to convert the pointer from nth_packed_object_sha1 to
to a pointer to struct object_id.  This behavior is compatible with GCC
and clang and explicitly sanctioned by C11.  The alternatives are to
just perform a cast, which would run afoul of strict aliasing rules, but
should just work, and changing the pointer into an instance of struct
object_id and copying the value.  The latter operation could seriously
bloat memory usage on fsck, which already uses a lot of memory on some
repositories.

Signed-off-by: brian m. carlson 
---
 builtin/fsck.c |  6 +++---
 pack-check.c   | 26 +++---
 pack.h |  2 +-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 14a216ee6..359d61de0 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -377,7 +377,7 @@ static int fsck_obj(struct object *obj)
return 0;
 }
 
-static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
+static int fsck_obj_buffer(const struct object_id *oid, enum object_type type,
   unsigned long size, void *buffer, int *eaten)
 {
/*
@@ -385,10 +385,10 @@ static int fsck_obj_buffer(const unsigned char *sha1, 
enum object_type type,
 * verify_packfile(), data_valid variable for details.
 */
struct object *obj;
-   obj = parse_object_buffer(sha1, type, size, buffer, eaten);
+   obj = parse_object_buffer(oid->hash, type, size, buffer, eaten);
if (!obj) {
errors_found |= ERROR_OBJECT;
-   return error("%s: object corrupt or missing", 
sha1_to_hex(sha1));
+   return error("%s: object corrupt or missing", oid_to_hex(oid));
}
obj->flags = HAS_OBJ;
return fsck_obj(obj);
diff --git a/pack-check.c b/pack-check.c
index 27f70d345..0b504d9c5 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -5,7 +5,10 @@
 
 struct idx_entry {
off_toffset;
-   const unsigned char *sha1;
+   union idx_entry_object {
+   const unsigned char *hash;
+   struct object_id *oid;
+   } oid;
unsigned int nr;
 };
 
@@ -51,7 +54,8 @@ static int verify_packfile(struct packed_git *p,
off_t index_size = p->index_size;
const unsigned char *index_base = p->index_data;
git_SHA_CTX ctx;
-   unsigned char sha1[20], *pack_sig;
+   struct object_id oid;
+   unsigned char *pack_sig;
off_t offset = 0, pack_sig_ofs = 0;
uint32_t nr_objects, i;
int err = 0;
@@ -71,9 +75,9 @@ static int verify_packfile(struct packed_git *p,
remaining -= (unsigned int)(offset - pack_sig_ofs);
git_SHA1_Update(, in, remaining);
} while (offset < pack_sig_ofs);
-   git_SHA1_Final(sha1, );
+   git_SHA1_Final(oid.hash, );
pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);
-   if (hashcmp(sha1, pack_sig))
+   if (hashcmp(oid.hash, pack_sig))
err = error("%s SHA1 checksum mismatch",
p->pack_name);
if (hashcmp(index_base + index_size - 40, pack_sig))
@@ -90,8 +94,8 @@ static int verify_packfile(struct packed_git *p,
entries[nr_objects].offset = pack_sig_ofs;
/* first sort entries by pack offset, since unpacking them is more 
efficient that way */
for (i = 0; i < nr_objects; i++) {
-   entries[i].sha1 = nth_packed_object_sha1(p, i);
-   if (!entries[i].sha1)
+   entries[i].oid.hash = nth_packed_object_sha1(p, i);
+   if (!entries[i].oid.hash)
die("internal error pack-check nth-packed-object");
entries[i].offset = nth_packed_object_offset(p, i);
entries[i].nr = i;
@@ -112,7 +116,7 @@ static int verify_packfile(struct packed_git *p,
if (check_pack_crc(p, w_curs, offset, len, nr))
err = error("index CRC mismatch for object %s "
"from %s at offset %"PRIuMAX"",
-   sha1_to_hex(entries[i].sha1),
+   oid_to_hex(entries[i].oid.oid),
p->pack_name, (uintmax_t)offset);
}
 
@@ -135,14 +139,14 @@ static int verify_packfile(struct packed_git *p,
 
if (data_valid && !data)
err = error("cannot unpack %s from %s at offset 
%"PRIuMAX"",
-   sha1_to_hex(entries[i].sha1), p->pack_name,
+   

[PATCH 40/53] http-push: convert process_ls_object and descendants to object_id

[brian m. carlson]

Rename one function to reflect that it now uses struct object_id.  This
conversion is a prerequisite for converting parse_object.

Note that while the use of a buffer that is exactly forty bytes long
looks questionable, get_oid_hex reads exactly the right number of bytes
and does not require the data to be NUL-terminated.

Signed-off-by: brian m. carlson 
---
 http-push.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/http-push.c b/http-push.c
index 7781f4078..4e7bd9e42 100644
--- a/http-push.c
+++ b/http-push.c
@@ -718,13 +718,13 @@ static int fetch_indices(void)
return ret;
 }
 
-static void one_remote_object(const unsigned char *sha1)
+static void one_remote_object(const struct object_id *oid)
 {
struct object *obj;
 
-   obj = lookup_object(sha1);
+   obj = lookup_object(oid->hash);
if (!obj)
-   obj = parse_object(sha1);
+   obj = parse_object(oid->hash);
 
/* Ignore remote objects that don't exist locally */
if (!obj)
@@ -1013,26 +1013,26 @@ static void remote_ls(const char *path, int flags,
  void *userData);
 
 /* extract hex from sharded "xx/x{40}" filename */
-static int get_sha1_hex_from_objpath(const char *path, unsigned char *sha1)
+static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)
 {
-   char hex[40];
+   char hex[GIT_MAX_HEXSZ];
 
-   if (strlen(path) != 41)
+   if (strlen(path) != GIT_SHA1_HEXSZ + 1)
return -1;
 
memcpy(hex, path, 2);
path += 2;
path++; /* skip '/' */
-   memcpy(hex, path, 38);
+   memcpy(hex, path, GIT_SHA1_HEXSZ - 2);
 
-   return get_sha1_hex(hex, sha1);
+   return get_oid_hex(hex, oid);
 }
 
 static void process_ls_object(struct remote_ls_ctx *ls)
 {
unsigned int *parent = (unsigned int *)ls->userData;
const char *path = ls->dentry_name;
-   unsigned char sha1[20];
+   struct object_id oid;
 
if (!strcmp(ls->path, ls->dentry_name) && (ls->flags & IS_DIR)) {
remote_dir_exists[*parent] = 1;
@@ -1040,10 +1040,10 @@ static void process_ls_object(struct remote_ls_ctx *ls)
}
 
if (!skip_prefix(path, "objects/", ) ||
-   get_sha1_hex_from_objpath(path, sha1))
+   get_oid_hex_from_objpath(path, ))
return;
 
-   one_remote_object(sha1);
+   one_remote_object();
 }
 
 static void process_ls_ref(struct remote_ls_ctx *ls)

[PATCH 43/53] upload-pack: convert remaining parse_object callers to object_id

[brian m. carlson]

Convert the remaining parse_object callers to struct object_id.  Use
named constants for several hard-coded values.  In addition, rename
got_sha1 to got_oid to reflect the new argument.

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

diff --git a/upload-pack.c b/upload-pack.c
index 20f87cd38..5b9d21089 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -286,19 +286,19 @@ static void create_pack_file(void)
die("git upload-pack: %s", abort_msg);
 }
 
-static int got_sha1(const char *hex, unsigned char *sha1)
+static int got_oid(const char *hex, struct object_id *oid)
 {
struct object *o;
int we_knew_they_have = 0;
 
-   if (get_sha1_hex(hex, sha1))
+   if (get_oid_hex(hex, oid))
die("git upload-pack: expected SHA1 object, got '%s'", hex);
-   if (!has_sha1_file(sha1))
+   if (!has_object_file(oid))
return -1;
 
-   o = parse_object(sha1);
+   o = parse_object(oid->hash);
if (!o)
-   die("oops (%s)", sha1_to_hex(sha1));
+   die("oops (%s)", oid_to_hex(oid));
if (o->type == OBJ_COMMIT) {
struct commit_list *parents;
struct commit *commit = (struct commit *)o;
@@ -382,8 +382,8 @@ static int ok_to_give_up(void)
 
 static int get_common_commits(void)
 {
-   unsigned char sha1[20];
-   char last_hex[41];
+   struct object_id oid;
+   char last_hex[GIT_MAX_HEXSZ + 1];
int got_common = 0;
int got_other = 0;
int sent_ready = 0;
@@ -416,11 +416,11 @@ static int get_common_commits(void)
continue;
}
if (skip_prefix(line, "have ", )) {
-   switch (got_sha1(arg, sha1)) {
+   switch (got_oid(arg, )) {
case -1: /* they have what we do not */
got_other = 1;
if (multi_ack && ok_to_give_up()) {
-   const char *hex = sha1_to_hex(sha1);
+   const char *hex = oid_to_hex();
if (multi_ack == 2) {
sent_ready = 1;
packet_write_fmt(1, "ACK %s 
ready\n", hex);
@@ -430,7 +430,7 @@ static int get_common_commits(void)
break;
default:
got_common = 1;
-   memcpy(last_hex, sha1_to_hex(sha1), 41);
+   memcpy(last_hex, oid_to_hex(), 41);
if (multi_ack == 2)
packet_write_fmt(1, "ACK %s common\n", 
last_hex);
else if (multi_ack)
@@ -492,7 +492,7 @@ static int do_reachable_revlist(struct child_process *cmd,
goto error;
 
namebuf[0] = '^';
-   namebuf[41] = '\n';
+   namebuf[GIT_SHA1_HEXSZ + 1] = '\n';
for (i = get_max_object_index(); 0 < i; ) {
o = get_indexed_object(--i);
if (!o)
@@ -502,10 +502,10 @@ static int do_reachable_revlist(struct child_process *cmd,
if (!is_our_ref(o))
continue;
memcpy(namebuf + 1, oid_to_hex(>oid), GIT_SHA1_HEXSZ);
-   if (write_in_full(cmd->in, namebuf, 42) < 0)
+   if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 2) < 0)
goto error;
}
-   namebuf[40] = '\n';
+   namebuf[GIT_SHA1_HEXSZ] = '\n';
for (i = 0; i < src->nr; i++) {
o = src->objects[i].item;
if (is_our_ref(o)) {
@@ -516,7 +516,7 @@ static int do_reachable_revlist(struct child_process *cmd,
if (reachable && o->type == OBJ_COMMIT)
o->flags |= TMP_MARK;
memcpy(namebuf, oid_to_hex(>oid), GIT_SHA1_HEXSZ);
-   if (write_in_full(cmd->in, namebuf, 41) < 0)
+   if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 1) < 0)
goto error;
}
close(cmd->in);
@@ -742,7 +742,7 @@ static void receive_needs(void)
for (;;) {
struct object *o;
const char *features;
-   unsigned char sha1_buf[20];
+   struct object_id oid_buf;
char *line = packet_read_line(0, NULL);
const char *arg;
 
@@ -751,15 +751,15 @@ static void receive_needs(void)
break;
 
if (skip_prefix(line, "shallow ", )) {
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *object;
-   

[PATCH 47/53] sequencer: convert fast_forward_to to struct object_id

[brian m. carlson]

fast_forward_to is required for checkout_fast_fowrard, which is required
for parse_tree_indirect.

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

diff --git a/sequencer.c b/sequencer.c
index 6957f3b4e..c673bb930 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -374,7 +374,7 @@ static void update_abort_safety_file(void)
write_file(git_path_abort_safety_file(), "%s", "");
 }
 
-static int fast_forward_to(const unsigned char *to, const unsigned char *from,
+static int fast_forward_to(const struct object_id *to, const struct object_id 
*from,
int unborn, struct replay_opts *opts)
 {
struct ref_transaction *transaction;
@@ -382,7 +382,7 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
struct strbuf err = STRBUF_INIT;
 
read_cache();
-   if (checkout_fast_forward(from, to, 1))
+   if (checkout_fast_forward(from->hash, to->hash, 1))
return -1; /* the callee should have complained already */
 
strbuf_addf(, _("%s: fast-forward"), _(action_name(opts)));
@@ -390,7 +390,7 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_update(transaction, "HEAD",
-  to, unborn ? null_sha1 : from,
+  to->hash, unborn ? null_sha1 : from->hash,
   0, sb.buf, ) ||
ref_transaction_commit(transaction, )) {
ref_transaction_free(transaction);
@@ -935,7 +935,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
 {
unsigned int flags = opts->edit ? EDIT_MSG : 0;
const char *msg_file = opts->edit ? NULL : git_path_merge_msg();
-   unsigned char head[20];
+   struct object_id head;
struct commit *base, *next, *parent;
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
@@ -949,12 +949,12 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
 * that represents the "current" state for merge-recursive
 * to work on.
 */
-   if (write_cache_as_tree(head, 0, NULL))
+   if (write_cache_as_tree(head.hash, 0, NULL))
return error(_("your index file is unmerged."));
} else {
-   unborn = get_sha1("HEAD", head);
+   unborn = get_oid("HEAD", );
if (unborn)
-   hashcpy(head, EMPTY_TREE_SHA1_BIN);
+   oidcpy(, _tree_oid);
if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 
0, 0))
return error_dirty_index(opts);
}
@@ -990,11 +990,11 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
oid_to_hex(>object.oid));
 
if (opts->allow_ff && !is_fixup(command) &&
-   ((parent && !hashcmp(parent->object.oid.hash, head)) ||
+   ((parent && !oidcmp(>object.oid, )) ||
 (!parent && unborn))) {
if (is_rebase_i(opts))
write_author_script(msg.message);
-   res = fast_forward_to(commit->object.oid.hash, head, unborn,
+   res = fast_forward_to(>object.oid, , unborn,
opts);
if (res || command != TODO_REWORD)
goto leave;
@@ -1081,7 +1081,7 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
res = -1;
else if (!opts->strategy || !strcmp(opts->strategy, "recursive") || 
command == TODO_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
-head, , opts);
+head.hash, , opts);
if (res < 0)
return res;
res |= write_message(msgbuf.buf, msgbuf.len,
@@ -1097,7 +1097,7 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
commit_list_insert(next, );
res |= try_merge_command(opts->strategy,
 opts->xopts_nr, (const char 
**)opts->xopts,
-   common, sha1_to_hex(head), remotes);
+   common, oid_to_hex(), remotes);
free_commit_list(common);
free_commit_list(remotes);
}

[PATCH 38/53] refs: convert struct ref_update to use struct object_id

[brian m. carlson]

Convert struct ref_array_item to use struct object_id by changing the
definition and applying the following semantic patch, plus the standard
object_id transforms:

@@
struct ref_update E1;
@@
- E1.new_sha1
+ E1.new_oid.hash

@@
struct ref_update *E1;
@@
- E1->new_sha1
+ E1->new_oid.hash

@@
struct ref_update E1;
@@
- E1.old_sha1
+ E1.old_oid.hash

@@
struct ref_update *E1;
@@
- E1->old_sha1
+ E1->old_oid.hash

This transformation allows us to convert write_ref_to_lockfile, which is
required to convert parse_object.

Signed-off-by: brian m. carlson 
---
 refs.c   |  4 ++--
 refs/files-backend.c | 29 +++--
 refs/refs-internal.h |  4 ++--
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/refs.c b/refs.c
index d1e1b4399..f76c42081 100644
--- a/refs.c
+++ b/refs.c
@@ -881,9 +881,9 @@ struct ref_update *ref_transaction_add_update(
update->flags = flags;
 
if (flags & REF_HAVE_NEW)
-   hashcpy(update->new_sha1, new_sha1);
+   hashcpy(update->new_oid.hash, new_sha1);
if (flags & REF_HAVE_OLD)
-   hashcpy(update->old_sha1, old_sha1);
+   hashcpy(update->old_oid.hash, old_sha1);
update->msg = xstrdup_or_null(msg);
return update;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 238a046e6..ccabf761e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3549,7 +3549,7 @@ static int split_head_update(struct ref_update *update,
new_update = ref_transaction_add_update(
transaction, "HEAD",
update->flags | REF_LOG_ONLY | REF_NODEREF,
-   update->new_sha1, update->old_sha1,
+   update->new_oid.hash, update->old_oid.hash,
update->msg);
 
item->util = new_update;
@@ -3606,7 +3606,7 @@ static int split_symref_update(struct files_ref_store 
*refs,
 
new_update = ref_transaction_add_update(
transaction, referent, new_flags,
-   update->new_sha1, update->old_sha1,
+   update->new_oid.hash, update->old_oid.hash,
update->msg);
 
new_update->parent_update = update;
@@ -3645,10 +3645,10 @@ static int check_old_oid(struct ref_update *update, 
struct object_id *oid,
 struct strbuf *err)
 {
if (!(update->flags & REF_HAVE_OLD) ||
-  !hashcmp(oid->hash, update->old_sha1))
+  !oidcmp(oid, >old_oid))
return 0;
 
-   if (is_null_sha1(update->old_sha1))
+   if (is_null_oid(>old_oid))
strbuf_addf(err, "cannot lock ref '%s': "
"reference already exists",
original_update_refname(update));
@@ -3656,13 +3656,13 @@ static int check_old_oid(struct ref_update *update, 
struct object_id *oid,
strbuf_addf(err, "cannot lock ref '%s': "
"reference is missing but expected %s",
original_update_refname(update),
-   sha1_to_hex(update->old_sha1));
+   oid_to_hex(>old_oid));
else
strbuf_addf(err, "cannot lock ref '%s': "
"is at %s but expected %s",
original_update_refname(update),
oid_to_hex(oid),
-   sha1_to_hex(update->old_sha1));
+   oid_to_hex(>old_oid));
 
return -1;
 }
@@ -3689,13 +3689,13 @@ static int lock_ref_for_update(struct files_ref_store 
*refs,
 {
struct strbuf referent = STRBUF_INIT;
int mustexist = (update->flags & REF_HAVE_OLD) &&
-   !is_null_sha1(update->old_sha1);
+   !is_null_oid(>old_oid);
int ret;
struct ref_lock *lock;
 
files_assert_main_repository(refs, "lock_ref_for_update");
 
-   if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
+   if ((update->flags & REF_HAVE_NEW) && is_null_oid(>new_oid))
update->flags |= REF_DELETING;
 
if (head_ref) {
@@ -3777,12 +3777,12 @@ static int lock_ref_for_update(struct files_ref_store 
*refs,
!(update->flags & REF_DELETING) &&
!(update->flags & REF_LOG_ONLY)) {
if (!(update->type & REF_ISSYMREF) &&
-   !hashcmp(lock->old_oid.hash, update->new_sha1)) {
+   !oidcmp(>old_oid, >new_oid)) {
/*
 * The reference already has the desired
 * value, so we don't need to write it.
 */
-   } else if (write_ref_to_lockfile(lock, update->new_sha1,
+   } else if (write_ref_to_lockfile(lock, update->new_oid.hash,
  

[PATCH 46/53] builtin/ls-files: convert overlay_tree_on_cache to object_id

[brian m. carlson]

This is another caller of parse_tree_indirect.

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

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index d449e46db..948c7066c 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -407,14 +407,14 @@ static void prune_cache(const char *prefix, size_t 
prefixlen)
 void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 {
struct tree *tree;
-   unsigned char sha1[20];
+   struct object_id oid;
struct pathspec pathspec;
struct cache_entry *last_stage0 = NULL;
int i;
 
-   if (get_sha1(tree_name, sha1))
+   if (get_oid(tree_name, ))
die("tree-ish %s not found.", tree_name);
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid.hash);
if (!tree)
die("bad tree-ish %s", tree_name);
 

[PATCH 32/53] Convert lookup_tree to struct object_id

[brian m. carlson]

Convert the lookup_tree function to take a pointer to struct object_id.

The commit was created with manual changes to tree.c, tree.h, and
object.c, plus the following semantic patch:

@@
@@
- lookup_tree(EMPTY_TREE_SHA1_BIN)
+ lookup_tree(_tree_oid)

@@
expression E1;
@@
- lookup_tree(E1.hash)
+ lookup_tree()

@@
expression E1;
@@
- lookup_tree(E1->hash)
+ lookup_tree(E1)

Signed-off-by: brian m. carlson 
---
 builtin/am.c| 4 ++--
 builtin/diff-tree.c | 2 +-
 builtin/diff.c  | 2 +-
 builtin/reflog.c| 2 +-
 cache-tree.c| 2 +-
 commit.c| 2 +-
 fsck.c  | 2 +-
 http-push.c | 2 +-
 list-objects.c  | 2 +-
 merge-recursive.c   | 6 +++---
 object.c| 2 +-
 reachable.c | 2 +-
 revision.c  | 4 ++--
 sequencer.c | 2 +-
 tag.c   | 2 +-
 tree.c  | 8 
 tree.h  | 2 +-
 walker.c| 2 +-
 18 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index a0979fab2..e12da7b3f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1457,9 +1457,9 @@ static void write_index_patch(const struct am_state 
*state)
FILE *fp;
 
if (!get_sha1_tree("HEAD", head.hash))
-   tree = lookup_tree(head.hash);
+   tree = lookup_tree();
else
-   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+   tree = lookup_tree(_tree_oid);
 
fp = xfopen(am_path(state, "patch"), "w");
init_revisions(_info, NULL);
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index e85a449df..95b8d1ba7 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -44,7 +44,7 @@ static int stdin_diff_trees(struct tree *tree1, const char *p)
struct tree *tree2;
if (!isspace(*p++) || parse_oid_hex(p, , ) || *p)
return error("Need exactly two trees, separated by a space");
-   tree2 = lookup_tree(oid.hash);
+   tree2 = lookup_tree();
if (!tree2 || parse_tree(tree2))
return -1;
printf("%s %s\n", oid_to_hex(>object.oid),
diff --git a/builtin/diff.c b/builtin/diff.c
index a25b4e4ae..895f92897 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -381,7 +381,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
add_head_to_pending();
if (!rev.pending.nr) {
struct tree *tree;
-   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+   tree = lookup_tree(_tree_oid);
add_pending_object(, >object, 
"HEAD");
}
break;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7866a0341..7e89e84dc 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -62,7 +62,7 @@ static int tree_is_complete(const struct object_id *oid)
int complete;
struct tree *tree;
 
-   tree = lookup_tree(oid->hash);
+   tree = lookup_tree(oid);
if (!tree)
return 0;
if (tree->object.flags & SEEN)
diff --git a/cache-tree.c b/cache-tree.c
index 35d507ed7..489241021 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -672,7 +672,7 @@ static void prime_cache_tree_rec(struct cache_tree *it, 
struct tree *tree)
cnt++;
else {
struct cache_tree_sub *sub;
-   struct tree *subtree = lookup_tree(entry.oid->hash);
+   struct tree *subtree = lookup_tree(entry.oid);
if (!subtree->object.parsed)
parse_tree(subtree);
sub = cache_tree_sub(it, entry.path);
diff --git a/commit.c b/commit.c
index 0f6c9b6bf..8004008bc 100644
--- a/commit.c
+++ b/commit.c
@@ -331,7 +331,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
if (get_sha1_hex(bufptr + 5, parent.hash) < 0)
return error("bad tree pointer in commit %s",
 oid_to_hex(>object.oid));
-   item->tree = lookup_tree(parent.hash);
+   item->tree = lookup_tree();
bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
pptr = >parents;
 
diff --git a/fsck.c b/fsck.c
index ab3016c0e..ee5224aea 100644
--- a/fsck.c
+++ b/fsck.c
@@ -358,7 +358,7 @@ static int fsck_walk_tree(struct tree *tree, void *data, 
struct fsck_options *op
continue;
 
if (S_ISDIR(entry.mode)) {
-   obj = _tree(entry.oid->hash)->object;
+   obj = _tree(entry.oid)->object;
if (name)
put_object_name(options, obj, "%s%s/", name,
entry.path);
diff --git a/http-push.c 

[PATCH 51/53] sequencer: convert do_recursive_merge to struct object_id

[brian m. carlson]

This conversion is required to convert parse_tree_indirect.

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

diff --git a/sequencer.c b/sequencer.c
index dfa44afa7..ed48c4c29 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -426,7 +426,7 @@ void append_conflicts_hint(struct strbuf *msgbuf)
 
 static int do_recursive_merge(struct commit *base, struct commit *next,
  const char *base_label, const char *next_label,
- unsigned char *head, struct strbuf *msgbuf,
+ struct object_id *head, struct strbuf *msgbuf,
  struct replay_opts *opts)
 {
struct merge_options o;
@@ -446,7 +446,7 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
if (is_rebase_i(opts))
o.buffer_output = 2;
 
-   head_tree = parse_tree_indirect(head);
+   head_tree = parse_tree_indirect(head->hash);
next_tree = next ? next->tree : empty_tree();
base_tree = base ? base->tree : empty_tree();
 
@@ -1081,7 +1081,7 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
res = -1;
else if (!opts->strategy || !strcmp(opts->strategy, "recursive") || 
command == TODO_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
-head.hash, , opts);
+, , opts);
if (res < 0)
return res;
res |= write_message(msgbuf.buf, msgbuf.len,

[PATCH 37/53] ref-filter: convert some static functions to struct object_id

[brian m. carlson]

Among the converted functions is a caller of parse_object_buffer, which
we will convert later.

Signed-off-by: brian m. carlson 
---
 ref-filter.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 77aee273f..56fc990a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -677,13 +677,13 @@ int verify_ref_format(const char *format)
  * by the "struct object" representation, set *eaten as well---it is a
  * signal from parse_object_buffer to us not to free the buffer.
  */
-static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned 
long *sz, int *eaten)
+static void *get_obj(const struct object_id *oid, struct object **obj, 
unsigned long *sz, int *eaten)
 {
enum object_type type;
-   void *buf = read_sha1_file(sha1, , sz);
+   void *buf = read_sha1_file(oid->hash, , sz);
 
if (buf)
-   *obj = parse_object_buffer(sha1, type, *sz, buf, eaten);
+   *obj = parse_object_buffer(oid->hash, type, *sz, buf, eaten);
else
*obj = NULL;
return buf;
@@ -1293,7 +1293,7 @@ static void populate_value(struct ref_array_item *ref)
struct object *obj;
int eaten, i;
unsigned long size;
-   const unsigned char *tagged;
+   const struct object_id *tagged;
 
ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
@@ -1370,10 +1370,10 @@ static void populate_value(struct ref_array_item *ref)
continue;
} else if (!strcmp(name, "HEAD")) {
const char *head;
-   unsigned char sha1[20];
+   struct object_id oid;
 
head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
- sha1, NULL);
+ oid.hash, NULL);
if (head && !strcmp(ref->refname, head))
v->s = "*";
else
@@ -1415,7 +1415,7 @@ static void populate_value(struct ref_array_item *ref)
return;
 
  need_obj:
-   buf = get_obj(ref->objectname.hash, , , );
+   buf = get_obj(>objectname, , , );
if (!buf)
die(_("missing object %s for %s"),
oid_to_hex(>objectname), ref->refname);
@@ -1438,7 +1438,7 @@ static void populate_value(struct ref_array_item *ref)
 * If it is a tag object, see if we use a value that derefs
 * the object, and if we do grab the object it refers to.
 */
-   tagged = ((struct tag *)obj)->tagged->oid.hash;
+   tagged = &((struct tag *)obj)->tagged->oid;
 
/*
 * NEEDSWORK: This derefs tag only once, which
@@ -1449,10 +1449,10 @@ static void populate_value(struct ref_array_item *ref)
buf = get_obj(tagged, , , );
if (!buf)
die(_("missing object %s for %s"),
-   sha1_to_hex(tagged), ref->refname);
+   oid_to_hex(tagged), ref->refname);
if (!obj)
die(_("parse_object_buffer failed on %s for %s"),
-   sha1_to_hex(tagged), ref->refname);
+   oid_to_hex(tagged), ref->refname);
grab_values(ref->value, 1, obj, buf, size);
if (!eaten)
free(buf);

[PATCH 29/53] Convert lookup_blob to struct object_id

[brian m. carlson]

Convert lookup_blob to take a pointer to struct object_id.

The commit was created with manual changes to blob.c and blob.h, plus
the following semantic patch:

@@
expression E1;
@@
- lookup_blob(E1.hash)
+ lookup_blob()

@@
expression E1;
@@
- lookup_blob(E1->hash)
+ lookup_blob(E1)

Signed-off-by: brian m. carlson 
---
 blob.c   | 6 +++---
 blob.h   | 2 +-
 builtin/fast-export.c| 2 +-
 builtin/fsck.c   | 2 +-
 builtin/index-pack.c | 2 +-
 builtin/merge-tree.c | 2 +-
 builtin/unpack-objects.c | 2 +-
 fsck.c   | 2 +-
 http-push.c  | 2 +-
 list-objects.c   | 2 +-
 object.c | 4 ++--
 reachable.c  | 2 +-
 revision.c   | 4 ++--
 tag.c| 2 +-
 walker.c | 2 +-
 15 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/blob.c b/blob.c
index 1fcb8e44b..fa2ab4f7a 100644
--- a/blob.c
+++ b/blob.c
@@ -3,11 +3,11 @@
 
 const char *blob_type = "blob";
 
-struct blob *lookup_blob(const unsigned char *sha1)
+struct blob *lookup_blob(const struct object_id *oid)
 {
-   struct object *obj = lookup_object(sha1);
+   struct object *obj = lookup_object(oid->hash);
if (!obj)
-   return create_object(sha1, alloc_blob_node());
+   return create_object(oid->hash, alloc_blob_node());
return object_as_type(obj, OBJ_BLOB, 0);
 }
 
diff --git a/blob.h b/blob.h
index 59b394eea..446061683 100644
--- a/blob.h
+++ b/blob.h
@@ -9,7 +9,7 @@ struct blob {
struct object object;
 };
 
-struct blob *lookup_blob(const unsigned char *sha1);
+struct blob *lookup_blob(const struct object_id *oid);
 
 int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);
 
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index b4521cb62..ae36b14db 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -232,7 +232,7 @@ static void export_blob(const struct object_id *oid)
 
if (anonymize) {
buf = anonymize_blob();
-   object = (struct object *)lookup_blob(oid->hash);
+   object = (struct object *)lookup_blob(oid);
eaten = 0;
} else {
buf = read_sha1_file(oid->hash, , );
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 543122f04..14a216ee6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -780,7 +780,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
mode = active_cache[i]->ce_mode;
if (S_ISGITLINK(mode))
continue;
-   blob = lookup_blob(active_cache[i]->oid.hash);
+   blob = lookup_blob(_cache[i]->oid);
if (!blob)
continue;
obj = >object;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2241ee68e..b75133f62 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -829,7 +829,7 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
if (strict) {
read_lock();
if (type == OBJ_BLOB) {
-   struct blob *blob = lookup_blob(oid->hash);
+   struct blob *blob = lookup_blob(oid);
if (blob)
blob->object.flags |= FLAG_CHECKED;
else
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index cdeb6562d..bad6735c7 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -168,7 +168,7 @@ static struct merge_list *create_entry(unsigned stage, 
unsigned mode, const stru
res->stage = stage;
res->path = path;
res->mode = mode;
-   res->blob = lookup_blob(oid->hash);
+   res->blob = lookup_blob(oid);
return res;
 }
 
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 3dc5e5691..7d5efa2b3 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -249,7 +249,7 @@ static void write_object(unsigned nr, enum object_type type,
added_object(nr, type, buf, size);
free(buf);
 
-   blob = lookup_blob(obj_list[nr].oid.hash);
+   blob = lookup_blob(_list[nr].oid);
if (blob)
blob->object.flags |= FLAG_WRITTEN;
else
diff --git a/fsck.c b/fsck.c
index e6152e4e6..ab3016c0e 100644
--- a/fsck.c
+++ b/fsck.c
@@ -365,7 +365,7 @@ static int fsck_walk_tree(struct tree *tree, void *data, 
struct fsck_options *op
result = options->walk(obj, OBJ_TREE, data, options);
}
else if (S_ISREG(entry.mode) || S_ISLNK(entry.mode)) {
-   obj = _blob(entry.oid->hash)->object;
+   obj = _blob(entry.oid)->object;
if 

[PATCH 34/53] Convert lookup_tag to struct object_id

[brian m. carlson]

Convert lookup_tag to take a pointer to struct object_id.

Signed-off-by: brian m. carlson 
---
 builtin/describe.c | 6 +++---
 builtin/pack-objects.c | 2 +-
 builtin/replace.c  | 2 +-
 log-tree.c | 2 +-
 object.c   | 2 +-
 sha1_name.c| 2 +-
 tag.c  | 8 
 tag.h  | 2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index f6032f593..893c8789f 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -79,13 +79,13 @@ static int replace_name(struct commit_name *e,
struct tag *t;
 
if (!e->tag) {
-   t = lookup_tag(e->oid.hash);
+   t = lookup_tag(>oid);
if (!t || parse_tag(t))
return 1;
e->tag = t;
}
 
-   t = lookup_tag(oid->hash);
+   t = lookup_tag(oid);
if (!t || parse_tag(t))
return 0;
*tag = t;
@@ -245,7 +245,7 @@ static unsigned long finish_depth_computation(
 static void display_name(struct commit_name *n)
 {
if (n->prio == 2 && !n->tag) {
-   n->tag = lookup_tag(n->oid.hash);
+   n->tag = lookup_tag(>oid);
if (!n->tag || parse_tag(n->tag))
die(_("annotated tag %s not available"), n->path);
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d76ff0542..7cebb5a7f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2348,7 +2348,7 @@ static void add_tag_chain(const struct object_id *oid)
if (packlist_find(_pack, oid->hash, NULL))
return;
 
-   tag = lookup_tag(oid->hash);
+   tag = lookup_tag(oid);
while (1) {
if (!tag || parse_tag(tag) || !tag->tagged)
die("unable to pack objects reachable from tag %s",
diff --git a/builtin/replace.c b/builtin/replace.c
index e2ea3d47b..205510b5b 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -354,7 +354,7 @@ static void check_one_mergetag(struct commit *commit,
int i;
 
hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
-   tag = lookup_tag(tag_oid.hash);
+   tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
if (parse_tag_buffer(tag, extra->value, extra->len))
diff --git a/log-tree.c b/log-tree.c
index 169fd039f..6532c892c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -488,7 +488,7 @@ static void show_one_mergetag(struct commit *commit,
size_t payload_size, gpg_message_offset;
 
hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), oid.hash);
-   tag = lookup_tag(oid.hash);
+   tag = lookup_tag();
if (!tag)
return; /* error message already given */
 
diff --git a/object.c b/object.c
index d23c5fad3..dd4d3a1ea 100644
--- a/object.c
+++ b/object.c
@@ -220,7 +220,7 @@ struct object *parse_object_buffer(const unsigned char 
*sha1, enum object_type t
obj = >object;
}
} else if (type == OBJ_TAG) {
-   struct tag *tag = lookup_tag(sha1);
+   struct tag *tag = lookup_tag();
if (tag) {
if (parse_tag_buffer(tag, buffer, size))
   return NULL;
diff --git a/sha1_name.c b/sha1_name.c
index 390a09c41..b7e09ac13 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -361,7 +361,7 @@ static int show_ambiguous_object(const struct object_id 
*oid, void *data)
format_commit_message(commit, " %ad - %s", , );
}
} else if (type == OBJ_TAG) {
-   struct tag *tag = lookup_tag(oid->hash);
+   struct tag *tag = lookup_tag(oid);
if (!parse_tag(tag) && tag->tag)
strbuf_addf(, " %s", tag->tag);
}
diff --git a/tag.c b/tag.c
index 062516b40..571798519 100644
--- a/tag.c
+++ b/tag.c
@@ -89,11 +89,11 @@ struct object *deref_tag_noverify(struct object *o)
return o;
 }
 
-struct tag *lookup_tag(const unsigned char *sha1)
+struct tag *lookup_tag(const struct object_id *oid)
 {
-   struct object *obj = lookup_object(sha1);
+   struct object *obj = lookup_object(oid->hash);
if (!obj)
-   return create_object(sha1, alloc_tag_node());
+   return create_object(oid->hash, alloc_tag_node());
return object_as_type(obj, OBJ_TAG, 0);
 }
 
@@ -148,7 +148,7 @@ int parse_tag_buffer(struct tag *item, const void *data, 
unsigned long size)
} else if (!strcmp(type, commit_type)) {
item->tagged = _commit()->object;
} else if (!strcmp(type, tag_type)) {
-   item->tagged = _tag(oid.hash)->object;
+   

[PATCH 36/53] Convert struct ref_array_item to struct object_id

[brian m. carlson]

Convert struct ref_array_item to use struct object_id by changing the
definition and applying the following semantic patch, plus the standard
object_id transforms:

@@
struct ref_array_item E1;
@@
- E1.objectname
+ E1.objectname.hash

@@
struct ref_array_item *E1;
@@
- E1->objectname
+ E1->objectname.hash

This transformation allows us to convert get_obj, which is needed to
convert parse_object_buffer.

Signed-off-by: brian m. carlson 
---
 ref-filter.c | 10 +-
 ref-filter.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index e1d18ac0d..77aee273f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1366,7 +1366,7 @@ static void populate_value(struct ref_array_item *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   } else if (!deref && grab_objectname(name, ref->objectname, v, 
atom)) {
+   } else if (!deref && grab_objectname(name, 
ref->objectname.hash, v, atom)) {
continue;
} else if (!strcmp(name, "HEAD")) {
const char *head;
@@ -1415,13 +1415,13 @@ static void populate_value(struct ref_array_item *ref)
return;
 
  need_obj:
-   buf = get_obj(ref->objectname, , , );
+   buf = get_obj(ref->objectname.hash, , , );
if (!buf)
die(_("missing object %s for %s"),
-   sha1_to_hex(ref->objectname), ref->refname);
+   oid_to_hex(>objectname), ref->refname);
if (!obj)
die(_("parse_object_buffer failed on %s for %s"),
-   sha1_to_hex(ref->objectname), ref->refname);
+   oid_to_hex(>objectname), ref->refname);
 
grab_values(ref->value, 0, obj, buf, size);
if (!eaten)
@@ -1704,7 +1704,7 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
 {
struct ref_array_item *ref;
FLEX_ALLOC_STR(ref, refname, refname);
-   hashcpy(ref->objectname, objectname);
+   hashcpy(ref->objectname.hash, objectname);
ref->flag = flag;
 
return ref;
diff --git a/ref-filter.h b/ref-filter.h
index c20167aa3..6552024f0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -34,7 +34,7 @@ struct ref_sorting {
 };
 
 struct ref_array_item {
-   unsigned char objectname[20];
+   struct object_id objectname;
int flag;
unsigned int kind;
const char *symref;

[PATCH 28/53] Convert remaining callers of lookup_blob to object_id

[brian m. carlson]

All but a few callers of lookup_blob have been converted to struct
object_id.  Introduce a temporary, which will be removed later, into
parse_object to ease the transition, and convert the remaining callers
so that we can update lookup_blob to take struct object_id *.

Signed-off-by: brian m. carlson 
---
 builtin/index-pack.c | 28 ++--
 builtin/merge-tree.c | 10 +-
 object.c |  9 ++---
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index fef0025e4..2241ee68e 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -785,7 +785,7 @@ static int check_collison(struct object_entry *entry)
 
 static void sha1_object(const void *data, struct object_entry *obj_entry,
unsigned long size, enum object_type type,
-   const unsigned char *sha1)
+   const struct object_id *oid)
 {
void *new_data = NULL;
int collision_test_needed = 0;
@@ -794,7 +794,7 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
 
if (startup_info->have_repository) {
read_lock();
-   collision_test_needed = has_sha1_file_with_flags(sha1, 
HAS_SHA1_QUICK);
+   collision_test_needed = has_sha1_file_with_flags(oid->hash, 
HAS_SHA1_QUICK);
read_unlock();
}
 
@@ -809,31 +809,31 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
enum object_type has_type;
unsigned long has_size;
read_lock();
-   has_type = sha1_object_info(sha1, _size);
+   has_type = sha1_object_info(oid->hash, _size);
if (has_type < 0)
-   die(_("cannot read existing object info %s"), 
sha1_to_hex(sha1));
+   die(_("cannot read existing object info %s"), 
oid_to_hex(oid));
if (has_type != type || has_size != size)
-   die(_("SHA1 COLLISION FOUND WITH %s !"), 
sha1_to_hex(sha1));
-   has_data = read_sha1_file(sha1, _type, _size);
+   die(_("SHA1 COLLISION FOUND WITH %s !"), 
oid_to_hex(oid));
+   has_data = read_sha1_file(oid->hash, _type, _size);
read_unlock();
if (!data)
data = new_data = get_data_from_pack(obj_entry);
if (!has_data)
-   die(_("cannot read existing object %s"), 
sha1_to_hex(sha1));
+   die(_("cannot read existing object %s"), 
oid_to_hex(oid));
if (size != has_size || type != has_type ||
memcmp(data, has_data, size) != 0)
-   die(_("SHA1 COLLISION FOUND WITH %s !"), 
sha1_to_hex(sha1));
+   die(_("SHA1 COLLISION FOUND WITH %s !"), 
oid_to_hex(oid));
free(has_data);
}
 
if (strict) {
read_lock();
if (type == OBJ_BLOB) {
-   struct blob *blob = lookup_blob(sha1);
+   struct blob *blob = lookup_blob(oid->hash);
if (blob)
blob->object.flags |= FLAG_CHECKED;
else
-   die(_("invalid blob object %s"), 
sha1_to_hex(sha1));
+   die(_("invalid blob object %s"), 
oid_to_hex(oid));
} else {
struct object *obj;
int eaten;
@@ -845,7 +845,7 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
 * we do not need to free the memory here, as the
 * buf is deleted by the caller.
 */
-   obj = parse_object_buffer(sha1, type, size, buf, 
);
+   obj = parse_object_buffer(oid->hash, type, size, buf, 
);
if (!obj)
die(_("invalid %s"), typename(type));
if (do_fsck_object &&
@@ -960,7 +960,7 @@ static void resolve_delta(struct object_entry *delta_obj,
   typename(delta_obj->real_type),
   delta_obj->idx.oid.hash);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
-   delta_obj->idx.oid.hash);
+   _obj->idx.oid);
counter_lock();
nr_resolved_deltas++;
counter_unlock();
@@ -1149,7 +1149,7 @@ static void parse_pack_objects(unsigned char *sha1)
nr_delays++;
} else
sha1_object(data, NULL, obj->size, obj->type,
-   obj->idx.oid.hash);
+   >idx.oid);
free(data);

[PATCH 25/53] Convert lookup_commit* to struct object_id

[brian m. carlson]

Convert lookup_commit, lookup_commit_or_die,
lookup_commit_reference, and lookup_commit_reference_gently to take
struct object_id arguments.

Introduce a temporary in parse_object buffer in order to convert this
function.  This is required since in order to convert parse_object and
parse_object_buffer, lookup_commit_reference_gently and
lookup_commit_or_die would need to be converted.  Not introducing a
temporary would therefore require that lookup_commit_or_die take a
struct object_id *, but lookup_commit would take unsigned char *,
leaving a confusing and hard-to-use interface.

parse_object_buffer will lose this temporary in a later patch.

This commit was created with manual changes to commit.c, commit.h, and
object.c, plus the following semantic patch:

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

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

@@
expression E1;
@@
- lookup_commit_reference(E1.hash)
+ lookup_commit_reference()

@@
expression E1;
@@
- lookup_commit_reference(E1->hash)
+ lookup_commit_reference(E1)

@@
expression E1;
@@
- lookup_commit(E1.hash)
+ lookup_commit()

@@
expression E1;
@@
- lookup_commit(E1->hash)
+ lookup_commit(E1)

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

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

Signed-off-by: brian m. carlson 
---
 archive.c   |  2 +-
 bisect.c|  2 +-
 branch.c|  2 +-
 builtin/am.c|  4 ++--
 builtin/blame.c |  4 ++--
 builtin/branch.c|  6 +++---
 builtin/checkout.c  |  6 +++---
 builtin/clone.c |  2 +-
 builtin/commit-tree.c   |  2 +-
 builtin/commit.c|  4 ++--
 builtin/describe.c  |  4 ++--
 builtin/diff-tree.c |  4 ++--
 builtin/fast-export.c   |  2 +-
 builtin/fetch.c |  7 ---
 builtin/fmt-merge-msg.c |  4 ++--
 builtin/log.c   |  8 
 builtin/merge-base.c|  6 +++---
 builtin/merge.c |  2 +-
 builtin/notes.c |  2 +-
 builtin/pull.c  | 10 +-
 builtin/reflog.c|  8 
 builtin/replace.c   |  4 ++--
 builtin/reset.c |  4 ++--
 builtin/rev-parse.c |  6 +++---
 builtin/show-branch.c   |  4 ++--
 builtin/tag.c   |  2 +-
 builtin/verify-commit.c |  2 +-
 bundle.c|  2 +-
 commit.c| 30 +++---
 commit.h| 12 ++--
 fast-import.c   |  4 ++--
 fetch-pack.c|  2 +-
 http-push.c |  5 +++--
 log-tree.c  |  2 +-
 notes-cache.c   |  2 +-
 notes-merge.c   |  4 ++--
 notes-utils.c   |  2 +-
 object.c|  5 -
 parse-options-cb.c  |  2 +-
 ref-filter.c|  4 ++--
 remote.c| 13 +++--
 revision.c  |  8 
 sequencer.c |  8 
 sha1_name.c | 10 +-
 shallow.c   | 20 ++--
 submodule.c | 14 +++---
 tag.c   |  2 +-
 tree.c  |  2 +-
 walker.c|  2 +-
 wt-status.c |  2 +-
 50 files changed, 138 insertions(+), 132 deletions(-)

diff --git a/archive.c b/archive.c
index 60b889198..54701e8bb 100644
--- a/archive.c
+++ b/archive.c
@@ -360,7 +360,7 @@ static void parse_treeish_arg(const char **argv,
if (get_sha1(name, oid.hash))
die("Not a valid object name");
 
-   commit = lookup_commit_reference_gently(oid.hash, 1);
+   commit = lookup_commit_reference_gently(, 1);
if (commit) {
commit_sha1 = commit->object.oid.hash;
archive_time = commit->date;
diff --git a/bisect.c b/bisect.c
index 03af06c66..1f273a85a 100644
--- a/bisect.c
+++ b/bisect.c
@@ -704,7 +704,7 @@ static int bisect_checkout(const unsigned char *bisect_rev, 
int no_checkout)
 
 static struct commit *get_commit_reference(const struct object_id *oid)
 {
-   struct commit *r = lookup_commit_reference(oid->hash);
+   struct commit *r = lookup_commit_reference(oid);
if (!r)
die(_("Not a valid commit name %s"), oid_to_hex(oid));
return r;
diff --git a/branch.c b/branch.c
index 1758c9708..4899144f5 100644
--- a/branch.c
+++ b/branch.c
@@ -286,7 +286,7 @@ void create_branch(const char *name, const char *start_name,
break;
}
 
-   if ((commit = lookup_commit_reference(oid.hash)) == NULL)
+   if ((commit = lookup_commit_reference()) == NULL)
die(_("Not a valid branch point: '%s'."), start_name);
oidcpy(, >object.oid);
 
diff --git a/builtin/am.c b/builtin/am.c
index f7a7a971f..a0979fab2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1492,7 +1492,7 @@ 

[PATCH 33/53] log-tree: convert to struct object_id

[brian m. carlson]

Convert the remaining functions to take pointers to struct object_id
instead of pointers to unsigned char, and update the internals of these
functions as well.  Among these functions is a caller of lookup_tag,
which we will convert shortly.

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

diff --git a/log-tree.c b/log-tree.c
index 7fb1a85d2..169fd039f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -184,7 +184,7 @@ static const struct name_decoration 
*current_pointed_by_HEAD(const struct name_d
 {
const struct name_decoration *list, *head = NULL;
const char *branch_name = NULL;
-   unsigned char unused[20];
+   struct object_id unused;
int rru_flags;
 
/* First find HEAD */
@@ -197,7 +197,7 @@ static const struct name_decoration 
*current_pointed_by_HEAD(const struct name_d
return NULL;
 
/* Now resolve and find the matching current branch */
-   branch_name = resolve_ref_unsafe("HEAD", 0, unused, _flags);
+   branch_name = resolve_ref_unsafe("HEAD", 0, unused.hash, _flags);
if (!(rru_flags & REF_ISSYMREF))
return NULL;
 
@@ -456,13 +456,13 @@ static void show_signature(struct rev_info *opt, struct 
commit *commit)
strbuf_release();
 }
 
-static int which_parent(const unsigned char *sha1, const struct commit *commit)
+static int which_parent(const struct object_id *oid, const struct commit 
*commit)
 {
int nth;
const struct commit_list *parent;
 
for (nth = 0, parent = commit->parents; parent; parent = parent->next) {
-   if (!hashcmp(parent->item->object.oid.hash, sha1))
+   if (!oidcmp(>item->object.oid, oid))
return nth;
nth++;
}
@@ -481,14 +481,14 @@ static void show_one_mergetag(struct commit *commit,
  void *data)
 {
struct rev_info *opt = (struct rev_info *)data;
-   unsigned char sha1[20];
+   struct object_id oid;
struct tag *tag;
struct strbuf verify_message;
int status, nth;
size_t payload_size, gpg_message_offset;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), sha1);
-   tag = lookup_tag(sha1);
+   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), oid.hash);
+   tag = lookup_tag(oid.hash);
if (!tag)
return; /* error message already given */
 
@@ -500,7 +500,7 @@ static void show_one_mergetag(struct commit *commit,
  >parents->next->item->object.oid))
strbuf_addf(_message,
"merged tag '%s'\n", tag->tag);
-   else if ((nth = which_parent(tag->tagged->oid.hash, commit)) < 0)
+   else if ((nth = which_parent(>tagged->oid, commit)) < 0)
strbuf_addf(_message, "tag %s names a non-parent %s\n",
tag->tag, tag->tagged->oid.hash);
else
@@ -536,7 +536,7 @@ void show_log(struct rev_info *opt)
struct strbuf msgbuf = STRBUF_INIT;
struct log_info *log = opt->loginfo;
struct commit *commit = log->commit, *parent = log->parent;
-   int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
+   int abbrev_commit = opt->abbrev_commit ? opt->abbrev : GIT_SHA1_HEXSZ;
const char *extra_headers = opt->extra_headers;
struct pretty_print_context ctx = {0};
 

[PATCH 27/53] builtin/unpack-objects: convert to struct object_id

[brian m. carlson]

Convert struct delta_info and struct object_info, as well as the various
functions, to use struct object_id.  Convert several hard-coded 20
values to GIT_SHA1_RAWSZ.  Among the functions converted is a caller of
lookup_blob, which we will convert shortly.

Signed-off-by: brian m. carlson 
---
 builtin/unpack-objects.c | 64 
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 4532aa083..3dc5e5691 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -127,7 +127,7 @@ static void *get_data(unsigned long size)
 }
 
 struct delta_info {
-   unsigned char base_sha1[20];
+   struct object_id base_oid;
unsigned nr;
off_t base_offset;
unsigned long size;
@@ -137,13 +137,13 @@ struct delta_info {
 
 static struct delta_info *delta_list;
 
-static void add_delta_to_list(unsigned nr, unsigned const char *base_sha1,
+static void add_delta_to_list(unsigned nr, const struct object_id *base_oid,
  off_t base_offset,
  void *delta, unsigned long size)
 {
struct delta_info *info = xmalloc(sizeof(*info));
 
-   hashcpy(info->base_sha1, base_sha1);
+   oidcpy(>base_oid, base_oid);
info->base_offset = base_offset;
info->size = size;
info->delta = delta;
@@ -154,7 +154,7 @@ static void add_delta_to_list(unsigned nr, unsigned const 
char *base_sha1,
 
 struct obj_info {
off_t offset;
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *obj;
 };
 
@@ -170,9 +170,9 @@ static unsigned nr_objects;
  */
 static void write_cached_object(struct object *obj, struct obj_buffer *obj_buf)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
 
-   if (write_sha1_file(obj_buf->buffer, obj_buf->size, 
typename(obj->type), sha1) < 0)
+   if (write_sha1_file(obj_buf->buffer, obj_buf->size, 
typename(obj->type), oid.hash) < 0)
die("failed to write object %s", oid_to_hex(>oid));
obj->flags |= FLAG_WRITTEN;
 }
@@ -237,19 +237,19 @@ static void write_object(unsigned nr, enum object_type 
type,
 void *buf, unsigned long size)
 {
if (!strict) {
-   if (write_sha1_file(buf, size, typename(type), 
obj_list[nr].sha1) < 0)
+   if (write_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash) < 0)
die("failed to write object");
added_object(nr, type, buf, size);
free(buf);
obj_list[nr].obj = NULL;
} else if (type == OBJ_BLOB) {
struct blob *blob;
-   if (write_sha1_file(buf, size, typename(type), 
obj_list[nr].sha1) < 0)
+   if (write_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash) < 0)
die("failed to write object");
added_object(nr, type, buf, size);
free(buf);
 
-   blob = lookup_blob(obj_list[nr].sha1);
+   blob = lookup_blob(obj_list[nr].oid.hash);
if (blob)
blob->object.flags |= FLAG_WRITTEN;
else
@@ -258,9 +258,9 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), obj_list[nr].sha1);
+   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
added_object(nr, type, buf, size);
-   obj = parse_object_buffer(obj_list[nr].sha1, type, size, buf, 
);
+   obj = parse_object_buffer(obj_list[nr].oid.hash, type, size, 
buf, );
if (!obj)
die("invalid %s", typename(type));
add_object_buffer(obj, buf, size);
@@ -296,7 +296,7 @@ static void added_object(unsigned nr, enum object_type type,
struct delta_info *info;
 
while ((info = *p) != NULL) {
-   if (!hashcmp(info->base_sha1, obj_list[nr].sha1) ||
+   if (!oidcmp(>base_oid, _list[nr].oid) ||
info->base_offset == obj_list[nr].offset) {
*p = info->next;
p = _list;
@@ -320,12 +320,12 @@ static void unpack_non_delta_entry(enum object_type type, 
unsigned long size,
free(buf);
 }
 
-static int resolve_against_held(unsigned nr, const unsigned char *base,
+static int resolve_against_held(unsigned nr, const struct object_id *base,
void *delta_data, unsigned long delta_size)
 {
struct object *obj;
struct obj_buffer *obj_buffer;
-   obj = lookup_object(base);
+   obj = lookup_object(base->hash);
if (!obj)
return 0;
obj_buffer = 

[PATCH 31/53] builtin/reflog: convert tree_is_complete to take struct object_id

[brian m. carlson]

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

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 4831116ea..7866a0341 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -55,14 +55,14 @@ struct collect_reflog_cb {
 #define STUDYING   (1u<<11)
 #define REACHABLE  (1u<<12)
 
-static int tree_is_complete(const unsigned char *sha1)
+static int tree_is_complete(const struct object_id *oid)
 {
struct tree_desc desc;
struct name_entry entry;
int complete;
struct tree *tree;
 
-   tree = lookup_tree(sha1);
+   tree = lookup_tree(oid->hash);
if (!tree)
return 0;
if (tree->object.flags & SEEN)
@@ -73,7 +73,7 @@ static int tree_is_complete(const unsigned char *sha1)
if (!tree->buffer) {
enum object_type type;
unsigned long size;
-   void *data = read_sha1_file(sha1, , );
+   void *data = read_sha1_file(oid->hash, , );
if (!data) {
tree->object.flags |= INCOMPLETE;
return 0;
@@ -85,7 +85,7 @@ static int tree_is_complete(const unsigned char *sha1)
complete = 1;
while (tree_entry(, )) {
if (!has_sha1_file(entry.oid->hash) ||
-   (S_ISDIR(entry.mode) && 
!tree_is_complete(entry.oid->hash))) {
+   (S_ISDIR(entry.mode) && !tree_is_complete(entry.oid))) {
tree->object.flags |= INCOMPLETE;
complete = 0;
}
@@ -152,7 +152,7 @@ static int commit_is_complete(struct commit *commit)
for (i = 0; i < found.nr; i++) {
struct commit *c =
(struct commit *)found.objects[i].item;
-   if (!tree_is_complete(c->tree->object.oid.hash)) {
+   if (!tree_is_complete(>tree->object.oid)) {
is_incomplete = 1;
c->object.flags |= INCOMPLETE;
}

[PATCH 30/53] tree: convert read_tree_1 to use struct object_id internally

[brian m. carlson]

Signed-off-by: brian m. carlson 
---
 tree.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tree.c b/tree.c
index 33fa7ee71..21fd80b80 100644
--- a/tree.c
+++ b/tree.c
@@ -58,7 +58,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,
 {
struct tree_desc desc;
struct name_entry entry;
-   unsigned char sha1[20];
+   struct object_id oid;
int len, oldlen = base->len;
enum interesting retval = entry_not_interesting;
 
@@ -87,7 +87,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,
}
 
if (S_ISDIR(entry.mode))
-   hashcpy(sha1, entry.oid->hash);
+   oidcpy(, entry.oid);
else if (S_ISGITLINK(entry.mode)) {
struct commit *commit;
 
@@ -102,7 +102,7 @@ static int read_tree_1(struct tree *tree, struct strbuf 
*base,
oid_to_hex(entry.oid),
base->buf, entry.path);
 
-   hashcpy(sha1, commit->tree->object.oid.hash);
+   oidcpy(, >tree->object.oid);
}
else
continue;
@@ -110,7 +110,7 @@ static int read_tree_1(struct tree *tree, struct strbuf 
*base,
len = tree_entry_len();
strbuf_add(base, entry.path, len);
strbuf_addch(base, '/');
-   retval = read_tree_1(lookup_tree(sha1),
+   retval = read_tree_1(lookup_tree(oid.hash),
 base, stage, pathspec,
 fn, context);
strbuf_setlen(base, oldlen);

[PATCH 22/53] sequencer: convert some functions to struct object_id

[brian m. carlson]

Convert update_squash_messages and is_index_unchanged to struct
object_id.  These are callers of lookup_commit and
lookup_commit_reference, which we want to convert.

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

diff --git a/sequencer.c b/sequencer.c
index e13a25b91..0562d7b9a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -482,13 +482,13 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
 
 static int is_index_unchanged(void)
 {
-   unsigned char head_sha1[20];
+   struct object_id head_oid;
struct commit *head_commit;
 
-   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, 
NULL))
return error(_("could not resolve HEAD commit\n"));
 
-   head_commit = lookup_commit(head_sha1);
+   head_commit = lookup_commit(head_oid.hash);
 
/*
 * If head_commit is NULL, check_commit, called from
@@ -835,13 +835,13 @@ static int update_squash_messages(enum todo_command 
command,
strbuf_splice(, 0, eol - buf.buf, header.buf, header.len);
strbuf_release();
} else {
-   unsigned char head[20];
+   struct object_id head;
struct commit *head_commit;
const char *head_message, *body;
 
-   if (get_sha1("HEAD", head))
+   if (get_oid("HEAD", ))
return error(_("need a HEAD to fixup"));
-   if (!(head_commit = lookup_commit_reference(head)))
+   if (!(head_commit = lookup_commit_reference(head.hash)))
return error(_("could not read HEAD"));
if (!(head_message = get_commit_buffer(head_commit, NULL)))
return error(_("could not read HEAD's commit message"));

[PATCH 07/53] branch: convert to struct object_id

[brian m. carlson]

This change is required to convert lookup_commit_reference later.

Signed-off-by: brian m. carlson 
---
 branch.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index ad5a2299b..1758c9708 100644
--- a/branch.c
+++ b/branch.c
@@ -191,9 +191,9 @@ int validate_new_branchname(const char *name, struct strbuf 
*ref,
 
if (!attr_only) {
const char *head;
-   unsigned char sha1[20];
+   struct object_id oid;
 
-   head = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
+   head = resolve_ref_unsafe("HEAD", 0, oid.hash, NULL);
if (!is_bare_repository() && head && !strcmp(head, ref->buf))
die(_("Cannot force update the current branch."));
}
@@ -233,7 +233,7 @@ void create_branch(const char *name, const char *start_name,
   int quiet, enum branch_track track)
 {
struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id oid;
char *real_ref;
struct strbuf ref = STRBUF_INIT;
int forcing = 0;
@@ -253,7 +253,7 @@ void create_branch(const char *name, const char *start_name,
}
 
real_ref = NULL;
-   if (get_sha1(start_name, sha1)) {
+   if (get_oid(start_name, )) {
if (explicit_tracking) {
if (advice_set_upstream_failure) {
error(_(upstream_missing), start_name);
@@ -265,7 +265,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), sha1, _ref)) {
+   switch (dwim_ref(start_name, strlen(start_name), oid.hash, _ref)) {
case 0:
/* Not branching from any existing branch */
if (explicit_tracking)
@@ -286,9 +286,9 @@ void create_branch(const char *name, const char *start_name,
break;
}
 
-   if ((commit = lookup_commit_reference(sha1)) == NULL)
+   if ((commit = lookup_commit_reference(oid.hash)) == NULL)
die(_("Not a valid branch point: '%s'."), start_name);
-   hashcpy(sha1, commit->object.oid.hash);
+   oidcpy(, >object.oid);
 
if (reflog)
log_all_ref_updates = LOG_REFS_NORMAL;
@@ -306,7 +306,7 @@ void create_branch(const char *name, const char *start_name,
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_update(transaction, ref.buf,
-  sha1, forcing ? NULL : null_sha1,
+  oid.hash, forcing ? NULL : null_sha1,
   0, msg, ) ||
ref_transaction_commit(transaction, ))
die("%s", err.buf);

[PATCH 20/53] revision: convert prepare_show_merge to struct object_id

[brian m. carlson]

This is a caller of lookup_commit_or_die, which we will convert later
on.

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

diff --git a/revision.c b/revision.c
index 2b56c3baf..945367034 100644
--- a/revision.c
+++ b/revision.c
@@ -1389,16 +1389,16 @@ static void prepare_show_merge(struct rev_info *revs)
 {
struct commit_list *bases;
struct commit *head, *other;
-   unsigned char sha1[20];
+   struct object_id oid;
const char **prune = NULL;
int i, prune_num = 1; /* counting terminating NULL */
 
-   if (get_sha1("HEAD", sha1))
+   if (get_oid("HEAD", ))
die("--merge without HEAD?");
-   head = lookup_commit_or_die(sha1, "HEAD");
-   if (get_sha1("MERGE_HEAD", sha1))
+   head = lookup_commit_or_die(oid.hash, "HEAD");
+   if (get_oid("MERGE_HEAD", ))
die("--merge without MERGE_HEAD?");
-   other = lookup_commit_or_die(sha1, "MERGE_HEAD");
+   other = lookup_commit_or_die(oid.hash, "MERGE_HEAD");
add_pending_object(revs, >object, "HEAD");
add_pending_object(revs, >object, "MERGE_HEAD");
bases = get_merge_bases(head, other);

[PATCH 12/53] submodule: convert merge_submodule to use struct object_id

[brian m. carlson]

This is a caller of lookup_commit_reference, which we will convert
later.

Signed-off-by: brian m. carlson 
---
 merge-recursive.c |  8 
 submodule.c   | 24 
 submodule.h   |  8 
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 9d6fd577e..1315a45b9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -994,11 +994,11 @@ static int merge_file_1(struct merge_options *o,
return ret;
result->clean = (merge_status == 0);
} else if (S_ISGITLINK(a->mode)) {
-   result->clean = merge_submodule(result->oid.hash,
+   result->clean = merge_submodule(>oid,
   one->path,
-  one->oid.hash,
-  a->oid.hash,
-  b->oid.hash,
+  >oid,
+  >oid,
+  >oid,
   !o->call_depth);
} else if (S_ISLNK(a->mode)) {
oidcpy(>oid, >oid);
diff --git a/submodule.c b/submodule.c
index 5615d7392..fb782c412 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1567,9 +1567,9 @@ static void print_commit(struct commit *commit)
 #define MERGE_WARNING(path, msg) \
warning("Failed to merge submodule %s (%s)", path, msg);
 
-int merge_submodule(unsigned char result[20], const char *path,
-   const unsigned char base[20], const unsigned char a[20],
-   const unsigned char b[20], int search)
+int merge_submodule(struct object_id *result, const char *path,
+   const struct object_id *base, const struct object_id *a,
+   const struct object_id *b, int search)
 {
struct commit *commit_base, *commit_a, *commit_b;
int parent_count;
@@ -1578,14 +1578,14 @@ int merge_submodule(unsigned char result[20], const 
char *path,
int i;
 
/* store a in result in case we fail */
-   hashcpy(result, a);
+   oidcpy(result, a);
 
/* we can not handle deletion conflicts */
-   if (is_null_sha1(base))
+   if (is_null_oid(base))
return 0;
-   if (is_null_sha1(a))
+   if (is_null_oid(a))
return 0;
-   if (is_null_sha1(b))
+   if (is_null_oid(b))
return 0;
 
if (add_submodule_odb(path)) {
@@ -1593,9 +1593,9 @@ int merge_submodule(unsigned char result[20], const char 
*path,
return 0;
}
 
-   if (!(commit_base = lookup_commit_reference(base)) ||
-   !(commit_a = lookup_commit_reference(a)) ||
-   !(commit_b = lookup_commit_reference(b))) {
+   if (!(commit_base = lookup_commit_reference(base->hash)) ||
+   !(commit_a = lookup_commit_reference(a->hash)) ||
+   !(commit_b = lookup_commit_reference(b->hash))) {
MERGE_WARNING(path, "commits not present");
return 0;
}
@@ -1609,11 +1609,11 @@ int merge_submodule(unsigned char result[20], const 
char *path,
 
/* Case #1: a is contained in b or vice versa */
if (in_merge_bases(commit_a, commit_b)) {
-   hashcpy(result, b);
+   oidcpy(result, b);
return 1;
}
if (in_merge_bases(commit_b, commit_a)) {
-   hashcpy(result, a);
+   oidcpy(result, a);
return 1;
}
 
diff --git a/submodule.h b/submodule.h
index 1277480ad..89c2ed219 100644
--- a/submodule.h
+++ b/submodule.h
@@ -84,10 +84,10 @@ extern int submodule_uses_gitfile(const char *path);
 #define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<1)
 #define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<2)
 extern int bad_to_remove_submodule(const char *path, unsigned flags);
-extern int merge_submodule(unsigned char result[20], const char *path,
-  const unsigned char base[20],
-  const unsigned char a[20],
-  const unsigned char b[20], int search);
+extern int merge_submodule(struct object_id *result, const char *path,
+  const struct object_id *base,
+  const struct object_id *a,
+  const struct object_id *b, int search);
 extern int find_unpushed_submodules(struct oid_array *commits,
const char *remotes_name,
struct string_list *needs_pushing);

[PATCH 16/53] builtin/verify-commit: convert to struct object_id

[brian m. carlson]

This is a prerequisite to convert to lookup_commit, which we will
convert later.

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

diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index 38bedf8f9..a5db1c427 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -18,14 +18,14 @@ static const char * const verify_commit_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned 
long size, unsigned flags)
+static int run_gpg_verify(const struct object_id *oid, const char *buf, 
unsigned long size, unsigned flags)
 {
struct signature_check signature_check;
int ret;
 
memset(_check, 0, sizeof(signature_check));
 
-   ret = check_commit_signature(lookup_commit(sha1), _check);
+   ret = check_commit_signature(lookup_commit(oid->hash), 
_check);
print_signature_buffer(_check, flags);
 
signature_check_clear(_check);
@@ -35,22 +35,22 @@ static int run_gpg_verify(const unsigned char *sha1, const 
char *buf, unsigned l
 static int verify_commit(const char *name, unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
+   struct object_id oid;
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
+   if (get_oid(name, ))
return error("commit '%s' not found.", name);
 
-   buf = read_sha1_file(sha1, , );
+   buf = read_sha1_file(oid.hash, , );
if (!buf)
return error("%s: unable to read file.", name);
if (type != OBJ_COMMIT)
return error("%s: cannot verify a non-commit object of type 
%s.",
name, typename(type));
 
-   ret = run_gpg_verify(sha1, buf, size, flags);
+   ret = run_gpg_verify(, buf, size, flags);
 
free(buf);
return ret;

[PATCH 26/53] pack: convert struct pack_idx_entry to struct object_id

[brian m. carlson]

Convert struct pack_idx_entry to use struct object_id by changing the
definition and applying the following semantic patch, plus the standard
object_id transforms:

@@
struct pack_idx_entry E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct pack_idx_entry *E1;
@@
- E1->sha1
+ E1->oid.hash

Signed-off-by: brian m. carlson 
---
 builtin/index-pack.c   | 33 +++---
 builtin/pack-objects.c | 63 +-
 bulk-checkin.c |  4 ++--
 fast-import.c  | 30 
 pack-bitmap-write.c|  8 ---
 pack-objects.c |  8 ---
 pack-write.c   | 10 
 pack.h |  2 +-
 8 files changed, 89 insertions(+), 69 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4ff567db4..fef0025e4 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -747,13 +747,13 @@ static int compare_objects(const unsigned char *buf, 
unsigned long size,
ssize_t len = read_istream(data->st, data->buf, size);
if (len == 0)
die(_("SHA1 COLLISION FOUND WITH %s !"),
-   sha1_to_hex(data->entry->idx.sha1));
+   oid_to_hex(>entry->idx.oid));
if (len < 0)
die(_("unable to read %s"),
-   sha1_to_hex(data->entry->idx.sha1));
+   oid_to_hex(>entry->idx.oid));
if (memcmp(buf, data->buf, len))
die(_("SHA1 COLLISION FOUND WITH %s !"),
-   sha1_to_hex(data->entry->idx.sha1));
+   oid_to_hex(>entry->idx.oid));
size -= len;
buf += len;
}
@@ -771,12 +771,12 @@ static int check_collison(struct object_entry *entry)
 
memset(, 0, sizeof(data));
data.entry = entry;
-   data.st = open_istream(entry->idx.sha1, , , NULL);
+   data.st = open_istream(entry->idx.oid.hash, , , NULL);
if (!data.st)
return -1;
if (size != entry->size || type != entry->type)
die(_("SHA1 COLLISION FOUND WITH %s !"),
-   sha1_to_hex(entry->idx.sha1));
+   oid_to_hex(>idx.oid));
unpack_data(entry, compare_objects, );
close_istream(data.st);
free(data.buf);
@@ -957,9 +957,10 @@ static void resolve_delta(struct object_entry *delta_obj,
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type), delta_obj->idx.sha1);
+  typename(delta_obj->real_type),
+  delta_obj->idx.oid.hash);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
-   delta_obj->idx.sha1);
+   delta_obj->idx.oid.hash);
counter_lock();
nr_resolved_deltas++;
counter_unlock();
@@ -989,7 +990,7 @@ static struct base_data *find_unresolved_deltas_1(struct 
base_data *base,
  struct base_data *prev_base)
 {
if (base->ref_last == -1 && base->ofs_last == -1) {
-   find_ref_delta_children(base->obj->idx.sha1,
+   find_ref_delta_children(base->obj->idx.oid.hash,
>ref_first, >ref_last,
OBJ_REF_DELTA);
 
@@ -1130,7 +1131,8 @@ static void parse_pack_objects(unsigned char *sha1)
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = [i];
void *data = unpack_raw_entry(obj, _delta->offset,
- ref_delta_sha1, obj->idx.sha1);
+ ref_delta_sha1,
+ obj->idx.oid.hash);
obj->real_type = obj->type;
if (obj->type == OBJ_OFS_DELTA) {
nr_ofs_deltas++;
@@ -1146,7 +1148,8 @@ static void parse_pack_objects(unsigned char *sha1)
obj->real_type = OBJ_BAD;
nr_delays++;
} else
-   sha1_object(data, NULL, obj->size, obj->type, 
obj->idx.sha1);
+   sha1_object(data, NULL, obj->size, obj->type,
+   obj->idx.oid.hash);
free(data);
display_progress(progress, i+1);
}
@@ -1172,7 +1175,8 @@ static void parse_pack_objects(unsigned char *sha1)
if (obj->real_type != OBJ_BAD)
continue;
obj->real_type = obj->type;
-   sha1_object(NULL, obj, obj->size, obj->type, obj->idx.sha1);
+   sha1_object(NULL, obj, obj->size, obj->type,
+   

[PATCH 06/53] bundle: convert to struct object_id

[brian m. carlson]

Convert the bundle code, plus the sole external user of struct
ref_list_entry, to use struct object_id.  Include cache.h from within
bundle.h to provide the definition.  Convert some of the hash parsing
code to use parse_oid_hex to avoid needing to hard-code constant values.

Signed-off-by: brian m. carlson 
---
 bundle.c| 33 +
 bundle.h|  4 +++-
 transport.c |  2 +-
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/bundle.c b/bundle.c
index bbf4efa0a..6e181bb3d 100644
--- a/bundle.c
+++ b/bundle.c
@@ -12,11 +12,11 @@
 
 static const char bundle_signature[] = "# v2 git bundle\n";
 
-static void add_to_ref_list(const unsigned char *sha1, const char *name,
+static void add_to_ref_list(const struct object_id *oid, const char *name,
struct ref_list *list)
 {
ALLOC_GROW(list->list, list->nr + 1, list->alloc);
-   hashcpy(list->list[list->nr].sha1, sha1);
+   oidcpy(>list[list->nr].oid, oid);
list->list[list->nr].name = xstrdup(name);
list->nr++;
 }
@@ -40,8 +40,9 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
/* The bundle header ends with an empty line */
while (!strbuf_getwholeline_fd(, fd, '\n') &&
   buf.len && buf.buf[0] != '\n') {
-   unsigned char sha1[20];
+   struct object_id oid;
int is_prereq = 0;
+   const char *p;
 
if (*buf.buf == '-') {
is_prereq = 1;
@@ -54,9 +55,9 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
 * Prerequisites have object name that is optionally
 * followed by SP and subject line.
 */
-   if (get_sha1_hex(buf.buf, sha1) ||
-   (buf.len > 40 && !isspace(buf.buf[40])) ||
-   (!is_prereq && buf.len <= 40)) {
+   if (parse_oid_hex(buf.buf, , ) ||
+   (*p && !isspace(*p)) ||
+   (!is_prereq && !*p)) {
if (report_path)
error(_("unrecognized header: %s%s (%d)"),
  (is_prereq ? "-" : ""), buf.buf, 
(int)buf.len);
@@ -64,9 +65,9 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
break;
} else {
if (is_prereq)
-   add_to_ref_list(sha1, "", 
>prerequisites);
+   add_to_ref_list(, "", 
>prerequisites);
else
-   add_to_ref_list(sha1, buf.buf + 41, 
>references);
+   add_to_ref_list(, p + 1, 
>references);
}
}
 
@@ -115,7 +116,7 @@ static int list_refs(struct ref_list *r, int argc, const 
char **argv)
if (j == argc)
continue;
}
-   printf("%s %s\n", sha1_to_hex(r->list[i].sha1),
+   printf("%s %s\n", oid_to_hex(>list[i].oid),
r->list[i].name);
}
return 0;
@@ -141,7 +142,7 @@ int verify_bundle(struct bundle_header *header, int verbose)
init_revisions(, NULL);
for (i = 0; i < p->nr; i++) {
struct ref_list_entry *e = p->list + i;
-   struct object *o = parse_object(e->sha1);
+   struct object *o = parse_object(e->oid.hash);
if (o) {
o->flags |= PREREQ_MARK;
add_pending_object(, o, e->name);
@@ -149,7 +150,7 @@ int verify_bundle(struct bundle_header *header, int verbose)
}
if (++ret == 1)
error("%s", message);
-   error("%s %s", sha1_to_hex(e->sha1), e->name);
+   error("%s %s", oid_to_hex(>oid), e->name);
}
if (revs.pending.nr != p->nr)
return ret;
@@ -285,16 +286,16 @@ static int compute_and_write_prerequisites(int bundle_fd,
return -1;
rls_fout = xfdopen(rls.out, "r");
while (strbuf_getwholeline(, rls_fout, '\n') != EOF) {
-   unsigned char sha1[20];
+   struct object_id oid;
if (buf.len > 0 && buf.buf[0] == '-') {
write_or_die(bundle_fd, buf.buf, buf.len);
-   if (!get_sha1_hex(buf.buf + 1, sha1)) {
-   struct object *object = 
parse_object_or_die(sha1, buf.buf);
+   if (!get_oid_hex(buf.buf + 1, )) {
+   struct object *object = 
parse_object_or_die(oid.hash, buf.buf);
object->flags |= UNINTERESTING;
add_pending_object(revs, object, buf.buf);
}
-   } else if 

[PATCH 18/53] http-push: convert some static functions to struct object_id

[brian m. carlson]

Among the functions converted is a caller of lookup_commit_or_die, which
we will convert later on.

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

diff --git a/http-push.c b/http-push.c
index f0e3108f7..f3dd0a560 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1536,7 +1536,7 @@ static int remote_exists(const char *path)
return ret;
 }
 
-static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
+static void fetch_symref(const char *path, char **symref, struct object_id 
*oid)
 {
char *url = xstrfmt("%s%s", repo->url, path);
struct strbuf buffer = STRBUF_INIT;
@@ -1549,7 +1549,7 @@ static void fetch_symref(const char *path, char **symref, 
unsigned char *sha1)
 
free(*symref);
*symref = NULL;
-   hashclr(sha1);
+   oidclr(oid);
 
if (buffer.len == 0)
return;
@@ -1561,15 +1561,15 @@ static void fetch_symref(const char *path, char 
**symref, unsigned char *sha1)
if (skip_prefix(buffer.buf, "ref: ", )) {
*symref = xmemdupz(name, buffer.len - (name - buffer.buf));
} else {
-   get_sha1_hex(buffer.buf, sha1);
+   get_oid_hex(buffer.buf, oid);
}
 
strbuf_release();
 }
 
-static int verify_merge_base(unsigned char *head_sha1, struct ref *remote)
+static int verify_merge_base(struct object_id *head_oid, struct ref *remote)
 {
-   struct commit *head = lookup_commit_or_die(head_sha1, "HEAD");
+   struct commit *head = lookup_commit_or_die(head_oid->hash, "HEAD");
struct commit *branch = lookup_commit_or_die(remote->old_oid.hash, 
remote->name);
 
return in_merge_bases(branch, head);
@@ -1579,7 +1579,7 @@ static int delete_remote_branch(const char *pattern, int 
force)
 {
struct ref *refs = remote_refs;
struct ref *remote_ref = NULL;
-   unsigned char head_sha1[20];
+   struct object_id head_oid;
char *symref = NULL;
int match;
int patlen = strlen(pattern);
@@ -1610,7 +1610,7 @@ static int delete_remote_branch(const char *pattern, int 
force)
 * Remote HEAD must be a symref (not exactly foolproof; a remote
 * symlink to a symref will look like a symref)
 */
-   fetch_symref("HEAD", , head_sha1);
+   fetch_symref("HEAD", , _oid);
if (!symref)
return error("Remote HEAD is not a symref");
 
@@ -1619,7 +1619,7 @@ static int delete_remote_branch(const char *pattern, int 
force)
if (!strcmp(remote_ref->name, symref))
return error("Remote branch %s is the current HEAD",
 remote_ref->name);
-   fetch_symref(symref, , head_sha1);
+   fetch_symref(symref, , _oid);
}
 
/* Run extra sanity checks if delete is not forced */
@@ -1627,10 +1627,10 @@ static int delete_remote_branch(const char *pattern, 
int force)
/* Remote HEAD must resolve to a known object */
if (symref)
return error("Remote HEAD symrefs too deep");
-   if (is_null_sha1(head_sha1))
+   if (is_null_oid(_oid))
return error("Unable to resolve remote HEAD");
-   if (!has_sha1_file(head_sha1))
-   return error("Remote HEAD resolves to object %s\nwhich 
does not exist locally, perhaps you need to fetch?", sha1_to_hex(head_sha1));
+   if (!has_object_file(_oid))
+   return error("Remote HEAD resolves to object %s\nwhich 
does not exist locally, perhaps you need to fetch?", oid_to_hex(_oid));
 
/* Remote branch must resolve to a known object */
if (is_null_oid(_ref->old_oid))
@@ -1640,7 +1640,7 @@ static int delete_remote_branch(const char *pattern, int 
force)
return error("Remote branch %s resolves to object 
%s\nwhich does not exist locally, perhaps you need to fetch?", 
remote_ref->name, oid_to_hex(_ref->old_oid));
 
/* Remote branch must be an ancestor of remote HEAD */
-   if (!verify_merge_base(head_sha1, remote_ref)) {
+   if (!verify_merge_base(_oid, remote_ref)) {
return error("The branch '%s' is not an ancestor "
 "of your current HEAD.\n"
 "If you are sure you want to delete it,"

[PATCH 24/53] Convert remaining callers of lookup_commit_reference* to object_id

[brian m. carlson]

There are a small number of remaining callers of lookup_commit_reference
and lookup_commit_reference_gently that still need to be converted to
struct object_id.  Convert these.

Signed-off-by: brian m. carlson 
---
 notes-merge.c | 26 +-
 ref-filter.c  |  6 +++---
 sequencer.c   | 20 ++--
 sha1_name.c   | 12 ++--
 shallow.c |  6 +++---
 submodule.c   |  8 
 6 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 5998605ac..7ac9bb9a4 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -535,7 +535,7 @@ int notes_merge(struct notes_merge_options *o,
struct notes_tree *local_tree,
unsigned char *result_sha1)
 {
-   unsigned char local_sha1[20], remote_sha1[20];
+   struct object_id local_oid, remote_oid;
struct commit *local, *remote;
struct commit_list *bases = NULL;
const unsigned char *base_sha1, *base_tree_sha1;
@@ -549,46 +549,46 @@ int notes_merge(struct notes_merge_options *o,
   o->local_ref, o->remote_ref);
 
/* Dereference o->local_ref into local_sha1 */
-   if (read_ref_full(o->local_ref, 0, local_sha1, NULL))
+   if (read_ref_full(o->local_ref, 0, local_oid.hash, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
else if (!check_refname_format(o->local_ref, 0) &&
-   is_null_sha1(local_sha1))
+   is_null_oid(_oid))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
-   else if (!(local = lookup_commit_reference(local_sha1)))
+   else if (!(local = lookup_commit_reference(local_oid.hash)))
die("Could not parse local commit %s (%s)",
-   sha1_to_hex(local_sha1), o->local_ref);
-   trace_printf("\tlocal commit: %.7s\n", sha1_to_hex(local_sha1));
+   oid_to_hex(_oid), o->local_ref);
+   trace_printf("\tlocal commit: %.7s\n", oid_to_hex(_oid));
 
/* Dereference o->remote_ref into remote_sha1 */
-   if (get_sha1(o->remote_ref, remote_sha1)) {
+   if (get_oid(o->remote_ref, _oid)) {
/*
 * Failed to get remote_sha1. If o->remote_ref looks like an
 * unborn ref, perform the merge using an empty notes tree.
 */
if (!check_refname_format(o->remote_ref, 0)) {
-   hashclr(remote_sha1);
+   oidclr(_oid);
remote = NULL;
} else {
die("Failed to resolve remote notes ref '%s'",
o->remote_ref);
}
-   } else if (!(remote = lookup_commit_reference(remote_sha1))) {
+   } else if (!(remote = lookup_commit_reference(remote_oid.hash))) {
die("Could not parse remote commit %s (%s)",
-   sha1_to_hex(remote_sha1), o->remote_ref);
+   oid_to_hex(_oid), o->remote_ref);
}
-   trace_printf("\tremote commit: %.7s\n", sha1_to_hex(remote_sha1));
+   trace_printf("\tremote commit: %.7s\n", oid_to_hex(_oid));
 
if (!local && !remote)
die("Cannot merge empty notes ref (%s) into empty notes ref "
"(%s)", o->remote_ref, o->local_ref);
if (!local) {
/* result == remote commit */
-   hashcpy(result_sha1, remote_sha1);
+   hashcpy(result_sha1, remote_oid.hash);
goto found_result;
}
if (!remote) {
/* result == local commit */
-   hashcpy(result_sha1, local_sha1);
+   hashcpy(result_sha1, local_oid.hash);
goto found_result;
}
assert(local && remote);
diff --git a/ref-filter.c b/ref-filter.c
index 3a640448f..47cce0a18 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2090,7 +2090,7 @@ int parse_opt_ref_sorting(const struct option *opt, const 
char *arg, int unset)
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset)
 {
struct ref_filter *rf = opt->value;
-   unsigned char sha1[20];
+   struct object_id oid;
int no_merged = starts_with(opt->long_name, "no");
 
if (rf->merge) {
@@ -2105,10 +2105,10 @@ int parse_opt_merge_filter(const struct option *opt, 
const char *arg, int unset)
? REF_FILTER_MERGED_OMIT
: REF_FILTER_MERGED_INCLUDE;
 
-   if (get_sha1(arg, sha1))
+   if (get_oid(arg, ))
die(_("malformed object name %s"), arg);
 
-   rf->merge_commit = lookup_commit_reference_gently(sha1, 0);
+   rf->merge_commit = lookup_commit_reference_gently(oid.hash, 0);
if (!rf->merge_commit)
return opterror(opt, "must point to a commit", 0);
 
diff --git a/sequencer.c b/sequencer.c
index 0562d7b9a..cfa544ab9 100644
--- 

[PATCH 17/53] tag: convert parse_tag_buffer to struct object_id

[brian m. carlson]

Specify some constants in terms of GIT_SHA1_HEXSZ, and convert a
get_sha1_hex into parse_oid_hex to avoid needing to specify additional
constants.

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

diff --git a/tag.c b/tag.c
index 243d1fdbb..625f5cd71 100644
--- a/tag.c
+++ b/tag.c
@@ -116,7 +116,7 @@ static unsigned long parse_tag_date(const char *buf, const 
char *tail)
 
 int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
char type[20];
const char *bufptr = data;
const char *tail = bufptr + size;
@@ -126,11 +126,10 @@ int parse_tag_buffer(struct tag *item, const void *data, 
unsigned long size)
return 0;
item->object.parsed = 1;
 
-   if (size < 64)
+   if (size < GIT_SHA1_HEXSZ + 24)
return -1;
-   if (memcmp("object ", bufptr, 7) || get_sha1_hex(bufptr + 7, sha1) || 
bufptr[47] != '\n')
+   if (memcmp("object ", bufptr, 7) || parse_oid_hex(bufptr + 7, , 
) || *bufptr++ != '\n')
return -1;
-   bufptr += 48; /* "object " + sha1 + "\n" */
 
if (!starts_with(bufptr, "type "))
return -1;
@@ -143,13 +142,13 @@ int parse_tag_buffer(struct tag *item, const void *data, 
unsigned long size)
bufptr = nl + 1;
 
if (!strcmp(type, blob_type)) {
-   item->tagged = _blob(sha1)->object;
+   item->tagged = _blob(oid.hash)->object;
} else if (!strcmp(type, tree_type)) {
-   item->tagged = _tree(sha1)->object;
+   item->tagged = _tree(oid.hash)->object;
} else if (!strcmp(type, commit_type)) {
-   item->tagged = _commit(sha1)->object;
+   item->tagged = _commit(oid.hash)->object;
} else if (!strcmp(type, tag_type)) {
-   item->tagged = _tag(sha1)->object;
+   item->tagged = _tag(oid.hash)->object;
} else {
error("Unknown type %s", type);
item->tagged = NULL;

[PATCH 23/53] builtin/tag: convert to struct object_id

[brian m. carlson]

Parts of this module call lookup_commit_reference, which we want to
convert.  The module is small and mostly self-contained, so convert the
rest of it while we're at it.

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

diff --git a/builtin/tag.c b/builtin/tag.c
index 222404522..597c925e3 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -66,7 +66,7 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1, const void *cb_data);
+   const struct object_id *oid, const void 
*cb_data);
 
 static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
 const void *cb_data)
@@ -74,17 +74,17 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn,
const char **p;
struct strbuf ref = STRBUF_INIT;
int had_error = 0;
-   unsigned char sha1[20];
+   struct object_id oid;
 
for (p = argv; *p; p++) {
strbuf_reset();
strbuf_addf(, "refs/tags/%s", *p);
-   if (read_ref(ref.buf, sha1)) {
+   if (read_ref(ref.buf, oid.hash)) {
error(_("tag '%s' not found."), *p);
had_error = 1;
continue;
}
-   if (fn(*p, ref.buf, sha1, cb_data))
+   if (fn(*p, ref.buf, , cb_data))
had_error = 1;
}
strbuf_release();
@@ -92,16 +92,16 @@ 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 unsigned char *sha1, const void *cb_data)
+ const struct object_id *oid, const void *cb_data)
 {
-   if (delete_ref(NULL, ref, sha1, 0))
+   if (delete_ref(NULL, ref, oid->hash, 0))
return 1;
-   printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
+   printf(_("Deleted tag '%s' (was %s)\n"), name, 
find_unique_abbrev(oid->hash, DEFAULT_ABBREV));
return 0;
 }
 
 static int verify_tag(const char *name, const char *ref,
- const unsigned char *sha1, const void *cb_data)
+ const struct object_id *oid, const void *cb_data)
 {
int flags;
const char *fmt_pretty = cb_data;
@@ -110,11 +110,11 @@ static int verify_tag(const char *name, const char *ref,
if (fmt_pretty)
flags = GPG_VERIFY_OMIT_STATUS;
 
-   if (gpg_verify_tag(sha1, name, flags))
+   if (gpg_verify_tag(oid->hash, name, flags))
return -1;
 
if (fmt_pretty)
-   pretty_print_ref(name, sha1, fmt_pretty);
+   pretty_print_ref(name, oid->hash, fmt_pretty);
 
return 0;
 }
@@ -182,13 +182,13 @@ static int git_tag_config(const char *var, const char 
*value, void *cb)
return git_default_config(var, value, cb);
 }
 
-static void write_tag_body(int fd, const unsigned char *sha1)
+static void write_tag_body(int fd, const struct object_id *oid)
 {
unsigned long size;
enum object_type type;
char *buf, *sp;
 
-   buf = read_sha1_file(sha1, , );
+   buf = read_sha1_file(oid->hash, , );
if (!buf)
return;
/* skip header */
@@ -204,11 +204,11 @@ static void write_tag_body(int fd, const unsigned char 
*sha1)
free(buf);
 }
 
-static int build_tag_object(struct strbuf *buf, int sign, unsigned char 
*result)
+static int build_tag_object(struct strbuf *buf, int sign, struct object_id 
*result)
 {
if (sign && do_sign(buf) < 0)
return error(_("unable to sign the tag"));
-   if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
+   if (write_sha1_file(buf->buf, buf->len, tag_type, result->hash) < 0)
return error(_("unable to write tag file"));
return 0;
 }
@@ -223,15 +223,15 @@ struct create_tag_options {
} cleanup_mode;
 };
 
-static void create_tag(const unsigned char *object, const char *tag,
+static void create_tag(const struct object_id *object, const char *tag,
   struct strbuf *buf, struct create_tag_options *opt,
-  unsigned char *prev, unsigned char *result)
+  struct object_id *prev, struct object_id *result)
 {
enum object_type type;
struct strbuf header = STRBUF_INIT;
char *path = NULL;
 
-   type = sha1_object_info(object, NULL);
+   type = sha1_object_info(object->hash, NULL);
if (type <= OBJ_NONE)
die(_("bad object type."));
 
@@ -240,7 +240,7 @@ static void create_tag(const unsigned char 

[PATCH 10/53] fast-import: convert internal structs to struct object_id

[brian m. carlson]

Convert struct tree_entry_ms, struct branch, struct tag, and struct
hash_list to use struct object_id by changing the definition and
applying the following semantic patch, plus the standard object_id
transforms:

@@
struct tree_entry_ms E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct tree_entry_ms *E1;
@@
- E1->sha1
+ E1->oid.hash

@@
struct branch E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct branch *E1;
@@
- E1->sha1
+ E1->oid.hash

@@
struct tag E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct tag *E1;
@@
- E1->sha1
+ E1->oid.hash

@@
struct hash_list E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct hash_list *E1;
@@
- E1->sha1
+ E1->oid.hash

Signed-off-by: brian m. carlson 
---
 fast-import.c | 182 +++---
 1 file changed, 96 insertions(+), 86 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 1ea3a0860..052c59d82 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -226,7 +226,7 @@ struct tree_entry {
struct atom_str *name;
struct tree_entry_ms {
uint16_t mode;
-   unsigned char sha1[20];
+   struct object_id oid;
} versions[2];
 };
 
@@ -252,19 +252,19 @@ struct branch {
unsigned active : 1;
unsigned delete : 1;
unsigned pack_id : PACK_ID_BITS;
-   unsigned char sha1[20];
+   struct object_id oid;
 };
 
 struct tag {
struct tag *next_tag;
const char *name;
unsigned int pack_id;
-   unsigned char sha1[20];
+   struct object_id oid;
 };
 
 struct hash_list {
struct hash_list *next;
-   unsigned char sha1[20];
+   struct object_id oid;
 };
 
 typedef enum {
@@ -386,13 +386,15 @@ static void write_branch_report(FILE *rpt, struct branch 
*b)
fputs(" active", rpt);
if (b->branch_tree.tree)
fputs(" loaded", rpt);
-   if (is_null_sha1(b->branch_tree.versions[1].sha1))
+   if (is_null_oid(>branch_tree.versions[1].oid))
fputs(" dirty", rpt);
fputc('\n', rpt);
 
-   fprintf(rpt, "  tip commit  : %s\n", sha1_to_hex(b->sha1));
-   fprintf(rpt, "  old tree: %s\n", 
sha1_to_hex(b->branch_tree.versions[0].sha1));
-   fprintf(rpt, "  cur tree: %s\n", 
sha1_to_hex(b->branch_tree.versions[1].sha1));
+   fprintf(rpt, "  tip commit  : %s\n", oid_to_hex(>oid));
+   fprintf(rpt, "  old tree: %s\n",
+   oid_to_hex(>branch_tree.versions[0].oid));
+   fprintf(rpt, "  cur tree: %s\n",
+   oid_to_hex(>branch_tree.versions[1].oid));
fprintf(rpt, "  commit clock: %" PRIuMAX "\n", b->last_commit);
 
fputs("  last pack   : ", rpt);
@@ -470,7 +472,7 @@ static void write_crash_report(const char *err)
fputs("Annotated Tags\n", rpt);
fputs("--\n", rpt);
for (tg = first_tag; tg; tg = tg->next_tag) {
-   fputs(sha1_to_hex(tg->sha1), rpt);
+   fputs(oid_to_hex(>oid), rpt);
fputc(' ', rpt);
fputs(tg->name, rpt);
fputc('\n', rpt);
@@ -876,7 +878,7 @@ static struct tree_content *dup_tree_content(struct 
tree_content *s)
a = s->entries[i];
b = new_tree_entry();
memcpy(b, a, sizeof(*a));
-   if (a->tree && is_null_sha1(b->versions[1].sha1))
+   if (a->tree && is_null_oid(>versions[1].oid))
b->tree = dup_tree_content(a->tree);
else
b->tree = NULL;
@@ -1041,12 +1043,14 @@ static void end_packfile(void)
for (i = 0; i < branch_table_sz; i++) {
for (b = branch_table[i]; b; b = 
b->table_next_branch) {
if (b->pack_id == pack_id)
-   fprintf(pack_edges, " %s", 
sha1_to_hex(b->sha1));
+   fprintf(pack_edges, " %s",
+   oid_to_hex(>oid));
}
}
for (t = first_tag; t; t = t->next_tag) {
if (t->pack_id == pack_id)
-   fprintf(pack_edges, " %s", 
sha1_to_hex(t->sha1));
+   fprintf(pack_edges, " %s",
+   oid_to_hex(>oid));
}
fputc('\n', pack_edges);
fflush(pack_edges);
@@ -1385,7 +1389,7 @@ static const char *get_mode(const char *str, uint16_t 
*modep)
 
 static void load_tree(struct tree_entry *root)
 {
-   unsigned char *sha1 = root->versions[1].sha1;
+   unsigned char *sha1 = root->versions[1].oid.hash;
struct object_entry *myoe;
struct 

[PATCH 08/53] builtin/blame: convert static function to struct object_id

[brian m. carlson]

This function is a caller of lookup_commit_reference_gently, which we
will convert later.

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

diff --git a/builtin/blame.c b/builtin/blame.c
index 07506a3e4..7d644d092 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2461,7 +2461,7 @@ static const char *dwim_reverse_initial(struct scoreboard 
*sb)
 */
struct object *obj;
struct commit *head_commit;
-   unsigned char head_sha1[20];
+   struct object_id head_oid;
 
if (sb->revs->pending.nr != 1)
return NULL;
@@ -2473,9 +2473,9 @@ static const char *dwim_reverse_initial(struct scoreboard 
*sb)
return NULL;
 
/* Do we have HEAD? */
-   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, 
NULL))
return NULL;
-   head_commit = lookup_commit_reference_gently(head_sha1, 1);
+   head_commit = lookup_commit_reference_gently(head_oid.hash, 1);
if (!head_commit)
return NULL;
 

[PATCH 11/53] fast-import: convert to struct object_id

[brian m. carlson]

Convert the remaining parts of fast-import.c to use struct object_id.
Convert several instances of get_sha1_hex to parse_oid_hex to avoid
needing to specify constants.  Convert other hardcoded values to named
constants.  Finally, use the is_empty_tree_oid function instead of a
direct comparison against a fixed string.

Note that the odd computation with GIT_MAX_HEXSZ is due to the insertion
of a slash between every two hex digits in the path, plus one for the
terminating NUL.

Signed-off-by: brian m. carlson 
---
 fast-import.c | 327 +-
 1 file changed, 161 insertions(+), 166 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 052c59d82..6b0117e51 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -391,10 +391,8 @@ static void write_branch_report(FILE *rpt, struct branch 
*b)
fputc('\n', rpt);
 
fprintf(rpt, "  tip commit  : %s\n", oid_to_hex(>oid));
-   fprintf(rpt, "  old tree: %s\n",
-   oid_to_hex(>branch_tree.versions[0].oid));
-   fprintf(rpt, "  cur tree: %s\n",
-   oid_to_hex(>branch_tree.versions[1].oid));
+   fprintf(rpt, "  old tree: %s\n", 
oid_to_hex(>branch_tree.versions[0].oid));
+   fprintf(rpt, "  cur tree: %s\n", 
oid_to_hex(>branch_tree.versions[1].oid));
fprintf(rpt, "  commit clock: %" PRIuMAX "\n", b->last_commit);
 
fputs("  last pack   : ", rpt);
@@ -557,7 +555,7 @@ static void alloc_objects(unsigned int cnt)
alloc_count += cnt;
 }
 
-static struct object_entry *new_object(unsigned char *sha1)
+static struct object_entry *new_object(struct object_id *oid)
 {
struct object_entry *e;
 
@@ -565,32 +563,32 @@ static struct object_entry *new_object(unsigned char 
*sha1)
alloc_objects(object_entry_alloc);
 
e = blocks->next_free++;
-   hashcpy(e->idx.sha1, sha1);
+   hashcpy(e->idx.sha1, oid->hash);
return e;
 }
 
-static struct object_entry *find_object(unsigned char *sha1)
+static struct object_entry *find_object(struct object_id *oid)
 {
-   unsigned int h = sha1[0] << 8 | sha1[1];
+   unsigned int h = oid->hash[0] << 8 | oid->hash[1];
struct object_entry *e;
for (e = object_table[h]; e; e = e->next)
-   if (!hashcmp(sha1, e->idx.sha1))
+   if (!hashcmp(oid->hash, e->idx.sha1))
return e;
return NULL;
 }
 
-static struct object_entry *insert_object(unsigned char *sha1)
+static struct object_entry *insert_object(struct object_id *oid)
 {
-   unsigned int h = sha1[0] << 8 | sha1[1];
+   unsigned int h = oid->hash[0] << 8 | oid->hash[1];
struct object_entry *e = object_table[h];
 
while (e) {
-   if (!hashcmp(sha1, e->idx.sha1))
+   if (!hashcmp(oid->hash, e->idx.sha1))
return e;
e = e->next;
}
 
-   e = new_object(sha1);
+   e = new_object(oid);
e->next = object_table[h];
e->idx.offset = 0;
object_table[h] = e;
@@ -1007,17 +1005,17 @@ static void end_packfile(void)
clear_delta_base_cache();
if (object_count) {
struct packed_git *new_p;
-   unsigned char cur_pack_sha1[20];
+   struct object_id cur_pack_oid;
char *idx_name;
int i;
struct branch *b;
struct tag *t;
 
close_pack_windows(pack_data);
-   sha1close(pack_file, cur_pack_sha1, 0);
+   sha1close(pack_file, cur_pack_oid.hash, 0);
fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
pack_data->pack_name, object_count,
-   cur_pack_sha1, pack_size);
+   cur_pack_oid.hash, pack_size);
 
if (object_count <= unpack_limit) {
if (!loosen_small_pack(pack_data)) {
@@ -1083,13 +1081,13 @@ static int store_object(
enum object_type type,
struct strbuf *dat,
struct last_object *last,
-   unsigned char *sha1out,
+   struct object_id *oidout,
uintmax_t mark)
 {
void *out, *delta;
struct object_entry *e;
unsigned char hdr[96];
-   unsigned char sha1[20];
+   struct object_id oid;
unsigned long hdrlen, deltalen;
git_SHA_CTX c;
git_zstream s;
@@ -1099,17 +1097,17 @@ static int store_object(
git_SHA1_Init();
git_SHA1_Update(, hdr, hdrlen);
git_SHA1_Update(, dat->buf, dat->len);
-   git_SHA1_Final(sha1, );
-   if (sha1out)
-   hashcpy(sha1out, sha1);
+   git_SHA1_Final(oid.hash, );
+   if (oidout)
+   oidcpy(oidout, );
 
-   e = insert_object(sha1);
+   e = insert_object();
if (mark)
insert_mark(mark, e);
  

[PATCH 15/53] reflog_expire: convert to struct object_id

[brian m. carlson]

Adjust the callback functions to take struct object_id * instead of
unsigned char *, and modify related static functions accordingly.

Introduce a temporary object_id instance into files_reflog_expire and
copy the SHA-1 value passed in.  This is necessary because the sha1
parameter can come indirectly from get_sha1.  Without the temporary, it
would require much more refactoring to be able to convert this function.

Signed-off-by: brian m. carlson 
---
 builtin/reflog.c | 22 +++---
 refs.h   |  6 +++---
 refs/files-backend.c |  7 +--
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 747277577..d6718d326 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -186,13 +186,13 @@ static int commit_is_complete(struct commit *commit)
return !is_incomplete;
 }
 
-static int keep_entry(struct commit **it, unsigned char *sha1)
+static int keep_entry(struct commit **it, struct object_id *oid)
 {
struct commit *commit;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 1;
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid->hash, 1);
if (!commit)
return 0;
 
@@ -251,17 +251,17 @@ static void mark_reachable(struct expire_reflog_policy_cb 
*cb)
cb->mark_list = leftover;
 }
 
-static int unreachable(struct expire_reflog_policy_cb *cb, struct commit 
*commit, unsigned char *sha1)
+static int unreachable(struct expire_reflog_policy_cb *cb, struct commit 
*commit, struct object_id *oid)
 {
/*
 * We may or may not have the commit yet - if not, look it
 * up using the supplied sha1.
 */
if (!commit) {
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid->hash, 1);
 
/* Not a commit -- keep it */
if (!commit)
@@ -283,7 +283,7 @@ static int unreachable(struct expire_reflog_policy_cb *cb, 
struct commit *commit
 /*
  * Return true iff the specified reflog entry should be expired.
  */
-static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int should_expire_reflog_ent(struct object_id *ooid, struct object_id 
*noid,
const char *email, unsigned long timestamp, 
int tz,
const char *message, void *cb_data)
 {
@@ -295,13 +295,13 @@ static int should_expire_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
 
old = new = NULL;
if (cb->cmd.stalefix &&
-   (!keep_entry(, osha1) || !keep_entry(, nsha1)))
+   (!keep_entry(, ooid) || !keep_entry(, noid)))
return 1;
 
if (timestamp < cb->cmd.expire_unreachable) {
if (cb->unreachable_expire_kind == UE_ALWAYS)
return 1;
-   if (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))
+   if (unreachable(cb, old, ooid) || unreachable(cb, new, noid))
return 1;
}
 
@@ -326,7 +326,7 @@ static int push_tip_to_list(const char *refname, const 
struct object_id *oid,
 }
 
 static void reflog_expiry_prepare(const char *refname,
- const unsigned char *sha1,
+ const struct object_id *oid,
  void *cb_data)
 {
struct expire_reflog_policy_cb *cb = cb_data;
@@ -335,7 +335,7 @@ static void reflog_expiry_prepare(const char *refname,
cb->tip_commit = NULL;
cb->unreachable_expire_kind = UE_HEAD;
} else {
-   cb->tip_commit = lookup_commit_reference_gently(sha1, 1);
+   cb->tip_commit = lookup_commit_reference_gently(oid->hash, 1);
if (!cb->tip_commit)
cb->unreachable_expire_kind = UE_ALWAYS;
else
diff --git a/refs.h b/refs.h
index 49e97d7d5..bbe16740f 100644
--- a/refs.h
+++ b/refs.h
@@ -611,10 +611,10 @@ enum expire_reflog_flags {
  * unlocked again.
  */
 typedef void reflog_expiry_prepare_fn(const char *refname,
- const unsigned char *sha1,
+ const struct object_id *oid,
  void *cb_data);
-typedef int reflog_expiry_should_prune_fn(unsigned char *osha1,
- unsigned char *nsha1,
+typedef int reflog_expiry_should_prune_fn(struct object_id *ooid,
+ struct object_id *noid,
  const char *email,
  unsigned long timestamp, int tz,
   

[PATCH 09/53] builtin/rev-parse: convert to struct object_id

[brian m. carlson]

Some of the functions converted are callers of lookup_commit_reference.
However, the changes involved in converting the entire thing are not too
large, so we might as well convert it all.

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

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 051333091..272bb13a0 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -121,7 +121,7 @@ static void show_with_type(int type, const char *arg)
 }
 
 /* Output a revision, only if filter allows it */
-static void show_rev(int type, const unsigned char *sha1, const char *name)
+static void show_rev(int type, const struct object_id *oid, const char *name)
 {
if (!(filter & DO_REVS))
return;
@@ -129,10 +129,10 @@ static void show_rev(int type, const unsigned char *sha1, 
const char *name)
 
if ((symbolic || abbrev_ref) && name) {
if (symbolic == SHOW_SYMBOLIC_FULL || abbrev_ref) {
-   unsigned char discard[20];
+   struct object_id discard;
char *full;
 
-   switch (dwim_ref(name, strlen(name), discard, )) {
+   switch (dwim_ref(name, strlen(name), discard.hash, 
)) {
case 0:
/*
 * Not found -- not a ref.  We could
@@ -158,9 +158,9 @@ static void show_rev(int type, const unsigned char *sha1, 
const char *name)
}
}
else if (abbrev)
-   show_with_type(type, find_unique_abbrev(sha1, abbrev));
+   show_with_type(type, find_unique_abbrev(oid->hash, abbrev));
else
-   show_with_type(type, sha1_to_hex(sha1));
+   show_with_type(type, oid_to_hex(oid));
 }
 
 /* Output a flag, only if filter allows it. */
@@ -180,11 +180,11 @@ static int show_default(void)
const char *s = def;
 
if (s) {
-   unsigned char sha1[20];
+   struct object_id oid;
 
def = NULL;
-   if (!get_sha1(s, sha1)) {
-   show_rev(NORMAL, sha1, s);
+   if (!get_oid(s, )) {
+   show_rev(NORMAL, , s);
return 1;
}
}
@@ -195,19 +195,19 @@ static int show_reference(const char *refname, const 
struct object_id *oid, int
 {
if (ref_excluded(ref_excludes, refname))
return 0;
-   show_rev(NORMAL, oid->hash, refname);
+   show_rev(NORMAL, oid, refname);
return 0;
 }
 
 static int anti_reference(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
-   show_rev(REVERSED, oid->hash, refname);
+   show_rev(REVERSED, oid, refname);
return 0;
 }
 
 static int show_abbrev(const struct object_id *oid, void *cb_data)
 {
-   show_rev(NORMAL, oid->hash, NULL);
+   show_rev(NORMAL, oid, NULL);
return 0;
 }
 
@@ -242,8 +242,8 @@ static int show_file(const char *arg, int output_prefix)
 static int try_difference(const char *arg)
 {
char *dotdot;
-   unsigned char sha1[20];
-   unsigned char end[20];
+   struct object_id oid;
+   struct object_id end;
const char *next;
const char *this;
int symmetric;
@@ -273,18 +273,18 @@ static int try_difference(const char *arg)
return 0;
}
 
-   if (!get_sha1_committish(this, sha1) && !get_sha1_committish(next, 
end)) {
-   show_rev(NORMAL, end, next);
-   show_rev(symmetric ? NORMAL : REVERSED, sha1, this);
+   if (!get_sha1_committish(this, oid.hash) && !get_sha1_committish(next, 
end.hash)) {
+   show_rev(NORMAL, , next);
+   show_rev(symmetric ? NORMAL : REVERSED, , this);
if (symmetric) {
struct commit_list *exclude;
struct commit *a, *b;
-   a = lookup_commit_reference(sha1);
-   b = lookup_commit_reference(end);
+   a = lookup_commit_reference(oid.hash);
+   b = lookup_commit_reference(end.hash);
exclude = get_merge_bases(a, b);
while (exclude) {
struct commit *commit = pop_commit();
-   show_rev(REVERSED, commit->object.oid.hash, 
NULL);
+   show_rev(REVERSED, >object.oid, NULL);
}
}
*dotdot = '.';
@@ -297,7 +297,7 @@ static int try_difference(const char *arg)
 static int try_parent_shorthands(const char *arg)
 {
char *dotdot;
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
struct 

[PATCH 21/53] shallow: convert shallow registration functions to object_id

[brian m. carlson]

Convert register_shallow and unregister_shallow to take struct
object_id.  register_shallow is a caller of lookup_commit, which we will
convert later.  It doesn't make sense for the registration and
unregistration functions to have incompatible interfaces, so convert
them both.

Signed-off-by: brian m. carlson 
---
 builtin/pack-objects.c |  6 +++---
 builtin/receive-pack.c |  2 +-
 commit.c   |  4 ++--
 commit.h   |  4 ++--
 fetch-pack.c   |  4 ++--
 shallow.c  | 12 ++--
 upload-pack.c  |  8 
 7 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0fe35d1b5..477070806 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2777,10 +2777,10 @@ static void get_object_list(int ac, const char **av)
continue;
}
if (starts_with(line, "--shallow ")) {
-   unsigned char sha1[20];
-   if (get_sha1_hex(line + 10, sha1))
+   struct object_id oid;
+   if (get_oid_hex(line + 10, ))
die("not an SHA-1 '%s'", line + 10);
-   register_shallow(sha1);
+   register_shallow();
use_bitmap_index = 0;
continue;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 3cba3fd27..38e5164f6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -859,7 +859,7 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
 * not lose these new roots..
 */
for (i = 0; i < extra.nr; i++)
-   register_shallow(extra.oid[i].hash);
+   register_shallow([i]);
 
si->shallow_ref[cmd->index] = 0;
oid_array_clear();
diff --git a/commit.c b/commit.c
index 73c78c2b8..ec41ba5e0 100644
--- a/commit.c
+++ b/commit.c
@@ -216,9 +216,9 @@ int for_each_commit_graft(each_commit_graft_fn fn, void 
*cb_data)
return ret;
 }
 
-int unregister_shallow(const unsigned char *sha1)
+int unregister_shallow(const struct object_id *oid)
 {
-   int pos = commit_graft_pos(sha1);
+   int pos = commit_graft_pos(oid->hash);
if (pos < 0)
return -1;
if (pos + 1 < commit_graft_nr)
diff --git a/commit.h b/commit.h
index 7b1986d5c..884177b8f 100644
--- a/commit.h
+++ b/commit.h
@@ -263,8 +263,8 @@ extern struct commit_list 
*get_merge_bases_many_dirty(struct commit *one, int n,
 
 struct oid_array;
 struct ref;
-extern int register_shallow(const unsigned char *sha1);
-extern int unregister_shallow(const unsigned char *sha1);
+extern int register_shallow(const struct object_id *oid);
+extern int unregister_shallow(const struct object_id *oid);
 extern int for_each_commit_graft(each_commit_graft_fn, void *);
 extern int is_repository_shallow(void);
 extern struct commit_list *get_shallow_commits(struct object_array *heads,
diff --git a/fetch-pack.c b/fetch-pack.c
index 1e6b03b6b..f3aae85d5 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -415,7 +415,7 @@ static int find_common(struct fetch_pack_args *args,
if (skip_prefix(line, "shallow ", )) {
if (get_oid_hex(arg, ))
die(_("invalid shallow line: %s"), 
line);
-   register_shallow(oid.hash);
+   register_shallow();
continue;
}
if (skip_prefix(line, "unshallow ", )) {
@@ -426,7 +426,7 @@ static int find_common(struct fetch_pack_args *args,
/* make sure that it is parsed as shallow */
if (!parse_object(oid.hash))
die(_("error in object: %s"), line);
-   if (unregister_shallow(oid.hash))
+   if (unregister_shallow())
die(_("no shallow found: %s"), line);
continue;
}
diff --git a/shallow.c b/shallow.c
index 25b6db989..c520ae3ae 100644
--- a/shallow.c
+++ b/shallow.c
@@ -27,13 +27,13 @@ void set_alternate_shallow_file(const char *path, int 
override)
alternate_shallow_file = xstrdup_or_null(path);
 }
 
-int register_shallow(const unsigned char *sha1)
+int register_shallow(const struct object_id *oid)
 {
struct commit_graft *graft =
xmalloc(sizeof(struct commit_graft));
-   struct commit *commit = lookup_commit(sha1);
+   struct commit *commit = lookup_commit(oid->hash);
 
-   hashcpy(graft->oid.hash, sha1);
+   

[PATCH 19/53] notes-utils: convert internals to struct object_id

[brian m. carlson]

Convert the internals of create_notes_comit and commit_notes to use
struct object_id.

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

diff --git a/notes-utils.c b/notes-utils.c
index 24a33616a..36c1490aa 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -7,18 +7,18 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 const char *msg, size_t msg_len,
 unsigned char *result_sha1)
 {
-   unsigned char tree_sha1[20];
+   struct object_id tree_oid;
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_sha1))
+   if (write_notes_tree(t, tree_oid.hash))
die("Failed to write notes tree to database");
 
if (!parents) {
/* Deduce parent commit from t->ref */
-   unsigned char parent_sha1[20];
-   if (!read_ref(t->ref, parent_sha1)) {
-   struct commit *parent = lookup_commit(parent_sha1);
+   struct object_id parent_oid;
+   if (!read_ref(t->ref, parent_oid.hash)) {
+   struct commit *parent = lookup_commit(parent_oid.hash);
if (parse_commit(parent))
die("Failed to find/parse commit %s", t->ref);
commit_list_insert(parent, );
@@ -26,14 +26,14 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
/* else: t->ref points to nothing, assume root/orphan commit */
}
 
-   if (commit_tree(msg, msg_len, tree_sha1, parents, result_sha1, NULL, 
NULL))
+   if (commit_tree(msg, msg_len, tree_oid.hash, parents, result_sha1, 
NULL, NULL))
die("Failed to commit notes tree to database");
 }
 
 void commit_notes(struct notes_tree *t, const char *msg)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char commit_sha1[20];
+   struct object_id commit_oid;
 
if (!t)
t = _notes_tree;
@@ -46,9 +46,9 @@ void commit_notes(struct notes_tree *t, const char *msg)
strbuf_addstr(, msg);
strbuf_complete_line();
 
-   create_notes_commit(t, NULL, buf.buf, buf.len, commit_sha1);
+   create_notes_commit(t, NULL, buf.buf, buf.len, commit_oid.hash);
strbuf_insert(, 0, "notes: ", 7); /* commit message starts at index 
7 */
-   update_ref(buf.buf, t->update_ref, commit_sha1, NULL, 0,
+   update_ref(buf.buf, t->update_ref, commit_oid.hash, NULL, 0,
   UPDATE_REFS_DIE_ON_ERR);
 
strbuf_release();

[PATCH 14/53] parse-options-cb: convert to struct object_id

[brian m. carlson]

This is a caller of lookup_commit_reference, which we will soon convert.

Signed-off-by: brian m. carlson 
---
 parse-options-cb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 7419780a9..35a941fdd 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -80,14 +80,14 @@ int parse_opt_verbosity_cb(const struct option *opt, const 
char *arg,
 
 int parse_opt_commits(const struct option *opt, const char *arg, int unset)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
 
if (!arg)
return -1;
-   if (get_sha1(arg, sha1))
+   if (get_oid(arg, ))
return error("malformed object name %s", arg);
-   commit = lookup_commit_reference(sha1);
+   commit = lookup_commit_reference(oid.hash);
if (!commit)
return error("no such commit %s", arg);
commit_list_insert(commit, opt->value);

[PATCH 13/53] notes-cache: convert to struct object_id

[brian m. carlson]

Convert as many instances of unsigned char [20] as possible,  Update the
callers of notes_cache_get and notes_cache_put to use the new interface.
Among the functions updated are callers of
lookup_commit_reference_gently, which we will soon convert.

Signed-off-by: brian m. carlson 
---
 diff.c|  4 ++--
 notes-cache.c | 29 ++---
 notes-cache.h |  4 ++--
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/diff.c b/diff.c
index 11eef1c85..3bd23ae4c 100644
--- a/diff.c
+++ b/diff.c
@@ -5244,7 +5244,7 @@ size_t fill_textconv(struct userdiff_driver *driver,
 
if (driver->textconv_cache && df->oid_valid) {
*outbuf = notes_cache_get(driver->textconv_cache,
- df->oid.hash,
+ >oid,
  );
if (*outbuf)
return size;
@@ -5256,7 +5256,7 @@ size_t fill_textconv(struct userdiff_driver *driver,
 
if (driver->textconv_cache && df->oid_valid) {
/* ignore errors, as we might be in a readonly repository */
-   notes_cache_put(driver->textconv_cache, df->oid.hash, *outbuf,
+   notes_cache_put(driver->textconv_cache, >oid, *outbuf,
size);
/*
 * we could save up changes and flush them all at the end,
diff --git a/notes-cache.c b/notes-cache.c
index 5dfc5cbd0..1cdd4984a 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -5,16 +5,16 @@
 
 static int notes_cache_match_validity(const char *ref, const char *validity)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
struct pretty_print_context pretty_ctx;
struct strbuf msg = STRBUF_INIT;
int ret;
 
-   if (read_ref(ref, sha1) < 0)
+   if (read_ref(ref, oid.hash) < 0)
return 0;
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid.hash, 1);
if (!commit)
return 0;
 
@@ -46,8 +46,7 @@ void notes_cache_init(struct notes_cache *c, const char *name,
 
 int notes_cache_write(struct notes_cache *c)
 {
-   unsigned char tree_sha1[20];
-   unsigned char commit_sha1[20];
+   struct object_id tree_oid, commit_oid;
 
if (!c || !c->tree.initialized || !c->tree.update_ref ||
!*c->tree.update_ref)
@@ -55,19 +54,19 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_sha1))
+   if (write_notes_tree(>tree, tree_oid.hash))
return -1;
-   if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,
-   commit_sha1, NULL, NULL) < 0)
+   if (commit_tree(c->validity, strlen(c->validity), tree_oid.hash, NULL,
+   commit_oid.hash, NULL, NULL) < 0)
return -1;
-   if (update_ref("update notes cache", c->tree.update_ref, commit_sha1,
+   if (update_ref("update notes cache", c->tree.update_ref, 
commit_oid.hash,
   NULL, 0, UPDATE_REFS_QUIET_ON_ERR) < 0)
return -1;
 
return 0;
 }
 
-char *notes_cache_get(struct notes_cache *c, unsigned char key_sha1[20],
+char *notes_cache_get(struct notes_cache *c, struct object_id *key_oid,
  size_t *outsize)
 {
const unsigned char *value_sha1;
@@ -75,7 +74,7 @@ char *notes_cache_get(struct notes_cache *c, unsigned char 
key_sha1[20],
char *value;
unsigned long size;
 
-   value_sha1 = get_note(>tree, key_sha1);
+   value_sha1 = get_note(>tree, key_oid->hash);
if (!value_sha1)
return NULL;
value = read_sha1_file(value_sha1, , );
@@ -84,12 +83,12 @@ char *notes_cache_get(struct notes_cache *c, unsigned char 
key_sha1[20],
return value;
 }
 
-int notes_cache_put(struct notes_cache *c, unsigned char key_sha1[20],
+int notes_cache_put(struct notes_cache *c, struct object_id *key_oid,
const char *data, size_t size)
 {
-   unsigned char value_sha1[20];
+   struct object_id value_oid;
 
-   if (write_sha1_file(data, size, "blob", value_sha1) < 0)
+   if (write_sha1_file(data, size, "blob", value_oid.hash) < 0)
return -1;
-   return add_note(>tree, key_sha1, value_sha1, NULL);
+   return add_note(>tree, key_oid->hash, value_oid.hash, NULL);
 }
diff --git a/notes-cache.h b/notes-cache.h
index 356f88fb3..a8409 100644
--- a/notes-cache.h
+++ b/notes-cache.h
@@ -12,9 +12,9 @@ void notes_cache_init(struct notes_cache *c, const char *name,
 const char *validity);
 int notes_cache_write(struct notes_cache *c);
 
-char *notes_cache_get(struct notes_cache *c, unsigned char sha1[20], size_t
+char *notes_cache_get(struct 

[PATCH 05/53] builtin/prune: convert to struct object_id

[brian m. carlson]

Convert the sole instance of unsigned char [20] to struct object_id.
cmd_prune is a caller of parse_object, which we will convert later.

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

diff --git a/builtin/prune.c b/builtin/prune.c
index 42633e0c6..96dca7d58 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -123,11 +123,11 @@ int cmd_prune(int argc, const char **argv, const char 
*prefix)
die(_("cannot prune in a precious-objects repo"));
 
while (argc--) {
-   unsigned char sha1[20];
+   struct object_id oid;
const char *name = *argv++;
 
-   if (!get_sha1(name, sha1)) {
-   struct object *object = parse_object_or_die(sha1, name);
+   if (!get_oid(name, )) {
+   struct object *object = parse_object_or_die(oid.hash, 
name);
add_pending_object(, object, "");
}
else

[PATCH 00/53] object_id part 8

[brian m. carlson]

This is the eighth series of patches to convert unsigned char [20] to
struct object_id.  This series converts lookup_commit, lookup_blob,
lookup_tree, lookup_tag, and finally parse_object to struct object_id.

A small number of functions have temporaries inserted during the
conversion in order to allow conversion of functions that still need to
take unsigned char *; they are removed either later in the series or
will be in a future series.

This series can be fetched from the object-id-part8 branch from either
of the follwing:

https://github.com/bk2204/git
https://git.crustytoothpaste.net/git/bmc/git.git

brian m. carlson (53):
  fetch-pack: convert to struct object_id
  Clean up outstanding object_id transforms.
  Convert struct cache_tree to use struct object_id
  builtin/name-rev: convert to struct object_id
  builtin/prune: convert to struct object_id
  bundle: convert to struct object_id
  branch: convert to struct object_id
  builtin/blame: convert static function to struct object_id
  builtin/rev-parse: convert to struct object_id
  fast-import: convert internal structs to struct object_id
  fast-import: convert to struct object_id
  submodule: convert merge_submodule to use struct object_id
  notes-cache: convert to struct object_id
  parse-options-cb: convert to struct object_id
  reflog_expire: convert to struct object_id
  builtin/verify-commit: convert to struct object_id
  tag: convert parse_tag_buffer to struct object_id
  http-push: convert some static functions to struct object_id
  notes-utils: convert internals to struct object_id
  revision: convert prepare_show_merge to struct object_id
  shallow: convert shallow registration functions to object_id
  sequencer: convert some functions to struct object_id
  builtin/tag: convert to struct object_id
  Convert remaining callers of lookup_commit_reference* to object_id
  Convert lookup_commit* to struct object_id
  pack: convert struct pack_idx_entry to struct object_id
  builtin/unpack-objects: convert to struct object_id
  Convert remaining callers of lookup_blob to object_id
  Convert lookup_blob to struct object_id
  tree: convert read_tree_1 to use struct object_id internally
  builtin/reflog: convert tree_is_complete to take struct object_id
  Convert lookup_tree to struct object_id
  log-tree: convert to struct object_id
  Convert lookup_tag to struct object_id
  Convert the verify_pack callback to struct object_id
  Convert struct ref_array_item to struct object_id
  ref-filter: convert some static functions to struct object_id
  refs: convert struct ref_update to use struct object_id
  refs/files-backend: convert many internals to struct object_id
  http-push: convert process_ls_object and descendants to object_id
  revision: rename add_pending_sha1 to add_pending_oid
  revision: convert remaining parse_object callers to object_id
  upload-pack: convert remaining parse_object callers to object_id
  sha1_name: convert internals of peel_onion to object_id
  builtin/read-tree: convert to struct object_id
  builtin/ls-files: convert overlay_tree_on_cache to object_id
  sequencer: convert fast_forward_to to struct object_id
  merge: convert checkout_fast_forward to struct object_id
  builtin/ls-tree: convert to struct object_id
  diff-lib: convert do_diff_cache to struct object_id
  sequencer: convert do_recursive_merge to struct object_id
  tree: convert parse_tree_indirect to struct object_id
  object: convert parse_object* to take struct object_id

 archive.c   |   6 +-
 bisect.c|   2 +-
 blob.c  |   6 +-
 blob.h  |   2 +-
 branch.c|  16 +-
 builtin/am.c|  18 +-
 builtin/blame.c |  14 +-
 builtin/branch.c|   6 +-
 builtin/checkout.c  |  18 +-
 builtin/clone.c |   4 +-
 builtin/commit-tree.c   |   2 +-
 builtin/commit.c|   8 +-
 builtin/describe.c  |  10 +-
 builtin/diff-tree.c |   8 +-
 builtin/diff.c  |   6 +-
 builtin/fast-export.c   |   8 +-
 builtin/fetch.c |   7 +-
 builtin/fmt-merge-msg.c |   8 +-
 builtin/fsck.c  |  16 +-
 builtin/grep.c  |   2 +-
 builtin/index-pack.c|  56 ++---
 builtin/log.c   |  10 +-
 builtin/ls-files.c  |   6 +-
 builtin/ls-tree.c   |   6 +-
 builtin/merge-base.c|   6 +-
 builtin/merge-tree.c|  10 +-
 builtin/merge.c |  12 +-
 builtin/name-rev.c  |  32 +--
 builtin/notes.c |   2 +-
 builtin/pack-objects.c  |  71 ---
 builtin/prune.c |   7 +-
 builtin/pull.c  |  14 +-
 builtin/read-tree.c |  10 +-
 builtin/receive-pack.c  |   8 +-
 builtin/reflog.c|  36 ++--
 builtin/replace.c   

[PATCH 03/53] Convert struct cache_tree to use struct object_id

[brian m. carlson]

Convert the sha1 member of struct cache_tree to struct object_id by
changing the definition and applying the following semantic patch, plus
the standard object_id transforms:

@@
struct cache_tree E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct cache_tree *E1;
@@
- E1->sha1
+ E1->oid.hash

Fix up one reference to active_cache_tree which was not automatically
caught by Coccinelle.  These changes are prerequisites for converting
parse_object.

Signed-off-by: brian m. carlson 
---
 builtin/commit.c|  2 +-
 builtin/fsck.c  |  4 ++--
 cache-tree.c| 31 ---
 cache-tree.h|  3 ++-
 merge-recursive.c   |  2 +-
 revision.c  |  2 +-
 sequencer.c |  3 ++-
 t/helper/test-dump-cache-tree.c |  4 ++--
 8 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 4e288bc51..05a0b609e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1758,7 +1758,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
+   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
 parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
rollback_index_files();
die(_("failed to write commit object"));
diff --git a/builtin/fsck.c b/builtin/fsck.c
index f76e4163a..543122f04 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -599,10 +599,10 @@ static int fsck_cache_tree(struct cache_tree *it)
fprintf(stderr, "Checking cache tree\n");
 
if (0 <= it->entry_count) {
-   struct object *obj = parse_object(it->sha1);
+   struct object *obj = parse_object(it->oid.hash);
if (!obj) {
error("%s: invalid sha1 pointer in cache-tree",
- sha1_to_hex(it->sha1));
+ oid_to_hex(>oid));
errors_found |= ERROR_REFS;
return 1;
}
diff --git a/cache-tree.c b/cache-tree.c
index 345ea3596..35d507ed7 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -225,7 +225,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
int i;
if (!it)
return 0;
-   if (it->entry_count < 0 || !has_sha1_file(it->sha1))
+   if (it->entry_count < 0 || !has_sha1_file(it->oid.hash))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
if (!cache_tree_fully_valid(it->down[i]->cache_tree))
@@ -253,7 +253,7 @@ static int update_one(struct cache_tree *it,
 
*skip_count = 0;
 
-   if (0 <= it->entry_count && has_sha1_file(it->sha1))
+   if (0 <= it->entry_count && has_sha1_file(it->oid.hash))
return it->entry_count;
 
/*
@@ -340,7 +340,7 @@ static int update_one(struct cache_tree *it,
die("cache-tree.c: '%.*s' in '%s' not found",
entlen, path + baselen, path);
i += sub->count;
-   sha1 = sub->cache_tree->sha1;
+   sha1 = sub->cache_tree->oid.hash;
mode = S_IFDIR;
contains_ita = sub->cache_tree->entry_count < 0;
if (contains_ita) {
@@ -402,12 +402,13 @@ static int update_one(struct cache_tree *it,
unsigned char sha1[20];
hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
if (has_sha1_file(sha1))
-   hashcpy(it->sha1, sha1);
+   hashcpy(it->oid.hash, sha1);
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
-   else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {
+   hash_sha1_file(buffer.buf, buffer.len, tree_type,
+  it->oid.hash);
+   else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
strbuf_release();
return -1;
}
@@ -417,7 +418,7 @@ static int update_one(struct cache_tree *it,
 #if DEBUG
fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
it->entry_count, it->subtree_nr,
-   sha1_to_hex(it->sha1));
+   oid_to_hex(>oid));
 #endif
return i;
 }
@@ -457,14 +458,14 @@ static void write_one(struct strbuf *buffer, struct 
cache_tree *it,
if (0 <= it->entry_count)
fprintf(stderr, "cache-tree <%.*s> (%d ent, %d subtree) %s\n",
pathlen, path, it->entry_count, it->subtree_nr,
- 

[PATCH 04/53] builtin/name-rev: convert to struct object_id

[brian m. carlson]

Convert all the uses of unsigned char [20] to struct object_id.  Also,
convert some hard-coded integers into constants.

name_rev_line accepts a wide variety of free-form input and only
interprets 40-character hex values, passing through everything else.
Consequently, it is not a good candidate for parse_oid_hex, which is
much stricter.

This change is a prerequisite for converting parse_object.

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

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 92a5d8a5d..00760ecc6 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -114,7 +114,7 @@ struct name_ref_data {
 
 static struct tip_table {
struct tip_table_entry {
-   unsigned char sha1[20];
+   struct object_id oid;
const char *refname;
} *table;
int nr;
@@ -122,13 +122,13 @@ static struct tip_table {
int sorted;
 } tip_table;
 
-static void add_to_tip_table(const unsigned char *sha1, const char *refname,
+static void add_to_tip_table(const struct object_id *oid, const char *refname,
 int shorten_unambiguous)
 {
refname = name_ref_abbrev(refname, shorten_unambiguous);
 
ALLOC_GROW(tip_table.table, tip_table.nr + 1, tip_table.alloc);
-   hashcpy(tip_table.table[tip_table.nr].sha1, sha1);
+   oidcpy(_table.table[tip_table.nr].oid, oid);
tip_table.table[tip_table.nr].refname = xstrdup(refname);
tip_table.nr++;
tip_table.sorted = 0;
@@ -137,7 +137,7 @@ static void add_to_tip_table(const unsigned char *sha1, 
const char *refname,
 static int tipcmp(const void *a_, const void *b_)
 {
const struct tip_table_entry *a = a_, *b = b_;
-   return hashcmp(a->sha1, b->sha1);
+   return oidcmp(>oid, >oid);
 }
 
 static int name_ref(const char *path, const struct object_id *oid, int flags, 
void *cb_data)
@@ -194,7 +194,7 @@ static int name_ref(const char *path, const struct 
object_id *oid, int flags, vo
return 0;
}
 
-   add_to_tip_table(oid->hash, path, can_abbreviate_output);
+   add_to_tip_table(oid, path, can_abbreviate_output);
 
while (o && o->type == OBJ_TAG) {
struct tag *t = (struct tag *) o;
@@ -216,7 +216,7 @@ static int name_ref(const char *path, const struct 
object_id *oid, int flags, vo
 static const unsigned char *nth_tip_table_ent(size_t ix, void *table_)
 {
struct tip_table_entry *table = table_;
-   return table[ix].sha1;
+   return table[ix].oid.hash;
 }
 
 static const char *get_exact_ref_match(const struct object *o)
@@ -301,9 +301,9 @@ static void name_rev_line(char *p, struct name_ref_data 
*data)
 #define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))
if (!ishex(*p))
forty = 0;
-   else if (++forty == 40 &&
+   else if (++forty == GIT_SHA1_HEXSZ &&
 !ishex(*(p+1))) {
-   unsigned char sha1[40];
+   struct object_id oid;
const char *name = NULL;
char c = *(p+1);
int p_len = p - p_start + 1;
@@ -311,9 +311,9 @@ static void name_rev_line(char *p, struct name_ref_data 
*data)
forty = 0;
 
*(p+1) = 0;
-   if (!get_sha1(p - 39, sha1)) {
+   if (!get_oid(p - (GIT_SHA1_HEXSZ - 1), )) {
struct object *o =
-   lookup_object(sha1);
+   lookup_object(oid.hash);
if (o)
name = get_rev_name(o, );
}
@@ -323,7 +323,7 @@ static void name_rev_line(char *p, struct name_ref_data 
*data)
continue;
 
if (data->name_only)
-   printf("%.*s%s", p_len - 40, p_start, name);
+   printf("%.*s%s", p_len - GIT_SHA1_HEXSZ, 
p_start, name);
else
printf("%.*s (%s)", p_len, p_start, name);
p_start = p + 1;
@@ -374,18 +374,18 @@ int cmd_name_rev(int argc, const char **argv, const char 
*prefix)
cutoff = 0;
 
for (; argc; argc--, argv++) {
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *object;
struct commit *commit;
 
-   if (get_sha1(*argv, sha1)) {
+   if (get_oid(*argv, )) {
fprintf(stderr, "Could not get sha1 for %s. 
Skipping.\n",
*argv);
continue;
}
 

[PATCH 01/53] fetch-pack: convert to struct object_id

[brian m. carlson]

Convert all uses of unsigned char [20] to struct object_id.  Switch one
use of get_sha1_hex to parse_oid_hex to avoid the need for a constant.
This change is necessary in order to convert parse_object.

Signed-off-by: brian m. carlson 
---
 fetch-pack.c | 89 ++--
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 42969353d..1e6b03b6b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -118,9 +118,9 @@ static void rev_list_push(struct commit *commit, int mark)
}
 }
 
-static int rev_list_insert_ref(const char *refname, const unsigned char *sha1)
+static int rev_list_insert_ref(const char *refname, const struct object_id 
*oid)
 {
-   struct object *o = deref_tag(parse_object(sha1), refname, 0);
+   struct object *o = deref_tag(parse_object(oid->hash), refname, 0);
 
if (o && o->type == OBJ_COMMIT)
rev_list_push((struct commit *)o, SEEN);
@@ -131,7 +131,7 @@ static int rev_list_insert_ref(const char *refname, const 
unsigned char *sha1)
 static int rev_list_insert_ref_oid(const char *refname, const struct object_id 
*oid,
   int flag, void *cb_data)
 {
-   return rev_list_insert_ref(refname, oid->hash);
+   return rev_list_insert_ref(refname, oid);
 }
 
 static int clear_marks(const char *refname, const struct object_id *oid,
@@ -183,7 +183,7 @@ static void mark_common(struct commit *commit,
   Get the next rev to send, ignoring the common.
 */
 
-static const unsigned char *get_rev(void)
+static const struct object_id *get_rev(void)
 {
struct commit *commit = NULL;
 
@@ -222,7 +222,7 @@ static const unsigned char *get_rev(void)
}
}
 
-   return commit->object.oid.hash;
+   return >object.oid;
 }
 
 enum ack_type {
@@ -251,7 +251,7 @@ static void consume_shallow_list(struct fetch_pack_args 
*args, int fd)
}
 }
 
-static enum ack_type get_ack(int fd, unsigned char *result_sha1)
+static enum ack_type get_ack(int fd, struct object_id *result_oid)
 {
int len;
char *line = packet_read_line(fd, );
@@ -262,7 +262,7 @@ static enum ack_type get_ack(int fd, unsigned char 
*result_sha1)
if (!strcmp(line, "NAK"))
return NAK;
if (skip_prefix(line, "ACK ", )) {
-   if (!get_sha1_hex(arg, result_sha1)) {
+   if (!get_oid_hex(arg, result_oid)) {
arg += 40;
len -= arg - line;
if (len < 1)
@@ -291,7 +291,7 @@ static void send_request(struct fetch_pack_args *args,
 
 static void insert_one_alternate_object(struct object *obj)
 {
-   rev_list_insert_ref(NULL, obj->oid.hash);
+   rev_list_insert_ref(NULL, >oid);
 }
 
 #define INITIAL_FLUSH 16
@@ -315,12 +315,12 @@ static int next_flush(struct fetch_pack_args *args, int 
count)
 }
 
 static int find_common(struct fetch_pack_args *args,
-  int fd[2], unsigned char *result_sha1,
+  int fd[2], struct object_id *result_oid,
   struct ref *refs)
 {
int fetching;
int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;
-   const unsigned char *sha1;
+   const struct object_id *oid;
unsigned in_vain = 0;
int got_continue = 0;
int got_ready = 0;
@@ -338,7 +338,7 @@ static int find_common(struct fetch_pack_args *args,
 
fetching = 0;
for ( ; refs ; refs = refs->next) {
-   unsigned char *remote = refs->old_oid.hash;
+   struct object_id *remote = >old_oid;
const char *remote_hex;
struct object *o;
 
@@ -352,12 +352,12 @@ static int find_common(struct fetch_pack_args *args,
 * interested in the case we *know* the object is
 * reachable and we have already scanned it.
 */
-   if (((o = lookup_object(remote)) != NULL) &&
+   if (((o = lookup_object(remote->hash)) != NULL) &&
(o->flags & COMPLETE)) {
continue;
}
 
-   remote_hex = sha1_to_hex(remote);
+   remote_hex = oid_to_hex(remote);
if (!fetching) {
struct strbuf c = STRBUF_INIT;
if (multi_ack == 2) strbuf_addstr(, " 
multi_ack_detailed");
@@ -408,25 +408,25 @@ static int find_common(struct fetch_pack_args *args,
if (args->deepen) {
char *line;
const char *arg;
-   unsigned char sha1[20];
+   struct object_id oid;
 
send_request(args, fd[1], _buf);
while ((line = packet_read_line(fd[0], NULL))) {
if (skip_prefix(line, "shallow ", )) {
-   if (get_sha1_hex(arg, sha1))
+ 

[PATCH 02/53] Clean up outstanding object_id transforms.

[brian m. carlson]

The semantic patch for standard object_id transforms found two
outstanding places where we could make a transformation automatically.
Apply these changes.

Signed-off-by: brian m. carlson 
---
 builtin/diff.c | 2 +-
 reflog-walk.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index d184aafab..a25b4e4ae 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -408,7 +408,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
} else if (obj->type == OBJ_BLOB) {
if (2 <= blobs)
die(_("more than two blobs given: '%s'"), name);
-   hashcpy(blob[blobs].oid.hash, obj->oid.hash);
+   oidcpy([blobs].oid, >oid);
blob[blobs].name = name;
blob[blobs].mode = entry->mode;
blobs++;
diff --git a/reflog-walk.c b/reflog-walk.c
index 99679f582..c8fdf051d 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -241,7 +241,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, 
struct commit *commit)
logobj = parse_object(reflog->ooid.hash);
} while (commit_reflog->recno && (logobj && logobj->type != 
OBJ_COMMIT));
 
-   if (!logobj && commit_reflog->recno >= 0 && 
is_null_sha1(reflog->ooid.hash)) {
+   if (!logobj && commit_reflog->recno >= 0 && is_null_oid(>ooid)) 
{
/* a root commit, but there are still more entries to show */
reflog = _reflog->reflogs->items[commit_reflog->recno];
logobj = parse_object(reflog->noid.hash);

Re: [PATCH v2] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Johannes Sixt]

Am 23.04.2017 um 16:51 schrieb Peter Krefting:

Johannes Sixt:

@@ -376,7 +397,7 @@ static int write_zip_entry(struct archiver_args
*args,
 copy_le16(dirent.comment_length, 0);
 copy_le16(dirent.disk, 0);
 copy_le32(dirent.attr2, attr2);
-copy_le32(dirent.offset, zip_offset);
+copy_le32(dirent.offset, clamp_max(zip_offset, 0xU,
));


I don't see any provisions to write the zip64 extra header in the
central directory when this offset is clamped. This means that ZIP
archives whose size exceed 4GB are still unsupported.


The clamped flag will trigger the inclusion of the zip64 central
directory, which contains the 64-bit offset. Should the central
directory entry also have the zip64 extra field?


There is no "zip64 central directory". There is a "zip64 end of central 
directory record"; it tells where to find the "central directory" in 
case that the ZIP archive exceeds 4GB. The central directory has the 
same format in a non-zip64 and a zip64 archive. But when size, 
compressed size, and offset overflow 4GB, it uses the same zip64 extra 
record like the local header records, except that it has to record an 
offset in addition to the uncompressed and compressed sizes.


The uncompressed and compressed sizes of entries are mentioned in both 
the central directory and the individual local headers. I think that the 
central directory's values are authorative; my reasoning is that it is 
possible that the local header can have a bit set that tells that the 
local header's values size values are garbage.


In summary, yes, when the central directory is constructed, it must use 
the zip64 extra record to note the values of uncompressed size, 
compressed size, and the offset to the local header when they overflow 4GB.


-- Hannes

[PATCH] doc: git-pull.txt use US spelling, fix minor typo

[René Genz]

---
Instead of using two command lines I could have replaced the comma with a 
semicolon.

I do not mind, if this patch is squashed into the other minor typo fixes of 
mine:
3c228f462d02e76956062b8d8572158cbcdbbc7b


 Documentation/git-pull.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 4470e4b..942af8e 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -67,7 +67,7 @@ with uncommitted changes is discouraged: while possible, it 
leaves you
 in a state that may be hard to back out of in the case of a conflict.
 
 If any of the remote changes overlap with local uncommitted changes,
-the merge will be automatically cancelled and the work tree untouched.
+the merge will be automatically canceled and the work tree untouched.
 It is generally best to get any local changes in working order before
 pulling or stash them away with linkgit:git-stash[1].
 
@@ -210,7 +210,8 @@ EXAMPLES
   current branch:
 +
 
-$ git pull, git pull origin
+$ git pull
+$ git pull origin
 
 +
 Normally the branch merged in is the HEAD of the remote repository,
-- 
1.9.1

Re: minor typos in documentation

[René Genz]

Hi Stefan,

I submitted the patch to the mailing list.
Based on my experience I submitted another patch to improve the documentation.

Thank you for guiding me.
--
Kind regards,
René

[PATCH] doc: update SubmittingPatches

[René Genz]

-use US English spelling
-based on my own experience:
 * add commands used to contribute a patch
 * minor wording change for better readability

Thanks-to: Stefan Beller 
---
 Documentation/SubmittingPatches | 31 +--
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index bc8ad00..ac027ba 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -51,7 +51,7 @@ If your description starts to get too long, that's a sign 
that you
 probably need to split up your commit to finer grained pieces.
 That being said, patches which plainly describe the things that
 help reviewers check the patch, and future maintainers understand
-the code, are the most beautiful patches.  Descriptions that summarise
+the code, are the most beautiful patches.  Descriptions that summarize
 the point in the subject well, and describe the motivation for the
 change, the approach taken by the change, and if relevant how this
 differs substantially from the prior version, are all good things
@@ -87,7 +87,7 @@ patches separate from other documentation changes.
 Oh, another thing.  We are picky about whitespaces.  Make sure your
 changes do not trigger errors with the sample pre-commit hook shipped
 in templates/hooks--pre-commit.  To help ensure this does not happen,
-run git diff --check on your changes before you commit.
+run "git diff --check" on your changes before you commit.
 
 
 (2) Describe your changes well.
@@ -111,10 +111,10 @@ Improve...".
 
 The body should provide a meaningful commit message, which:
 
-  . explains the problem the change tries to solve, iow, what is wrong
+  . explains the problem the change tries to solve, i.e. what is wrong
 with the current code without the change.
 
-  . justifies the way the change solves the problem, iow, why the
+  . justifies the way the change solves the problem, i.e. why the
 result with the change is better.
 
   . alternate solutions considered but discarded, if any.
@@ -122,7 +122,7 @@ The body should provide a meaningful commit message, which:
 Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
 instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
 to do frotz", as if you are giving orders to the codebase to change
-its behaviour.  Try to make sure your explanation can be understood
+its behavior.  Try to make sure your explanation can be understood
 without external resources. Instead of giving a URL to a mailing list
 archive, summarize the relevant points of the discussion.
 
@@ -261,7 +261,7 @@ smaller project it is a good discipline to follow it.
 The sign-off is a simple line at the end of the explanation for
 the patch, which certifies that you wrote it or otherwise have
 the right to pass it on as a open-source patch.  The rules are
-pretty simple: if you can certify the below:
+pretty simple: if you can certify the below D-C-O:
 
 Developer's Certificate of Origin 1.1
 
@@ -376,6 +376,25 @@ from the list and queue it to 'pu', in order to make it 
easier for
 people play with it without having to pick up and apply the patch to
 their trees themselves.
 
+
+An oversimplified summary of the commands to run:
+* clone repo
+  $ git clone https://github.com/git/git
+
+* change files in your local repo copy
+
+* commit your changes
+  $ git commit -a
+
+* create '.patch' file for the latest commit
+  $ git format-patch HEAD^
+
+* install 'git-email' package and configure it, f.e.
+https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/
+  send an email to yourself using your MUA in order to find out the value
+  for the "--smtp-domain" option; look at the 'Received' header option
+  $ git send-email --annotate --smtp-domain=LONGSTRING 
--to=git@vger.kernel.org --cc=MAINTAINER --smtp-debug=1 NAME.patch
+
 
 Know the status of your patch after submission
 
-- 
1.9.1

Logical inconsistency in git reset

[vvs]

Dear list

Like it was suggested on git-users list I'm forwarding the original
message here.

I found out that "git reset --hard" produced different outcome
depending on current index content, i.e. when there is no entry for a
file in working tree it actually changed that file. While on the
contrary, if you use "git reset --mixed" right before that, the file
won't be touched. This affects make, which thinks that the file was
changed.

More detailed discussion and test cases can be found here:
https://groups.google.com/forum/?hl=en#!topic/git-users/9ziZ6yq-BfU

I could just add to that argument that this issue is certainly looks
like a logical inconsistency to an average user. The git operations
are checking an index entry for the file in working tree but they
doesn't fill that entry in the first place. Shouldn't this be
considered as a kind of uninitialized pointer problem? And there is
that useful abstraction that --hard should include --mixed semantics.

Re: [PATCH v2] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Peter Krefting]

Johannes Sixt:

Let's get the naming straight: There is no "zip64 local header". There is a 
"zip64 extra record" for the "zip local header".


Indeed, sorry for the confusion. That's what I get for trying to write 
coherent email at half past midnight :-)


The zip64 extra data record has an offset field, but since the local 
header does not have an offset field, the offset field in the 
corresponding zip64 extra data record is always omitted.


Ah, now I understand, I was a bit confused, as the same code generates 
the central directory entry as the local entry.



@@ -376,7 +397,7 @@ static int write_zip_entry(struct archiver_args *args,
copy_le16(dirent.comment_length, 0);
copy_le16(dirent.disk, 0);
copy_le32(dirent.attr2, attr2);
-   copy_le32(dirent.offset, zip_offset);
+	copy_le32(dirent.offset, clamp_max(zip_offset, 0xU, 
));


I don't see any provisions to write the zip64 extra header in the central 
directory when this offset is clamped. This means that ZIP archives whose 
size exceed 4GB are still unsupported.


The clamped flag will trigger the inclusion of the zip64 central 
directory, which contains the 64-bit offset. Should the central 
directory entry also have the zip64 extra field?


These are wrong, I think. Entries that did not overflow must be omitted 
entirely from the zip64 extra record, not filled with 0. This implies that 
the payload size (.extra_size) is dynamic.


That is what I was trying to figure out, APPNOTE is extremely vague on 
the subject, but thinking back I recall that you are correct.


--
\\// Peter - http://www.softwolves.pp.se/

git v2.13.0-rc0 test failures on cygwin

[Ramsay Jones]

Hi Junio, Adam,

[Adam, if you are no longer the git package maintainer for cygwin, then
please ignore this email and sorry for the noise!]

On thursday evening, I ran the test-suite on the newly minted v2.13.0-rc0
release on cygwin, which unfortunately failed (the 'test-out' files are
from the most recent test runs for v2.13.0-rc0, v2.12.0-rc0, v2.12.0-rc1
and v2.12.0 - I missed the v2.12.0-rc2 release!):

$ ls -l test-out*
-rw-r--r-- 1 ramsay None 104K Apr 20 22:29 test-out
-rw-r--r-- 1 ramsay None 103K Feb  5 00:57 test-out.old1
-rw-r--r-- 1 ramsay None 103K Feb 12 00:29 test-out.old2
-rw-r--r-- 1 ramsay None 103K Feb 25 01:14 test-out.old3
$ 
$ grep FAIL test-out*
test-out:Result: FAIL
$ 
$ tail -15 test-out
[22:29:51]

Test Summary Report
---
t0301-credential-cache.sh(Wstat: 256 Tests: 29 Failed: 
6)
  Failed tests:  12, 24-28
  Non-zero exit status: 1
t8010-cat-file-filters.sh(Wstat: 256 Tests: 8 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=780, Tests=14700, 10398 wallclock secs ( 1.27 usr  0.78 sys + 1265.08 
cusr 4076.38 csys = 5343.50 CPU)
Result: FAIL
make[1]: *** [Makefile:45: prove] Error 1
make[1]: Leaving directory '/home/ramsay/git/t'
make: *** [Makefile:2313: test] Error 2
$

I was a bit surprised about the 'credential-cache' failures (I didn't think
they were being run on cygwin!), but I haven't spent any time looking at
that yet. So, looking at the t8010 test failure:

$ grep t8010 test-out*
test-out:[22:20:20] t8010-cat-file-filters.sh ..
test-out:t8010-cat-file-filters.sh(Wstat: 256 Tests: 8 
Failed: 1)
test-out.old1:[00:46:05] t8010-cat-file-filters.sh .. 
ok 2169 ms ( 0.00 usr  0.00 sys +  0.21 cusr  0.54 csys =  0.75 CPU)
test-out.old2:[00:20:09] t8010-cat-file-filters.sh .. 
ok 1831 ms ( 0.00 usr  0.00 sys +  0.32 cusr  0.74 csys =  1.06 CPU)
test-out.old3:[01:05:04] t8010-cat-file-filters.sh .. 
ok 1845 ms ( 0.00 usr  0.00 sys +  0.44 cusr  0.59 csys =  1.03 CPU)
$

So, this test was passing on the three previous runs and nothing seems to
have changed in the interval. Hmm, except that the previous evening I had
updated my cygwin installation (according to my setup.log file, the cygwin
dll version went from 2.7.0-1 to 2.8.0-1). As a quick test, I rebuilt the
v2.12.0 version, which has previously worked, and ran the t8010 test
repeatedly; it failed consistently. Hmm, back to the v2.13.0-rc0 failure:

$ cd t
$ ./t8010-cat-file-filters.sh -i -v

...

ok 7 - --path= complains without --textconv/--filters

expecting success:
sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
git cat-file --textconv --batch >actual &&
printf "%s blob 6\nuryyb\r\n\n%s blob 6\nhello\n\n" \
$sha1 $sha1 >expect &&
test_cmp expect actual

--- expect  2017-04-23 12:40:07.899086200 +
+++ actual  2017-04-23 12:40:07.883459900 +
@@ -1,6 +1,3 @@
 ce013625030ba8dba906f756967f9e9ca394464a blob 6
 uryyb

-ce013625030ba8dba906f756967f9e9ca394464a blob 6
-hello
-
not ok 8 - cat-file --textconv --batch works
#
#   sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
#   test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
#   printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
#   git cat-file --textconv --batch >actual &&
#   printf "%s blob 6\nuryyb\r\n\n%s blob 6\nhello\n\n" \
#   $sha1 $sha1 >expect &&
#   test_cmp expect actual
#
$

OK, lets go run it by hand:

$ cd trash\ directory.t8010-cat-file-filters/
$ sha1=$(git rev-parse -q --verify HEAD:world.txt)
$ printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
> git cat-file --textconv --batch
ce013625030ba8dba906f756967f9e9ca394464a blob 6
uryyb

$

Having run it in the debugger, the loop in batch_objects() (builtin/cat-file.c
lines #489-505) only reads one line from stdin, then gets EOF on the stream.

$ printf "%s hello.txt\n%s hello\n" $sha1 $sha1 | cat
ce013625030ba8dba906f756967f9e9ca394464a hello.txt
ce013625030ba8dba906f756967f9e9ca394464a hello
$

$ printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
> git cat-file --batch
ce013625030ba8dba906f756967f9e9ca394464a hello.txt missing
ce013625030ba8dba906f756967f9e9ca394464a hello missing
$

$ printf "%s hello.txt\n%s hello\n" $sha1 $sha1 >input
$ git cat-file --textconv --batch 8 --

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1890d7a63..da5e45d75 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -426,12 +426,31 @@ static int batch_packed_object(const struct object_id 
*oid,
return 0;
 }
 
+static struct strbuf **strbuf_getlines(FILE *fp)
+{
+   struct strbuf **ret = NULL;
+   size_t nr = 0, 

[PATCH] fix minor typing mistakes

[René Genz]

Thanks-to: Stefan Beller 
---
 Documentation/git-commit.txt| 4 ++--
 Documentation/gitremote-helpers.txt | 2 +-
 ci/run-windows-build.sh | 2 +-
 diff.c  | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index ed0f5b9..afb06ad 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -95,7 +95,7 @@ OPTIONS
 
 --reset-author::
When used with -C/-c/--amend options, or when committing after a
-   a conflicting cherry-pick, declare that the authorship of the
+   conflicting cherry-pick, declare that the authorship of the
resulting commit now belongs to the committer. This also renews
the author timestamp.
 
@@ -112,7 +112,7 @@ OPTIONS
`--dry-run`.
 
 --long::
-   When doing a dry-run, give the output in a the long-format.
+   When doing a dry-run, give the output in the long-format.
Implies `--dry-run`.
 
 -z::
diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index e4b785e..4a584f3 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -463,7 +463,7 @@ set by Git if the remote helper has the 'option' capability.
GPG sign pushes.
 
 'option push-option ::
-   Transmit  as a push option. As the a push option
+   Transmit  as a push option. As the push option
must not contain LF or NUL characters, the string is not encoded.
 
 SEE ALSO
diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
index 4e3a50b..9f89d54 100755
--- a/ci/run-windows-build.sh
+++ b/ci/run-windows-build.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #
-# Script to trigger the a Git for Windows build and test run.
+# Script to trigger the Git for Windows build and test run.
 # Set the $GFW_CI_TOKEN as environment variable.
 # Pass the branch (only branches on https://github.com/git/git are
 # supported) and a commit hash.
diff --git a/diff.c b/diff.c
index 11eef1c..74283d9 100644
--- a/diff.c
+++ b/diff.c
@@ -911,7 +911,7 @@ static int fn_out_diff_words_write_helper(FILE *fp,
 /*
  * '--color-words' algorithm can be described as:
  *
- *   1. collect a the minus/plus lines of a diff hunk, divided into
+ *   1. collect the minus/plus lines of a diff hunk, divided into
  *  minus-lines and plus-lines;
  *
  *   2. break both minus-lines and plus-lines into words and
-- 
1.9.1

Re: [PATCH v2] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Johannes Sixt]

Am 23.04.2017 um 00:41 schrieb Peter Krefting:

Indeed. Last time I implemented zip64 support it was on the reading side,
and I remember this was a mess...


It is indeed!


 static void set_zip_header_data_desc(struct zip_local_header *header,
 unsigned long size,
 unsigned long compressed_size,
-unsigned long crc)
+unsigned long crc,
+int *clamped)
 {
copy_le32(header->crc32, crc);
-   copy_le32(header->compressed_size, compressed_size);
-   copy_le32(header->size, size);
+   copy_le32(header->compressed_size, clamp_max(compressed_size, 
0xU, clamped));
+   copy_le32(header->size, clamp_max(size, 0xU, clamped));
+   if (clamped)
+   zip_zip64 = 1;


This must be

if (*clamped)


 }

 static int has_only_ascii(const char *s)
@@ -279,6 +299,7 @@ static int write_zip_entry(struct archiver_args *args,
int is_binary = -1;
const char *path_without_prefix = path + args->baselen;
unsigned int creator_version = 0;
+   int clamped = 0;

crc = crc32(0, NULL, 0);

@@ -376,7 +397,7 @@ static int write_zip_entry(struct archiver_args *args,
copy_le16(dirent.comment_length, 0);
copy_le16(dirent.disk, 0);
copy_le32(dirent.attr2, attr2);
-   copy_le32(dirent.offset, zip_offset);
+   copy_le32(dirent.offset, clamp_max(zip_offset, 0xU, ));


I don't see any provisions to write the zip64 extra header in the 
central directory when this offset is clamped. This means that ZIP 
archives whose size exceed 4GB are still unsupported.




copy_le32(header.magic, 0x04034b50);
copy_le16(header.version, 10);
@@ -384,15 +405,26 @@ static int write_zip_entry(struct archiver_args *args,
copy_le16(header.compression_method, method);
copy_le16(header.mtime, zip_time);
copy_le16(header.mdate, zip_date);
-   set_zip_header_data_desc(, size, compressed_size, crc);
+   set_zip_header_data_desc(, size, compressed_size, crc, );
copy_le16(header.filename_length, pathlen);
-   copy_le16(header.extra_length, ZIP_EXTRA_MTIME_SIZE);
+   copy_le16(header.extra_length, ZIP_EXTRA_MTIME_SIZE + (clamped ? 
ZIP_EXTRA_ZIP64_SIZE : 0));
write_or_die(1, , ZIP_LOCAL_HEADER_SIZE);
zip_offset += ZIP_LOCAL_HEADER_SIZE;
write_or_die(1, path, pathlen);
zip_offset += pathlen;
write_or_die(1, , ZIP_EXTRA_MTIME_SIZE);
zip_offset += ZIP_EXTRA_MTIME_SIZE;
+   if (clamped) {
+   struct zip_extra_zip64 extra_zip64;
+   copy_le16(extra_zip64.magic, 0x0001);
+   copy_le16(extra_zip64.extra_size, ZIP_EXTRA_ZIP64_PAYLOAD_SIZE);
+   copy_le64(extra_zip64.size, size >= 0xU ? size : 0);
+   copy_le64(extra_zip64.compressed_size, compressed_size >= 
0xU ? compressed_size : 0);
+   copy_le64(extra_zip64.offset, zip_offset >= 0xU ? 
zip_offset : 0);
+   copy_le32(extra_zip64.disk, 0);


These are wrong, I think. Entries that did not overflow must be omitted 
entirely from the zip64 extra record, not filled with 0. This implies 
that the payload size (.extra_size) is dynamic.


As René pointed out, the offset is only written in the central 
directory, but not in the local header for the current file. Therefore, 
it must be omitted here. The disk number also never exceeds 0x and 
must be omitted as well.



+   write_or_die(1, _zip64, ZIP_EXTRA_ZIP64_SIZE);
+   zip_offset += ZIP_EXTRA_ZIP64_SIZE;
+   }
if (stream && method == 0) {
unsigned char buf[STREAM_BUFFER_SIZE];
ssize_t readlen;
@@ -538,7 +570,7 @@ static void write_zip_trailer(const unsigned char *sha1)
copy_le16(trailer.comment_length, sha1 ? GIT_SHA1_HEXSZ : 0);

write_or_die(1, zip_dir, zip_dir_offset);
-   if (clamped)
+   if (clamped || zip_zip64)
write_zip64_trailer();
write_or_die(1, , ZIP_DIR_TRAILER_SIZE);
if (sha1)



-- Hannes

Re: [PATCH] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Johannes Sixt]

Am 23.04.2017 um 08:42 schrieb Peter Krefting:

René Scharfe:

The offset is only needed in the ZIP64 extra record for the central
header (in zip_dir) -- the local header has no offset field.


Good point.


The zip64 local header does have an offset field, though. I thought that
was the zip_offset value, but that doesn't make sense, I'm not quite
sure what it is supposed to store. I need to investigate that further, I
assume.


Let's get the naming straight: There is no "zip64 local header". There 
is a "zip64 extra record" for the "zip local header". The zip64 extra 
data record has an offset field, but since the local header does not 
have an offset field, the offset field in the corresponding zip64 extra 
data record is always omitted.


-- Hannes

Re: [PATCH] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Peter Krefting]

René Scharfe:


The offset is declared as unsigned int, so will wrap on most platforms
before reaching the clamp check.  At least InfoZIP's unzip can handle
that, but it's untidy.


Right, that needs to be changed into unsigned long and clamped, just 
like the original and compressed file sizes already are.


The offset is only needed in the ZIP64 extra record for the central 
header (in zip_dir) -- the local header has no offset field.


The zip64 local header does have an offset field, though. I thought 
that was the zip_offset value, but that doesn't make sense, I'm not 
quite sure what it is supposed to store. I need to investigate that 
further, I assume.


--
\\// Peter - http://www.softwolves.pp.se/

git log --ignore-space-change -L start,end:file does not ignore indentation changes

[Дилян Палаузов]

Hello,

$ git version
git version 2.12.2.89.g49800c940

$ git init
Initialized empty Git repository in .git/

$ cat a.c
int main() {
 int result;
}

$git add a.c

$git commit -m I
[master (root-commit) ef9ab63] I
 1 file changed, 3 insertions(+)
 create mode 100644 a.c

$cat a.c  # now the indentation of result changed
cat a.c
int main() {
   int result;
   return 0;
}

$git commit -am J
[master 1bed130] J
 1 file changed, 2 insertions(+), 1 deletion(-)

$git -oneline -b -L:main:a.c
1bed130 J

diff --git a/a.c b/a.c
--- a/a.c
+++ b/a.c
@@ -1,3 +1,4 @@
 int main() {
- int result;
+   int result;
+   return 0;
 }
ef9ab63 I

diff --git a/a.c b/a.c
--- /dev/null
+++ b/a.c
@@ -0,0 +1,3 @@
+int main() {
+ int result;
+}

I expect that the result-line is not displayed as changed, when git log 
is called with -b.


Likewise:
git log --oneline  -b -L2,2:a.c
1bed130 J

diff --git a/a.c b/a.c
--- a/a.c
+++ b/a.c
@@ -2,1 +2,1 @@
- int result;
+   int result;
ef9ab63 I

diff --git a/a.c b/a.c
--- /dev/null
+++ b/a.c
@@ -0,0 +2,1 @@
+ int result;

Regards
  Дилян