To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_whitespace_option() should return -1 instead
of calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_single_patch() should return a negative
integer instead of calling die().
Let's do that by using error() and let's adjust the related test
cases accordingly.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing or exit()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_chunk() should return a negative integer
instead of calling die() or exit().
As parse_chunk() is called only by apply_patch() which already
returns either -1 or -128 when an error happened, let's make it also
return -1 or -128.
This makes it compatible with what find_header() and parse_binary()
already return.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, let's make find_header() return -128 instead of
calling die().
We could make it return -1, unfortunately find_header() already
returns -1 when no header is found.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing. Let's do that by returning -1 instead of
die()ing in read_patch_file().
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we have to signal errors
to the caller instead of die()ing.
As a first step in this direction, let's make apply_patch() return
-1 or -128 in case of errors instead of dying. For now its only
caller apply_all_patches() will exit(128) when apply_patch()
return -128 and it will exit(1) when it returns -1.
We exit() with code 128 because that was what die() was doing
and we want to keep the distinction between exiting with code 1
and exiting with code 128.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To libify `git apply` functionality we must make 'struct apply_state'
usable outside "builtin/apply.c".
Let's do that by creating a new "apply.h" and moving
'struct apply_state' there.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To prepare for some structs and constants being moved from
builtin/apply.c to apply.h, we should give them some more
specific names to avoid possible name collisions in the global
namespace.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit message is long and has lots of background and
numbers. The summary is: the current default of 250 doesn't
save much space, and costs CPU. It's not a good tradeoff.
Read on for details.
The "--aggressive" flag to git-gc does three things:
1. use "-f" to throw out existing deltas and recompute from
scratch
2. use "--window=250" to look harder for deltas
3. use "--depth=250" to make longer delta chains
Items (1) and (2) are good matches for an "aggressive"
repack. They ask the repack to do more computation work in
the hopes of getting a better pack. You pay the costs during
the repack, and other operations see only the benefit.
Item (3) is not so clear. Allowing longer chains means fewer
restrictions on the deltas, which means potentially finding
better ones and saving some space. But it also means that
operations which access the deltas have to follow longer
chains, which affects their performance. So it's a tradeoff,
and it's not clear that the tradeoff is even a good one.
The existing "250" numbers for "--aggressive" come
originally from this thread:
http://public-inbox.org/git/alpine.LFD.0.9999.0712060803430.13796@woody.linux-foundation.org/
where Linus says:
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.
There are some numbers in that thread, but they're mostly
focused on the improved window size, and measure the
improvement from --depth=250 and --window=250 together.
E.g.:
http://public-inbox.org/git/9e4733910712062006l651571f3w7f76ce64c6650dff@mail.gmail.com/
talks about the improved run-time of "git-blame", which
comes from the reduced pack size. But most of that reduction
is coming from --window=250, whereas most of the extra costs
come from --depth=250. There's a link in that thread showing
that increasing the depth beyond 50 doesn't seem to help
much with the size:
https://vcscompare.blogspot.com/2008/06/git-repack-parameters.html
but again, no discussion of the timing impact.
In an earlier thread from Ted Ts'o which discussed setting
the non-aggressive default (from 10 to 50):
http://public-inbox.org/git/20070509134958.GA21489%40thunk.org/
we have more numbers, with the conclusion that going past 50
does not help size much, and hurts the speed of normal
operations.
So from that, we might guess that 50 is actually a sweet
spot, even for aggressive, if we interpret aggressive to
"spend time now to make a better pack". It is not clear that
"--depth=250" is actually a better pack. It may be slightly
_smaller_, but it carries a run-time penalty.
Here are some more recent timings I did to verify that. They
show three things:
- the size of the resulting pack (so disk saved to store,
bandwidth saved on clones/fetches)
- the cost of "rev-list --objects --all", which shows the
effect of the delta chains on trees (commits typically
don't delta, and the command doesn't touch the blobs at
all)
- the cost of "log -Sfoo", which will additionally access
each blob
All cases were repacked with "git repack -adf --depth=$d
--window=250" (so basically, what would happen if we tweaked
the "gc --aggressive" default depth).
The timings are all wall-clock best-of-3. The machine itself
has plenty of RAM compared to the repositories (which is
probably typical of most workstations these days), so we're
really measuring CPU usage, as the whole thing will be in
disk cache after the first run.
The core.deltaBaseCacheLimit is at its default of 96MiB.
It's possible that tweaking it would have some impact on the
tests, as some of them (especially "log -S" on a large repo)
are likely to overflow that. But bumping that carries a
run-time memory cost, so for these tests, I focused on what
we could do just with the on-disk pack tradeoffs.
Each test is done for four depths: 250 (the current value),
50 (the current default that tested well previously), 100
(to show something on the larger side, which previous tests
showed was not a good tradeoff), and 10 (the very old
default, which previous tests showed was worse than 50).
Here are the numbers for linux.git:
depth | size | % | rev-list | % | log -Sfoo | %
-------+-------+-------+----------+--------+-----------+-------
250 | 967MB | n/a | 48.159s | n/a | 378.088 | n/a
100 | 971MB | +0.4% | 41.471s | -13.9% | 342.060 | -9.5%
50 | 979MB | +1.2% | 37.778s | -21.6% | 311.040s | -17.7%
10 | 1.1GB | +6.6% | 32.518s | -32.5% | 279.890s | -25.9%
and for git.git:
depth | size | % | rev-list | % | log -Sfoo | %
-------+-------+-------+----------+--------+-----------+-------
250 | 48MB | n/a | 2.215s | n/a | 20.922s | n/a
100 | 49MB | +0.5% | 2.140s | -3.4% | 17.736s | -15.2%
50 | 49MB | +1.7% | 2.099s | -5.2% | 15.418s | -26.3%
10 | 53MB | +9.3% | 2.001s | -9.7% | 12.677s | -39.4%
You can see that that the CPU savings for regular operations improves as we
decrease the depth. The savings are less for "rev-list" on a smaller repository
than they are for blob-accessing operations, or even rev-list on a larger
repository. This may mean that a larger delta cache would help (though setting
core.deltaBaseCacheLimit by itself doesn't).
But we can also see that the space savings are not that great as the depth goes
higher. Saving 5-10% between 10 and 50 is probably worth the CPU tradeoff.
Saving 1% to go from 50 to 100, or another 0.5% to go from 100 to 250 is
probably not.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add lf_to_nul helper function to test-lib-functions.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update status manpage to include information about
porcelain v2 format.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Expand porcelain v2 output to include branch and tracking
branch information. This includes the commit id, the branch,
the upstream branch, and the ahead and behind counts.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Print per-file information in porcelain v2 format.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Collect extra per-file data for porcelain V2 format.
The output of `git status --porcelain` leaves out many
details about the current status that clients might like
to have. This can force them to be less efficient as they
may need to launch secondary commands (and try to match
the logic within git) to accumulate this extra information.
For example, a GUI IDE might want the file mode to display
the correct icon for a changed item (without having to stat
it afterwards).
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A few updates to "git submodule update".
Use of "| wc -l" break with BSD variant of 'wc'.
* sb/submodule-update-dot-branch:
t7406: fix breakage on OSX
submodule update: allow '.' for branch value
submodule--helper: add remote-branch helper
submodule-config: keep configured branch around
submodule--helper: fix usage string for relative-path
submodule update: narrow scope of local variable
submodule update: respect depth in subsequent fetches
t7406: future proof tests with hard coded depth
"git am -3" calls "git merge-recursive" when it needs to fall back
to a three-way merge; this call has been turned into an internal
subroutine call instead of spawning a separate subprocess.
* js/am-3-merge-recursive-direct:
merge-recursive: flush output buffer even when erroring out
merge_trees(): ensure that the callers release output buffer
merge-recursive: offer an option to retain the output in 'obuf'
merge-recursive: write the commit title in one go
merge-recursive: flush output buffer before printing error messages
am -3: use merge_recursive() directly again
merge-recursive: switch to returning errors instead of dying
merge-recursive: handle return values indicating errors
merge-recursive: allow write_tree_from_memory() to error out
merge-recursive: avoid returning a wholesale struct
merge_recursive: abort properly upon errors
prepare the builtins for a libified merge_recursive()
merge-recursive: clarify code in was_tracked()
die(_("BUG")): avoid translating bug messages
die("bug"): report bugs consistently
t5520: verify that `pull --rebase` shows the helpful advice when failing
"git format-patch" learned format.from configuration variable to
specify the default settings for its "--from" option.
* jt/format-patch-from-config:
format-patch: format.from gives the default for --from
"git push --force-with-lease" already had enough logic to allow
ensuring that such a push results in creation of a ref (i.e. the
receiving end did not have another push from sideways that would be
discarded by our force-pushing), but didn't expose this possibility
to the users. It does so now.
* jk/push-force-with-lease-creation:
t5533: make it pass on case-sensitive filesystems
push: allow pushing new branches with --force-with-lease
push: add shorthand for --force-with-lease branch creation
Documentation/git-push: fix placeholder formatting
Not-so-recent rewrite of "git am" that started making internal
calls into the commit machinery had an unintended regression, in
that no matter how many seconds it took to apply many patches, the
resulting committer timestamp for the resulting commits were all
the same.
* jk/reset-ident-time-per-commit:
am: reset cached ident date for each patch
A minor documentation update.
This was split out from a stalled jh/clean-smudge-annex topic
before discarding it.
* jh/clean-smudge-f-doc:
clarify %f documentation
The API documentation for hashmap was unclear if hashmap_entry
can be safely discarded without any other consideration. State
that it is safe to do so.
* jc/hashmap-doc-init:
hashmap: clarify that hashmap_entry can safely be discarded
FreeBSD can lie when asked mtime of a directory, which made the
untracked cache code to fall back to a slow-path, which in turn
caused tests in t7063 to fail because it wanted to verify the
behaviour of the fast-path.
* nd/fbsd-lazy-mtime:
t7063: work around FreeBSD's lazy mtime update feature
The characters in the label shown for tags/refs for commits in
"gitweb" output are now properly escaped for proper HTML output.
* ab/gitweb-link-html-escape:
gitweb: escape link body in format_ref_marker
Windows port was failing some tests in t4130, due to the lack of
inum in the returned values by its lstat(2) emulation.
* js/t4130-rename-without-ino:
t4130: work around Windows limitation
"git -c grep.patternType=extended log --basic-regexp" misbehaved
because the internal API to access the grep machinery was not
designed well.
* jc/grep-commandline-vs-configuration:
grep: further simplify setting the pattern type
There is an optimization used in "git diff $treeA $treeB" to borrow
an already checked-out copy in the working tree when it is known to
be the same as the blob being compared, expecting that open/mmap of
such a file is faster than reading it from the object store, which
involves inflating and applying delta. This however kicked in even
when the checked-out copy needs to go through the convert-to-git
conversion (including the clean filter), which defeats the whole
point of the optimization. The optimization has been disabled when
the conversion is necessary.
* jk/diff-do-not-reuse-wtf-needs-cleaning:
diff: do not reuse worktree files that need "clean" conversion
The build procedure for "git persistent-https" helper (in contrib/)
has been updated so that it can be built with more recent versions
of Go.
* pm/build-persistent-https-with-recent-go:
contrib/persistent-https: use Git version for build label
contrib/persistent-https: update ldflags syntax for Go 1.7+
"git merge" in Git v2.9 was taught to forbid merging an unrelated
lines of history by default, but that is exactly the kind of thing
the "--rejoin" mode of "git subtree" (in contrib/) wants to do.
"git subtree" has been taught to use the "--allow-unrelated-histories"
option to override the default.
* da/subtree-2.9-regression:
subtree: fix "git subtree split --rejoin"
t7900-subtree.sh: fix quoting and broken && chains
"git commit --help" said "--no-verify" is only about skipping the
pre-commit hook, and failed to say that it also skipped the
commit-msg hook.
* os/no-verify-skips-commit-msg-too:
commit: describe that --no-verify skips the commit-msg hook in the help text
The use of strbuf in "git rm" to build filename to remove was a bit
suboptimal, which has been fixed.
* rs/rm-strbuf-optim:
rm: reuse strbuf for all remove_dir_recursively() calls
Users of the parse_options_concat() API function need to allocate
extra slots in advance and fill them with OPT_END() when they want
to decide the set of supported options dynamically, which makes the
code error-prone and hard to read. This has been corrected by tweaking
the API to allocate and return a new copy of "struct option" array.
* jk/parse-options-concat:
parse_options: allocate a new array when concatenating
Existing autoconf generated test for the need to link with pthread
library did not check all the functions from pthread libraries;
recent FreeBSD has some functions in libc but not others, and we
mistakenly thought linking with libc is enough when it is not.
* ew/autoconf-pthread:
configure.ac: stronger test for pthread linkage
The .c/.h sources are marked as such in our .gitattributes file so
that "git diff -W" and friends would work better.
* rs/help-c-source-with-gitattributes:
.gitattributes: set file type for C files
"git status" learned to suggest "merge --abort" during a conflicted
merge, just like it already suggests "rebase --abort" during a
conflicted rebase.
* mm/status-suggest-merge-abort:
status: suggest 'git merge --abort' when appropriate
On OSX `wc` prefixes the output of numbers with whitespace, such
that the `commit_count` would be "SP <NUMBER>". When using that in
git submodule update --init --depth=$commit_count
the depth would be empty and the number is interpreted as the
pathspec. Fix this by not using `wc` and rather instruct rev-list
to count.
Another way to fix this is to remove the `=` sign after the
`--depth` argument as then we are allowed to have more than just one
whitespace between `--depth` and the actual number. Prefer the
solution of rev-list counting as that is expected to be slightly
faster and more self-contained within Git.
Reported-by: Lars Schneider <larsxschneider@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>,
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Avoid waking up the readers for unnecessary context switches for
each line of header data being written, as all the headers are
written in short succession.
It is unlikely any HTTP/1.x server would want to read a CGI
response one-line-at-a-time and trickle each to the client.
Instead, I'd expect HTTP servers want to minimize syscall and
TCP/IP framing overhead by trying to send all of its response
headers in a single syscall or even combining the headers and
first chunk of the body with MSG_MORE or writev.
Verified by strace-ing response parsing on the CGI side.
Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When developing another patch series I had a temporary state in
which git-clone would segfault, when the call was prepared in
prepare_to_clone_next_submodule. This lead to the call failing,
i.e. in `update_clone_task_finished` the task was scheduled to be
tried again. The second call to prepare_to_clone_next_submodule
would return 0, as the segfaulted clone did create the .git file
already, such that was not considered to need to be cloned again. I
was seeing the "BUG: ce was a submodule before?\n" message, which
was the correct behavior at the time as my local code was
buggy. When trying to debug this failure, I tried to use printing
messages into the strbuf that is passed around, but these messages
were never printed as the die(..) doesn't flush the `err` strbuf.
When implementing the die() in 665b35ecc (2016-06-09, "submodule--helper:
initial clone learns retry logic"), I considered this condition to be
a severe condition, which should lead to an immediate abort as we do not
trust ourselves any more. However the queued messages in `err` are valuable
so let's not toss them out by immediately dying, but a graceful return.
Another thing to note: The error message itself was misleading. A return
value of 0 doesn't indicate the passed in `ce` is not a submodule any more,
but just that we do not consider cloning it any more.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
