Re: [PATCH RFC 02/24] ref-filter: add return value to some functions

[Оля Тележная]

2018-01-27 0:05 GMT+03:00 Junio C Hamano :
> Olga Telezhnaya  writes:
>
>> Add return flag to format_ref_array_item, show_ref_array_item,
>> get_ref_array_info and populate_value for further using.
>> Need it to handle situations when item is broken but we can not invoke
>> die() because we are in batch mode and all items need to be processed.
>>
>> Signed-off-by: Olga Telezhnaia 
>> Mentored-by: Christian Couder 
>> Mentored by: Jeff King 
>> ---
>>  ref-filter.c | 21 +
>>  ref-filter.h |  4 ++--
>>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> Makes sense as a preparatory step to pass the return status
> throughout the call chain.  The functions that actually will detect
> issues should probably gain
>
> /*
>  * a comment before the function
>  * that documents what it does and what values it returns
>  * to signal the failure.
>  */
>
> before them; those that only pass the errorcode through to the
> caller do not necessarily have to, though.

I will add comments, agree, thank you.

>
>> -void show_ref_array_item(struct ref_array_item *info,
>> +int show_ref_array_item(struct ref_array_item *info,
>>const struct ref_format *format)
>>  {
>>   struct strbuf final_buf = STRBUF_INIT;
>> + int retval = format_ref_array_item(info, format, _buf);
>>
>> - format_ref_array_item(info, format, _buf);
>>   fwrite(final_buf.buf, 1, final_buf.len, stdout);
>>   strbuf_release(_buf);
>> - putchar('\n');
>> + if (!retval)
>> + putchar('\n');
>> + return retval;
>
> This is questionable.  Why does it write final_buf out regardless of
> the return value, even though it refrains from writing terminating LF
> upon non-zero return from the same function that prepared final_buf?
>
> "Because final_buf is left unmodified when formatter returns an
> error, and calling fwrite on an empty buffer ends up being a no-op"
> is a wrong answer---it relies on having too intimate knowledge on
> how the callee happens to work currently.

I will change that piece of code by putting fwrite into if statement
also, thank you. We really do not put anything to buffer if retval is
not equal to zero (checked that).

Thank you for all your comments, waiting for further review.
Olga

Re: [PATCH 2/3] perf/aggregate: add --reponame option

[Christian Couder]

On Sun, Jan 28, 2018 at 8:57 PM, Eric Sunshine  wrote:
>
> Not a big deal, but the extra indentation (and noisy diff) could be
> avoided like this:
>
> my $environment;
> if ($reponame) {
> $environment = $reponame;
> } else if (exists ...) {
> ...as before
> }

Ok I will reroll with something like this and also the typo fix
suggested by Philip.

Thanks both.

Re: [PATCH RFC 03/24] cat-file: split expand_atom into 2 functions

[Оля Тележная]

2018-01-27 0:46 GMT+03:00 Junio C Hamano :
> Olga Telezhnaya  writes:
>
>> Split expand_atom function into 2 different functions,
>> expand_atom_into_fields prepares variable for further filling,
>> (new) expand_atom creates resulting string.
>> Need that for further reusing of formatting logic from ref-filter.
>>
>> Signed-off-by: Olga Telezhnaia 
>> Mentored-by: Christian Couder 
>> Mentored by: Jeff King 
>> ---
>>  builtin/cat-file.c | 73 
>> +-
>>  1 file changed, 39 insertions(+), 34 deletions(-)
>
> As expand_atom() is file-scope static and its callers are well
> isolated, it is OK to change its meaning while restructuring the
> code like this patch does (as opposed to a public function to which
> new callers may be added on other topics in flight).
>
> The split itself looks sensible, but expand_atom_into_fields() is a
> questionable name.  expand_atom() does fill the data in sb, but
> calling expand_atom_into_fields() does not fill any data into
> separated fields---it merely prepares somebody else to do so.
>
> Helped by this comment:
>
> /*
>  * If mark_query is true, we do not expand anything, but rather
>  * just mark the object_info with items we wish to query.
>  */
> int mark_query;
>
> we can guess that a better name would mention or hint "object_info",
> "query" and probably "prepare" (because we would do so before
> actually querying).

OK, I will rename that function. Actually, not sure that we really
need to have ideal name here because both functions will be deleted by
the end of this patch.
"mark_atom_in_object_info"?

>
> I am not sure if separating the logic into these two functions is a
> good way to organize things.  When a new %(atom) is introduced, it
> is more likely that a programmer adds it to one but forgets to make
> a matching change to the other, no?  (here, "I am not sure" is just
> that.  It is very different from "I am sure this is wrong").

In the end of the patch we don't have both of those functions, so
there would not be such problem.
But, I need that split, it helps me to go further and apply new
changes step-by-step.

>
> Thanks.

Re: [PATCH RFC 01/24] ref-filter: get rid of goto

[Оля Тележная]

2018-01-26 23:19 GMT+03:00 Junio C Hamano :
> Olga Telezhnaya  writes:
>
>> Get rid of goto command in ref-filter for better readability.
>>
>> Signed-off-by: Olga Telezhnaia 
>> Mentored-by: Christian Couder 
>> Mentored by: Jeff King 
>> ---
>
> How was this patch "mentored by" these two folks?  Have they already
> reviewed and gave you OK, or are you asking them to also help reviewing
> with this message?  Mostly just being curious.

Christian and Jeff help me when I have different sort of difficulties.
Not sure that they were helping me with that commit separately.
Both of them reviewed my code and said that it's ready for a final
review (actually, Christian said, but it's usual situation when I ask
for help/review and one of them helps me. The other one could add
something, but, as I understand, if he totally agree, he will keep
silence, and I find that behavior logical).
Do I need to delete these lines from some of commits where I do not
remember help from them?

>
> It is not convincning that this splitting the last part of a single
> function into a separate helper function that is called from only
> one place improves readability.  If better readability is the
> purpose, I would even say
>
>  for (i = 0; i < used_atom_cnt; i++) {
> if (...)
> -   goto need_obj;
> +   break;
> }
> -   return;
> +   if (used_atom_cnt <= i)
> return;
>
> -need_obj:
>
> would make the result easier to follow with a much less impact.

It's hard for me to read the code with goto, and as I know, it's not
only my problem, it's usual situation to try to get rid of gotos. I
always need to re-check whether we use that piece of code somewhere
else or not, and how we do that. I also think that it's good that most
of variables in the beginning of the function populate_value go to new
function.

>
> If we later in the series will use this new helper function from
> other places, it certainly makes sense to create a new helper like
> this patch does, but then "get rid of goto for readability" is not
> the justification for such a change.

We don't use that new function anywhere else further. So, I can delete
this commit or I can change commit message (if so, please give me some
ideas what I need to mention there).

>
>>  ref-filter.c | 103 
>> ++-
>>  1 file changed, 53 insertions(+), 50 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index f9e25aea7a97e..37337b57aacf4 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1354,16 +1354,60 @@ static const char *get_refname(struct used_atom 
>> *atom, struct ref_array_item *re
>>   return show_ref(>u.refname, ref->refname);
>>  }
>>
>> +static void need_object(struct ref_array_item *ref) {
>
> Style.  The opening brace at the beginning of the function sits on
> its own line alone.

Thanks, I will fix that when we decide how to finally improve that commit.

Olga

Dear Sir/Madam!

[Mimi ibrahim Couibaly]

Dear Sir/Madam,

Good day !

Please permit me to introduce myself, I am MIMI IBRAHIM COULIBALY 17 years old 
female from the Republic of Ivory Coast,  in Abidjan; I'm the Daughter of Late 
Chief Sgt. Ibrahim Coulibaly (A.K.A General  Warlord IB ). My late Father was a 
well known Ivory Coast military leader. He died on Thursday 28 April 2011 
following a fight with the Republican Forces of Ivory Coast (FRCI).

You can read more about my late father in the news graduating of ivory cost  in 
bouake.

www.guardian.co.uk/world/2011/apr/28/ivory-coast-renegade-warlord-ibrahim-coulibaly

Please I want to leave my situation here to come over to your country to 
continue my education.

I have made research before contacting you and please I want you to help me 
come to your country to start my new life.
I will tell you more about my condition if you are willing to help me come over 
to your country to continue my education.
I will truly appreciate your help to my life for my education.

I will be happy to hear from you

e-mail.mimiibrahimcouib...@gmail.com

Miss. MIMI IBRAHIM COULIBALY

Re: t9128 failing randomly with svn 1.9?

[Todd Zullinger]

brian m. carlson wrote:
> While running tests for my object_id part 11 series, I noticed a random
> segfault in git svn while running t9128.  I've reproduced this on a
> different machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian
> stable and unstable).  It is reproducible on master.
> 
> When the test fails, it fails on the "git svn tag tag3" step, and I get
> the following:
> 
> Copying 
> file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk
>  at r2 to 
> file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3...
> Found possible branch point: 
> file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk
>  => 
> file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3,
>  2
> Found branch parent: (refs/remotes/origin/tags/tag3) 
> 0604824a81a121ad05aaf8caea65d8ca8f86c018
> Following parent with do_switch
> Successfully followed parent
> r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)
> error: git-svn died of signal 11
> 
> Doing the following three times, I had two crashes.
> 
> (set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh 
> --verbose); done)
> 
> I'm not really familiar with git svn or its internals, and I didn't see
> anything recently on the list about this.  Is this issue known?

I see the same test fail randomly on Fedora (and I think on
CentOS/RHEL, but I run the full tests more often on Fedora).

For me, it's tests 3 and 4 which fail with the same error.
I thought it was a failure in subversion or the perl
bindings rather than git, so I simply disabled them in the
Fedora builds.  The Debian packages skip 9128 as well (and
9167, which fails similarly).

I've seen the failures in t9141 from 'git svn branch' as
well.  I made a note to re-enable those tests after Jeff's
work to make it easier to run with shell tracing enabled by
default, but have not done so yet.

The 'git svn' tests are not run in Travis because the perl
subversion bindings are not installed.  I haven't made time
to try installing them and running the tests in Travis to
see if the failures occur there, but I suspect they would.

-- 
Todd
~~
Going to trial with a lawyer who considers your whole life-style a
Crime in Progress is not a happy prospect.
-- Hunter S. Thompson



signature.asc
Description: PGP signature

t9128 failing randomly with svn 1.9?

[brian m. carlson]

While running tests for my object_id part 11 series, I noticed a random
segfault in git svn while running t9128.  I've reproduced this on a
different machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian
stable and unstable).  It is reproducible on master.

When the test fails, it fails on the "git svn tag tag3" step, and I get
the following:

Copying 
file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk
 at r2 to 
file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3...
Found possible branch point: 
file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk
 => 
file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3,
 2
Found branch parent: (refs/remotes/origin/tags/tag3) 
0604824a81a121ad05aaf8caea65d8ca8f86c018
Following parent with do_switch
Successfully followed parent
r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)
error: git-svn died of signal 11

Doing the following three times, I had two crashes.

(set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh 
--verbose); done)

I'm not really familiar with git svn or its internals, and I didn't see
anything recently on the list about this.  Is this issue known?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Jan 28, 2018 at 5:58 PM, Lucas Werkmeister
 wrote:
> On 28.01.2018 07:40, Eric Sunshine wrote:
>> On Sat, Jan 27, 2018 at 1:31 PM, Lucas Werkmeister
>>  wrote:
>>> This makes it possible to use --inetd while still logging to standard
>>> error. --syslog is retained as an alias for --send-log-to=syslog. A mode
>>> to disable logging explicitly is also provided.
>>>
>>> The combination of --inetd with --send-log-to=stderr is useful, for
>>> instance, when running `git daemon` as an instanced systemd service
>>> (with associated socket unit). In this case, log messages sent via
>>> syslog are received by the journal daemon, but run the risk of being
>>> processed at a time when the `git daemon` process has already exited
>>> (especially if the process was very short-lived, e.g. due to client
>>> error), so that the journal daemon can no longer read its cgroup and
>>> attach the message to the correct systemd unit (see systemd/systemd#2913
>>> [1]). Logging to stderr instead can solve this problem, because systemd
>>> can connect stderr directly to the journal daemon, which then already
>>> knows which unit is associated with this stream.
>>
>> The purpose of this patch would be easier to fathom if the problem was
>> presented first (systemd race condition), followed by the solution
>> (ability to log to stderr even when using --inetd), followed finally
>> by incidental notes ("--syslog is retained as an alias..." and ability
>> to disable logging).
>>
>> Not sure, though, if it's worth a re-roll.
>
> I didn’t want to sound like I was just scratching my own itch ;) I hope
> this option is useful for other use-cases as well?

If the reader does not know that --inetd implies --syslog, then

This makes it possible to use --inetd while still logging to
standard error.

leads to a "Huh?" moment since it is not self-contained. Had it said

Add new option --send-log-to=(stderr|syslog|none) which
allows the implied --syslog by --inetd to be overridden.

it would have provided enough information to understand the purpose of
the patch at a glance. Talking about the systemd race-condition first
would also have explained the patch's purpose, and since the proposed
solution is general (not specific to your use-case), scratching an
itch is not a point against it.

Anyhow, it's not that big of a deal, but it did give me a bit of a
pause when reading the first paragraph since it's customary on this
project to start by explaining the problem.

>> I understand that Junio suggested the name --send-log-to=, but I
>> wonder if the more concise --log= would be an possibility.
>
> --log sounds to me like it could also indicate *what* to log (e. g. “log
> verbosely” or “don’t log client IPs”). But perhaps --log-to= ?

Perhaps we can take into consideration precedent by other (non-Git)
daemon-like commands when naming this option. (None come to my mind
immediately, but I'm sure they're out there.)

>>> if (!strcmp(arg, "--inetd")) {
>>> inetd_mode = 1;
>>> -   log_syslog = 1;
>>> +   log_destination = LOG_TO_SYSLOG;
>>
>> Hmm, so an invocation "--inetd --send-log-to=stderr" works as
>> expected, but "--send-log-to=stderr --inetd" doesn't; output goes to
>> syslog despite the explicit request for stderr. Counterintuitive. This
>> should probably distinguish between 'log_destination' being unset and
>> set explicitly; if unset, then, and only then, have --inetd imply
>> syslog. Perhaps something like this:
>>
>> static enum log_destination {
>> LOG_TO_UNSET = -1
>> LOG_TO_NONE,
>> LOG_TO_STDERR,
>> LOG_TO_SYSLOG,
>> } log_destination = LOG_TO_UNSET;
>>
>> if (!strcmp(arg, "--inetd")) {
>> inetd_mode = 1;
>> if (log_destination == LOG_TO_UNSET)
>> log_destination = LOG_TO_SYSLOG;
>> ...
>> }
>> ...
>> if (log_destination == LOG_TO_UNSET)
>> log_destination = LOG_TO_STDERR
>>
>
> I’m not sure if that’s worth the extra complication… some existing
> options behave the same way already, e. g. in `git rebase --stat
> --quiet`, the `--stat` is ignored.

I took "last one wins" into consideration when writing the above but
was not convinced that it applies to this case since --inetd and
--send-log-to= have no obvious relation to one another (unlike, say,
--verbose and --quiet or other similar combinations). Unless one reads
the documentation very closely, output ending up in syslog despite
"--send-log-to=stderr --inetd" is just way too counterintuitive and
may well lead to bug reports later on. Therefore, doing the additional
work now to stave off such bug reports is likely worthwhile.

>>> +   }
>>> +   else if (!strcmp(v, "stderr")) {
>>
>> Style: cuddle 'else' with the braces: } else if (...) {
>>
>
> Is that a general rule? I couldn’t find anything 

Urgent Message

[Lucy Nnani]

Dearest

I am Mrs. Lucy Nnani, 68 years of age from England, married to an Australian 
citizen I came across your profile and decided to mail you, I am a cancer 
patient . looking for somebody I can trust to join hands together and use my 
life savings to help the less privilege since I know my days are numbered.

I look forward to your reply for further explanation

Yours Faithfully
Mrs. Lucy Nnani
Email: lucynn...@gmail.com

Re: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Lucas Werkmeister]

On 28.01.2018 07:40, Eric Sunshine wrote:
> On Sat, Jan 27, 2018 at 1:31 PM, Lucas Werkmeister
>  wrote:
>> This makes it possible to use --inetd while still logging to standard
>> error. --syslog is retained as an alias for --send-log-to=syslog. A mode
>> to disable logging explicitly is also provided.
>>
>> The combination of --inetd with --send-log-to=stderr is useful, for
>> instance, when running `git daemon` as an instanced systemd service
>> (with associated socket unit). In this case, log messages sent via
>> syslog are received by the journal daemon, but run the risk of being
>> processed at a time when the `git daemon` process has already exited
>> (especially if the process was very short-lived, e.g. due to client
>> error), so that the journal daemon can no longer read its cgroup and
>> attach the message to the correct systemd unit (see systemd/systemd#2913
>> [1]). Logging to stderr instead can solve this problem, because systemd
>> can connect stderr directly to the journal daemon, which then already
>> knows which unit is associated with this stream.
> 
> The purpose of this patch would be easier to fathom if the problem was
> presented first (systemd race condition), followed by the solution
> (ability to log to stderr even when using --inetd), followed finally
> by incidental notes ("--syslog is retained as an alias..." and ability
> to disable logging).
> 
> Not sure, though, if it's worth a re-roll.

I didn’t want to sound like I was just scratching my own itch ;) I hope
this option is useful for other use-cases as well?

> 
>> Signed-off-by: Lucas Werkmeister 
>> Helped-by: Ævar Arnfjörð Bjarmason 
>> Helped-by: Junio C Hamano 
>> ---
>>
>> Notes:
>> This was originally “daemon: add --no-syslog to undo implicit
>> --syslog”, but Junio pointed out that combining --no-syslog with
>> --detach isn’t especially useful and suggested --send-log-to=
>> instead. Is Helped-by: the right credit for this or should it be
>> something else?
> 
> Helped-by: is fine, though typically your Signed-off-by: would be last.
> 
> I understand that Junio suggested the name --send-log-to=, but I
> wonder if the more concise --log= would be an possibility.

--log sounds to me like it could also indicate *what* to log (e. g. “log
verbosely” or “don’t log client IPs”). But perhaps --log-to= ?

> 
> More below...
> 
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> @@ -110,8 +111,26 @@ OPTIONS
>> +--send-log-to=::
>> +   Send log messages to the specified destination.
>> +   Note that this option does not imply --verbose,
>> +   thus by default only error conditions will be logged.
>> +   The  defaults to `stderr`, and must be one of:
> 
> Perhaps also update the documentation of --inetd to mention that its
> implied --syslog can be overridden by --send-log-to=.

Very good idea!

> 
>> diff --git a/daemon.c b/daemon.c
>> @@ -74,11 +79,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>>
>>  static void logreport(int priority, const char *err, va_list params)
>>  {
>> -   if (log_syslog) {
>> +   switch (log_destination) {
>> +   case LOG_TO_SYSLOG: {
>> char buf[1024];
>> vsnprintf(buf, sizeof(buf), err, params);
>> syslog(priority, "%s", buf);
>> -   } else {
>> +   break;
>> +   }
>> +   case LOG_TO_STDERR: {
> 
> There aren't many instances of:
> 
> case FOO: {
> 
> in the code-base, but those that exist don't use braces around cases
> which don't need it, so perhaps drop it from the STDERR and NONE
> cases. (Probably not worth a re-roll, though.)

Good point, forgot that part of the coding guidelines. Only the syslog
case needs it because the buf declaration can’t be labeled directly.

> 
>> /*
>>  * Since stderr is set to buffered mode, the
>>  * logging of different processes will not overlap
>> @@ -88,6 +96,11 @@ static void logreport(int priority, const char *err, 
>> va_list params)
>> vfprintf(stderr, err, params);
>> fputc('\n', stderr);
>> fflush(stderr);
>> +   break;
>> +   }
>> +   case LOG_TO_NONE: {
>> +   break;
>> +   }
>> }
> 
> Consecutive lines with braces at the same indentation level is rather
> odd (but see previous comment).
> 
>>  }
>>
>> @@ -1289,7 +1302,7 @@ int cmd_main(int argc, const char **argv)
>> }
>> if (!strcmp(arg, "--inetd")) {
>> inetd_mode = 1;
>> -   log_syslog = 1;
>> +   log_destination = LOG_TO_SYSLOG;
> 
> Hmm, so an invocation "--inetd --send-log-to=stderr" works as
> expected, but "--send-log-to=stderr --inetd" doesn't; output goes to
> syslog despite the explicit request for stderr. 

Re: Some rough edges of core.fsmonitor

[Ævar Arnfjörð Bjarmason]

On Sun, Jan 28, 2018 at 9:44 PM, Johannes Schindelin
 wrote:
> Hi,
>
> On Sat, 27 Jan 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> I just got around to testing this since it landed, for context some
>> previous poking of mine in [1].
>>
>> Issues / stuff I've noticed:
>>
>> 1) We end up invalidating the untracked cache because stuff in .git/
>> changed. For example:
>>
>> 01:09:24.975524 fsmonitor.c:173 fsmonitor process 
>> '.git/hooks/fsmonitor-watchman' returned success
>> 01:09:24.975548 fsmonitor.c:138 fsmonitor_refresh_callback '.git'
>> 01:09:24.975556 fsmonitor.c:138 fsmonitor_refresh_callback 
>> '.git/config'
>> 01:09:24.975568 fsmonitor.c:138 fsmonitor_refresh_callback 
>> '.git/index'
>> 01:09:25.122726 fsmonitor.c:91  write fsmonitor extension 
>> successful
>>
>> Am I missing something or should we do something like:
>>
>> diff --git a/fsmonitor.c b/fsmonitor.c
>> index 0af7c4edba..5067b89bda 100644
>> --- a/fsmonitor.c
>> +++ b/fsmonitor.c
>> @@ -118,7 +118,12 @@ static int query_fsmonitor(int version, uint64_t 
>> last_update, struct strbuf *que
>>
>>  static void fsmonitor_refresh_callback(struct index_state *istate, 
>> const char *name)
>>  {
>> -   int pos = index_name_pos(istate, name, strlen(name));
>> +   int pos;
>> +
>> +   if (!strcmp(name, ".git") || starts_with(name, ".git/"))
>> +   return;
>> +
>> +   pos = index_name_pos(istate, name, strlen(name));
>
> I would much rather have the fsmonitor hook already exclude those.

As documented the fsmonitor-watchman hook we ship doesn't work as
described in githooks(5), unless "files in the working directory" is
taken to include .git/, but I haven't seen that ever used.

On the other hand relying on arbitrary user-provided hooks to do the
right thing at the cost of silent performance degradation is bad. If
we're going to expect the hook to remove these we should probably
warn/die here if it does send us .git/* files.

> If you *must* add these comparisons, you have to use fspathcmp() and
> fspathncmp() instead (because case-insensitivity).

Thanks.

Re: git send-email sets date

[Ævar Arnfjörð Bjarmason]

On Fri, Jan 26, 2018 at 6:32 PM, Michal Suchánek  wrote:
> This is wrong because the message will most likely not get delivered
> when the author date differs from current time.

Others have covered other bases here, but I just wanted to ask about
this. Are there really mail setups that refuse to deliver or accept
messages whose Date headers don't match what the expect? I would think
that such issues wouldn't be present in the wild since SMTP daemons
need to deal with messages that are e.g. held locally somewhere, or
the only make it to your server days afterwards due to your own
downtime + client retries.

Now if by "not get delivered" you mean they'll show up on the Nth page
of your mailer because it sorts by the Date header, sure. That's a
problem and quite common, tricky to solve due to the issues others
have noted though.

Re: git send-email sets date

[Theodore Ts'o]

On Sun, Jan 28, 2018 at 03:56:57PM -, Philip Oakley wrote:
> Michal, you may want to hack up an option that can automatically create 
> that format if it is of use. I sometimes find the sort order an issue in 
> some of my mail clients.

If there is a From: header in the beginning of the mail body, it is
used as the Author instead of the From: header in the mail header.  It
would make sense if there is a Date: header in the beginning of the
mail body, it should be used instead of Date: field in the mail header.

The problem is that if existing git clients don't support this, it
wouldn't be safe to start emmiting patches with that format for at
least a year or two until the prerequisite version of git gets wide
adoption.  Alternatively, there could be a git option which causes
something like X-Git-Author-Date: to be set in the mail header.

- Ted

Re: [PATCH 00/12] object_id part 11 (the_hash_algo)

[brian m. carlson]

On Sun, Jan 28, 2018 at 09:48:28PM +0100, Patryk Obara wrote:
> I looked at your branch object-id-part-12, and it conflicts with my next
> batch of object_id conversions in quite many places (mostly through
> formatting). Therefore I'll hold my horses and postpone my conversion
> patches at least until part 12 will be sent.

Okay.  I've just rebased it on top of your object_id series, so I'll
reroll this series and then once both series get picked up, rebase and
submit it on top.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 08/12] pack-write: switch various SHA-1 values to abstract forms

[brian m. carlson]

On Sun, Jan 28, 2018 at 09:30:36PM +0100, Patryk Obara wrote:
> On 28/01/2018 16:57, brian m. carlson wrote:
> > if (partial_pack_offset == 0) {
> > -   unsigned char sha1[20];
> > -   git_SHA1_Final(sha1, _sha1_ctx);
> > -   if (hashcmp(sha1, partial_pack_sha1) != 0)
> > +   unsigned char hash[GIT_MAX_RAWSZ];
> > +   the_hash_algo->final_fn(hash, _hash_ctx);
> > +   if (hashcmp(hash, partial_pack_hash) != 0)
> 
> Maybe "hash" should be struct object_id here?

In this case, I opted not to do that because it's specifically not an
object ID.  It's a checksum for the pack, which isn't a normal Git
object, so I tried to preserve that distinction.

> >   char *index_pack_lockfile(int ip_out)
> >   {
> > -   char packname[46];
> > +   char packname[GIT_MAX_HEXSZ + 6];
> > +   int len = the_hash_algo->hexsz + 6;
> 
> Just me nitpicking, but "len" can be const :)

I wanted it to be const, too, but I recall getting feedback discouraging
it.  I"m happy to make the change; after all, it can only help the
compiler and any future readers.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 00/12] object_id part 11 (the_hash_algo)

[Patryk Obara]

I looked at your branch object-id-part-12, and it conflicts with my next
batch of object_id conversions in quite many places (mostly through
formatting). Therefore I'll hold my horses and postpone my conversion
patches at least until part 12 will be sent.

--
| ← Ceci n'est pas une pipe
Patryk Obara

Re: Some rough edges of core.fsmonitor

[Johannes Schindelin]

Hi,

On Sat, 27 Jan 2018, Ævar Arnfjörð Bjarmason wrote:

> I just got around to testing this since it landed, for context some
> previous poking of mine in [1].
> 
> Issues / stuff I've noticed:
> 
> 1) We end up invalidating the untracked cache because stuff in .git/
> changed. For example:
> 
> 01:09:24.975524 fsmonitor.c:173 fsmonitor process 
> '.git/hooks/fsmonitor-watchman' returned success
> 01:09:24.975548 fsmonitor.c:138 fsmonitor_refresh_callback '.git'
> 01:09:24.975556 fsmonitor.c:138 fsmonitor_refresh_callback 
> '.git/config'
> 01:09:24.975568 fsmonitor.c:138 fsmonitor_refresh_callback 
> '.git/index'
> 01:09:25.122726 fsmonitor.c:91  write fsmonitor extension 
> successful
> 
> Am I missing something or should we do something like:
> 
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 0af7c4edba..5067b89bda 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -118,7 +118,12 @@ static int query_fsmonitor(int version, uint64_t 
> last_update, struct strbuf *que
> 
>  static void fsmonitor_refresh_callback(struct index_state *istate, const 
> char *name)
>  {
> -   int pos = index_name_pos(istate, name, strlen(name));
> +   int pos;
> +
> +   if (!strcmp(name, ".git") || starts_with(name, ".git/"))
> +   return;
> +
> +   pos = index_name_pos(istate, name, strlen(name));

I would much rather have the fsmonitor hook already exclude those.

If you *must* add these comparisons, you have to use fspathcmp() and
fspathncmp() instead (because case-insensitivity).

Ciao,
Dscho

Re: [PATCH 08/12] pack-write: switch various SHA-1 values to abstract forms

[Patryk Obara]

On 28/01/2018 16:57, brian m. carlson wrote:

if (partial_pack_offset == 0) {
-   unsigned char sha1[20];
-   git_SHA1_Final(sha1, _sha1_ctx);
-   if (hashcmp(sha1, partial_pack_sha1) != 0)
+   unsigned char hash[GIT_MAX_RAWSZ];
+   the_hash_algo->final_fn(hash, _hash_ctx);
+   if (hashcmp(hash, partial_pack_hash) != 0)


Maybe "hash" should be struct object_id here?


  char *index_pack_lockfile(int ip_out)
  {
-   char packname[46];
+   char packname[GIT_MAX_HEXSZ + 6];
+   int len = the_hash_algo->hexsz + 6;


Just me nitpicking, but "len" can be const :)

--
| ← Ceci n'est pas une pipe
Patryk Obara

Re: [PATCH 02/12] hash: create union for hash context allocation

[brian m. carlson]

On Sun, Jan 28, 2018 at 08:57:21PM +0100, Patryk Obara wrote:
> On 28/01/2018 16:57, brian m. carlson wrote:
> > In various parts of our code, we want to allocate a structure
> > representing the internal state of a hash algorithm.  The original
> > implementation of the hash algorithm abstraction assumed we would do
> > that using heap allocations, and added a context size element to struct
> > git_hash_algo.  However, most of the existing code uses stack
> > allocations and conversion would needlessly complicate various parts of
> > the code.  Add a union for the purpose of allocating hash contexts on
> > the stack and a typedef for ease of use.  Remove the ctxsz element for
> > struct git_hash_algo, which is no longer very useful.
> 
> Overall, I am OK with this approach (it's straightforward change and
> cleanest way to replace direct calls to git_SHA1_* functions), but just to
> play devil's advocate: OpenSSL decided to sway users into heap allocated
> contexts, citing binary compatibility issues if they change the size of
> context structure. [1]
> 
> I think we might need to revisit this design decision in future - perhaps as
> soon as we'll transition away from calling git_SHA1_* functions directly.

The approach I took was to keep the code as similar as possible to
what's there already.  If our hash implementation wants to use pointers,
it's okay for it to define git_SHA1_CTX to a pointer type, and
everything should still work.  We treat the type as fully opaque anyway
(outside of the actual hash implementation).

> > +/* A suitably aligned type for stack allocations of hash contexts. */
> > +union git_hash_ctx {
> > +   git_SHA_CTX sha1;
> > +};
> > +typedef union git_hash_ctx git_hash_ctx;
> > +
> >   typedef void (*git_hash_init_fn)(void *ctx);
> >   typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
> >   typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);
> 
> I think it would be appropriate to replace "void *ctx" with "git_hash_ctx
> *ctx". This way we can avoid unnecessary casting in git_hash_sha1_*
> functions.

Yeah, that does make more sense.  I'll make that change.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 09/12] read-cache: abstract away uses of SHA-1

[brian m. carlson]

On Sun, Jan 28, 2018 at 02:50:18PM -0500, Eric Sunshine wrote:
> On Sun, Jan 28, 2018 at 10:57 AM, brian m. carlson
>  wrote:
> > Convert various uses of direct calls to SHA-1 and 20- and 40-based
> > constants to use the_hash_algo instead.  Don't yet convert the on-disk
> > data structures, which will be handled in a future commit.
> >
> > Signed-off-by: brian m. carlson 
> > ---
> > diff --git a/read-cache.c b/read-cache.c
> > @@ -2000,26 +2000,26 @@ static int write_index_ext_header(git_SHA_CTX 
> > *context, int fd,
> > /* Flush first if not enough space for SHA1 signature */
> 
> Did you want to update the comment to remove the SHA1 reference also?
> (Or was the omission intentional per the commit message?)

These weren't intentional, so I'll fix them in a reroll.

The only reason I didn't touch the ondisk data structures is because
they're directly mmap'd.  So if I change them to use struct object_id,
as soon as we expand the structure size, the ondisk structures become
unusable.  Some sort of union hack or multiple subroutines will probably
be required.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 06/12] fast-import: switch various uses of SHA-1 to the_hash_algo

[Patryk Obara]

On 28/01/2018 16:57, brian m. carlson wrote:

-   if (last && last->data.buf && last->depth < max_depth && dat->len > 20) 
{
+   if (last && last->data.buf && last->depth < max_depth && dat->len > 
the_hash_algo->rawsz) {


At this point line is almost 100 characters long - maybe it's time to
break it ;)

--
| ← Ceci n'est pas une pipe
Patryk Obara

Re: [PATCH 02/12] hash: create union for hash context allocation

[Patryk Obara]

On 28/01/2018 16:57, brian m. carlson wrote:

In various parts of our code, we want to allocate a structure
representing the internal state of a hash algorithm.  The original
implementation of the hash algorithm abstraction assumed we would do
that using heap allocations, and added a context size element to struct
git_hash_algo.  However, most of the existing code uses stack
allocations and conversion would needlessly complicate various parts of
the code.  Add a union for the purpose of allocating hash contexts on
the stack and a typedef for ease of use.  Remove the ctxsz element for
struct git_hash_algo, which is no longer very useful.


Overall, I am OK with this approach (it's straightforward change and 
cleanest way to replace direct calls to git_SHA1_* functions), but just 
to play devil's advocate: OpenSSL decided to sway users into heap 
allocated contexts, citing binary compatibility issues if they change 
the size of context structure. [1]


I think we might need to revisit this design decision in future - 
perhaps as soon as we'll transition away from calling git_SHA1_* 
functions directly.



+/* A suitably aligned type for stack allocations of hash contexts. */
+union git_hash_ctx {
+   git_SHA_CTX sha1;
+};
+typedef union git_hash_ctx git_hash_ctx;
+
  typedef void (*git_hash_init_fn)(void *ctx);
  typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
  typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);


I think it would be appropriate to replace "void *ctx" with 
"git_hash_ctx *ctx". This way we can avoid unnecessary casting in 
git_hash_sha1_* functions.


[1] https://wiki.openssl.org/index.php/Manual:EVP_DigestInit(3)#NOTES

--
| ← Ceci n'est pas une pipe
Patryk Obara

Re: [PATCH 2/3] perf/aggregate: add --reponame option

[Eric Sunshine]

On Sun, Jan 28, 2018 at 6:18 AM, Christian Couder
 wrote:
> This makes it easier to use the aggregate script
> on the command line when one wants to get the
> "environment" fields set in the codespeed output.
>
> Previously setting GIT_REPO_NAME was needed
> for this purpose.
>
> Signed-off-by: Christian Couder 
> ---
> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> @@ -209,15 +218,17 @@ sub print_codespeed_results {
> -   my $environment;
> -   if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne 
> "") {
> -   $environment = $ENV{GIT_PERF_REPO_NAME};
> -   } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} 
> ne "") {
> -   $environment = $ENV{GIT_TEST_INSTALLED};
> -   $environment =~ s|/bin-wrappers$||;
> -   } else {
> -   $environment = `uname -r`;
> -   chomp $environment;
> +   my $environment = $reponame;
> +   if (! $environment) {
> +   if (exists $ENV{GIT_PERF_REPO_NAME} and 
> $ENV{GIT_PERF_REPO_NAME} ne "") {
> +   $environment = $ENV{GIT_PERF_REPO_NAME};
> +   } elsif (exists $ENV{GIT_TEST_INSTALLED} and 
> $ENV{GIT_TEST_INSTALLED} ne "") {
> +   $environment = $ENV{GIT_TEST_INSTALLED};
> +   $environment =~ s|/bin-wrappers$||;
> +   } else {
> +   $environment = `uname -r`;
> +   chomp $environment;
> +   }
> }

Not a big deal, but the extra indentation (and noisy diff) could be
avoided like this:

my $environment;
if ($reponame) {
$environment = $reponame;
} else if (exists ...) {
...as before
}

Re: [PATCH 09/12] read-cache: abstract away uses of SHA-1

[Eric Sunshine]

On Sun, Jan 28, 2018 at 10:57 AM, brian m. carlson
 wrote:
> Convert various uses of direct calls to SHA-1 and 20- and 40-based
> constants to use the_hash_algo instead.  Don't yet convert the on-disk
> data structures, which will be handled in a future commit.
>
> Signed-off-by: brian m. carlson 
> ---
> diff --git a/read-cache.c b/read-cache.c
> @@ -2000,26 +2000,26 @@ static int write_index_ext_header(git_SHA_CTX 
> *context, int fd,
> /* Flush first if not enough space for SHA1 signature */

Did you want to update the comment to remove the SHA1 reference also?
(Or was the omission intentional per the commit message?)

> -   if (left + 20 > WRITE_BUFFER_SIZE) {
> +   if (left + the_hash_algo->rawsz > WRITE_BUFFER_SIZE) {
> if (write_in_full(fd, write_buffer, left) < 0)
> return -1;
> left = 0;
> }
>
> /* Append the SHA1 signature at the end */

Ditto.

> -   git_SHA1_Final(write_buffer + left, context);
> -   hashcpy(sha1, write_buffer + left);
> -   left += 20;
> +   the_hash_algo->final_fn(write_buffer + left, context);
> +   hashcpy(hash, write_buffer + left);
> +   left += the_hash_algo->rawsz;

[PATCH] doc: mention 'git show' defaults to HEAD

[Todd Zullinger]

When 'git show' is called without any object it defaults to HEAD.  This
has been true since d4ed9793fd ("Simplify common default options setup
for built-in log family.", 2006-04-16).

The SYNOPSIS suggests that the object argument is required.  Clarify
that it is not required and note the default.

Signed-off-by: Todd Zullinger 
---
This was mentioned in #git today by qaz.  It seems reasonable to document the
default.

 Documentation/git-show.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index 82a4125a2d..e73ef54017 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -9,7 +9,7 @@ git-show - Show various types of objects
 SYNOPSIS
 
 [verse]
-'git show' [options] ...
+'git show' [options] [...]
 
 DESCRIPTION
 ---
@@ -35,7 +35,7 @@ This manual page describes only the most frequently used 
options.
 OPTIONS
 ---
 ...::
-   The names of objects to show.
+   The names of objects to show (defaults to 'HEAD').
For a more complete list of ways to spell object names, see
"SPECIFYING REVISIONS" section in linkgit:gitrevisions[7].
 
-- 
2.16.1

Re: [RFC PATCH 0/2] alternate hash test

[brian m. carlson]

On Sun, Jan 28, 2018 at 07:58:10PM +0100, Ævar Arnfjörð Bjarmason wrote:
> On Sun, Jan 28, 2018 at 6:06 PM, brian m. carlson
>  wrote:
> If the goal is to smoke out hardcoded SHA1s in tests, isn't it easier
> to instrument SHA-1 (e.g. our blk_sha1 copy, or our wrappers) to
> pretend that whenever we ask for the hash for STRING to pretend we
> asked for SOME_PREFIX + STRING?
> 
> Such an approach would have the advantage of being more portable
> (easier to run these mock test), and also that if we ever move to
> NewHash we could still test for this, we'd just always set the prefix
> to compilation time(), and could thus guarantee that the hashes would
> change every time git was built.

That's certainly a possibility.  We could simply call the update
function from the init function and prepend a NUL byte or something like
that, which would definitely produce different results.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/2] alternate hash test

[Ævar Arnfjörð Bjarmason]

On Sun, Jan 28, 2018 at 6:06 PM, brian m. carlson
 wrote:
> For this test, I picked BLAKE2b-160 for a couple reasons:
> * Debian ships a libb2-1 package which can be used easily (in other
>   words, I was lazy and didn't want to add a crypto implementation just
>   for test purposes);
> [...]
> This isn't an endorsement for or against any particular algorithm
> choice, just an artifact of the tools that were easily available to me.
> Provoking discussion of which hash to pick for NewHash is explicitly
> *not* a goal for this series.  I'm only interested in the ability to
> identify and fix tests.

If the goal is to smoke out hardcoded SHA1s in tests, isn't it easier
to instrument SHA-1 (e.g. our blk_sha1 copy, or our wrappers) to
pretend that whenever we ask for the hash for STRING to pretend we
asked for SOME_PREFIX + STRING?

Such an approach would have the advantage of being more portable
(easier to run these mock test), and also that if we ever move to
NewHash we could still test for this, we'd just always set the prefix
to compilation time(), and could thus guarantee that the hashes would
change every time git was built.

Re: [RFC PATCH 0/2] alternate hash test

[SZEDER Gábor]

> This series wires up an alternate hash implementation, namely
> BLAKE2b-160.  The goal is to allow us to identify tests which rely on
> the hash algorithm in use so that we can fix those tests.

> t9903-bash-prompt.sh (Wstat: 256 Tests: 66 
> Failed: 53)
>   Failed tests:  4, 6-10, 14-34, 36, 39-62, 65
>   Non-zero exit status: 1

I didn't recall seeing hardcoded SHA-1s in the bash prompt tests, so
went to have a look.  And indeed, these failures are not the test's
fault, but the result of a segfault in 'git checkout' while trying to
return from an orphan branch in test 4.  The rest of the failures are
mostly fallout caused by the wrong branch being checked out or the
leftover index.lock.

$ ./t9903-bash-prompt.sh -v -i
<...>
expecting success: 
  printf " (unborn)" >expected &&
  git checkout --orphan unborn &&
  test_when_finished "git checkout master" &&
  __git_ps1 >"$actual" &&
  test_cmp expected "$actual"

Switched to a new branch 'unborn'
./test-lib.sh: line 609: 29342 Segmentation fault  git checkout master
not ok 4 - prompt - unborn branch


$ gdb --args ../../git checkout master
<...>
Program received signal SIGSEGV, Segmentation fault.
0x0041b636 in merge_working_tree (opts=0x7fffd200, 
old=0x7fffd0d0, new=0x7fffd1e0,
writeout_error=0x7fffd0c0)
at builtin/checkout.c:525
525 init_tree_desc([0], tree->buffer,
tree->size);
(gdb) p tree 
$1 = (struct tree *) 0x0


The root cause is that patch 2/2 misses places where EMPTY_TREE_SHA1_*
constants should have been replaced with their EMPTY_TREE_SBLAKE2B_*
counterparts.

[RFC PATCH 2/2] Switch default hash algorithm to short BLAKE2b for testing

[brian m. carlson]

Set the default hash algorithm to short (160-bit) BLAKE2b so we can
identify tests that rely on a given hash algorithm.

This is a test commit and should not be used in production.  Widespread
test breakage occurs when using this commit.
---
 repository.c  | 2 +-
 setup.c   | 2 +-
 t/test-lib.sh | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/repository.c b/repository.c
index f66fcb1342..95c0ff74d6 100644
--- a/repository.c
+++ b/repository.c
@@ -5,7 +5,7 @@
 
 /* The main repository */
 static struct repository the_repo = {
-   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
_algos[GIT_HASH_SHA1], 0, 0
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
_algos[GIT_HASH_SBLAKE2], 0, 0
 };
 struct repository *the_repository = _repo;
 
diff --git a/setup.c b/setup.c
index 8cc34186ce..72eaee9c4e 100644
--- a/setup.c
+++ b/setup.c
@@ -488,7 +488,7 @@ int read_repository_format(struct repository_format 
*format, const char *path)
memset(format, 0, sizeof(*format));
format->version = -1;
format->is_bare = -1;
-   format->hash_algo = GIT_HASH_SHA1;
+   format->hash_algo = GIT_HASH_SBLAKE2;
string_list_init(>unknown_extensions, 1);
git_config_from_file(check_repo_format, path, format);
return format->version;
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9a0a21f49a..cd257040ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -183,8 +183,8 @@ _x40="$_x35$_x05"
 # Zero SHA-1
 _z40=
 
-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+EMPTY_TREE=f44422a644bfa5212387098f253a1e89eba94548
+EMPTY_BLOB=a706650a477f63b9b00eba41272bf36ef5a7dfa2
 
 # Line feed
 LF='

[RFC PATCH 1/2] Base test implementation for short BLAKE2b support

[brian m. carlson]

Test the implementation of a second hash algorithm with "short"
(160-bit) BLAKE2b support.  Simply add an additional algorithm to the
codebase without changing the use of any hash function and add a test
helper.

BLAKE2b was chosen simply because it provides a readily accessible hash
algorithm with arbitrary length support.  A 160-bit (20-byte) hash was
chosen to allow identification of tests that depend on the hash
algorithm in use while not causing buffer overflows, since parts of the
codebase still assume a 20-byte hash.

This is a preliminary commit for test purposes only and should not be
used in production in any way.
---
 Makefile |  3 +++
 cache.h  | 16 ++
 hash.h   |  9 +++-
 sha1_file.c  | 33 
 t/helper/test-sblake2b.c | 56 
 5 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-sblake2b.c

diff --git a/Makefile b/Makefile
index 1a9b23b679..d46247586b 100644
--- a/Makefile
+++ b/Makefile
@@ -677,6 +677,7 @@ TEST_PROGRAMS_NEED_X += test-ref-store
 TEST_PROGRAMS_NEED_X += test-regex
 TEST_PROGRAMS_NEED_X += test-revision-walking
 TEST_PROGRAMS_NEED_X += test-run-command
+TEST_PROGRAMS_NEED_X += test-sblake2b
 TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sha1-array
@@ -1539,6 +1540,8 @@ endif
 endif
 endif
 
+EXTLIBS += -lb2
+
 ifdef SHA1_MAX_BLOCK_SIZE
LIB_OBJS += compat/sha1-chunked.o
BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
diff --git a/cache.h b/cache.h
index bfde6f757a..638d832350 100644
--- a/cache.h
+++ b/cache.h
@@ -45,6 +45,10 @@ unsigned long git_deflate_bound(git_zstream *, unsigned 
long);
 #define GIT_SHA1_RAWSZ 20
 #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
 
+/* The length in bytes and in hex digits of an object name (short BLAKE2b 
value). */
+#define GIT_SBLAKE2B_RAWSZ 20
+#define GIT_SBLAKE2B_HEXSZ (2 * GIT_SBLAKE2B_RAWSZ)
+
 /* The length in byte and in hex digits of the largest possible hash value. */
 #define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
 #define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
@@ -1013,7 +1017,13 @@ static inline void oidclr(struct object_id *oid)
 #define EMPTY_TREE_SHA1_BIN_LITERAL \
 "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
 "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
+#define EMPTY_TREE_SBLAKE2B_HEX \
+   "f44422a644bfa5212387098f253a1e89eba94548"
+#define EMPTY_TREE_SBLAKE2B_BIN_LITERAL \
+   "\xf4\x44\x22\xa6\x44\xbf\xa5\x21\x23\x87" \
+   "\x09\x8f\x25\x3a\x1e\x89\xeb\xa9\x45\x48"
 extern const struct object_id empty_tree_oid;
+const struct object_id empty_tree_oid_sblake2b;
 #define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
 
 #define EMPTY_BLOB_SHA1_HEX \
@@ -1021,7 +1031,13 @@ extern const struct object_id empty_tree_oid;
 #define EMPTY_BLOB_SHA1_BIN_LITERAL \
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
+#define EMPTY_BLOB_SBLAKE2B_HEX \
+   "a706650a477f63b9b00eba41272bf36ef5a7dfa2"
+#define EMPTY_BLOB_SBLAKE2B_BIN_LITERAL \
+   "\xa7\x06\x65\x0a\x47\x7f\x63\xb9\xb0\x0e" \
+   "\xba\x41\x27\x2b\xf3\x6e\xf5\xa7\xdf\xa2"
 extern const struct object_id empty_blob_oid;
+const struct object_id empty_blob_oid_sblake2b;
 #define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
 
 
diff --git a/hash.h b/hash.h
index 365846a6b5..ef510bd51a 100644
--- a/hash.h
+++ b/hash.h
@@ -15,6 +15,8 @@
 #include "block-sha1/sha1.h"
 #endif
 
+#include 
+
 #ifndef platform_SHA_CTX
 /*
  * platform's underlying implementation of SHA-1; could be OpenSSL,
@@ -40,6 +42,8 @@
 #define git_SHA1_Updategit_SHA1_Update_Chunked
 #endif
 
+#define git_BLAKE2B_CTXblake2b_state
+
 /*
  * Note that these constants are suitable for indexing the hash_algos array and
  * comparing against each other, but are otherwise arbitrary, so they should 
not
@@ -52,12 +56,15 @@
 #define GIT_HASH_UNKNOWN 0
 /* SHA-1 */
 #define GIT_HASH_SHA1 1
+/* 20-byte BLAKE2b ("short" BLAKE2b) */
+#define GIT_HASH_SBLAKE2 2
 /* Number of algorithms supported (including unknown). */
-#define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)
+#define GIT_HASH_NALGOS (GIT_HASH_SBLAKE2 + 1)
 
 /* A suitably aligned type for stack allocations of hash contexts. */
 union git_hash_ctx {
git_SHA_CTX sha1;
+   git_BLAKE2B_CTX blake2b;
 };
 typedef union git_hash_ctx git_hash_ctx;
 
diff --git a/sha1_file.c b/sha1_file.c
index d9e2b1f285..5067b22a30 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -38,6 +38,12 @@ const struct object_id empty_tree_oid = {
 const struct object_id empty_blob_oid = {
EMPTY_BLOB_SHA1_BIN_LITERAL
 };
+const struct object_id empty_tree_oid_sblake2b = {
+   EMPTY_TREE_SBLAKE2B_BIN_LITERAL
+};
+const struct object_id empty_blob_oid_sblake2b = {
+   EMPTY_BLOB_SBLAKE2B_BIN_LITERAL
+};
 
 static void 

[RFC PATCH 0/2] alternate hash test

[brian m. carlson]

This series wires up an alternate hash implementation, namely
BLAKE2b-160.  The goal is to allow us to identify tests which rely on
the hash algorithm in use so that we can fix those tests.

For this test, I picked BLAKE2b-160 for a couple reasons:
* Debian ships a libb2-1 package which can be used easily (in other
  words, I was lazy and didn't want to add a crypto implementation just
  for test purposes);
* The API of the libb2 package easily allows arbitrary hash lengths, so
  I didn't have to manage truncation myself;
* Our codebase isn't yet ready for a hash function larger than 20 bytes,
  as there's still more work to do on the object_id conversions.

This isn't an endorsement for or against any particular algorithm
choice, just an artifact of the tools that were easily available to me.
Provoking discussion of which hash to pick for NewHash is explicitly
*not* a goal for this series.  I'm only interested in the ability to
identify and fix tests.

The first patch does no feature detection and just assumes you have
libb2 installed.  For obvious reasons, this series is not meant for
production use.

This series applies on top of the object_id part 11 series I just sent
out.

Below is a list of the prove output indicating which tests failed on my
system with these patches.

Test Summary Report
---
t0002-gitfile.sh (Wstat: 256 Tests: 14 Failed: 
3)
  Failed tests:  12-14
  Non-zero exit status: 1
t0005-signals.sh (Wstat: 256 Tests: 5 Failed: 2)
  Failed tests:  4-5
  Non-zero exit status: 1
t-basic.sh   (Wstat: 256 Tests: 78 Failed: 
12)
  Failed tests:  47, 54, 56, 58, 60, 62, 64, 66, 71, 74-76
  Non-zero exit status: 1
t1007-hash-object.sh (Wstat: 256 Tests: 38 Failed: 
15)
  Failed tests:  6, 8, 10-11, 19-29
  Non-zero exit status: 1
t1011-read-tree-sparse-checkout.sh   (Wstat: 256 Tests: 21 Failed: 
3)
  Failed tests:  1-2, 5
  Non-zero exit status: 1
t1300-repo-config.sh (Wstat: 256 Tests: 146 Failed: 
1)
  Failed test:  144
  Non-zero exit status: 1
t1304-default-acl.sh (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
t1405-main-ref-store.sh  (Wstat: 256 Tests: 17 Failed: 
1)
  Failed test:  10
  Non-zero exit status: 1
t1411-reflog-show.sh (Wstat: 256 Tests: 18 Failed: 
6)
  Failed tests:  3-5, 7, 10, 13
  Non-zero exit status: 1
t1450-fsck.sh(Wstat: 256 Tests: 71 Failed: 
3)
  Failed tests:  18, 26, 59
  Non-zero exit status: 1
t1507-rev-parse-upstream.sh  (Wstat: 256 Tests: 28 Failed: 
2)
  Failed tests:  25-26
  Non-zero exit status: 1
t1512-rev-parse-disambiguation.sh(Wstat: 256 Tests: 33 Failed: 
19)
  Failed tests:  2-5, 7-12, 16, 22, 26-32
  Non-zero exit status: 1
t1700-split-index.sh (Wstat: 256 Tests: 22 Failed: 
9)
  Failed tests:  1-2, 5-7, 13-16
  Non-zero exit status: 1
t2011-checkout-invalid-head.sh   (Wstat: 256 Tests: 10 Failed: 
5)
  Failed tests:  3, 6-7, 9-10
  Non-zero exit status: 1
t2015-checkout-unborn.sh (Wstat: 256 Tests: 6 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 1
t1013-read-tree-submodule.sh (Wstat: 256 Tests: 64 Failed: 
15)
  Failed tests:  1-2, 4-7, 19-20, 22-25, 30, 34-35
  Non-zero exit status: 1
t2017-checkout-orphan.sh (Wstat: 256 Tests: 13 Failed: 
7)
  Failed tests:  7-13
  Non-zero exit status: 1
t2020-checkout-detach.sh (Wstat: 256 Tests: 24 Failed: 
2)
  Failed tests:  23-24
  Non-zero exit status: 1
t2101-update-index-reupdate.sh   (Wstat: 256 Tests: 7 Failed: 6)
  Failed tests:  1-3, 5-7
  Non-zero exit status: 1
t2107-update-index-basic.sh  (Wstat: 256 Tests: 9 Failed: 1)
  Failed test:  9
  Non-zero exit status: 1
t2203-add-intent.sh  (Wstat: 256 Tests: 16 Failed: 
4)
  Failed tests:  3, 12, 15-16
  Non-zero exit status: 1
t3033-merge-toplevel.sh  (Wstat: 256 Tests: 13 Failed: 
11)
  Failed tests:  3-13
  Non-zero exit status: 1
t3103-ls-tree-misc.sh(Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t3201-branch-contains.sh (Wstat: 256 Tests: 19 Failed: 
1)
  Failed test:  18
  Non-zero exit status: 1
t3301-notes.sh   (Wstat: 256 Tests: 134 Failed: 
50)
  Failed tests:  10, 18-23, 42-44, 46, 55-57, 60, 65-78
80, 83-84, 86-91, 93-104
  Non-zero exit status: 1
t2013-checkout-submodule.sh  (Wstat: 256 Tests: 70 Failed: 
16)
  Failed tests:  7-8, 10-13, 19, 25-26, 28-31, 36, 40-41
  Non-zero exit status: 1

[PATCH 08/12] pack-write: switch various SHA-1 values to abstract forms

[brian m. carlson]

Convert various uses of hardcoded 20- and 40-based numbers to use
the_hash_algo, along with direct calls to SHA-1.  Adjust the names of
variables to refer to "hash" instead of "sha1".

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

diff --git a/pack-write.c b/pack-write.c
index fea6284192..fe33f7464c 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -122,7 +122,7 @@ const char *write_idx_file(const char *index_name, struct 
pack_idx_entry **objec
uint32_t offset = htonl(obj->offset);
sha1write(f, , 4);
}
-   sha1write(f, obj->oid.hash, 20);
+   sha1write(f, obj->oid.hash, the_hash_algo->rawsz);
if ((opts->flags & WRITE_IDX_STRICT) &&
(i && !oidcmp([-2]->oid, >oid)))
die("The same object %s appears twice in the pack",
@@ -169,7 +169,7 @@ const char *write_idx_file(const char *index_name, struct 
pack_idx_entry **objec
}
}
 
-   sha1write(f, sha1, 20);
+   sha1write(f, sha1, the_hash_algo->rawsz);
sha1close(f, NULL, ((opts->flags & WRITE_IDX_VERIFY)
? CSUM_CLOSE : CSUM_FSYNC));
return index_name;
@@ -203,20 +203,20 @@ off_t write_pack_header(struct sha1file *f, uint32_t 
nr_entries)
  * interested in the resulting SHA1 of pack data above partial_pack_offset.
  */
 void fixup_pack_header_footer(int pack_fd,
-unsigned char *new_pack_sha1,
+unsigned char *new_pack_hash,
 const char *pack_name,
 uint32_t object_count,
-unsigned char *partial_pack_sha1,
+unsigned char *partial_pack_hash,
 off_t partial_pack_offset)
 {
int aligned_sz, buf_sz = 8 * 1024;
-   git_SHA_CTX old_sha1_ctx, new_sha1_ctx;
+   git_hash_ctx old_hash_ctx, new_hash_ctx;
struct pack_header hdr;
char *buf;
ssize_t read_result;
 
-   git_SHA1_Init(_sha1_ctx);
-   git_SHA1_Init(_sha1_ctx);
+   the_hash_algo->init_fn(_hash_ctx);
+   the_hash_algo->init_fn(_hash_ctx);
 
if (lseek(pack_fd, 0, SEEK_SET) != 0)
die_errno("Failed seeking to start of '%s'", pack_name);
@@ -228,9 +228,9 @@ void fixup_pack_header_footer(int pack_fd,
  pack_name);
if (lseek(pack_fd, 0, SEEK_SET) != 0)
die_errno("Failed seeking to start of '%s'", pack_name);
-   git_SHA1_Update(_sha1_ctx, , sizeof(hdr));
+   the_hash_algo->update_fn(_hash_ctx, , sizeof(hdr));
hdr.hdr_entries = htonl(object_count);
-   git_SHA1_Update(_sha1_ctx, , sizeof(hdr));
+   the_hash_algo->update_fn(_hash_ctx, , sizeof(hdr));
write_or_die(pack_fd, , sizeof(hdr));
partial_pack_offset -= sizeof(hdr);
 
@@ -238,28 +238,28 @@ void fixup_pack_header_footer(int pack_fd,
aligned_sz = buf_sz - sizeof(hdr);
for (;;) {
ssize_t m, n;
-   m = (partial_pack_sha1 && partial_pack_offset < aligned_sz) ?
+   m = (partial_pack_hash && partial_pack_offset < aligned_sz) ?
partial_pack_offset : aligned_sz;
n = xread(pack_fd, buf, m);
if (!n)
break;
if (n < 0)
die_errno("Failed to checksum '%s'", pack_name);
-   git_SHA1_Update(_sha1_ctx, buf, n);
+   the_hash_algo->update_fn(_hash_ctx, buf, n);
 
aligned_sz -= n;
if (!aligned_sz)
aligned_sz = buf_sz;
 
-   if (!partial_pack_sha1)
+   if (!partial_pack_hash)
continue;
 
-   git_SHA1_Update(_sha1_ctx, buf, n);
+   the_hash_algo->update_fn(_hash_ctx, buf, n);
partial_pack_offset -= n;
if (partial_pack_offset == 0) {
-   unsigned char sha1[20];
-   git_SHA1_Final(sha1, _sha1_ctx);
-   if (hashcmp(sha1, partial_pack_sha1) != 0)
+   unsigned char hash[GIT_MAX_RAWSZ];
+   the_hash_algo->final_fn(hash, _hash_ctx);
+   if (hashcmp(hash, partial_pack_hash) != 0)
die("Unexpected checksum for %s "
"(disk corruption?)", pack_name);
/*
@@ -267,23 +267,24 @@ void fixup_pack_header_footer(int pack_fd,
 * pack, which also means making partial_pack_offset
 * big enough not to matter anymore.
 */
-   git_SHA1_Init(_sha1_ctx);
+   

[PATCH 10/12] csum-file: rename sha1file to hashfile

[brian m. carlson]

Rename struct sha1file to struct hashfile, along with all of its related
functions.

The transformation in this commit was made by global search-and-replace.
---
 builtin/index-pack.c   | 20 +--
 builtin/pack-objects.c | 52 +-
 bulk-checkin.c | 16 
 csum-file.c| 36 +-
 csum-file.h| 34 -
 fast-import.c  | 28 +--
 pack-bitmap-write.c| 30 ++---
 pack-write.c   | 32 +++
 pack.h |  4 ++--
 9 files changed, 126 insertions(+), 126 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 40c000aca8..0bb520c731 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1241,7 +1241,7 @@ static void resolve_deltas(void)
  * - append objects to convert thin pack to full pack if required
  * - write the final pack hash
  */
-static void fix_unresolved_deltas(struct sha1file *f);
+static void fix_unresolved_deltas(struct hashfile *f);
 static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned 
char *pack_hash)
 {
if (nr_ref_deltas + nr_ofs_deltas == nr_resolved_deltas) {
@@ -1252,7 +1252,7 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
}
 
if (fix_thin_pack) {
-   struct sha1file *f;
+   struct hashfile *f;
unsigned char read_hash[GIT_MAX_RAWSZ], 
tail_hash[GIT_MAX_RAWSZ];
struct strbuf msg = STRBUF_INIT;
int nr_unresolved = nr_ofs_deltas + nr_ref_deltas - 
nr_resolved_deltas;
@@ -1262,7 +1262,7 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
REALLOC_ARRAY(objects, nr_objects + nr_unresolved + 1);
memset(objects + nr_objects + 1, 0,
   nr_unresolved * sizeof(*objects));
-   f = sha1fd(output_fd, curr_pack);
+   f = hashfd(output_fd, curr_pack);
fix_unresolved_deltas(f);
strbuf_addf(, Q_("completed with %d local object",
 "completed with %d local objects",
@@ -1270,7 +1270,7 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
nr_objects - nr_objects_initial);
stop_progress_msg(, msg.buf);
strbuf_release();
-   sha1close(f, tail_hash, 0);
+   hashclose(f, tail_hash, 0);
hashcpy(read_hash, pack_hash);
fixup_pack_header_footer(output_fd, pack_hash,
 curr_pack, nr_objects,
@@ -1286,7 +1286,7 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
nr_ofs_deltas + nr_ref_deltas - nr_resolved_deltas);
 }
 
-static int write_compressed(struct sha1file *f, void *in, unsigned int size)
+static int write_compressed(struct hashfile *f, void *in, unsigned int size)
 {
git_zstream stream;
int status;
@@ -1300,7 +1300,7 @@ static int write_compressed(struct sha1file *f, void *in, 
unsigned int size)
stream.next_out = outbuf;
stream.avail_out = sizeof(outbuf);
status = git_deflate(, Z_FINISH);
-   sha1write(f, outbuf, sizeof(outbuf) - stream.avail_out);
+   hashwrite(f, outbuf, sizeof(outbuf) - stream.avail_out);
} while (status == Z_OK);
 
if (status != Z_STREAM_END)
@@ -1310,7 +1310,7 @@ static int write_compressed(struct sha1file *f, void *in, 
unsigned int size)
return size;
 }
 
-static struct object_entry *append_obj_to_pack(struct sha1file *f,
+static struct object_entry *append_obj_to_pack(struct hashfile *f,
   const unsigned char *sha1, void *buf,
   unsigned long size, enum object_type type)
 {
@@ -1327,7 +1327,7 @@ static struct object_entry *append_obj_to_pack(struct 
sha1file *f,
}
header[n++] = c;
crc32_begin(f);
-   sha1write(f, header, n);
+   hashwrite(f, header, n);
obj[0].size = size;
obj[0].hdr_size = n;
obj[0].type = type;
@@ -1335,7 +1335,7 @@ static struct object_entry *append_obj_to_pack(struct 
sha1file *f,
obj[1].idx.offset = obj[0].idx.offset + n;
obj[1].idx.offset += write_compressed(f, buf, size);
obj[0].idx.crc32 = crc32_end(f);
-   sha1flush(f);
+   hashflush(f);
hashcpy(obj->idx.oid.hash, sha1);
return obj;
 }
@@ -1347,7 +1347,7 @@ static int delta_pos_compare(const void *_a, const void 
*_b)
return a->obj_no - b->obj_no;
 }
 
-static void fix_unresolved_deltas(struct sha1file *f)
+static void fix_unresolved_deltas(struct hashfile *f)
 {
struct 

[PATCH 03/12] builtin/index-pack: improve hash function abstraction

[brian m. carlson]

Convert several uses of unsigned char [20] to struct object_id and
convert various hard-coded constants and uses of SHA-1 functions to use
the_hash_algo.

Signed-off-by: brian m. carlson 
---
 builtin/index-pack.c | 90 ++--
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..40c000aca8 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -91,7 +91,7 @@ static unsigned int input_offset, input_len;
 static off_t consumed_bytes;
 static off_t max_input_size;
 static unsigned deepest_delta;
-static git_SHA_CTX input_ctx;
+static git_hash_ctx input_ctx;
 static uint32_t input_crc32;
 static int input_fd, output_fd;
 static const char *curr_pack;
@@ -253,7 +253,7 @@ static void flush(void)
if (input_offset) {
if (output_fd >= 0)
write_or_die(output_fd, input_buffer, input_offset);
-   git_SHA1_Update(_ctx, input_buffer, input_offset);
+   the_hash_algo->update_fn(_ctx, input_buffer, 
input_offset);
memmove(input_buffer, input_buffer + input_offset, input_len);
input_offset = 0;
}
@@ -326,7 +326,7 @@ static const char *open_pack_file(const char *pack_name)
output_fd = -1;
nothread_data.pack_fd = input_fd;
}
-   git_SHA1_Init(_ctx);
+   the_hash_algo->init_fn(_ctx);
return pack_name;
 }
 
@@ -437,22 +437,22 @@ static int is_delta_type(enum object_type type)
 }
 
 static void *unpack_entry_data(off_t offset, unsigned long size,
-  enum object_type type, unsigned char *sha1)
+  enum object_type type, struct object_id *oid)
 {
static char fixed_buf[8192];
int status;
git_zstream stream;
void *buf;
-   git_SHA_CTX c;
+   git_hash_ctx c;
char hdr[32];
int hdrlen;
 
if (!is_delta_type(type)) {
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), 
size) + 1;
-   git_SHA1_Init();
-   git_SHA1_Update(, hdr, hdrlen);
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, hdr, hdrlen);
} else
-   sha1 = NULL;
+   oid = NULL;
if (type == OBJ_BLOB && size > big_file_threshold)
buf = fixed_buf;
else
@@ -469,8 +469,8 @@ static void *unpack_entry_data(off_t offset, unsigned long 
size,
stream.avail_in = input_len;
status = git_inflate(, 0);
use(input_len - stream.avail_in);
-   if (sha1)
-   git_SHA1_Update(, last_out, stream.next_out - 
last_out);
+   if (oid)
+   the_hash_algo->update_fn(, last_out, stream.next_out 
- last_out);
if (buf == fixed_buf) {
stream.next_out = buf;
stream.avail_out = sizeof(fixed_buf);
@@ -479,15 +479,15 @@ static void *unpack_entry_data(off_t offset, unsigned 
long size,
if (stream.total_out != size || status != Z_STREAM_END)
bad_object(offset, _("inflate returned %d"), status);
git_inflate_end();
-   if (sha1)
-   git_SHA1_Final(sha1, );
+   if (oid)
+   the_hash_algo->final_fn(oid->hash, );
return buf == fixed_buf ? NULL : buf;
 }
 
 static void *unpack_raw_entry(struct object_entry *obj,
  off_t *ofs_offset,
- unsigned char *ref_sha1,
- unsigned char *sha1)
+ struct object_id *ref_oid,
+ struct object_id *oid)
 {
unsigned char *p;
unsigned long size, c;
@@ -515,8 +515,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
 
switch (obj->type) {
case OBJ_REF_DELTA:
-   hashcpy(ref_sha1, fill(20));
-   use(20);
+   hashcpy(ref_oid->hash, fill(the_hash_algo->rawsz));
+   use(the_hash_algo->rawsz);
break;
case OBJ_OFS_DELTA:
p = fill(1);
@@ -546,7 +546,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
}
obj->hdr_size = consumed_bytes - obj->idx.offset;
 
-   data = unpack_entry_data(obj->idx.offset, obj->size, obj->type, sha1);
+   data = unpack_entry_data(obj->idx.offset, obj->size, obj->type, oid);
obj->idx.crc32 = input_crc32;
return data;
 }
@@ -1119,11 +1119,11 @@ static void *threaded_second_pass(void *data)
  * - calculate SHA1 of all non-delta objects;
  * - remember base (SHA1 or offset) for all deltas.
  */
-static void parse_pack_objects(unsigned char *sha1)
+static void parse_pack_objects(unsigned char *hash)
 {
int i, nr_delays = 0;

[PATCH 02/12] hash: create union for hash context allocation

[brian m. carlson]

In various parts of our code, we want to allocate a structure
representing the internal state of a hash algorithm.  The original
implementation of the hash algorithm abstraction assumed we would do
that using heap allocations, and added a context size element to struct
git_hash_algo.  However, most of the existing code uses stack
allocations and conversion would needlessly complicate various parts of
the code.  Add a union for the purpose of allocating hash contexts on
the stack and a typedef for ease of use.  Remove the ctxsz element for
struct git_hash_algo, which is no longer very useful.

This does mean that stack allocations will grow slightly as additional
hash functions are added, but this should not be a significant problem,
since we don't allocate many hash contexts.  The improved usability and
benefits from avoiding dynamic allocation outweigh this small downside.

Signed-off-by: brian m. carlson 
---
 hash.h  | 9 ++---
 sha1_file.c | 2 --
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hash.h b/hash.h
index 7122dea7b3..365846a6b5 100644
--- a/hash.h
+++ b/hash.h
@@ -55,6 +55,12 @@
 /* Number of algorithms supported (including unknown). */
 #define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)
 
+/* A suitably aligned type for stack allocations of hash contexts. */
+union git_hash_ctx {
+   git_SHA_CTX sha1;
+};
+typedef union git_hash_ctx git_hash_ctx;
+
 typedef void (*git_hash_init_fn)(void *ctx);
 typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
 typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);
@@ -69,9 +75,6 @@ struct git_hash_algo {
/* A four-byte version identifier, used in pack indices. */
uint32_t format_id;
 
-   /* The size of a hash context (e.g. git_SHA_CTX). */
-   size_t ctxsz;
-
/* The length of the hash in binary. */
size_t rawsz;
 
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..e61d93a6e8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -75,7 +75,6 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
0x,
0,
0,
-   0,
git_hash_unknown_init,
git_hash_unknown_update,
git_hash_unknown_final,
@@ -86,7 +85,6 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
"sha-1",
/* "sha1", big-endian */
0x73686131,
-   sizeof(git_SHA_CTX),
GIT_SHA1_RAWSZ,
GIT_SHA1_HEXSZ,
git_hash_sha1_init,

[PATCH 07/12] pack-check: convert various uses of SHA-1 to abstract forms

[brian m. carlson]

Convert various explicit calls to use SHA-1 functions and constants to
references to the_hash_algo.  Make several strings more generic with
respect to the hash algorithm used.

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

diff --git a/pack-check.c b/pack-check.c
index 073c1fbd46..403a572567 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -41,7 +41,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window 
**w_curs,
} while (len);
 
index_crc = p->index_data;
-   index_crc += 2 + 256 + p->num_objects * (20/4) + nr;
+   index_crc += 2 + 256 + p->num_objects * (the_hash_algo->rawsz/4) + nr;
 
return data_crc != ntohl(*index_crc);
 }
@@ -54,7 +54,7 @@ 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;
+   git_hash_ctx ctx;
unsigned char hash[GIT_MAX_RAWSZ], *pack_sig;
off_t offset = 0, pack_sig_ofs = 0;
uint32_t nr_objects, i;
@@ -64,24 +64,24 @@ static int verify_packfile(struct packed_git *p,
if (!is_pack_valid(p))
return error("packfile %s cannot be accessed", p->pack_name);
 
-   git_SHA1_Init();
+   the_hash_algo->init_fn();
do {
unsigned long remaining;
unsigned char *in = use_pack(p, w_curs, offset, );
offset += remaining;
if (!pack_sig_ofs)
-   pack_sig_ofs = p->pack_size - 20;
+   pack_sig_ofs = p->pack_size - the_hash_algo->rawsz;
if (offset > pack_sig_ofs)
remaining -= (unsigned int)(offset - pack_sig_ofs);
-   git_SHA1_Update(, in, remaining);
+   the_hash_algo->update_fn(, in, remaining);
} while (offset < pack_sig_ofs);
-   git_SHA1_Final(hash, );
+   the_hash_algo->final_fn(hash, );
pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);
if (hashcmp(hash, pack_sig))
-   err = error("%s SHA1 checksum mismatch",
+   err = error("%s pack checksum mismatch",
p->pack_name);
-   if (hashcmp(index_base + index_size - 40, pack_sig))
-   err = error("%s SHA1 does not match its index",
+   if (hashcmp(index_base + index_size - the_hash_algo->hexsz, pack_sig))
+   err = error("%s pack checksum does not match its index",
p->pack_name);
unuse_pack(w_curs);
 
@@ -165,8 +165,8 @@ int verify_pack_index(struct packed_git *p)
 {
off_t index_size;
const unsigned char *index_base;
-   git_SHA_CTX ctx;
-   unsigned char sha1[20];
+   git_hash_ctx ctx;
+   unsigned char hash[GIT_MAX_RAWSZ];
int err = 0;
 
if (open_pack_index(p))
@@ -175,11 +175,11 @@ int verify_pack_index(struct packed_git *p)
index_base = p->index_data;
 
/* Verify SHA1 sum of the index file */
-   git_SHA1_Init();
-   git_SHA1_Update(, index_base, (unsigned int)(index_size - 20));
-   git_SHA1_Final(sha1, );
-   if (hashcmp(sha1, index_base + index_size - 20))
-   err = error("Packfile index for %s SHA1 mismatch",
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, index_base, (unsigned int)(index_size - 
the_hash_algo->rawsz));
+   the_hash_algo->final_fn(hash, );
+   if (hashcmp(hash, index_base + index_size - the_hash_algo->rawsz))
+   err = error("Packfile index for %s hash mismatch",
p->pack_name);
return err;
 }

[PATCH 11/12] csum-file: abstract uses of SHA-1

[brian m. carlson]

Convert several direct uses of SHA-1 to use the_hash_algo instead.
Convert one use of the constant 20 as well.

Signed-off-by: brian m. carlson 
---
 csum-file.c | 10 +-
 csum-file.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/csum-file.c b/csum-file.c
index e4ad6337dc..5eda7fb6af 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -47,7 +47,7 @@ void hashflush(struct hashfile *f)
unsigned offset = f->offset;
 
if (offset) {
-   git_SHA1_Update(>ctx, f->buffer, offset);
+   the_hash_algo->update_fn(>ctx, f->buffer, offset);
flush(f, f->buffer, offset);
f->offset = 0;
}
@@ -58,12 +58,12 @@ int hashclose(struct hashfile *f, unsigned char *result, 
unsigned int flags)
int fd;
 
hashflush(f);
-   git_SHA1_Final(f->buffer, >ctx);
+   the_hash_algo->final_fn(f->buffer, >ctx);
if (result)
hashcpy(result, f->buffer);
if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
/* write checksum and close fd */
-   flush(f, f->buffer, 20);
+   flush(f, f->buffer, the_hash_algo->rawsz);
if (flags & CSUM_FSYNC)
fsync_or_die(f->fd, f->name);
if (close(f->fd))
@@ -110,7 +110,7 @@ void hashwrite(struct hashfile *f, const void *buf, 
unsigned int count)
buf = (char *) buf + nr;
left -= nr;
if (!left) {
-   git_SHA1_Update(>ctx, data, offset);
+   the_hash_algo->update_fn(>ctx, data, offset);
flush(f, data, offset);
offset = 0;
}
@@ -149,7 +149,7 @@ struct hashfile *hashfd_throughput(int fd, const char 
*name, struct progress *tp
f->tp = tp;
f->name = name;
f->do_crc = 0;
-   git_SHA1_Init(>ctx);
+   the_hash_algo->init_fn(>ctx);
return f;
 }
 
diff --git a/csum-file.h b/csum-file.h
index ceb3e5712d..992e5c0141 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -8,7 +8,7 @@ struct hashfile {
int fd;
int check_fd;
unsigned int offset;
-   git_SHA_CTX ctx;
+   git_hash_ctx ctx;
off_t total;
struct progress *tp;
const char *name;
@@ -20,7 +20,7 @@ struct hashfile {
 /* Checkpoint */
 struct hashfile_checkpoint {
off_t offset;
-   git_SHA_CTX ctx;
+   git_hash_ctx ctx;
 };
 
 extern void hashfile_checkpoint(struct hashfile *, struct hashfile_checkpoint 
*);

[PATCH 12/12] bulk-checkin: abstract SHA-1 usage

[brian m. carlson]

Convert uses of the direct SHA-1 functions to use the_hash_algo instead.

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

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 5f79ed6ea3..8bcd1c8665 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -93,7 +93,7 @@ static int already_written(struct bulk_checkin_state *state, 
unsigned char sha1[
  * with a new pack.
  */
 static int stream_to_pack(struct bulk_checkin_state *state,
- git_SHA_CTX *ctx, off_t *already_hashed_to,
+ git_hash_ctx *ctx, off_t *already_hashed_to,
  int fd, size_t size, enum object_type type,
  const char *path, unsigned flags)
 {
@@ -127,7 +127,7 @@ static int stream_to_pack(struct bulk_checkin_state *state,
if (rsize < hsize)
hsize = rsize;
if (hsize)
-   git_SHA1_Update(ctx, ibuf, hsize);
+   the_hash_algo->update_fn(ctx, ibuf, 
hsize);
*already_hashed_to = offset;
}
s.next_in = ibuf;
@@ -192,7 +192,7 @@ static int deflate_to_pack(struct bulk_checkin_state *state,
   unsigned flags)
 {
off_t seekback, already_hashed_to;
-   git_SHA_CTX ctx;
+   git_hash_ctx ctx;
unsigned char obuf[16384];
unsigned header_len;
struct hashfile_checkpoint checkpoint;
@@ -204,8 +204,8 @@ static int deflate_to_pack(struct bulk_checkin_state *state,
 
header_len = xsnprintf((char *)obuf, sizeof(obuf), "%s %" PRIuMAX,
   typename(type), (uintmax_t)size) + 1;
-   git_SHA1_Init();
-   git_SHA1_Update(, obuf, header_len);
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, obuf, header_len);
 
/* Note: idx is non-NULL when we are writing */
if ((flags & HASH_WRITE_OBJECT) != 0)
@@ -236,7 +236,7 @@ static int deflate_to_pack(struct bulk_checkin_state *state,
if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
return error("cannot seek back");
}
-   git_SHA1_Final(result_sha1, );
+   the_hash_algo->final_fn(result_sha1, );
if (!idx)
return 0;
 

[PATCH 06/12] fast-import: switch various uses of SHA-1 to the_hash_algo

[brian m. carlson]

Switch various uses of explicit calls to SHA-1 to use the_hash_algo.
Convert various uses of 20 and the GIT_SHA1 constants as well.

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

diff --git a/fast-import.c b/fast-import.c
index b70ac025e0..1b8ab8ea29 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1092,15 +1092,15 @@ static int store_object(
unsigned char hdr[96];
struct object_id oid;
unsigned long hdrlen, deltalen;
-   git_SHA_CTX c;
+   git_hash_ctx c;
git_zstream s;
 
hdrlen = xsnprintf((char *)hdr, sizeof(hdr), "%s %lu",
   typename(type), (unsigned long)dat->len) + 1;
-   git_SHA1_Init();
-   git_SHA1_Update(, hdr, hdrlen);
-   git_SHA1_Update(, dat->buf, dat->len);
-   git_SHA1_Final(oid.hash, );
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, hdr, hdrlen);
+   the_hash_algo->update_fn(, dat->buf, dat->len);
+   the_hash_algo->final_fn(oid.hash, );
if (oidout)
oidcpy(oidout, );
 
@@ -1118,11 +1118,11 @@ static int store_object(
return 1;
}
 
-   if (last && last->data.buf && last->depth < max_depth && dat->len > 20) 
{
+   if (last && last->data.buf && last->depth < max_depth && dat->len > 
the_hash_algo->rawsz) {
delta_count_attempts_by_type[type]++;
delta = diff_delta(last->data.buf, last->data.len,
dat->buf, dat->len,
-   , dat->len - 20);
+   , dat->len - the_hash_algo->rawsz);
} else
delta = NULL;
 
@@ -1231,7 +1231,7 @@ static void stream_blob(uintmax_t len, struct object_id 
*oidout, uintmax_t mark)
struct object_id oid;
unsigned long hdrlen;
off_t offset;
-   git_SHA_CTX c;
+   git_hash_ctx c;
git_zstream s;
struct sha1file_checkpoint checkpoint;
int status = Z_OK;
@@ -1246,8 +1246,8 @@ static void stream_blob(uintmax_t len, struct object_id 
*oidout, uintmax_t mark)
 
hdrlen = xsnprintf((char *)out_buf, out_sz, "blob %" PRIuMAX, len) + 1;
 
-   git_SHA1_Init();
-   git_SHA1_Update(, out_buf, hdrlen);
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, out_buf, hdrlen);
 
crc32_begin(pack_file);
 
@@ -1265,7 +1265,7 @@ static void stream_blob(uintmax_t len, struct object_id 
*oidout, uintmax_t mark)
if (!n && feof(stdin))
die("EOF in data (%" PRIuMAX " bytes 
remaining)", len);
 
-   git_SHA1_Update(, in_buf, n);
+   the_hash_algo->update_fn(, in_buf, n);
s.next_in = in_buf;
s.avail_in = n;
len -= n;
@@ -1291,7 +1291,7 @@ static void stream_blob(uintmax_t len, struct object_id 
*oidout, uintmax_t mark)
}
}
git_deflate_end();
-   git_SHA1_Final(oid.hash, );
+   the_hash_algo->final_fn(oid.hash, );
 
if (oidout)
oidcpy(oidout, );
@@ -1350,11 +1350,11 @@ static void *gfi_unpack_entry(
 {
enum object_type type;
struct packed_git *p = all_packs[oe->pack_id];
-   if (p == pack_data && p->pack_size < (pack_size + 20)) {
+   if (p == pack_data && p->pack_size < (pack_size + 
the_hash_algo->rawsz)) {
/* The object is stored in the packfile we are writing to
 * and we have modified it since the last time we scanned
 * back to read a previously written object.  If an old
-* window covered [p->pack_size, p->pack_size + 20) its
+* window covered [p->pack_size, p->pack_size + rawsz) its
 * data is stale and is not valid.  Closing all windows
 * and updating the packfile length ensures we can read
 * the newly written data.
@@ -1362,13 +1362,13 @@ static void *gfi_unpack_entry(
close_pack_windows(p);
sha1flush(pack_file);
 
-   /* We have to offer 20 bytes additional on the end of
+   /* We have to offer rawsz bytes additional on the end of
 * the packfile as the core unpacker code assumes the
 * footer is present at the file end and must promise
-* at least 20 bytes within any window it maps.  But
+* at least rawsz bytes within any window it maps.  But
 * we don't actually create the footer here.
 */
-   p->pack_size = pack_size + 20;
+   p->pack_size = pack_size + the_hash_algo->rawsz;
}
return unpack_entry(p, oe->idx.offset, , sizep);
 }
@@ -2204,7 +2204,7 @@ static void construct_path_with_fanout(const char 
*hex_sha1,
 

[PATCH 04/12] builtin/unpack-objects: switch uses of SHA-1 to the_hash_algo

[brian m. carlson]

Switch various uses of explicit calls to SHA-1 into references to
the_hash_algo to better abstract away the various uses of it.

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

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..813ca31979 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -21,7 +21,7 @@ static unsigned char buffer[4096];
 static unsigned int offset, len;
 static off_t consumed_bytes;
 static off_t max_input_size;
-static git_SHA_CTX ctx;
+static git_hash_ctx ctx;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
 
 /*
@@ -62,7 +62,7 @@ static void *fill(int min)
if (min > sizeof(buffer))
die("cannot fill %d bytes", min);
if (offset) {
-   git_SHA1_Update(, buffer, offset);
+   the_hash_algo->update_fn(, buffer, offset);
memmove(buffer, buffer + offset, len);
offset = 0;
}
@@ -345,8 +345,8 @@ static void unpack_delta_entry(enum object_type type, 
unsigned long delta_size,
struct object_id base_oid;
 
if (type == OBJ_REF_DELTA) {
-   hashcpy(base_oid.hash, fill(GIT_SHA1_RAWSZ));
-   use(GIT_SHA1_RAWSZ);
+   hashcpy(base_oid.hash, fill(the_hash_algo->rawsz));
+   use(the_hash_algo->rawsz);
delta_data = get_data(delta_size);
if (dry_run || !delta_data) {
free(delta_data);
@@ -564,15 +564,15 @@ int cmd_unpack_objects(int argc, const char **argv, const 
char *prefix)
/* We don't take any non-flag arguments now.. Maybe some day */
usage(unpack_usage);
}
-   git_SHA1_Init();
+   the_hash_algo->init_fn();
unpack_all();
-   git_SHA1_Update(, buffer, offset);
-   git_SHA1_Final(oid.hash, );
+   the_hash_algo->update_fn(, buffer, offset);
+   the_hash_algo->final_fn(oid.hash, );
if (strict)
write_rest();
-   if (hashcmp(fill(GIT_SHA1_RAWSZ), oid.hash))
+   if (hashcmp(fill(the_hash_algo->rawsz), oid.hash))
die("final sha1 did not match");
-   use(GIT_SHA1_RAWSZ);
+   use(the_hash_algo->rawsz);
 
/* Write the last part of the buffer to stdout */
while (len) {

[PATCH 09/12] read-cache: abstract away uses of SHA-1

[brian m. carlson]

Convert various uses of direct calls to SHA-1 and 20- and 40-based
constants to use the_hash_algo instead.  Don't yet convert the on-disk
data structures, which will be handled in a future commit.

Signed-off-by: brian m. carlson 
---
 read-cache.c | 54 +++---
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 2eb81a66b9..4f7aac23af 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1545,8 +1545,8 @@ int verify_ce_order;
 
 static int verify_hdr(struct cache_header *hdr, unsigned long size)
 {
-   git_SHA_CTX c;
-   unsigned char sha1[20];
+   git_hash_ctx c;
+   unsigned char hash[GIT_MAX_RAWSZ];
int hdr_version;
 
if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
@@ -1558,10 +1558,10 @@ static int verify_hdr(struct cache_header *hdr, 
unsigned long size)
if (!verify_index_checksum)
return 0;
 
-   git_SHA1_Init();
-   git_SHA1_Update(, hdr, size - 20);
-   git_SHA1_Final(sha1, );
-   if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, hdr, size - the_hash_algo->rawsz);
+   the_hash_algo->final_fn(hash, );
+   if (hashcmp(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz))
return error("bad index file sha1 signature");
return 0;
 }
@@ -1791,7 +1791,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
die_errno("cannot stat the open index");
 
mmap_size = xsize_t(st.st_size);
-   if (mmap_size < sizeof(struct cache_header) + 20)
+   if (mmap_size < sizeof(struct cache_header) + the_hash_algo->rawsz)
die("index file smaller than expected");
 
mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
@@ -1803,7 +1803,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
if (verify_hdr(hdr, mmap_size) < 0)
goto unmap;
 
-   hashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 20);
+   hashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 
the_hash_algo->rawsz);
istate->version = ntohl(hdr->hdr_version);
istate->cache_nr = ntohl(hdr->hdr_entries);
istate->cache_alloc = alloc_nr(istate->cache_nr);
@@ -1831,7 +1831,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
istate->timestamp.sec = st.st_mtime;
istate->timestamp.nsec = ST_MTIME_NSEC(st);
 
-   while (src_offset <= mmap_size - 20 - 8) {
+   while (src_offset <= mmap_size - the_hash_algo->rawsz - 8) {
/* After an array of active_nr index entries,
 * there can be arbitrary number of extended
 * sections, each of which is prefixed with
@@ -1957,11 +1957,11 @@ int unmerged_index(const struct index_state *istate)
 static unsigned char write_buffer[WRITE_BUFFER_SIZE];
 static unsigned long write_buffer_len;
 
-static int ce_write_flush(git_SHA_CTX *context, int fd)
+static int ce_write_flush(git_hash_ctx *context, int fd)
 {
unsigned int buffered = write_buffer_len;
if (buffered) {
-   git_SHA1_Update(context, write_buffer, buffered);
+   the_hash_algo->update_fn(context, write_buffer, buffered);
if (write_in_full(fd, write_buffer, buffered) < 0)
return -1;
write_buffer_len = 0;
@@ -1969,7 +1969,7 @@ static int ce_write_flush(git_SHA_CTX *context, int fd)
return 0;
 }
 
-static int ce_write(git_SHA_CTX *context, int fd, void *data, unsigned int len)
+static int ce_write(git_hash_ctx *context, int fd, void *data, unsigned int 
len)
 {
while (len) {
unsigned int buffered = write_buffer_len;
@@ -1991,7 +1991,7 @@ static int ce_write(git_SHA_CTX *context, int fd, void 
*data, unsigned int len)
return 0;
 }
 
-static int write_index_ext_header(git_SHA_CTX *context, int fd,
+static int write_index_ext_header(git_hash_ctx *context, int fd,
  unsigned int ext, unsigned int sz)
 {
ext = htonl(ext);
@@ -2000,26 +2000,26 @@ static int write_index_ext_header(git_SHA_CTX *context, 
int fd,
(ce_write(context, fd, , 4) < 0)) ? -1 : 0;
 }
 
-static int ce_flush(git_SHA_CTX *context, int fd, unsigned char *sha1)
+static int ce_flush(git_hash_ctx *context, int fd, unsigned char *hash)
 {
unsigned int left = write_buffer_len;
 
if (left) {
write_buffer_len = 0;
-   git_SHA1_Update(context, write_buffer, left);
+   the_hash_algo->update_fn(context, write_buffer, left);
}
 
/* Flush first if not enough space for SHA1 signature */
-   if (left + 20 > WRITE_BUFFER_SIZE) {
+   if (left + the_hash_algo->rawsz > 

[PATCH 05/12] sha1_file: switch uses of SHA-1 to the_hash_algo

[brian m. carlson]

Switch various uses of explicit calls to SHA-1 into references to
the_hash_algo for better abstraction.  Convert some calls to use struct
object_id.

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

diff --git a/sha1_file.c b/sha1_file.c
index e61d93a6e8..d9e2b1f285 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -786,16 +786,16 @@ void *xmmap(void *start, size_t length,
 int check_sha1_signature(const unsigned char *sha1, void *map,
 unsigned long size, const char *type)
 {
-   unsigned char real_sha1[20];
+   struct object_id real_oid;
enum object_type obj_type;
struct git_istream *st;
-   git_SHA_CTX c;
+   git_hash_ctx c;
char hdr[32];
int hdrlen;
 
if (map) {
-   hash_sha1_file(map, size, type, real_sha1);
-   return hashcmp(sha1, real_sha1) ? -1 : 0;
+   hash_sha1_file(map, size, type, real_oid.hash);
+   return hashcmp(sha1, real_oid.hash) ? -1 : 0;
}
 
st = open_istream(sha1, _type, , NULL);
@@ -806,8 +806,8 @@ int check_sha1_signature(const unsigned char *sha1, void 
*map,
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(obj_type), 
size) + 1;
 
/* Sha1.. */
-   git_SHA1_Init();
-   git_SHA1_Update(, hdr, hdrlen);
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, hdr, hdrlen);
for (;;) {
char buf[1024 * 16];
ssize_t readlen = read_istream(st, buf, sizeof(buf));
@@ -818,11 +818,11 @@ int check_sha1_signature(const unsigned char *sha1, void 
*map,
}
if (!readlen)
break;
-   git_SHA1_Update(, buf, readlen);
+   the_hash_algo->update_fn(, buf, readlen);
}
-   git_SHA1_Final(real_sha1, );
+   the_hash_algo->final_fn(real_oid.hash, );
close_istream(st);
-   return hashcmp(sha1, real_sha1) ? -1 : 0;
+   return hashcmp(sha1, real_oid.hash) ? -1 : 0;
 }
 
 int git_open_cloexec(const char *name, int flags)
@@ -1421,16 +1421,16 @@ static void write_sha1_file_prepare(const void *buf, 
unsigned long len,
 const char *type, unsigned char *sha1,
 char *hdr, int *hdrlen)
 {
-   git_SHA_CTX c;
+   git_hash_ctx c;
 
/* Generate the header */
*hdrlen = xsnprintf(hdr, *hdrlen, "%s %lu", type, len)+1;
 
/* Sha1.. */
-   git_SHA1_Init();
-   git_SHA1_Update(, hdr, *hdrlen);
-   git_SHA1_Update(, buf, len);
-   git_SHA1_Final(sha1, );
+   the_hash_algo->init_fn();
+   the_hash_algo->update_fn(, hdr, *hdrlen);
+   the_hash_algo->update_fn(, buf, len);
+   the_hash_algo->final_fn(sha1, );
 }
 
 /*
@@ -1552,8 +1552,8 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
int fd, ret;
unsigned char compressed[4096];
git_zstream stream;
-   git_SHA_CTX c;
-   unsigned char parano_sha1[20];
+   git_hash_ctx c;
+   struct object_id parano_oid;
static struct strbuf tmp_file = STRBUF_INIT;
const char *filename = sha1_file_name(sha1);
 
@@ -1569,14 +1569,14 @@ static int write_loose_object(const unsigned char 
*sha1, char *hdr, int hdrlen,
git_deflate_init(, zlib_compression_level);
stream.next_out = compressed;
stream.avail_out = sizeof(compressed);
-   git_SHA1_Init();
+   the_hash_algo->init_fn();
 
/* First header.. */
stream.next_in = (unsigned char *)hdr;
stream.avail_in = hdrlen;
while (git_deflate(, 0) == Z_OK)
; /* nothing */
-   git_SHA1_Update(, hdr, hdrlen);
+   the_hash_algo->update_fn(, hdr, hdrlen);
 
/* Then the data itself.. */
stream.next_in = (void *)buf;
@@ -1584,7 +1584,7 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
do {
unsigned char *in0 = stream.next_in;
ret = git_deflate(, Z_FINISH);
-   git_SHA1_Update(, in0, stream.next_in - in0);
+   the_hash_algo->update_fn(, in0, stream.next_in - in0);
if (write_buffer(fd, compressed, stream.next_out - compressed) 
< 0)
die("unable to write sha1 file");
stream.next_out = compressed;
@@ -1596,8 +1596,8 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
ret = git_deflate_end_gently();
if (ret != Z_OK)
die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), 
ret);
-   git_SHA1_Final(parano_sha1, );
-   if (hashcmp(sha1, parano_sha1) != 0)
+   the_hash_algo->final_fn(parano_oid.hash, );
+   if (hashcmp(sha1, parano_oid.hash) != 0)

[PATCH 01/12] hash: move SHA-1 macros to hash.h

[brian m. carlson]

Most of the other code dealing with SHA-1 and other hashes is located in
hash.h, which is in turn loaded by cache.h.  Move the SHA-1 macros to
hash.h as well, so we can use them in additional hash-related items in
the future.

Signed-off-by: brian m. carlson 
---
 cache.h | 25 -
 hash.h  | 25 +
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/cache.h b/cache.h
index d8b975a571..bfde6f757a 100644
--- a/cache.h
+++ b/cache.h
@@ -16,31 +16,6 @@
 #include "sha1-array.h"
 #include "repository.h"
 
-#ifndef platform_SHA_CTX
-/*
- * platform's underlying implementation of SHA-1; could be OpenSSL,
- * blk_SHA, Apple CommonCrypto, etc...  Note that including
- * SHA1_HEADER may have already defined platform_SHA_CTX for our
- * own implementations like block-sha1 and ppc-sha1, so we list
- * the default for OpenSSL compatible SHA-1 implementations here.
- */
-#define platform_SHA_CTX   SHA_CTX
-#define platform_SHA1_Init SHA1_Init
-#define platform_SHA1_Update   SHA1_Update
-#define platform_SHA1_FinalSHA1_Final
-#endif
-
-#define git_SHA_CTXplatform_SHA_CTX
-#define git_SHA1_Init  platform_SHA1_Init
-#define git_SHA1_Updateplatform_SHA1_Update
-#define git_SHA1_Final platform_SHA1_Final
-
-#ifdef SHA1_MAX_BLOCK_SIZE
-#include "compat/sha1-chunked.h"
-#undef git_SHA1_Update
-#define git_SHA1_Updategit_SHA1_Update_Chunked
-#endif
-
 #include 
 typedef struct git_zstream {
z_stream z;
diff --git a/hash.h b/hash.h
index 7d7a864f5d..7122dea7b3 100644
--- a/hash.h
+++ b/hash.h
@@ -15,6 +15,31 @@
 #include "block-sha1/sha1.h"
 #endif
 
+#ifndef platform_SHA_CTX
+/*
+ * platform's underlying implementation of SHA-1; could be OpenSSL,
+ * blk_SHA, Apple CommonCrypto, etc...  Note that including
+ * SHA1_HEADER may have already defined platform_SHA_CTX for our
+ * own implementations like block-sha1 and ppc-sha1, so we list
+ * the default for OpenSSL compatible SHA-1 implementations here.
+ */
+#define platform_SHA_CTX   SHA_CTX
+#define platform_SHA1_Init SHA1_Init
+#define platform_SHA1_Update   SHA1_Update
+#define platform_SHA1_FinalSHA1_Final
+#endif
+
+#define git_SHA_CTXplatform_SHA_CTX
+#define git_SHA1_Init  platform_SHA1_Init
+#define git_SHA1_Updateplatform_SHA1_Update
+#define git_SHA1_Final platform_SHA1_Final
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+#include "compat/sha1-chunked.h"
+#undef git_SHA1_Update
+#define git_SHA1_Updategit_SHA1_Update_Chunked
+#endif
+
 /*
  * Note that these constants are suitable for indexing the hash_algos array and
  * comparing against each other, but are otherwise arbitrary, so they should 
not

[PATCH 00/12] object_id part 11 (the_hash_algo)

[brian m. carlson]

This series includes various changes to adopt the use of the_hash_algo
for abstracting hash algorithms away.

The series moves much of the hash-related code to hash.h from cache.h,
drops the ctxsz member in favor of allowing stack-allocated hash
contexts, and switches object-related code to use the_hash_algo for
hashing.

Note that not all instances of calls to git_SHA1_* have been converted.
The diff line code, the push cert code, and patch IDs all have been left
alone for the moment because they are not related to object handling.
Pack checksums, on the other hand, have been converted.

The series is based off master, and has one minor conflict with Patryk
Obara's recent object_id series.

I will also be sending out preliminary test patches on top of this
series that wire up an alternate hash algorithm so that we can see what
tests break as a result.  (Hint: there's a lot of them.)

brian m. carlson (12):
  hash: move SHA-1 macros to hash.h
  hash: create union for hash context allocation
  builtin/index-pack: improve hash function abstraction
  builtin/unpack-objects: switch uses of SHA-1 to the_hash_algo
  sha1_file: switch uses of SHA-1 to the_hash_algo
  fast-import: switch various uses of SHA-1 to the_hash_algo
  pack-check: convert various uses of SHA-1 to abstract forms
  pack-write: switch various SHA-1 values to abstract forms
  read-cache: abstract away uses of SHA-1
  csum-file: rename sha1file to hashfile
  csum-file: abstract uses of SHA-1
  bulk-checkin: abstract SHA-1 usage

 builtin/index-pack.c | 108 +++
 builtin/pack-objects.c   |  52 +++
 builtin/unpack-objects.c |  18 
 bulk-checkin.c   |  28 ++--
 cache.h  |  25 ---
 csum-file.c  |  46 ++--
 csum-file.h  |  38 -
 fast-import.c|  68 ++---
 hash.h   |  34 +--
 pack-bitmap-write.c  |  30 ++---
 pack-check.c |  32 +++---
 pack-write.c |  77 -
 pack.h   |   4 +-
 read-cache.c |  54 
 sha1_file.c  |  54 
 15 files changed, 335 insertions(+), 333 deletions(-)

RE: git send-email sets date

[Philip Oakley]

Behalf Of brian m. carlson
> On Fri, Jan 26, 2018 at 06:32:30PM +0100, Michal Suchánek wrote:
> > git send-email sets the message date to author date.
> >
> > This is wrong because the message will most likely not get delivered
> > when the author date differs from current time. It might give slightly
> > better results with commit date instead of author date but can't is
> > just skip that header and leave it to the mailer?
> >
> > It does not even seem to have an option to suppress adding the date
> > header.
> 
> I'm pretty sure it's intended to work this way.
> 
> Without the Date header, we have no way of providing the author date
> when sending a patch.  git am will read this date and use it as the
> author date when applying patches, so if it's omitted, the author date
> will be wrong.
> 
> If you want to send patches with a different date, you can always insert
> the patch inline in your mailer using the scissors notation, which will
> allow your mailer to insert its own date while keeping the patch date
> separate.
> --

Michal, you may want to hack up an option that can automatically create 
that format if it is of use. I sometimes find the sort order an issue in 
some of my mail clients.
--
Philip

recommendations for log enhancement

[牛旭]

Our team studies the consistent edits of git during evolution. And we find 
several missed edits in the latest release of git. For example, there are two 
consist edits we have figured out from historical commits:
1) . Version: git 2.3.9 – git-2.3.10
   File: builtin/merge-tree.c

dst.size = size;
-   xdi_diff(, , , , );
+   if (xdi_diff(, , , , ))
+   die("unable to generate diff");
free(src.ptr);
free(dst.ptr);
 }

2) .Version: git 2.3.9 – git-2.3.10
  File: combine-diff.c
  
-   xdi_diff_outf(_file, result_file, consume_line, ,
- , );
+   if (xdi_diff_outf(_file, result_file, consume_line, ,
+ , ))
+   die("unable to generate combined diff for %s",
+   sha1_to_hex(parent));
free(parent_file.ptr);

Those two commits both add if structure and log messages for handling the 
return value of xdi_diff_outf(). 
And in the latest release, we find one candidate that may also need log 
statements inserted:
1)  File: git-2.14.2/builtin/rerere.c

 1  static int diff_two(const char *file1, const char *label1,
 2  const char *file2, const char *label2)
 3  {
….
20  ret = xdi_diff(, , , , );
21  
22  free(minus.ptr);
23  free(plus.ptr);
24  return ret;
25  }
...
}

There are more examples of consistent update and corresponding suggestions in 
attachment. It is so nice of you to read them and share me with your opinion on 
the correctness of our suggestion. Thanks a lot. <>
<>
<>

Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat

[brian m. carlson]

On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:
> This extension selects which hashing algorithm from vtable should be
> used for reading and writing objects in the object store.  At the moment
> supports only single value (sha-1).

I think you want an "it" here: "At the moment *it* supports".

> In case value of objectFormat is an unknown hashing algorithm, Git
> command will fail with following message:
> 
>   fatal: unknown repository extensions found:
> objectformat = 
> 
> To indicate, that this specific objectFormat value is not recognised.
> 
> The objectFormat extension is not allowed in repository marked as
> version 0 to prevent any possibility of accidentally writing a NewHash
> object in the sha-1 object store. This extension behaviour is different
> than preciousObjects extension (which is allowed in repo version 0).
> 
> Add tests and documentation note about new extension.
> 
> Signed-off-by: Patryk Obara 

Other than that, the patch looks good to me.  I like that we reject
invalid values immediately.  Adding documentation is good, too.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Cloned repository has file changes -> bug?

[brian m. carlson]

On Sat, Jan 27, 2018 at 07:35:49PM +, Filip Jorissen wrote:
> I think our git repository is bugged. The reason why I say this is the 
> following. When cloning the repository, the newly cloned repository 
> immediately has file changes. Steps to reproduce and illustration is at the 
> end of this email. Git checkout does not work to remove the file changes. 
> This behavior seems to be reproducible across multiple computers. Is this a 
> bug? How can I fix the repository?
> 
> Changes not staged for commit:
>   (use "git add ..." to update what will be committed)
>   (use "git checkout -- ..." to discard changes in working directory)
> 
>   modified:   
> IDEAS/Resources/ReferenceResults/Dymola/IDEAS_Fluid_HeatExchangers_GroundHeatExchangers_Borefield_Examples_MultipleBoreholesWithHeatPump.txt
>   modified:   
> IDEAS/Resources/ReferenceResults/Dymola/IDEAS_Utilities_Psychrometrics_Functions_Examples_SaturationPressure.txt

This repository has multiple files that differ only in case.  I take it
from your hostname that you're on a Mac, and by default macOS uses a
case-insensitive file system.

Since the two files aren't identical, one or the other will show as
modified.  You probably want to adjust the repository so that it doesn't
have files differing only in case or use a case-sensitive file system.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: git send-email sets date

[brian m. carlson]

On Fri, Jan 26, 2018 at 06:32:30PM +0100, Michal Suchánek wrote:
> git send-email sets the message date to author date.
> 
> This is wrong because the message will most likely not get delivered
> when the author date differs from current time. It might give slightly
> better results with commit date instead of author date but can't is
> just skip that header and leave it to the mailer?
> 
> It does not even seem to have an option to suppress adding the date
> header.

I'm pretty sure it's intended to work this way.

Without the Date header, we have no way of providing the author date
when sending a patch.  git am will read this date and use it as the
author date when applying patches, so if it's omitted, the author date
will be wrong.

If you want to send patches with a different date, you can always insert
the patch inline in your mailer using the scissors notation, which will
allow your mailer to insert its own date while keeping the patch date
separate.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Git For Aix 6 and 7

[Michael Felt]

I have git on my portal www.aixtools.net (See
http://www.aixtools.net/index.php/git)

These are installp packages - and what you install, you can uninstall.

There are some dependencies (e.g., python, gnu grep, and a few
others). Can't say I use it daily, but I do use it weekly.

For additional questions or issues with this packaging - please post
on http://forums.rootvg.net/aixtools - I see that a lot sooner than
any email (on gmail).

HTH,
Michael

On Thu, Jan 18, 2018 at 3:47 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Thu, Jan 18 2018, raikrishna jotted:
>
>> Hi Team,
>>
>> I have an urgent requirement to install Git client for Aix 6 and 7, could
>> you please help send me or navigate me to the correct url.
>> My present infrastructure comprise of Aix and Linux servers , I am
>> successfully using Git on Linux however I am struggling to find correct
>> package for AIX platform.
>>
>> Appreciate your quick response.
>
> Hi raikrishna. The git-packagers list is a rather small list so perhaps
> someone on the general git list (CC'd) knows the answer to this.
>
> I'm not aware of anyone providing binary git packages for AIX, but I
> don't use it so maybe they exist.
>
> The last mention on the mailing list I could find of someone packaging
> it was this from Michael Felt's (CC'd)
> https://public-inbox.org/git/canvxnixkbakgjm+nz0cyyctoeyp23kd8s4yxsquosauahjs...@mail.gmail.com/
>
> The last AIX-related patch to git is actually mine, but I haven't logged
> into an AIX box in over a decade, see
> https://github.com/chef/omnibus-software/commit/e247e36761#diff-3df898345d670979b74acc0bf71d8c47
>
> So it looks like there's a chef build recipe for it, maybe that's
> something you can use?
>
> I would not be surprised if building git on AIX, particularly with a
> non-GNU toolchain, fails in all sorts of interesting ways. People here
> on the list would be happy to help you work through those failures,
> we're keen to port git to whatever we can get our hands on, but these
> platforms experience quite a bit of bitrot.

Re: [PATCH 3/3] perf/aggregate: sort JSON fields in output

[Philip Oakley]

From: "Christian Couder" 

It is much easier to diff the output against a preivous


s/preivous/previous/


one when the fields are sorted.

Signed-off-by: Christian Couder 
---
t/perf/aggregate.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index d616d31ca8..fcc0313e65 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -253,7 +253,7 @@ sub print_codespeed_results {
 }
 }

- print to_json(\@data, {utf8 => 1, pretty => 1}), "\n";
+ print to_json(\@data, {utf8 => 1, pretty => 1, canonical => 1}), "\n";
}

binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
--
2.16.0.rc2.45.g09a1bbd803

[PATCH 2/3] perf/aggregate: add --reponame option

[Christian Couder]

This makes it easier to use the aggregate script
on the command line when one wants to get the
"environment" fields set in the codespeed output.

Previously setting GIT_REPO_NAME was needed
for this purpose.

Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 31 +--
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index bbf0f30898..d616d31ca8 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -37,7 +37,7 @@ sub format_times {
 }
 
 my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests,
-$codespeed, $subsection);
+$codespeed, $subsection, $reponame);
 while (scalar @ARGV) {
my $arg = $ARGV[0];
my $dir;
@@ -55,6 +55,15 @@ while (scalar @ARGV) {
}
next;
}
+   if ($arg eq "--reponame") {
+   shift @ARGV;
+   $reponame = $ARGV[0];
+   shift @ARGV;
+   if (! $reponame) {
+   die "empty reponame";
+   }
+   next;
+   }
last if -f $arg or $arg eq "--";
if (! -d $arg) {
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -209,15 +218,17 @@ sub print_codespeed_results {
$executable .= ", " . $subsection;
}
 
-   my $environment;
-   if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") 
{
-   $environment = $ENV{GIT_PERF_REPO_NAME};
-   } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} 
ne "") {
-   $environment = $ENV{GIT_TEST_INSTALLED};
-   $environment =~ s|/bin-wrappers$||;
-   } else {
-   $environment = `uname -r`;
-   chomp $environment;
+   my $environment = $reponame;
+   if (! $environment) {
+   if (exists $ENV{GIT_PERF_REPO_NAME} and 
$ENV{GIT_PERF_REPO_NAME} ne "") {
+   $environment = $ENV{GIT_PERF_REPO_NAME};
+   } elsif (exists $ENV{GIT_TEST_INSTALLED} and 
$ENV{GIT_TEST_INSTALLED} ne "") {
+   $environment = $ENV{GIT_TEST_INSTALLED};
+   $environment =~ s|/bin-wrappers$||;
+   } else {
+   $environment = `uname -r`;
+   chomp $environment;
+   }
}
 
my @data;
-- 
2.16.0.rc2.45.g09a1bbd803

[PATCH 1/3] perf/aggregate: add --subsection option

[Christian Couder]

This makes it easier to use the aggregate script
on the command line, to get results from
subsections.

Previously setting GIT_PERF_SUBSECTION was needed
for this purpose.

Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

This small patch series contains a few small Codespeed related
usability improvements of the perf/aggregate.perl script on top the
cc/codespeed patch series that was recently merged into master.

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 5c439f6bc2..bbf0f30898 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -36,7 +36,8 @@ sub format_times {
return $out;
 }
 
-my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed);
+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests,
+$codespeed, $subsection);
 while (scalar @ARGV) {
my $arg = $ARGV[0];
my $dir;
@@ -45,6 +46,15 @@ while (scalar @ARGV) {
shift @ARGV;
next;
}
+   if ($arg eq "--subsection") {
+   shift @ARGV;
+   $subsection = $ARGV[0];
+   shift @ARGV;
+   if (! $subsection) {
+   die "empty subsection";
+   }
+   next;
+   }
last if -f $arg or $arg eq "--";
if (! -d $arg) {
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -76,10 +86,15 @@ if (not @tests) {
 }
 
 my $resultsdir = "test-results";
-my $results_section = "";
-if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
-   $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
-   $results_section = $ENV{GIT_PERF_SUBSECTION};
+
+if (! $subsection and
+exists $ENV{GIT_PERF_SUBSECTION} and
+$ENV{GIT_PERF_SUBSECTION} ne "") {
+   $subsection = $ENV{GIT_PERF_SUBSECTION};
+}
+
+if ($subsection) {
+   $resultsdir .= "/" . $subsection;
 }
 
 my @subtests;
@@ -183,15 +198,15 @@ sub print_default_results {
 }
 
 sub print_codespeed_results {
-   my ($results_section) = @_;
+   my ($subsection) = @_;
 
my $project = "Git";
 
my $executable = `uname -s -m`;
chomp $executable;
 
-   if ($results_section ne "") {
-   $executable .= ", " . $results_section;
+   if ($subsection) {
+   $executable .= ", " . $subsection;
}
 
my $environment;
@@ -233,7 +248,7 @@ sub print_codespeed_results {
 binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
 
 if ($codespeed) {
-   print_codespeed_results($results_section);
+   print_codespeed_results($subsection);
 } else {
print_default_results();
 }
-- 
2.16.0.rc2.45.g09a1bbd803

[PATCH 3/3] perf/aggregate: sort JSON fields in output

[Christian Couder]

It is much easier to diff the output against a preivous
one when the fields are sorted.

Signed-off-by: Christian Couder 
---
 t/perf/aggregate.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index d616d31ca8..fcc0313e65 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -253,7 +253,7 @@ sub print_codespeed_results {
}
}
 
-   print to_json(\@data, {utf8 => 1, pretty => 1}), "\n";
+   print to_json(\@data, {utf8 => 1, pretty => 1, canonical => 1}), "\n";
 }
 
 binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
-- 
2.16.0.rc2.45.g09a1bbd803

Re: Creating sparse checkout in a new linked git worktree

[Duy Nguyen]

On Sun, Jan 28, 2018 at 2:25 PM, Eric Sunshine  wrote:
> On Wed, Jan 24, 2018 at 11:11 AM, Jessie Hernandez
>  wrote:
>> I am trying to get a sparse checkout in a linked worktree but cannot get
>> it working. I have tried the following
>>
>> * git worktree add /some/new/path/new-branch --no-checkout
>> * git config core.sparseCheckout true
>> * > $GIT_DIR/info/sparse-checkout>
>> * cd  /some/new/path/new-branch
>> * git read-tree -mu sparse-checkout
>>
>> But I still end up with a fully populated worktree.
>> Is there something I am missing or doing wrong?
>
> The sparse-checkout file is specific to each worktree, which allows
> you to control "sparsity" on a worktree by worktree basis. Therefore,
> you should create $GIT_DIR/worktrees//info/sparse-checkout instead
> (where  is "new-branch" in your example).

Nit. Do

$EDITOR `git rev-parse --git-path info/sparse-checkout`

if you're already in the right worktree so you don't have to find out
about  or the full path.

I wanted to add "git checkout --edit-sparse" for some time. Using it
in multiple worktrees may be a good push for me to eventually do it.
-- 
Duy