gc --aggressive: make --depth configurable

When 1c192f3 (gc --aggressive: make it really aggressive - 2007-12-06)
made --depth=250 the default value, it didn't really explain the
reason behind, especially the pros and cons of --depth=250.

An old mail from Linus below explains it at length. Long story short,
--depth=250 is a disk saver and a performance killer. Not everybody
agrees on that aggressiveness. Let the user configure it.

    From: Linus Torvalds <torvalds@linux-foundation.org>
    Subject: Re: [PATCH] gc --aggressive: make it really aggressive
    Date: Thu, 6 Dec 2007 08:19:24 -0800 (PST)
    Message-ID: <alpine.LFD.0.9999.0712060803430.13796@woody.linux-foundation.org>
    Gmane-URL: http://article.gmane.org/gmane.comp.gcc.devel/94637

    On Thu, 6 Dec 2007, Harvey Harrison wrote:
    >
    > 7:41:25elapsed 86%CPU

    Heh. And this is why you want to do it exactly *once*, and then just
    export the end result for others ;)

    > -r--r--r-- 1 hharrison hharrison 324094684 2007-12-06 07:26 pack-1d46...pack

    But yeah, especially if you allow longer delta chains, the end result can
    be much smaller (and what makes the one-time repack more expensive is the
    window size, not the delta chain - you could make the delta chains longer
    with no cost overhead at packing time)

    HOWEVER.

    The longer delta chains do make it potentially much more expensive to then
    use old history. So there's a trade-off. And quite frankly, a delta depth
    of 250 is likely going to cause overflows in the delta cache (which is
    only 256 entries in size *and* it's a hash, so it's going to start having
    hash conflicts long before hitting the 250 depth limit).

    So when I said "--depth=250 --window=250", I chose those numbers more as
    an example of extremely aggressive packing, and I'm not at all sure that
    the end result is necessarily wonderfully usable. It's going to save disk
    space (and network bandwidth - the delta's will be re-used for the network
    protocol too!), but there are definitely downsides too, and using long
    delta chains may simply not be worth it in practice.

    (And some of it might just want to have git tuning, ie if people think
    that long deltas are worth it, we could easily just expand on the delta
    hash, at the cost of some more memory used!)

    That said, the good news is that working with *new* history will not be
    affected negatively, and if you want to be _really_ sneaky, there are ways
    to say "create a pack that contains the history up to a version one year
    ago, and be very aggressive about those old versions that we still want to
    have around, but do a separate pack for newer stuff using less aggressive
    parameters"

    So this is something that can be tweaked, although we don't really have
    any really nice interfaces for stuff like that (ie the git delta cache
    size is hardcoded in the sources and cannot be set in the config file, and
    the "pack old history more aggressively" involves some manual scripting
    and knowing how "git pack-objects" works rather than any nice simple
    command line switch).

    So the thing to take away from this is:
     - git is certainly flexible as hell
     - .. but to get the full power you may need to tweak things
     - .. happily you really only need to have one person to do the tweaking,
       and the tweaked end results will be available to others that do not
       need to know/care.

    And whether the difference between 320MB and 500MB is worth any really
    involved tweaking (considering the potential downsides), I really don't
    know. Only testing will tell.

			    Linus

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment.c: fix constness for odb_pack_keep()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff-no-index: correctly diagnose error return from diff_opt_parse()

diff_opt_parse() returns the number of options parsed, or often
returns error() which is defined to return -1.  Yes, return value of
0 is "I did not process that option at all", which should cause the
caller to say that, but negative return should not be forgotten.

This bug caused "diff --no-index" to infinitely show the same error
message because the returned value was used to decrement the loop
control variable, e.g.

        $ git diff --no-index --color=words a b
        error: option `color' expects "always", "auto", or "never"
        error: option `color' expects "always", "auto", or "never"
        ...

Instead, make it act like so:

        $ git diff --no-index --color=words a b
        error: option `color' expects "always", "auto", or "never"
        fatal: invalid diff option/value: --color=words

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dir.c: make git_fnmatch() not inline

Now that it calls a static inline function, it cannot be an inline
definition with external linkage. Remove inline and make it an
external definition.

Signed-off-by: Charles Bailey <cbailey32@bloomberg.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
tests: don't rely on strerror text when testing rmdir failure

AIX doesn't make a distiction between EEXIST and ENOTEMPTY; relying
on the strerror string for the rmdir failure is fragile. Just test
that the start of the string matches the Git controlled "failed to
rmdir..."  error. The exact text of the OS generated error string
isn't important to the test.

Signed-off-by: Charles Bailey <cbailey32@bloomberg.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
MSVC: fix t0040-parse-options crash

On 64-bit MSVC, pointers are 64 bit but `long` is only 32.
Thus, casting string to `unsigned long`, which is redundand on other
platforms, throws away important bits and when later cast to `intptr_t`
results in corrupt pointer.

This patch fixes test-parse-options by replacing harming cast with
correct one.

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse-options: add cast to correct pointer type to OPT_SET_PTR

Do not force users of OPT_SET_PTR to cast pointer to correct
underlying pointer type by integrating cast into OPT_SET_PTR macro.

Cast is required to prevent 'initialization makes integer from pointer
without a cast' compiler warning.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
patch-id: make it stable against hunk reordering

Patch id changes if users
1. reorder file diffs that make up a patch
or
2. split a patch up to multiple diffs that touch the same path
(keeping hunks within a single diff ordered to make patch valid).

As the result is functionally equivalent, a different patch id is
surprising to many users.
In particular, reordering files using diff -O is helpful to make patches
more readable (e.g. API header diff before implementation diff).

Change patch-id behaviour making it stable against these two kinds
of patch change:
1. calculate SHA1 hash for each hunk separately and sum all hashes
(using a symmetrical sum) to get patch id
2. hash the file-level headers together with each hunk (not just the
first hunk)

We use a 20byte sum and not xor - since xor would give 0 output
for patches that have two identical diffs, which isn't all that
unlikely (e.g. append the same line in two places).

Add a new flag --unstable to get the historical behaviour.

Add --stable which is a nop, for symmetry.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
patch-id: document new behaviour

Clarify that patch ID is now a sum of hashes, not a hash.
Document --stable and --unstable flags.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
patch-id-test: test --stable and --unstable flags

Verify that patch ID is now stable against diff split and reordering.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse-options: remove unused OPT_SET_PTR

OPT_SET_PTR was never used since its creation at db7244bd
(parse-options new features., 2007-11-07).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
t1400: fix test name and expected result of one test

The test

    stdin -z create ref fails with zero new value

actually passes an empty new value, not a zero new value.  So rename
the test s/zero/empty/, and change the expected error from

    fatal: create $c given zero new value

to

    fatal: create $c missing <newvalue>

Of course, this makes the test fail now, so mark it
test_expect_failure.  The failure will be fixed later in this patch
series.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t1400: provide more usual input to the command

The old version was passing (among other things)

    update SP refs/heads/c NUL NUL 0{40} NUL

to "git update-ref -z --stdin" to test whether the old-value check for
c is working.  But the <newvalue> is empty, which is a bit off the
beaten track.

So, to be sure that we are testing what we want to test, provide an
actual <newvalue> on the "update" line.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse_arg(): really test that argument is properly terminated

Test that the argument is properly terminated by either whitespace or
a NUL character, even if it is quoted, to be consistent with the
non-quoted case.  Adjust the tests to expect the new error message.
Add a docstring to the function, incorporating the comments that were
formerly within the function plus some added information.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t1400: add some more tests involving quoted arguments

Previously there were no good tests of C-quoted arguments.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs.h: rename the action_on_err constants

Given that these constants are only being used when updating
references, it is inappropriate to give them such generic names as
"DIE_ON_ERR".  So prefix their names with "UPDATE_REFS_".

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
update_refs(): tighten constness

Since full const correctness is beyond the ability of C's type system,
just put the const where it doesn't do any harm.  A (struct ref_update
**) can be passed to a (struct ref_update * const *) argument, but not
to a (const struct ref_update **) argument.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
update-ref --stdin: read the whole input at once

Read the whole input into a strbuf at once, and then parse it from
there.  This might also be a tad faster, but that is not the point.
The point is to decouple the parsing code from the input source (the
old parsing code had to read new data even in the middle of commands).
Add docstrings for the parsing functions.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse_cmd_verify(): copy old_sha1 instead of evaluating <oldvalue> twice

Aside from avoiding a tiny bit of work, this makes it transparently
obvious that old_sha1 and new_sha1 are identical.  It is arguably a
bit silly to have to set new_sha1 in order to verify old_sha1, but
that is a problem for another day.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
update-ref.c: extract a new function, parse_refname()

There is no reason to obscure the fact that parse_first_arg() always
parses refnames.  Form the new function by combining parse_first_arg()
and update_store_ref_name().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>