| Age | Commit message (Collapse) | Author | Files | Lines |
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Build with gettext breaks on recent macOS w/ Homebrew when
/usr/local/bin is not on PATH, which has been corrected.
* js/macos-gettext-build:
macOS: make sure that gettext is found
|
|
The recently added feature to add addresses that are on
anything-by: trailers in 'git send-email' was found to be way too
eager and considered nonsense strings as if they can be legitimate
beginning of *-by: trailer. This has been tightened.
* bs/sendemail-tighten-anything-by:
send-email: don't cc *-by lines with '-' prefix
|
|
"git send-email" has been taught to use quoted-printable when the
payload contains carriage-return. The use of the mechanism is in
line with the design originally added the codepath that chooses QP
when the payload has overly long lines.
* bc/send-email-qp-cr:
send-email: default to quoted-printable when CR is present
|
|
"git submodule foreach <command> --quiet" did not pass the option
down correctly, which has been corrected.
* nd/submodule-foreach-quiet:
submodule foreach: fix "<command> --quiet" not being respected
|
|
Test fix on APFS that is incapable of store paths in Latin-1.
* js/iso8895-test-on-apfs:
t9822: skip tests if file names cannot be ISO-8859-1 encoded
|
|
The GETTEXT_POISON test option has been quite broken ever since it
was made runtime-tunable, which has been fixed.
* jc/gettext-test-fix:
gettext tests: export the restored GIT_TEST_GETTEXT_POISON
|
|
Code clean-up and a fix for "git fetch" by an explicit object name
(as opposed to fetching refs by name).
* jk/fetch-reachability-error-fix:
fetch: do not consider peeled tags as advertised tips
remote.c: make singular free_ref() public
fetch: use free_refs()
pkt-line: prepare buffer before handling ERR packets
upload-pack: send ERR packet for non-tip objects
t5530: check protocol response for "not our ref"
t5516: drop ok=sigpipe from unreachable-want tests
|
|
The code is updated to check the result of memory allocation before
it is used in more places, by using xmalloc and/or xcalloc calls.
* jk/xmalloc:
progress: use xmalloc/xcalloc
xdiff: use xmalloc/xrealloc
xdiff: use git-compat-util
test-prio-queue: use xmalloc
|
|
A test update.
* km/t3000-retitle:
t3000 (ls-files -o): widen description to reflect current tests
|
|
An underallocation in the code to read the untracked cache
extension has been corrected.
* js/untracked-cache-allocfix:
untracked cache: fix off-by-one
|
|
Test fix.
* js/t3301-unbreak-notes-test:
t3301: fix false negative
|
|
Doc formatting fix.
* tz/doc-apostrophe-no-longer-needed:
Documentation/git-show-branch: avoid literal {apostrophe}
|
|
"git blame -- path" in a non-bare repository starts blaming from
the working tree, and the same command in a bare repository errors
out because there is no working tree by definition. The command
has been taught to instead start blaming from the commit at HEAD,
which is more useful.
* sg/blame-in-bare-start-at-head:
blame: default to HEAD in a bare repo when no start commit is given
|
|
Debugging code fix.
* tg/ls-files-debug-format-fix:
ls-files: use correct format string
|
|
Docfix.
* po/describe-not-necessarily-7:
describe doc: remove '7-char' abbreviation reference
|
|
Docfix.
* po/rerere-doc-fmt:
rerere doc: quote `rerere.enabled`
|
|
Updating the display with progress message has been cleaned up to
deal better with overlong messages.
* sg/overlong-progress-fix:
progress: break too long progress bar lines
progress: clear previous progress update dynamically
progress: assemble percentage and counters in a strbuf before printing
progress: make display_progress() return void
|
|
While running "git diff" in a lazy clone, we can upfront know which
missing blobs we will need, instead of waiting for the on-demand
machinery to discover them one by one. Aim to achieve better
performance by batching the request for these promised blobs.
* jt/batch-fetch-blobs-in-diff:
diff: batch fetching of missing blobs
sha1-file: support OBJECT_INFO_FOR_PREFETCH
|
|
Update docs around "gc".
* ab/gc-docs:
gc docs: remove incorrect reference to gc.auto=0
gc docs: clarify that "gc" doesn't throw away referenced objects
gc docs: note "gc --aggressive" in "fast-import"
gc docs: downplay the usefulness of --aggressive
gc docs: note how --aggressive impacts --window & --depth
gc docs: fix formatting for "gc.writeCommitGraph"
gc docs: re-flow the "gc.*" section in "config"
gc docs: include the "gc.*" section from "config" in "gc"
gc docs: clean grammar for "gc.bigPackThreshold"
gc docs: stop noting "repack" flags
gc docs: modernize the advice for manually running "gc"
|
|
"git submodule" learns "set-branch" subcommand that allows the
submodule.*.branch settings to be modified.
* dl/submodule-set-branch:
submodule: teach set-branch subcommand
submodule--helper: teach config subcommand --unset
git-submodule.txt: "--branch <branch>" option defaults to 'master'
|
|
Performance fix for "rev-list --parents -- pathspec".
* jk/revision-rewritten-parents-in-prio-queue:
revision: use a prio_queue to hold rewritten parents
|
|
Performance fix around "git blame", especially in a linear history
(which is the norm we should optimize for).
* dk/blame-keep-origin-blob:
blame.c: don't drop origin blobs as eagerly
|
|
Conversion from unsigned char[20] to struct object_id continues.
* bc/hash-transition-16: (35 commits)
gitweb: make hash size independent
Git.pm: make hash size independent
read-cache: read data in a hash-independent way
dir: make untracked cache extension hash size independent
builtin/difftool: use parse_oid_hex
refspec: make hash size independent
archive: convert struct archiver_args to object_id
builtin/get-tar-commit-id: make hash size independent
get-tar-commit-id: parse comment record
hash: add a function to lookup hash algorithm by length
remote-curl: make hash size independent
http: replace sha1_to_hex
http: compute hash of downloaded objects using the_hash_algo
http: replace hard-coded constant with the_hash_algo
http-walker: replace sha1_to_hex
http-push: remove remaining uses of sha1_to_hex
http-backend: allow 64-character hex names
http-push: convert to use the_hash_algo
builtin/pull: make hash-size independent
builtin/am: make hash size independent
...
|
|
"git fast-import" update.
* en/fast-import-parsing-fix:
fast-import: fix erroneous handling of get-mark with empty orphan commits
fast-import: only allow cat-blob requests where it makes sense
fast-import: check most prominent commands first
git-fast-import.txt: fix wording about where ls command can appear
t9300: demonstrate bug with get-mark and empty orphan commits
|
|
Fix for protocol v2 support in "git fetch-pack" of shallow clones.
* jt/fetch-no-update-shallow-in-proto-v2:
fetch-pack: respect --no-update-shallow in v2
fetch-pack: call prepare_shallow_info only if v0
|
|
Performance fix around "git fetch" that grabs many refs.
* jt/fetch-pack-wanted-refs-optim:
fetch-pack: binary search when storing wanted-refs
|
|
Allow tracing of Git executable while running the testsuite.
* ab/test-lib-pass-trace2-env:
test-lib: whitelist GIT_TR2_* in the environment
|
|
A progress indicator has been added to the "index-pack" step, which
often makes users wait for completion during "git clone".
* sg/index-pack-progress:
index-pack: show progress while checking objects
|
|
Code cleanup with more careful error checking before using data
read from the commit-graph file.
* ab/commit-graph-fixes:
commit-graph: improve & i18n error messages
commit-graph write: don't die if the existing graph is corrupt
commit-graph verify: detect inability to read the graph
commit-graph: don't pass filename to load_commit_graph_one_fd_st()
commit-graph: don't early exit(1) on e.g. "git status"
commit-graph: fix segfault on e.g. "git status"
commit-graph tests: test a graph that's too small
commit-graph tests: split up corrupt_graph_and_verify()
|
|
Fix various glitches in "git gc" around reflog handling.
* ab/gc-reflog:
gc: handle & check gc.reflogExpire config
reflog tests: assert lack of early exit with expiry="never"
reflog tests: test for the "points nowhere" warning
reflog tests: make use of "test_config" idiom
gc: refactor a "call me once" pattern
gc: convert to using the_hash_algo
gc: remove redundant check for gc_auto_threshold
|
|
"git checkout -m <other>" was about carrying the differences
between HEAD and the working-tree files forward while checking out
another branch, and ignored the differences between HEAD and the
index. The command has been taught to abort when the index and the
HEAD are different.
* nd/checkout-m:
checkout: prevent losing staged changes with --merge
read-tree: add --quiet
unpack-trees: rename "gently" flag to "quiet"
unpack-trees: keep gently check inside add_rejected_path
|
|
"git difftool" can now run outside a repository.
* js/difftool-no-index:
difftool: allow running outside Git worktrees with --no-index
parse-options: make OPT_ARGUMENT() more useful
difftool: remove obsolete (and misleading) comment
|
|
"git cherry-pick --options A..B", after giving control back to the
user to ask help resolving a conflicted step, did not honor the
options it originally received, which has been corrected.
* pw/cherry-pick-continue:
cherry-pick --continue: remember options
cherry-pick: demonstrate option amnesia
sequencer: break some long lines
|
|
The message given when "git commit -a <paths>" errors out has been
updated.
* nd/commit-a-with-paths-msg-update:
commit: improve error message in "-a <paths>" case
|
|
Code clean-up around a much-less-important-than-it-used-to-be
update_server_info() funtion.
* jk/server-info-rabbit-hole:
update_info_refs(): drop unused force parameter
server-info: drop objdirlen pointer arithmetic
server-info: drop nr_alloc struct member
server-info: use strbuf to read old info/packs file
server-info: simplify cleanup in parse_pack_def()
server-info: fix blind pointer arithmetic
http: simplify parsing of remote objects/info/packs
packfile: fix pack basename computation
midx: check both pack and index names for containment
t5319: drop useless --buffer from cat-file
t5319: fix bogus cat-file argument
pack-revindex: open index if necessary
packfile.h: drop extern from function declarations
|
|
Fourth batch to teach the diff machinery to use the parse-options
API.
* nd/diff-parseopt-4:
am: avoid diff_opt_parse()
diff --no-index: use parse_options() instead of diff_opt_parse()
range-diff: use parse_options() instead of diff_opt_parse()
diff.c: allow --no-color-moved-ws
diff-parseopt: convert --color-moved-ws
diff-parseopt: convert --[no-]color-moved
diff-parseopt: convert --inter-hunk-context
diff-parseopt: convert --no-prefix
diff-parseopt: convert --line-prefix
diff-parseopt: convert --[src|dst]-prefix
diff-parseopt: convert --[no-]abbrev
diff-parseopt: convert --diff-filter
diff-parseopt: convert --find-object
diff-parseopt: convert -O
diff-parseopt: convert --pickaxe-all|--pickaxe-regex
diff-parseopt: convert -S|-G
diff-parseopt: convert -l
diff-parseopt: convert -z
diff-parseopt: convert --ita-[in]visible-in-index
diff-parseopt: convert --ws-error-highlight
|
|
Code cleanup.
* jk/unused-params-even-more:
parse_opt_ref_sorting: always use with NONEG flag
pretty: drop unused strbuf from parse_padding_placeholder()
pretty: drop unused "type" parameter in needs_rfc2047_encoding()
parse-options: drop unused ctx parameter from show_gitcomp()
fetch_pack(): drop unused parameters
report_path_error(): drop unused prefix parameter
unpack-trees: drop unused error_type parameters
unpack-trees: drop name_entry from traverse_by_cache_tree()
test-date: drop unused "now" parameter from parse_dates()
update-index: drop unused prefix_length parameter from do_reupdate()
log: drop unused "len" from show_tagger()
log: drop unused rev_info from early output
revision: drop some unused "revs" parameters
|
|
Test framework update to more robustly clean up leftover files and
processes after tests are done.
* sg/test-atexit:
t9811-git-p4-label-import: fix pipeline negation
git p4 test: disable '-x' tracing in the p4d watchdog loop
git p4 test: simplify timeout handling
git p4 test: clean up the p4d cleanup functions
git p4 test: use 'test_atexit' to kill p4d and the watchdog process
t0301-credential-cache: use 'test_atexit' to stop the credentials helper
tests: use 'test_atexit' to stop httpd
git-daemon: use 'test_atexit` to stop 'git-daemon'
test-lib: introduce 'test_atexit'
t/lib-git-daemon: make sure to kill the 'git-daemon' process
test-lib: fix interrupt handling with 'dash' and '--verbose-log -x'
|
|
The scripted version of "git rebase -i" wrote and rewrote the todo
list many times during a single step of its operation, and the
recent C-rewrite made a faithful conversion of the logic to C. The
implementation has been updated to carry necessary information
around in-core to avoid rewriting the same file over and over
unnecessarily.
* ag/sequencer-reduce-rewriting-todo:
rebase--interactive: move transform_todo_file()
sequencer: use edit_todo_list() in complete_action()
rebase-interactive: rewrite edit_todo_list() to handle the initial edit
rebase-interactive: append_todo_help() changes
rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
sequencer: refactor skip_unnecessary_picks() to work on a todo_list
rebase--interactive: move rearrange_squash_in_todo_file()
rebase--interactive: move sequencer_add_exec_commands()
sequencer: change complete_action() to use the refactored functions
sequencer: make sequencer_make_script() write its script to a strbuf
sequencer: refactor rearrange_squash() to work on a todo_list
sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
sequencer: refactor check_todo_list() to work on a todo_list
sequencer: introduce todo_list_write_to_file()
sequencer: refactor transform_todos() to work on a todo_list
sequencer: remove the 'arg' field from todo_item
sequencer: make the todo_list structure public
sequencer: changes in parse_insn_buffer()
|
|
A new hook "post-index-change" is called when the on-disk index
file changes, which can help e.g. a virtualized working tree
implementation.
* bp/post-index-change-hook:
read-cache: add post-index-change hook
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"git mergetool" learned to offer Sublime Merge (smerge) as one of
its backends.
* da/smerge:
contrib/completion: add smerge to the mergetool completion candidates
mergetools: add support for smerge (Sublime Merge)
|
|
Code clean-up.
* dl/flex-str-cocci:
cocci: FLEX_ALLOC_MEM to FLEX_ALLOC_STR
midx.c: convert FLEX_ALLOC_MEM to FLEX_ALLOC_STR
|
|
The tests have been updated not to rely on the abbreviated option
names the parse-options API offers, to protect us from an
abbreviated form of an option that used to be unique within the
command getting non-unique when a new option that share the same
prefix is added.
* js/spell-out-options-in-tests:
tests: disallow the use of abbreviated options (by default)
tests (pack-objects): use the full, unabbreviated `--revs` option
tests (status): spell out the `--find-renames` option in full
tests (push): do not abbreviate the `--follow-tags` option
t5531: avoid using an abbreviated option
t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match`
tests (rebase): spell out the `--force-rebase` option
tests (rebase): spell out the `--keep-empty` option
|
|
The documentation for "git read-tree --reset -u" has been updated.
* nd/read-tree-reset-doc:
read-tree.txt: clarify --reset and worktree changes
|
|
Doc update.
* nd/interpret-trailers-docfix:
interpret-trailers.txt: start the desc line with a capital letter
|
|
Update our support to format documentation in the CI environment,
either with AsciiDoc ro Asciidoctor.
* sg/asciidoctor-in-ci:
ci: fix AsciiDoc/Asciidoctor stderr check in the documentation build job
ci: stick with Asciidoctor v1.5.8 for now
ci: install Asciidoctor in 'ci/install-dependencies.sh'
Documentation/technical/protocol-v2.txt: fix formatting
Documentation/technical/api-config.txt: fix formatting
Documentation/git-diff-tree.txt: fix formatting
|
|
Doc updates.
* tz/asciidoctor-fixes:
Documentation/git-status: fix titles in porcelain v2 section
Documentation/rev-list-options: wrap --date=<format> block with "--"
|
|
Dev support update.
* js/check-docs-exe:
check-docs: fix for setups where executables have an extension
check-docs: do not expect guide pages to correspond to commands
check-docs: really look at the documented commands again
docs: do not document the `git remote-testgit` command
docs: move gitremote-helpers into section 7
|
|
A buglet in configuration parser has been fixed.
* nd/include-if-wildmatch:
config: correct '**' matching in includeIf patterns
|
|
Clean-up markup in the documentation suite.
* cb/doco-mono:
doc: format pathnames and URLs as monospace.
doc/CodingGuidelines: URLs and paths as monospace
|
|
A regression fix.
* tg/stash-in-c-show-default-to-p-fix:
stash: setup default diff output format if necessary
|
|
Further fixes to "git stash" reimplemented in C.
* js/stash-in-c-pathspec-fix:
stash: pass pathspec as pointer
built-in stash: handle :(glob) pathspecs again
legacy stash: fix "rudimentary backport of -q"
|
|
Code clean-up.
* tb/stash-in-c-unused-param-fix:
stash: drop unused parameter
|
|
"git stash" rewritten in C.
* ps/stash-in-c: (28 commits)
tests: add a special setup where stash.useBuiltin is off
stash: optionally use the scripted version again
stash: add back the original, scripted `git stash`
stash: convert `stash--helper.c` into `stash.c`
stash: replace all `write-tree` child processes with API calls
stash: optimize `get_untracked_files()` and `check_changes()`
stash: convert save to builtin
stash: make push -q quiet
stash: convert push to builtin
stash: convert create to builtin
stash: convert store to builtin
stash: convert show to builtin
stash: convert list to builtin
stash: convert pop to builtin
stash: convert branch to builtin
stash: convert drop and clear to builtin
stash: convert apply to builtin
stash: mention options in `show` synopsis
stash: add tests for `git stash show` config
stash: rename test cases to be more descriptive
...
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Code cleanup.
* sg/t5318-cleanup:
t5318-commit-graph: remove unused variable
|
|
Test update.
* jt/t5551-protocol-v2-does-not-have-half-auth:
t5551: mark half-auth no-op fetch test as v0-only
|
|
dumb-http walker has been updated to share more error recovery
strategy with the normal codepath.
* jk/http-walker-status-fix:
http: use normalize_curl_result() instead of manual conversion
http: normalize curl results for dumb loose and alternates fetches
http: factor out curl result code normalization
|
|
"git multi-pack-index verify" did not scale well with the number of
packfiles, which is being improved.
* jh/midx-verify-too-many-packs:
midx: during verify group objects by packfile to speed verification
midx: add progress indicators in multi-pack-index verify
trace2:data: add trace2 data to midx
progress: add sparse mode to force 100% complete message
|
|
Unicode update.
* bb/unicode-12:
unicode: update the width tables to Unicode 12
|
|
A corner case bug in the refs API has been corrected.
* jk/refs-double-abort:
refs/files-backend: don't look at an aborted transaction
refs/files-backend: handle packed transaction prepare failure
|
|
Doc about the above.
* nd/checkout-m-doc-update:
checkout.txt: note about losing staged changes with --merge
|
|
Fix some code that passed a NULL when a va_list was expected.
* tb/trace2-va-list-fix:
trace2: NULL is not allowed for va_list
|
|
The completion helper code now pays attention to repository-local
configuration (when available), which allows --list-cmds to honour
a repository specific setting of completion.commands, for example.
* tz/completion:
completion: use __git when calling --list-cmds
completion: fix multiple command removals
t9902: test multiple removals via completion.commands
git: read local config in --list-cmds
|
|
Dev support update to make it easier to compare two formatted
results from our documentation.
* ma/doc-diff-doc-vs-doctor-comparison:
doc-diff: add `--cut-header-footer`
doc-diff: support diffing from/to AsciiDoc(tor)
doc-diff: let `render_tree()` take an explicit directory name
Doc: auto-detect changed build flags
|
|
Work-around extra warning from bash in our tests.
* tz/t4038-bash-redirect-target-workaround:
t4038-diff-combined: quote paths with whitespace
|
|
Retire scripted "git rebase" implementation.
* ab/drop-scripted-rebase:
rebase: remove the rebase.useBuiltin setting
|
|
Code cleanup in the test framework.
* jk/perf-lib-tee:
perf-lib.sh: rely on test-lib.sh for --tee handling
|
|
Typofixes.
* ab/doc-misc-typofixes:
doc: fix typos in man pages
|
|
Docfix.
* mh/pack-protocol-doc-fix:
fix pack protocol example client/server communication
|
|
Doc update.
* tg/glossary-overlay:
glossary: add definition for overlay
|
|
"git checkout -f <branch>" while the index has an unmerged path
incorrectly left some paths in an unmerged state, which has been
corrected.
* nd/checkout-f-while-conflicted-fix:
unpack-trees: fix oneway_merge accidentally carry over stage index
|
|
Doc updates.
* pw/rerere-autoupdate:
merge: tweak --rerere-autoupdate documentation
am/cherry-pick/rebase/revert: document --rerere-autoupdate
|
|
"git format-patch" used overwrite an existing patch/cover-letter
file. A new "--no-clobber" option stops it.
* jc/format-patch-error-check:
format-patch: notice failure to open cover letter for writing
builtin/log: downcase the beginning of error messages
|
|
A corner-case object name ambiguity while the sequencer machinery
is working (e.g. "rebase -i -x") has been (half) fixed.
* js/get-short-oid-drop-cache:
get_oid(): when an object was not found, try harder
sequencer: move stale comment into correct location
sequencer: improve error message when an OID could not be parsed
rebase -i: demonstrate obscure loose object cache bug
|
|
"git subtree" (in contrib/) update.
* dl/subtree-limit-to-one-rev:
contrib/subtree: ensure only one rev is provided
|
|
"git init" forgot to read platform-specific repository
configuration, which made Windows port to ignore settings of
core.hidedotfiles, for example.
* js/init-db-update-for-mingw:
mingw: respect core.hidedotfiles = false in git-init again
|
|
Error messages given from the http transport have been updated so
that they can be localized.
* js/remote-curl-i18n:
remote-curl: mark all error messages for translation
|
|
remote-http transport did not anonymize URLs reported in its error
messages at places.
* js/anonymize-remote-curl-diag:
curl: anonymize URLs in error messages and warnings
|
|
Documentation mark-up fixes.
* ma/asciidoctor-fixes-more:
Documentation: turn middle-of-line tabs into spaces
git-svn.txt: drop escaping '\' that ends up being rendered
git.txt: remove empty line before list continuation
config/fsck.txt: avoid starting line with dash
config/diff.txt: drop spurious backtick
|
|
Build fix around use of asciidoctor instead of asciidoc
* ma/asciidoctor-fixes:
asciidoctor-extensions: fix spurious space after linkgit
Documentation/Makefile: add missing dependency on asciidoctor-extensions
Documentation/Makefile: add missing xsl dependencies for manpages
|
|
Help developers by making it easier to run most of the tests under
different versions of over-the-wire protocols.
* jt/test-protocol-version:
t5552: compensate for v2 filtering ref adv.
tests: fix protocol version for overspecifications
t5700: only run with protocol version 1
t5512: compensate for v0 only sending HEAD symrefs
t5503: fix overspecification of trace expectation
tests: always test fetch of unreachable with v0
t5601: check ssh command only with protocol v0
tests: define GIT_TEST_PROTOCOL_VERSION
|
|
Third batch to teach the diff machinery to use the parse-options
API.
* nd/diff-parseopt-3:
diff-parseopt: convert --submodule
diff-parseopt: convert --ignore-submodules
diff-parseopt: convert --textconv
diff-parseopt: convert --ext-diff
diff-parseopt: convert --quiet
diff-parseopt: convert --exit-code
diff-parseopt: convert --color-words
diff-parseopt: convert --word-diff-regex
diff-parseopt: convert --word-diff
diff-parseopt: convert --[no-]color
diff-parseopt: convert --[no-]follow
diff-parseopt: convert -R
diff-parseopt: convert -a|--text
diff-parseopt: convert --full-index
diff-parseopt: convert --binary
diff-parseopt: convert --anchored
diff-parseopt: convert --diff-algorithm
diff-parseopt: convert --histogram
diff-parseopt: convert --patience
diff-parseopt: convert --[no-]indent-heuristic
|
|
Once upon a time the force flag meant something when writing info/refs,
but it hasn't done anything since 60d0526aaa (Unoptimize info/refs
creation., 2005-09-14).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When writing objects/info/packs, we use the basename of each pack
(i.e., just the "pack-1234abcd.pack" part). We compute that manually by
adding "objdirlen + 6" to the name.
This _should_ work consistently, as we do not include non-local packs,
meaning everything should be in $objdir/pack/. Before f13d7db4af
(server-info.c: use pack_local like everybody else., 2005-12-05), this
was definitely true, since we computed "local" based on comparing the
objdir string. Since then, we're relying on the code on packfile.c to
match our expectations of p->pack_name and p->local.
I think our expectations do still hold today, but we can be a bit more
defensive by just using pack_basename() to get the base. That
future-proofs us, and should hopefully be more obviously safe to
somebody reading the code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We keep an array of struct pointers, with each one representing a single
packfile. But for some reason there is a nr_alloc parameter inside each
struct, which has never been used.
This is probably cruft left over from development, where we might have
wanted a nr_alloc to dynamically grow the list. But as it turns out, we
do not dynamically grow the list at all, but rather count up the total
number of packs and use that as a maximum size. So while we're thinking
of this, let's add an assert() that documents (and checks!) that our
allocation and fill loops stay in sync.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This old code uses fgets with a fixed-size buffer. Let's use a strbuf
instead, so we don't have to wonder if "1000" is big enough, or what
happens if we see a long line.
This also lets us drop our custom code to trim the newline.
Probably nobody actually cares about the 1000-char limit (after all, the
lines generally only say "P pack-[0-9a-f]{40}.pack"), so this is mostly
just about cleanup/readability.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We have two exits from the function: either we jump to the out_stale
label or not. But in both exits we repeat our cleanup, and the only
difference is our return value. Let's just use a variable for the return
value to avoid repeating ourselves.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When we're writing out a new objects/info/packs file, we read back the
old one to try to keep the ordering the same. When we see a line
starting with "P", we expect "P pack-1234..." and blindly jump to "line
+ 2" to parse the pack name. If we saw a line with _just_ "P" and
nothing else, we'd jump past the end of the buffer and start reading
arbitrary memory.
This shouldn't be a big attack vector, as the files are local to the
repository and written by us, but it's clearly worth fixing (we do read
remote copies of the file for dumb-http fetches, but using a totally
different parser!).
Let's instead use skip_prefix() here, which avoids pointer arithmetic
altogether. Note that this converts our switch statement to an if/else
chain, making it slightly more verbose. But it will also make it easier
to do a few follow-on cleanups.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We can use skip_prefix() and parse_oid_hex() to continuously increment
our pointer, rather than dealing with magic numbers. This also fixes a
few small shortcomings:
- if we see a line with the right prefix, suffix, and length, i.e.
matching /P pack-.{40}.pack\n/, we'll interpret the middle part as
hex without checking if it could be parsed. This could lead to us
looking at uninitialized garbage in the hash array. In practice this
means we'll just make a garbage request to the server which will
fail, though it's interesting that a malicious server could convince
us to leak 40 bytes of uninitialized stack to them.
- the current code is picky about seeing a newline at the end of file,
but we can easily be more liberal
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When we have a multi-pack-index that covers many packfiles, we try to
avoid opening the .idx for those packfiles. To do that we feed the pack
name to midx_contains_pack(). But that function wants to see only the
basename, which we compute using strrchr() to find the final slash. But
that leaves an extra "/" at the start of our string.
We can fix this by incrementing the pointer. That also raises the
question of what to do when the name does not have a '/' at all. This
should generally not happen (we always find files in "pack/"), but it
doesn't hurt to be defensive here.
Let's wrap all of that up in a helper function and make it publicly
available, since a later patch will need to use it, too.
The tests don't notice because there's nothing about opening those .idx
files that would cause us to give incorrect output. It's just a little
slower. The new test checks this case by corrupting the covered .idx,
and then making sure we don't complain about it.
We also have to tweak t5570, which intentionally corrupts a .idx file
and expects us to notice it. When run with GIT_TEST_MULTI_PACK_INDEX,
this will fail since we now will (correctly) not bother opening the .idx
at all. We can fix that by unconditionally dropping any midx that's
there, which ensures we'll have to read the .idx.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
A midx file (and the struct we parse from it) contains a list of all of
the covered packfiles, mentioned by their ".idx" names (e.g.,
"pack-1234.idx", etc). And thus calls to midx_contains_pack() expect
callers to provide the idx name.
This works for most of the calls, but the one in open_packed_git_1()
tries to feed a packed_git->pack_name, which is the ".pack" name,
meaning we'll never find a match (even if the pack is covered by the
midx).
We can fix this by converting the ".pack" to ".idx" in the caller.
However, that requires allocating a new string. Instead, let's make
midx_contains_pack() a bit friendlier, and allow it take _either_ the
.pack or .idx variant.
All cleverness in the matching code is credited to René. Bugs are mine.
There's no test here, because while this does fix _a_ bug, it's masked
by another bug in that same caller. That will be covered (with a test)
in the next patch.
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The cat-file --buffer option is the default already when using
--batch-all-objects. It doesn't hurt to specify it, but it's nice for
the test scripts to model good usage.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There's no such argument as "--unsorted"; it's spelled "--unordered".
But our test failed to notice that cat-file didn't run at all because:
1. It lost the exit code of git on the left-hand side of a pipe.
2. It was comparing two runs of the broken invocation with and without
a particular config variable (and indeed, both cases produced no
output!).
Let's fix the option, but also tweak the helper function to check the
exit code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We can't create a pack revindex if we haven't actually looked at the
index. Normally we would never get as far as creating a revindex without
having already been looking in the pack, so this code never bothered to
double-check that pack->index_data had been loaded.
But with the new multi-pack-index feature, many code paths might not
load the individual pack .idx at all (they'd find objects via the midx
and then open the .pack, but not its index).
This can't yet be triggered in practice, because a bug in the midx code
means we accidentally open up the individual .idx files anyway. But in
preparation for fixing that, let's have the revindex code check that
everything it needs has been loaded.
In most cases this will just be a quick noop. But note that this does
introduce a possibility of error (if we have to open the index and it's
corrupt), so load_pack_revindex() now returns a result code, and callers
need to handle the error.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As CodingGuidelines recommends, we do not need an "extern" when
declaring a public function. Let's drop these. Note that we leave the
extern on report_garbage(), as that is actually a function pointer, not
a function itself.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Our filter_refs() function accidentally considers the target of a peeled
tag to be advertised by the server, even though upload-pack on the
server side does not consider it so. This can result in the client
making a bogus fetch to the server, which will end with the server
complaining "not our ref". Whereas the correct behavior is for the
client to notice that the server will not allow the request and error
out immediately.
So as bugs go, this is not very serious (the outcome is the same either
way -- the fetch fails). But it's worth making the logic here correct
and consistent with other related cases (e.g., fetching an oid that the
server did not mention at all).
The crux of the issue comes from fdb69d33c4 (fetch-pack: always allow
fetching of literal SHA1s, 2017-05-15). After that, the strategy of
filter_refs() is basically:
- for each advertised ref, try to match it with a "sought" ref
provided by the user. Skip any malformed refs (which includes
peeled values like "refs/tags/foo^{}"), and place any unmatched
items onto the unmatched list.
- if there are unmatched sought refs, then put all of the advertised
tips into an oidset, including the unmatched ones.
- for each sought ref, see if it's in the oidset, in which case it's
legal for us to ask the server for it
The problem is in the second step. Our list of unmatched refs includes
the peeled refs, even though upload-pack does not allow them to be
directly fetched. So the simplest fix would be to exclude them during
that step.
However, we can observe that the unmatched list isn't used for anything
else, and is freed at the end. We can just free those malformed refs
immediately. That saves us having to check each ref a second time to see
if it's malformed.
Note that this code only kicks in when "strict" is in effect. I.e., if
we are using the v0 protocol and uploadpack.allowReachableSHA1InWant is
not in effect. With v2, all oids are allowed, and we do not bother
creating or consulting the oidset at all. To future-proof our test
against the upcoming GIT_TEST_PROTOCOL_VERSION flag, we'll manually mark
it as a v0-only test.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We provide a free_refs() function to free a list, but there's no easy
way for a caller to free a single ref. Let's make our singular
free_ref() function public. Since its name is so similar to the
list-freeing free_refs(), and because both of those functions have the
same signature, it might be easy to accidentally use the wrong one.
Let's call the singular version the more verbose "free_one_ref()" to
distinguish it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There's no need for us to write this loop manually when a helper
function can already do it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since 2d103c31c2 (pack-protocol.txt: accept error packets in any
context, 2018-12-29), the pktline code will detect an ERR packet and die
automatically, saving the caller from dealing with it. But we do so too
early in the function, before we have terminated the buffer with a NUL.
As a result, passing the ERR message to die() may result in us printing
random cruft from a previous packet. This doesn't trigger memory tools
like ASan because we reuse the same buffer over and over (so the
contents are valid and initialized; they're just stale).
We can see demonstrate this by tightening the regex we use to match the
error message in t5516; without this patch, git-fetch will accidentally
print the capabilities from the (much longer) initial packet we
received.
By moving the ERR code later in the function we get a few other
benefits, too:
- we'll now chomp any newline sent by the other side (which is what we
want, since die() will add its own newline)
- we'll now mention the ERR packet with GIT_TRACE_PACKET
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Commit bdb31eada7 (upload-pack: report "not our ref" to client,
2017-02-23) catches the case where a client asks for an object we don't
have, and issues a message that the client can show to the user (in
addition to dying and writing to stderr).
There's a similar case (with the same message) when the client asks for
an object which we _do_ have, but which isn't a ref tip (or isn't
reachable, when uploadpack.allowReachableSHA1InWant is true). Let's give
that one the same treatment, for the same reason (namely that it's more
informative to the client than just hanging up, since they won't see our
stderr over some protocols).
There are two tests here. We cover it most directly in t5530 by invoking
upload-pack, which matches the existing "not our ref" test.
But a more end-to-end check is that "git fetch" actually shows the
message to the client. We're already checking in t5516 that this case
fails, so we can just check stderr there, too. Note that even after we
started ignoring SIGPIPE in 8bf4becf0c, this could in theory still be
racy as described in that commit (because we die() on write failures
before pumping the connection for any ERR packets).
In practice this should be OK for this case. The server will not
actually check reachability until it has received our whole group of
"want" lines. And since we have no objects in the repository, we won't
send any "have" lines, meaning we're always waiting to read the server
response.
Note also that this case cannot happen in the v2 protocol, since it
allows any available object to be requested. However, we don't have to
take any steps to protect against the upcoming GIT_TEST_PROTOCOL_VERSION
in our tests:
- the tests in t5516 would already need to be skipped under v2, and
that is covered by ab0c5f5096 (tests: always test fetch of
unreachable with v0, 2019-02-25)
- the tests in t5530 invoke upload-pack directly, which will continue
to default to v0. Eventually we may have a test setting which uses
v2 even for bare upload-pack calls, but we can't override it here
until we know what the setting looks like.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Back in 9f9aa76130 (upload-pack: Improve error message when bad ref
requested, 2010-07-31), we added a test to make sure that we die with a
sensible message when the client asks for an object we don't have.
Much later, in bdb31eada7 (upload-pack: report "not our ref" to client,
2017-02-23), we started reporting that information via an "ERR" line in
the protocol. Let's check that part, as well.
While we're touching this test, let's drop the "-q" on the grep calls.
Our usual test style just relies on --verbose to control output.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We annotated our test_must_fail calls in 8bf4becf0c (add "ok=sigpipe" to
test_must_fail and use it to fix flaky tests, 2015-11-27) because the
abrupt hangup of the server meant that we'd sometimes fail on read() and
sometimes get SIGPIPE on write().
But since 143588949c (fetch: ignore SIGPIPE during network operation,
2019-03-03), we make sure that we end up with a real die(), and our
tests no longer need to work around the race.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
6cdccfce ("i18n: make GETTEXT_POISON a runtime option", 2018-11-08)
made the gettext-poison test a runtime option (which was a good
move) and adjusted the test framework so that Git commands we run as
part of the framework, as opposed to the ones that are part of the
test proper, are not affected by the setting. The original value
for the GIT_TEST_GETTEXT_POISON environment variable is saved away
in another variable and gets unset, and then later the saved value
is restored to the environment variable.
But the code forgot to export the variable again, which is necessary
to restore the "export" bit that was lost when the variable was unset.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Some of the recently added progress indicators have quite long titles,
which might be even longer when translated to some languages, and when
they are shown while operating on bigger repositories, then the
progress bar grows longer than the default 80 column terminal width.
When the progress bar exceeds the width of the terminal it gets
line-wrapped, and after that the CR at the end doesn't return to the
beginning of the progress bar, but to the first column of its last
line. Consequently, the first line of the previously shown progress
bar is not overwritten by the next, and we end up with a bunch of
truncated progress bar lines scrolling past:
$ LANG=es_ES.UTF-8 git commit-graph write
Encontrando commits para commit graph entre los objetos empaquetados: 2% (1599
Encontrando commits para commit graph entre los objetos empaquetados: 3% (1975
Encontrando commits para commit graph entre los objetos empaquetados: 4% (2633
Encontrando commits para commit graph entre los objetos empaquetados: 5% (3292
[...]
Prevent this by breaking progress bars after the title once they
exceed the width of the terminal, so the counter and optional
percentage and throughput, i.e. all changing parts, are on the last
line. Subsequent updates will from then on only refresh the changing
parts, but not the title, and it will look like this:
$ LANG=es_ES.UTF-8 ~/src/git/git commit-graph write
Encontrando commits para commit graph entre los objetos empaquetados:
100% (6584502/6584502), listo.
Calculando números de generación de commit graph: 100% (824705/824705), listo.
Escribiendo commit graph en 4 pasos: 100% (3298820/3298820), listo.
Note that the number of columns in the terminal is cached by
term_columns(), so this might not kick in when it should when a
terminal window is resized while the operation is running.
Furthermore, this change won't help if the terminal is so narrow that
the counters don't fit on one line, but I would put this in the "If it
hurts, don't do it" box.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When the progress bar includes throughput, its length can shorten as
the unit of display changes from KiB to MiB. To cover up remnants of
the previous progress bar when such a change of units happens we
always print three spaces at the end of the progress bar.
Alas, covering only three characters is not quite enough: when both
the total and the throughput happen to change units from KiB to MiB in
the same update, then the progress bar's length is shortened by four
characters (or maybe even more!):
Receiving objects: 25% (2901/11603), 772.01 KiB | 733.00 KiB/s
Receiving objects: 27% (3133/11603), 1.43 MiB | 1.16 MiB/s s
and a stray 's' is left behind.
So instead of hard-coding the three characters to cover, let's compare
the length of the current progress bar with the previous one, and
cover up as many characters as needed.
Sure, it would be much simpler to just print more spaces at the end of
the progress bar, but this approach is more future-proof, and it won't
print extra spaces when none are needed, notably when the progress bar
doesn't show throughput and thus never shrinks.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Due to reasons (some XCode versions seem to include gettext, some
don't?), Homebrew does not expose the libraries and headers in
/usr/local/ by default anymore.
Let's help find them again.
Note: for some reason, this is a change of behavior caused by the
upgrade to Mojave, identified in our Azure Pipeline; it seems that
Homebrew used to add the /usr/local/ directories to the include and link
search path before, but now it no longer does.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Most notably, it seems that macOS' APFS does not allow that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Robin reported that
git submodule foreach --quiet git pull --quiet origin
is not really quiet anymore [1]. "git pull" behaves as if --quiet is not
given.
This happens because parseopt in submodule--helper will try to parse
both --quiet options as if they are foreach's options, not git-pull's.
The parsed options are removed from the command line. So when we do
pull later, we execute just this
git pull origin
When calling submodule helper, adding "--" in front of "git pull" will
stop parseopt for parsing options that do not really belong to
submodule--helper foreach.
PARSE_OPT_KEEP_UNKNOWN is removed as a safety measure. parseopt should
never see unknown options or something has gone wrong. There are also
a couple usage string update while I'm looking at them.
While at it, I also add "--" to other subcommands that pass "$@" to
submodule--helper. "$@" in these cases are paths and less likely to be
--something-like-this. But the point still stands, git-submodule has
parsed and classified what are options, what are paths. submodule--helper
should never consider paths passed by git-submodule to be options even
if they look like one.
The test case is also contributed by Robin.
[1] it should be quiet before fc1b9243cd (submodule: port submodule
subcommand 'foreach' from shell to C, 2018-05-10) because parseopt
can't accidentally eat options then.
Reported-by: Robin H. Johnson <robbat2@gentoo.org>
Tested-by: Robin H. Johnson <robbat2@gentoo.org>
Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Git's command-line parsers support uniquely abbreviated options, e.g.
`git init --ba` would automatically expand `--ba` to `--bare`.
This is a very convenient feature in every day life for Git users, in
particular when tab completion is not available.
However, it is not a good idea to rely on that in Git's test suite, as
something that is a unique abbreviation of a command line option today
might no longer be a unique abbreviation tomorrow.
For example, if a future contribution added a new mode
`git init --babyproofing` and a previously-introduced test case used the
fact that `git init --ba` expanded to `git init --bare`, that future
contribution would now have to touch seemingly unrelated tests just to
keep the test suite from failing.
So let's disallow abbreviated options in the test suite by default.
Note: for ease of implementation, this patch really only touches the
`parse-options` machinery: more and more hand-rolled option parsers are
converted to use that internal API, and more and more scripts are
converted to built-ins (naturally using the parse-options API, too), so
in practice this catches most issues, and is definitely the biggest bang
for the buck.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 7a36987fff ("send-email: add an auto option for transfer encoding",
2018-07-08), git send-email learned how to automatically determine the
transfer encoding for a patch. However, the only criterion considered
was the length of the lines.
Another case we need to consider is that of carriage returns. Because
emails have CRLF endings when canonicalized, we don't want to write raw
carriage returns into a patch, lest they be stripped off as an artifact
of the transport. Ensure that we choose quoted-printable encoding if the
patch we're sending contains carriage returns.
Note that we are guaranteed to always correctly encode carriage returns
when writing quoted-printable since we explicitly specify the line
ending as "\n", forcing MIME::QuotedPrint to encode our carriage return
as "=0D".
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since the early days of Git, the progress code allocates its struct with
a bare malloc(), not xmalloc(). If the allocation fails, we just avoid
showing progress at all.
While perhaps a noble goal not to fail the whole operation because of
optional progress, in practice:
1. Any failure to allocate a few dozen bytes here means critical path
allocations are likely to fail, too.
2. These days we use a strbuf for throughput progress (and there's a
patch under discussion to do the same for non-throughput cases,
too). And that uses xmalloc() under the hood, which means we'd
still die on some allocation failures.
Let's switch to xmalloc(). That makes us consistent with the rest of Git
and makes it easier to audit for other (less careful) bare mallocs.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Most of xdiff uses a bare malloc() to allocate memory, and returns an
error when we get NULL. However, there are a few spots which don't check
the return value and may segfault, including at least xdl_merge() and
xpatience.c's find_longest_common_sequence().
Let's use xmalloc() everywhere instead, so that we get a graceful die()
for these cases, without having to do further auditing. This does mean
the existing cases which check errors will now die() instead of
returning an error up the stack. But:
- that's how the rest of Git behaves already for malloc errors
- all of the callers of xdi_diff(), etc, die upon seeing an error
So while we might one day want to fully lib-ify the diff code and make
it possible to use as part of a long-running process, we're not close to
that now. And because we're just tweaking the xdl_malloc() macro here,
we're not really moving ourselves any further away from that. We
could, for example, simplify some of the functions which handle malloc()
errors which can no longer occur. But that would probably be taking us
in the wrong direction.
This also makes our malloc handling more consistent with the rest of
Git, including enforcing GIT_ALLOC_LIMIT and trying to reclaim pack
memory when needed.
Reported-by: 王健强 <jianqiang.wang@securitygossip.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since the xdiff library was not originally part of Git, it does its own
system includes. Let's instead use git-compat-util, which has two
benefits:
1. It adjusts for any system-specific quirks in how or what we should
include (though xdiff's needs are light enough that this hasn't
been a problem in the past).
2. It lets us use wrapper functions like xmalloc().
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
test-prio-queue.c doesn't check the return value of malloc, and could
segfault.
It's unlikely for this to matter in practice; it's a small allocation,
and this code isn't even installed alongside the rest of Git. But let's
use xmalloc(), which makes auditing for other accidental uses of bare
malloc() easier.
Reported-by: 王健强 <jianqiang.wang@securitygossip.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Remove the mention of symlinks from the test description because
several tests that are not related to symlinks have been added since
this file was introduced long ago.
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In f9e6c649589e (untracked cache: load from UNTR index extension,
2015-03-08), code was added to read back the untracked cache from an
index extension.
Probably in the endeavor to avoid the `calloc()` implied by
`FLEX_ALLOC_STR()` (it is hard to know why exactly, the commit message
of that commit is a bit parsimonious with information), it calls
`malloc()` manually and then `memcpy()`s the bits and pieces into place.
It allocates the size of `struct untracked_cache_dir` plus the string
length of the untracked file name, then copies the information in two
steps: first the fixed-size metadata, then the name. And here lies the
rub: it includes the trailing NUL byte in the name.
If `FLEX_ARRAY` is defined as 0, this results in a buffer overrun.
To fix this, let's just add 1, for the trailing NUL byte. Technically,
this overallocates on platforms where `FLEX_ARRAY` is 1, but it should
not matter much in reality, as `malloc()` usually overallocates anyway,
unless the size to allocate aligns exactly with some internal chunk size
(see below for more on that).
The real strange thing is that neither valgrind nor DrMemory catches
this bug. In this developer's tests, a `memcpy()` (but not a
`memset()`!) could write up to 4 bytes after the allocated memory range
before valgrind would start reporting an issue.
However, when running Git built with nedmalloc as allocator, under rare
conditions (and inconsistently at that), this bug triggered an `abort()`
because nedmalloc rounds up the size to be `malloc()`ed to a multiple of
a certain chunk size, then adds a few bytes to be used for storing some
internal state. If there is no rounding up to do (because the size is
already a multiple of that chunk size), and if the buffer is overrun as
in the code patched in this commit, the internal state is corrupted.
The scenario that triggered this here bug fix entailed a git.git
checkout with an extra copy of the source code in an untracked
subdirectory, meaning that there was an untracked subdirectory called
"thunderbird-patch-inline" whose name's length is exactly 24 bytes,
which, added to the size of above-mentioned `struct untracked_cache_dir`
that weighs in with 104 bytes on a 64-bit system, amounts to 128,
aligning perfectly with nedmalloc's chunk size.
As there is no obvious way to trigger this bug reliably, on all
platforms supported by Git, and as the bug is obvious enough, this patch
comes without a regression test.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The {apostrophe} was needed at the time of a521845800 ("Documentation:
remove stray backslash in show-branch discussion", 2010-08-20). All
other uses of {apostrophe} were removed in 6cf378f0cb ("docs: stop using
asciidoc no-inline-literal", 2012-04-26).
Unfortunately, the {apostrophe} is rendered literally with Asciidoctor
(at least with 1.5.5-2.0.3). Avoid this by using single-quotes.
Escaping the leading single-quote allows the content to render properly
in AsciiDoc and Asciidoctor.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Error message update.
* jt/submodule-fetch-errmsg:
submodule: explain first attempt failure clearly
|
|
Build update for SHA-1 with collision detection.
* jk/sha1dc:
Makefile: fix unaligned loads in sha1dc with UBSan
|
|
Suggest GitGitGadget instead of submitGit as a way to submit
patches based on GitHub PR to us.
* jk/promote-ggg:
point pull requesters to GitGitGadget
|
|
Test cleanup.
* ar/t4150-remove-cruft:
t4150: remove unused variable
|
|
"git rebase --rebase-merges" replaces its old "--preserve-merges"
option; the latter is now marked as deprecated.
* js/rebase-deprecate-preserve-merges:
rebase: deprecate --preserve-merges
|
|
"git worktree add" used to do a "find an available name with stat
and then mkdir", which is race-prone. This has been fixed by using
mkdir and reacting to EEXIST in a loop.
* ms/worktree-add-atomic-mkdir:
worktree: fix worktree add race
|
|
"git log -L<from>,<to>:<path>" with "-s" did not suppress the patch
output as it should. This has been corrected.
* jk/line-log-with-patch:
line-log: detect unsupported formats
line-log: suppress diff output with "-s"
|
|
A GSoC micro.
* ra/t3600-test-path-funcs:
t3600: use helpers to replace test -d/f/e/s <path>
t3600: modernize style
test functions: add function `test_file_not_empty`
|
|
"git rebase" uses the refs/rewritten/ hierarchy to store its
intermediate states, which inherently makes the hierarchy per
worktree, but it didn't quite work well.
* nd/rewritten-ref-is-per-worktree:
Make sure refs/rewritten/ is per-worktree
files-backend.c: reduce duplication in add_per_worktree_entries_to_dir()
files-backend.c: factor out per-worktree code in loose_fill_ref_dir()
|
|
When the "clean" filter can reduce the size of a huge file in the
working tree down to a small "token" (a la Git LFS), there is no
point in allocating a huge scratch area upfront, but the buffer is
sized based on the original file size. The convert mechanism now
allocates very minimum and reallocates as it receives the output
from the clean filter process.
* jh/resize-convert-scratch-buffer:
convert: avoid malloc of original file size
|
|
Doc update.
* dl/ignore-docs:
docs: move core.excludesFile from git-add to gitignore
git-clean.txt: clarify ignore pattern files
|
|
Doc update.
* ja/dir-rename-doc-markup-fix:
Doc: fix misleading asciidoc formating
|
|
Doc update.
* dl/reset-doc-no-wrt-abbrev:
git-reset.txt: clarify documentation
|
|
In 6956f858f6 (notes: implement helpers needed for note copying during
rewrite, 2010-03-12), we introduced a test case that verifies that the
config setting `notes.rewriteRef` can be overridden via the environment
variable `GIT_NOTES_REWRITE_REF`.
Back when it was introduced, it relied on a side effect of an earlier
test case that configured `core.noteRef` to point to `refs/notes/other`.
In 908a320363 (t3301: modernize style, 2014-11-12), this side effect was
removed.
The test case *still* passed, but for the wrong reason: we no longer
overrode the rewrite ref, but there simply was nothing to rewrite
anymore, as the overridden notes ref was "modernized" away.
Let's let that test case pass for the correct reason again.
To make sure of that, let's change the idea of the original test case:
it configured `notes.rewriteRef` to point to the actual notes ref,
forced that to be ignored and then verified that the notes were *not*
rewritten.
By turning that idea upside down (configure the `notes.rewriteRef` to
another notes ref, override it via the environment variable to force the
notes to be copied, and then verify that the notes *were* rewritten), we
make it much harder for that test case to pass for the wrong reason.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
While the minimum is 7-char, the unambiguous length can be longer.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When 'git blame' is invoked without specifying the commit to start
blaming from, it starts from the given file's state in the work tree.
However, when invoked in a bare repository without a start commit,
then there is no work tree state to start from, and it dies with the
following error message:
$ git rev-parse --is-bare-repository
true
$ git blame file.c
fatal: this operation must be run in a work tree
This is misleading, because it implies that 'git blame' doesn't work
in bare repositories at all, but it does, in fact, work just fine when
it is given a commit to start from.
We could improve the error message, of course, but let's just default
to HEAD in a bare repository instead, as most likely that is what the
user wanted anyway (if they wanted to start from an other commit, then
they would have specified that in the first place).
'git annotate' is just a thin wrapper around 'git blame', so in the
same situation it printed the same misleading error message, and this
patch fixes it, too.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
struct stat_data and struct cache_time both use unsigned ints for all
their members. However the format string for 'git ls-files --debug'
currently uses %d for formatting these numbers. This means that we
potentially print these values incorrectly if they are greater than
INT_MAX.
This has been the case since the --debug option was introduced in 'git
ls-files' in 8497421715 ("ls-files: learn a debugging dump format",
2010-07-31).
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The chance of a repository being corrupted due to a "gc" has nothing
to do with whether or not that "gc" was invoked via "gc --auto", but
whether there's other concurrent operations happening.
This is already noted earlier in the paragraph, so there's no reason
to suggest this here. The user can infer from the rest of the
documentation that "gc" will run automatically unless gc.auto=0 is
set, and we shouldn't confuse the issue by implying that "gc --auto"
is somehow more prone to produce corruption than a normal "gc".
Well, it is in the sense that a blocking "gc" would stop you from
doing anything else in *that* particular terminal window, but users
are likely to have another window, or to be worried about how
concurrent "gc" on a server might cause corruption.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Amend the "NOTES" section to fix up wording that's been with us since
3ffb58be0a ("doc/git-gc: add a note about what is collected",
2008-04-23).
I can't remember when/where anymore (I think Freenode #Git), but at
some point I was having a conversation with someone who was convinced
that "gc" would prune things only referenced by e.g. refs/pull/*, and
pointed to this section as proof.
It turned out that they'd read the "branches and tags" wording here
and thought just refs/{heads,tags}/* and refs/remotes/* etc. would be
kept, which is what we enumerate explicitly.
So let's say "other refs", even though just above we say "objects that
are referenced anywhere in your repository".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Amend the "PACKFILE OPTIMIZATION" section in "fast-import" to explain
that simply running "git gc --aggressive" after a "fast-import" should
properly optimize the repository. This is simpler and more effective
than the existing "repack" advice (which I'm keeping as it helps
explain things) because it e.g. also packs the newly imported refs.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The existing "gc --aggressive" docs come just short of recommending to
users that they run it regularly. I've personally talked to many users
who've taken these docs as an advice to use this option, and have,
usually it's (mostly) a waste of time.
So let's clarify what it really does, and let the user draw their own
conclusions.
Let's also clarify the "The effects [...] are persistent" to
paraphrase a brief version of Jeff King's explanation at [1].
1. https://public-inbox.org/git/20190318235356.GK29661@sigill.intra.peff.net/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) we
somewhat confusingly use the same depth under --aggressive as we do by
default.
As noted in that commit that makes sense, it was wrong to make more
depth the default for "aggressive", and thus save disk space at the
expense of runtime performance, which is usually the opposite of
someone who'd like "aggressive gc" wants.
But that's left us with a mostly-redundant configuration variable, so
let's clearly note in its documentation that it doesn't change the
default.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Change the AsciiDoc formatting so that an example of "gc --auto" isn't
rendered as "git-gc(1) --auto", but as "git gc --auto". This is
consistent with the rest of the links and command examples in this
documentation.
The formatting I'm changing was initially introduced in
d5d5d7b641 ("gc: automatically write commit-graph files", 2018-06-27).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Re-flow the "gc.*" section in "config". A previous commit moved this
over from the "gc" docs, but tried to keep as many of the lines
identical to benefit from diff's move detection.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Rather than duplicating the documentation for the various "gc" options
let's include the "gc" docs from git-config. They were mostly better
already, and now we don't have the same docs in two places with subtly
different wording.
In the cases where the git-gc(1) docs were saying something the "gc"
docs in git-config(1) didn't cover move the relevant section over to
the git-config(1) docs.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When running a command like "git show" or "git diff" in a partial clone,
batch all missing blobs to be fetched as one request.
This is similar to c0c578b33c ("unpack-trees: batch fetching of missing
blobs", 2017-12-08), but for another command.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The following patches in this series want to handle the progress bar's
title and changing parts (i.e. the counter and the optional percentage
and throughput combined) differently, and need to know the length
of the changing parts of the previously displayed progress bar.
To prepare for those changes assemble the changing parts in a separate
strbuf kept in 'struct progress' before printing.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Ever since the progress infrastructure was introduced in 96a02f8f6d
(common progress display support, 2007-04-18), display_progress() has
returned an int, telling callers whether it updated the progress bar
or not. However, this is:
- useless, because over the last dozen years there has never been a
single caller that cared about that return value.
- not quite true, because it doesn't print a progress bar when
running in the background, yet it returns 1; see 85cb8906f0
(progress: no progress in background, 2015-04-13).
The related display_throughput() function returned void already upon
its introduction in cf84d51c43 (add throughput to progress display,
2007-10-30).
Let's make display_progress() return void, too. While doing so
several return statements in display() become unnecessary, remove
them.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 'ci/test-documentation.sh' we save the standard error of 'make
doc', and, in an attempt to make sure that neither AsciiDoc nor
Asciidoctor printed any warnings, we check the emptiness of the
resulting file with '! test -s stderr.log'. This check has never
actually worked, because in our 'ci/*' build scripts we rely on 'set
-e' aborting the build job when a command exits with error, and,
unfortunately, the combination of the two doesn't work as intended.
According to POSIX [1]:
"The -e setting shall be ignored when executing [...] a pipeline
beginning with the ! reserved word" [2]
Watch and learn:
$ echo unexpected >file
$ ( set -e; ! test -s file ; echo "should not reach this" ) ; echo $?
should not reach this
0
This is why we haven't noticed the warnings from Asciidoctor that were
fixed in the first patches of this patch series, though some of them
were already there in the build of v2.18.0-rc0 [3].
Check the emptiness of that file with 'test ! -s' instead, which works
properly with 'set -e':
$ ( set -e; test ! -s file ; echo "should not reach this" ) ; echo $?
1
Furthermore, dump the contents of that file to the log for our
convenience, so if it were to unexpectedly end up being non-empty,
then we wouldn't have to scroll through all that long build log
looking for warnings, but could see them right away near the end of
the log.
Note that we are only really interested in the standard error of
AsciiDoc and Asciidoctor, but by saving the stderr of 'make doc' we
also save any error output from the make rules. Currently there is
only one such line: we build the docs with Asciidoctor right after a
'make clean', meaning that 'make USE_ASCIIDOCTOR=1 doc' always starts
with running 'GIT-VERSION-GEN', which in turn prints the version to
stderr. A 'sed' command was supposed to remove this version line to
prevent it from triggering that (previously defunct) emptiness check,
but, unfortunately, this command doesn't work as intended, either,
because it leaves the file to be checked intact, but that defunct
emptiness check hid this issue, too... Furthermore, in the near
future there will be an other line on stderr, because commit
9a71722b4d (Doc: auto-detect changed build flags, 2019-03-17) in the
currently cooking branch 'ma/doc-diff-doc-vs-doctor-comparison' will
print "* new asciidoc flags" at the beginning of both 'make doc'
invokations.
Extend that 'sed' command to remove this line, too, wrap it in a
helper function so the output of both 'make doc' is filtered the same
way, and change its invokation to actually write the logfile to be
checked.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#set
[2] POSIX doesn't discuss the meaning of '! cmd' in case of simple
commands, but it defines that "A pipeline is a sequence of one or
more commands separated by the control operator '|'", so
apparently a simple command is considered as pipeline as well.
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_02
[3] https://travis-ci.org/git/git/jobs/385932007#L1463
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The recent release of Asciidoctor v2.0.0 broke our documentation
build job on Travis CI, where we 'gem install asciidoctor', which
always brings us the latest and (supposedly) greatest. Alas, we are
not ready for that just yet, because it removed support for DocBook
4.5, and we have been requiring that particular DocBook version to
build 'user-manual.xml' with Asciidoctor, resulting in:
ASCIIDOC user-manual.xml
asciidoctor: FAILED: missing converter for backend 'docbook45'. Processing aborted.
Use --trace for backtrace
make[1]: *** [user-manual.xml] Error 1
Unfortunately, we can't simply switch to DocBook 5 right away, as
doing so leads to validation errors from 'xmlto', and working around
those leads to yet another errors... [1]
So let's stick with Asciidoctor v1.5.8 (latest stable release before
v2.0.0) in our documentation build job on Travis CI for now, until we
figure out how to deal with the fallout from Asciidoctor v2.0.0.
[1] https://public-inbox.org/git/20190324162131.GL4047@pobox.com/
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since commit ef0cc1df90f6b ("send-email: also pick up cc addresses from
-by trailers") in git version 2.20, git send-email adds to cc list
addresses from all *-by lines. As a side effect a line with
'-Signed-off-by' is now also added to cc. This makes send-email pick
lines from patches that remove patch files from the git repo. This is
common in the Buildroot project that often removes (and adds) patch
files that have 'Signed-off-by' in their patch description part.
Consider only *-by lines that start with [a-z] (case insensitive) to
avoid unrelated addresses in cc.
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Acked-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Ensure that a FLEX_MALLOC_MEM that uses 'strlen' for its 'len' uses
FLEX_ALLOC_STR instead, since these are equivalent forms.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This patch fixes a quadratic list insertion in rewrite_one() when
pathspec limiting is combined with --parents. What happens is something
like this:
1. We see that some commit X touches the path, so we try to rewrite
its parents.
2. rewrite_one() loops forever, rewriting parents, until it finds a
relevant parent (or hits the root and decides there are none). The
heavy lifting is done by process_parent(), which uses
try_to_simplify_commit() to drop parents.
3. process_parent() puts any intermediate parents into the
&revs->commits list, inserting by commit date as usual.
So if commit X is recent, and then there's a large chunk of history that
doesn't touch the path, we may add a lot of commits to &revs->commits.
And insertion by commit date is O(n) in the worst case, making the whole
thing quadratic.
We tried to deal with this long ago in fce87ae538 (Fix quadratic
performance in rewrite_one., 2008-07-12). In that scheme, we cache the
oldest commit in the list; if the new commit to be added is older, we
can start our linear traversal there. This often works well in practice
because parents are older than their descendants, and thus we tend to
add older and older commits as we traverse.
But this isn't guaranteed, and in fact there's a simple case where it is
not: merges. Imagine we look at the first parent of a merge and see a
very old commit (let's say 3 years old). And on the second parent, as we
go back 3 years in history, we might have many commits. That one
first-parent commit has polluted our oldest-commit cache; it will remain
the oldest while we traverse a huge chunk of history, during which we
have to fall back to the slow, linear method of adding to the list.
Naively, one might imagine that instead of caching the oldest commit,
we'd start at the last-added one. But that just makes some cases faster
while making others slower (and indeed, while it made a real-world test
case much faster, it does quite poorly in the perf test include here).
Fundamentally, these are just heuristics; our worst case is still
quadratic, and some cases will approach that.
Instead, let's use a data structure with better worst-case performance.
Swapping out revs->commits for something else would have repercussions
all over the code base, but we can take advantage of one fact: for the
rewrite_one() case, nobody actually needs to see those commits in
revs->commits until we've finished generating the whole list.
That leaves us with two obvious options:
1. We can generate the list _unordered_, which should be O(n), and
then sort it afterwards, which would be O(n log n) total. This is
"sort-after" below.
2. We can insert the commits into a separate data structure, like a
priority queue. This is "prio-queue" below.
I expected that sort-after would be the fastest (since it saves us the
extra step of copying the items into the linked list), but surprisingly
the prio-queue seems to be a bit faster.
Here are timings for the new p0001.6 for all three techniques across a
few repositories, as compared to master:
master cache-last sort-after prio-queue
--------------------------------------------------------------------------------------------
GIT_PERF_REPO=git.git
0.52(0.50+0.02) 0.53(0.51+0.02) +1.9% 0.37(0.33+0.03) -28.8% 0.37(0.32+0.04) -28.8%
GIT_PERF_REPO=linux.git
20.81(20.74+0.07) 20.31(20.24+0.07) -2.4% 0.94(0.86+0.07) -95.5% 0.91(0.82+0.09) -95.6%
GIT_PERF_REPO=llvm-project.git
83.67(83.57+0.09) 4.23(4.15+0.08) -94.9% 3.21(3.15+0.06) -96.2% 2.98(2.91+0.07) -96.4%
A few items to note:
- the cache-list tweak does improve the bad case for llvm-project.git
that started my digging into this problem. But it performs terribly
on linux.git, barely helping at all.
- the sort-after and prio-queue techniques work well. They approach
the timing for running without --parents at all, which is what you'd
expect (see below for more data).
- prio-queue just barely outperforms sort-after. As I said, I'm not
really sure why this is the case, but it is. You can see it even
more prominently in this real-world case on llvm-project.git:
git rev-list --parents 07ef786652e7 -- llvm/test/CodeGen/Generic/bswap.ll
where prio-queue routinely outperforms sort-after by about 7%. One
guess is that the prio-queue may just be more efficient because it
uses a compact array.
There are three new perf tests:
- "rev-list --parents" gives us a baseline for running with --parents.
This isn't sped up meaningfully here, because the bad case is
triggered only with simplification. But it's good to make sure we
don't screw it up (now, or in the future).
- "rev-list -- dummy" gives us a baseline for just traversing with
pathspec limiting. This gives a lower bound for the next test (and
it's also a good thing for us to be checking in general for
regressions, since we don't seem to have any existing tests).
- "rev-list --parents -- dummy" shows off the problem (and our fix)
Here are the timings for those three on llvm-project.git, before and
after the fix:
Test master prio-queue
------------------------------------------------------------------------------
0001.3: rev-list --parents 2.24(2.12+0.12) 2.22(2.11+0.11) -0.9%
0001.5: rev-list -- dummy 2.89(2.82+0.07) 2.92(2.89+0.03) +1.0%
0001.6: rev-list --parents -- dummy 83.67(83.57+0.09) 2.98(2.91+0.07) -96.4%
Changes in the first two are basically noise, and you can see we
approach our lower bound in the final one.
Note that we can't fully get rid of the list argument from
process_parents(). Other callers do have lists, and it would be hard to
convert them. They also don't seem to have this problem (probably
because they actually remove items from the list as they loop, meaning
it doesn't grow so large in the first place). So this basically just
drops the "cache_ptr" parameter (which was used only by the one caller
we're fixing here) and replaces it with a prio_queue. Callers are free
to use either data structure, depending on what they're prepared to
handle.
Reported-by: Björn Pettersson A <bjorn.a.pettersson@ericsson.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Teach difftool and mergetool about the Sublime Merge "smerge" command.
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When a parent blob already has chunks queued up for blaming, dropping
the blob at the end of one blame step will cause it to get reloaded
right away, doubling the amount of I/O and unpacking when processing a
linear history.
Keeping such parent blobs in memory seems like a reasonable optimization
that should incur additional memory pressure mostly when processing the
merges from old branches.
Signed-off-by: David Kastrup <dak@gnu.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The description of --reset stays true to the first implementation in
438195cced (git-read-tree: add "--reset" flag, 2005-06-09). That is,
--reset discards unmerged entries. Or at least true to the commit
message because I can't be sure about read-tree's behavior regarding
local changes.
But in fcc387db9b (read-tree -m -u: do not overwrite or remove untracked
working tree files., 2006-05-17), it is clear that "-m -u" tries to keep
local changes, while --reset is singled out and will keep overwriting
worktree files. It's not stated in the commit message, but it's obvious
from the patch.
I went this far back not because I had a lot of free time, but because I
did not trust my reading of unpack-trees.c code. So far I think the
related changes in history agree with my understanding of the current
code, that "--reset" loses local changes.
This behavior is not mentioned in git-read-tree.txt, even though
old-timers probably can just guess it based on the "reset" name. Update
git-read-tree.txt about this.
Side note. There's another change regarding --reset that is not
obviously about local changes, b018ff6085 (unpack-trees: fix "read-tree
-u --reset A B" with conflicted index, 2012-12-29). But I'm pretty sure
this is about the first function of --reset, to discard unmerged entries
correctly.
PS. The patch changes one more line than necessary because the first
line uses spaces instead of tab.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
To use the singular form of a word, when the option wants the plural
form (and quietly expands it because it thinks it was abbreviated), is
an easy mistake to make, and t5317 contains almost two dozen of them.
However, using abbreviated options in tests is a bit fragile, so we will
disallow use of abbreviated options in our test suite.
In preparation for this change, let's fix
`t5317-pack-objects-filter-objects.sh`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
To avoid future ambiguities, we really want to use full option names in
the test suite. `t7525-status-rename.sh` used an abbreviated form of the
`--find-renames` option, though, so let's change that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We really want to spell out the option in the full form, to avoid any
ambiguity that might be introduced by future patches.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
It was probably just an oversight: the `--recurse-submodules` option
puts the term "submodules" in the plural form, not the singular one.
To avoid future problems in case that another option is introduced that
starts with the prefix `--recurse-submodule`, let's just fix this.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This script used abbreviated options, which is unnecessarily fragile.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In quite a few test cases, we were sloppy and used the abbreviation
`--force`, but we really should be precise in what we want to test.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This test wants to run `git rebase` with the `--keep-empty` option, but
it really only spelled out `--keep` and trusted Git's option parsing to
determine that this was a unique abbreviation of the real option.
However, Denton Liu contributed a patch series in
https://public-inbox.org/git/cover.1553354374.git.liu.denton@gmail.com/
that introduces a new `git rebase` option called `--keep-base`, which
makes this previously unique abbreviation non-unique.
Whether this patch series is accepted or not, it is actually a bad
practice to use abbreviated options in our test suite, because of the
issue that those unique option names are not guaranteed to stay unique
in the future.
So let's just not use abbreviated options in the test suite.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When 'git index-pack' is run by 'git clone', its check_objects()
function usually doesn't take long enough to be a concern, but I just
run into a situation where it took about a minute or so: I
inadvertently put some memory pressure on my tiny laptop while cloning
linux.git, and then there was quite a long silence between the
"Resolving deltas" and "Checking connectivity" progress bars.
Show a progress bar during the loop of check_objects() to let the user
know that something is still going on.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Asciidoc uses either one-line or two-line syntax for document/section
titles[1]. The two-line form is used in git-status. Fix a few section
titles in the porcelain v2 section which were inadvertently using
markdown syntax.
[1] http://asciidoc.org/userguide.html#X17
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Using "+" to continue multiple list items is more tedious and
error-prone than wrapping the entire block with "--" block markers.
When using asciidoctor, the list items after the --date=iso list items
are incorrectly formatted when using "+" continuation. Use "--" block
markers to correctly format the block.
When using asciidoc there is no change in how the content is rendered.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Add GIT_TR2_* to the whitelist of environment variables that we don't
clear when running the test suite.
This allows us to use the test suite to produce trace2 test data,
which is handy to e.g. write consumers that collate the trace data
itself.
One caveat here is that we produce trace output for not *just* the
tests, but also e.g. from this line in test-lib.sh:
# It appears that people try to run tests without building...
"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
[...]
I consider this not just OK but a feature. Let's log *all* the git
commands we're going to execute, not just those within
test_expect_*().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In do_fetch_pack_v2(), the "sought" array is sorted by name, and it is
not subsequently reordered (within the function). Therefore,
receive_wanted_refs() can assume that "sought" is sorted, and can thus
use a binary search when storing wanted-refs retrieved from the server.
Replace the existing linear search with a binary search. This improves
performance significantly when mirror cloning a repository with more
than 1 million refs.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This description line is shown in 'git help -a' and all other commands
description starts with an uppercase character. This just makes that
printout a bit nicer.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Teach oid_object_info_extended() to support a new flag that inhibits
fetching of missing objects. This is equivalent to setting
fetch_is_missing to 0, calling oid_object_info_extended(), then setting
fetch_if_missing to whatever it was before. Update unpack-trees.c to use
this new flag instead of repeatedly setting fetch_if_missing.
This new flag complicates things slightly in that there are now 2 ways
to do the same thing. But this eliminates the need to repeatedly set a
global variable, and more importantly, allows prefetching to be done in
parallel (in the future); hence, this patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In protocol v0, when sending "shallow" lines, the server distinguishes
between lines caused by the remote repo being shallow and lines caused
by client-specified depth settings. Unless "--update-shallow" is
specified, there is a difference in behavior: refs that reach the former
"shallow" lines, but not the latter, are rejected. But in v2, the server
does not, and the client treats all "shallow" lines like lines caused by
client-specified depth settings.
Full restoration of v0 functionality is not possible without protocol
change, but we can implement a heuristic: if we specify any depth
setting, treat all "shallow" lines like lines caused by client-specified
depth settings (that is, unaffected by "--no-update-shallow"), but
otherwise, treat them like lines caused by the remote repo being shallow
(that is, affected by "--no-update-shallow"). This restores most of v0
behavior, except in the case where a client fetches from a shallow
repository with depth settings.
This patch causes a test that previously failed with
GIT_TEST_PROTOCOL_VERSION=2 to pass.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In fetch_pack(), be clearer that there is no shallow information before
the fetch when v2 is used - memset the struct shallow_info to 0 instead
of calling prepare_shallow_info().
This patch is in preparation for a future patch in which a v2 fetch
might call prepare_shallow_info() after shallow info has been retrieved
during the fetch, so I needed to ensure that prepare_shallow_info() is
not called before the fetch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
jt/fetch-no-update-shallow-in-proto-v2
* jt/test-protocol-version:
t5552: compensate for v2 filtering ref adv.
tests: fix protocol version for overspecifications
t5700: only run with protocol version 1
t5512: compensate for v0 only sending HEAD symrefs
t5503: fix overspecification of trace expectation
tests: always test fetch of unreachable with v0
t5601: check ssh command only with protocol v0
tests: define GIT_TEST_PROTOCOL_VERSION
|
|
When our '.travis.yml' was split into several 'ci/*' scripts [1], the
installation of the 'asciidoctor' gem somehow ended up in
'ci/test-documentation.sh'.
Install it in 'ci/install-dependencies.sh', where we install other
dependencies of the Documentation build job as well (asciidoc,
xmlto).
[1] 657343a602 (travis-ci: move Travis CI code into dedicated scripts,
2017-09-10)
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Asciidoctor versions v1.5.7 or later print the following warning while
building the documentation:
ASCIIDOC technical/protocol-v2.html
asciidoctor: WARNING: protocol-v2.txt: line 38: unterminated listing block
This highlights an issue (even with older Asciidoctor versions) where
the 'Initial Client Request' header is not rendered as a header but in
monospace. I'm not sure what exactly causes this issue and why it's
an issue only with this particular header, but all headers in
'protocol-v2.txt' are written like this:
Initial Client Request
------------------------
i.e. the header itself is indented by a space, and the "underline" is
two characters longer than the header.
Dropping that indentation and making the length of the underline match
the length of the header apparently fixes this issue.
While at it, adjust all other headers 'protocol-v2.txt' as well, to
match the style we use everywhere else.
The page rendered with AsciiDoc doesn't have this formatting issue.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Asciidoctor versions v1.5.7 or later print the following warning while
building the documentation:
ASCIIDOC technical/api-config.html
asciidoctor: WARNING: api-config.txt: line 232: unterminated listing block
This highlight an issue (even with older Asciidoctor versions) where
the length of the '----' lines surrounding a code example don't match,
and the rest of the document is rendered in monospace.
Fix this by making sure that the length of those lines match.
The page rendered with AsciiDoc doesn't have this formatting issue.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Asciidoctor versions v1.5.7 or later print the following warning while
building the documentation:
ASCIIDOC git-diff-tree.xml
asciidoctor: WARNING: diff-format.txt: line 2: unterminated listing block
This highlights an issue (even with older Asciidoctor versions) where
the "Raw output format" header is not rendered as a header, and the
rest of the document is rendered in monospace. This is not caused by
'diff-format.txt' in itself, but rather by 'git-diff-tree.txt'
including 'pretty-formats.txt' and 'diff-format.txt' on subsequent
lines, while the former happens to end with monospace-formatted
example commands.
Fix this by inserting an empty line between the two include::
directives.
The page rendered with AsciiDoc doesn't have this formatting issue.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The current wildmatch() call for includeIf's gitdir pattern does not
pass the WM_PATHNAME flag. Without this flag, '*' is treated _almost_
the same as '**' (because '*' also matches slashes) with one exception:
'/**/' can match a single slash. The pattern 'foo/**/bar' matches
'foo/bar'.
But '/*/', which is essentially what wildmatch engine sees without
WM_PATHNAME, has to match two slashes (and '*' matches nothing). Which
means 'foo/*/bar' cannot match 'foo/bar'. It can only match 'foo//bar'.
The result of this is the current wildmatch() call works most of the
time until the user depends on '/**/' matching no path component. And
also '*' matches slashes while it should not, but people probably
haven't noticed this yet. The fix is straightforward.
Reported-by: Jason Karns <jason.karns@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
On Windows, for example, executables (must) have the extension `.exe`.
Our `check-docs` target was not prepared for that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When we want to see what commands are listed in `command-list.txt` but
not installed, we currently include lines that refer to guides, e.g.
`gitattributes` or `gitcli`.
Let's not include those lines, as they are not referring to commands.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As part of the `check-docs` target, we verify that commands that are
documented are actually in the current list of commands to be built.
However, this logic broke in 5fafce0b78 (check-docs: get documented
command list from Makefile, 2012-08-08), when we tried to make the logic
safer by not looking at the files in the worktree, but at the list of
files to be generated in `Documentation/Makefile`. While this was the
right thing to do, it failed to accommodate for the fact that `make -C
Documentation/ print-man1`, unlike `ls Documentation/*.txt`, would *not*
print lines starting with the prefix `Documentation/`.
At long last, let's fix this.
Note: This went undetected due to a funny side effect of the
`ALL_PROGRAMS` variable starting with a space. That space, together with
the extra space we inserted before `$(ALL_PROGRAMS)` in the
case " $(ALL_PROGRAMS)" in
*" $$cmd ")
[...]
construct, is responsible that this case arm is used when `cmd` is empty
(which was clearly not intended to be the case).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Since 7ded055401 (build: do not install git-remote-testgit, 2013-06-07),
we do not install it. Therefore it makes no sense to document it,
either.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
It is currently in section 1, but that section is intended for
"Executable programs or shell commands".
A more appropriate place is section 7: "Miscellaneous (including macro
packages and conventions), e.g. man(7), groff(7)".
This issue should have been detected earlier by `make check-docs`, but
was missed due to a bug in that Makefile target (that we are about to
fix).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Don't redundantly run "git reflog expire --all" when gc.reflogExpire
and gc.reflogExpireUnreachable are set to "never", and die immediately
if those configuration valuer are bad.
As an earlier "assert lack of early exit" change to the tests for "git
reflog expire" shows, an early check of gc.reflogExpire{Unreachable,}
isn't wanted in general for "git reflog expire", but it makes sense
for "gc" because:
1) Similarly to 8ab5aa4bd8 ("parseopt: handle malformed --expire
arguments more nicely", 2018-04-21) we'll now die early if the
config variables are set to invalid values.
We run "pack-refs" before "reflog expire", which can take a while,
only to then die on an invalid gc.reflogExpire{Unreachable,}
configuration.
2) Not invoking the command at all means it won't show up in trace
output, which makes what's going on more obvious when the two are
set to "never".
3) As a later change documents we lock the refs when looping over the
refs to expire, even in cases where we end up doing nothing due to
this config.
For the reasons noted in the earlier "assert lack of early exit"
change I don't think it's worth it to bend over backwards in "git
reflog expire" itself to carefully detect if we'll really do
nothing given the combination of all its possible options and skip
that locking, but that's easy to detect here in "gc" where we'll
only run "reflog expire" in a relatively simple mode.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When gc.reflogExpire and gc.reflogExpireUnreachable are set to "never"
and --stale-fix isn't in effect we *could* exit early without
pointlessly looping over all the reflogs.
However, as an earlier change to add a test for the "points nowhere"
warning shows even in such a mode we might want to print out a
warning.
So while it's conceivable to implement this, I don't think it's worth
it. It's going to be too easy to inadvertently add some flag that'll
make the expiry happen anyway, and even with "never" we'd like to see
all the lines we're going to keep.
So let's assert that we're going to loop over all the references even
when this configuration is in effect.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Change the error emitted when a commit-graph file is corrupt so that
we actually mention the commit-graph, e.g. change errors like:
error: improper chunk offset 0000000000385e0c
To:
error: commit-graph improper chunk offset 0000000000385e0c
As discussed in the commits leading up to this one the commit-graph
machinery is now used by common commands like "status". If the graph
was corrupt we'd often emit some error that gave no indication what
was wrong. Now some of them are still cryptic, but they'll at least
mention "commit-graph" to give the user a hint as to where to look.
While I'm at it mark some of the strings that hadn't been marked for
translation. It's clear from the commit history and the code that this
was merely forgotten at the time, and wasn't intentional.p5
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When the commit-graph is written we end up calling
parse_commit(). This will in turn invoke code that'll consult the
existing commit-graph about the commit, if the graph is corrupted we
die.
We thus get into a state where a failing "commit-graph verify" can't
be followed-up with a "commit-graph write" if core.commitGraph=true is
set, the graph either needs to be manually removed to proceed, or
core.commitGraph needs to be set to "false".
Change the "commit-graph write" codepath to use a new
parse_commit_no_graph() helper instead of parse_commit() to avoid
this. The latter will call repo_parse_commit_internal() with
use_commit_graph=1 as seen in 177722b344 ("commit: integrate commit
graph with commit parsing", 2018-04-10).
Not using the old graph at all slows down the writing of the new graph
by some small amount, but is a sensible way to prevent an error in the
existing commit-graph from spreading.
Just fixing the current issue would be likely to result in code that's
inadvertently broken in the future. New code might use the
commit-graph at a distance. To detect such cases introduce a
"GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD" setting used when we do our
corruption tests, and test that a "write/verify" combo works after
every one of our current test cases where we now detect commit-graph
corruption.
Some of the code changes here might be strictly unnecessary, e.g. I
was unable to find cases where the parse_commit() called from
write_graph_chunk_data() didn't exit early due to
"item->object.parsed" being true in
repo_parse_commit_internal() (before the use_commit_graph=1 has any
effect). But let's also convert those cases for good measure, we do
not have exhaustive tests for all possible types of commit-graph
corruption.
This might need to be re-visited if we learn to write the commit-graph
incrementally, but probably not. Hopefully we'll just start by finding
out what commits we have in total, then read the old graph(s) to see
what they cover, and finally write a new graph file with everything
that's missing. In that case the new graph writing code just needs to
continue to use e.g. a parse_commit() that doesn't consult the
existing commit-graphs.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Change "commit-graph verify" to error on open() failures other than
ENOENT. As noted in the third paragraph of 283e68c72f ("commit-graph:
add 'verify' subcommand", 2018-06-27) and the test it added it's
intentional that "commit-graph verify" doesn't error out when the file
doesn't exist.
But let's not be overly promiscuous in what we accept. If we can't
read the file for other reasons, e.g. permission errors, bad file
descriptor etc. we'd like to report an error to the user.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
An earlier change implemented load_commit_graph_one_fd_st() in a way
that was bug-compatible with earlier code in terms of the "graph file
%s is too small" error message printing out the path to the
commit-graph (".git/objects/info/commit-graph").
But change that, because:
* A function that takes an already-open file descriptor also needing
the filename isn't very intuitive.
* The vast majority of errors we might emit when loading the graph
come from parse_commit_graph(), which doesn't report the
filename. Let's not do that either in this case for consistency.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Make the commit-graph loading code work as a library that returns an
error code instead of calling exit(1) when the commit-graph is
corrupt. This means that e.g. "status" will now report commit-graph
corruption as an "error: [...]" at the top of its output, but then
proceed to work normally.
This required splitting up the load_commit_graph_one() function so
that the code that deals with open()-ing and stat()-ing the graph can
now be called independently as open_commit_graph().
This is needed because "commit-graph verify" where the graph doesn't
exist isn't an error. See the third paragraph in
283e68c72f ("commit-graph: add 'verify' subcommand",
2018-06-27). There's a bug in that logic where we conflate the
intended ENOENT with other errno values (e.g. EACCES), but this change
doesn't address that. That'll be addressed in a follow-up change.
I'm then splitting most of the logic out of load_commit_graph_one()
into load_commit_graph_one_fd_st(), which allows for providing an
existing file descriptor and stat information to the loading
code. This isn't strictly needed, but it would be redundant and
confusing to open() and stat() the file twice for some of the
codepaths, this allows for calling open_commit_graph() followed by
load_commit_graph_one_fd_st(). The "graph_file" still needs to be
passed to that function for the the "graph file %s is too small" error
message.
This leaves load_commit_graph_one() unused by everything except the
internal prepare_commit_graph_one() function, so let's mark it as
"static". If someone needs it in the future we can remove the "static"
attribute. I could also rewrite its sole remaining
user ("prepare_commit_graph_one()") to use
load_commit_graph_one_fd_st() instead, but let's leave it at this.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When core.commitGraph=true is set, various common commands now consult
the commit graph. Because the commit-graph code is very trusting of
its input data, it's possibly to construct a graph that'll cause an
immediate segfault on e.g. "status" (and e.g. "log", "blame", ...). In
some other cases where git immediately exits with a cryptic error
about the graph being broken.
The root cause of this is that while the "commit-graph verify"
sub-command exhaustively verifies the graph, other users of the graph
simply trust the graph, and will e.g. deference data found at certain
offsets as pointers, causing segfaults.
This change does the bare minimum to ensure that we don't segfault in
the common fill_commit_in_graph() codepath called by
e.g. setup_revisions(), to do this instrument the "commit-graph
verify" tests to always check if "status" would subsequently
segfault. This fixes the following tests which would previously
segfault:
not ok 50 - detect low chunk count
not ok 51 - detect missing OID fanout chunk
not ok 52 - detect missing OID lookup chunk
not ok 53 - detect missing commit data chunk
Those happened because with the commit-graph enabled setup_revisions()
would eventually call fill_commit_in_graph(), where e.g.
g->chunk_commit_data is used early as an offset (and will be
0x0). With this change we get far enough to detect that the graph is
broken, and show an error instead. E.g.:
$ git status; echo $?
error: commit-graph is missing the Commit Data chunk
1
That also sucks, we should *warn* and not hard-fail "status" just
because the commit-graph is corrupt, but fixing is left to a follow-up
change.
A side-effect of changing the reporting from graph_report() to error()
is that we now have an "error: " prefix for these even for
"commit-graph verify". Pseudo-diff before/after:
$ git commit-graph verify
-commit-graph is missing the Commit Data chunk
+error: commit-graph is missing the Commit Data chunk
Changing that is OK. Various errors it emits now early on are prefixed
with "error: ", moving these over and changing the output doesn't
break anything.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When get-mark was introduced in commit 28c7b1f7b7b7 ("fast-import: add a
get-mark command", 2015-07-01), it followed the precedent of the
cat-blob command to be allowed on any line other than in the middle of a
data directive; see commit 777f80d7429b ("fast-import: Allow cat-blob
requests at arbitrary points in stream", 2010-11-28). It was useful to
allow cat-blob directives in the middle of a commit to get more data
that would be used in writing the current commit object. get-mark is
not similarly useful since fast-import can already use either object id
or mark. Further, trying to allow this command anywhere caused parsing
bugs. Fix the parsing problems by only allowing get-mark commands to
appear when other commands have completed.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In commit 777f80d7429b ("fast-import: Allow cat-blob requests at
arbitrary points in stream", 2010-11-28), fast-import started allowing
cat-blob commands to appear on the start of any line except in the
middle of a "data" command. It could be in the middle of various
directives that were part of a tag command, or in the middle of
checkpoints or progresses (each of which allow an optional second empty
newline), or even immediately after the mark command of a blob before
the data directive appeared (raising the question of what if it used the
mark for the blob that just barely appeared in the stream that we do not
yet have the data for). None of these locations make any sense as
places to put cat-blob requests.
The purpose of this change as stated in that commit message was to
[save] frontends from having to loop over everything they want to
commit in the next commit and cat-ing the necessary objects in
advance.
However, that can be achieved by simply allowing cat-blob requests to
appear whenever a filemodify directive is allowed. Further, it avoids
setting a bad precedent for other commands to follow (e.g. get-mark); a
precedent which caused parsing problems in corner cases.
Technically, inline filemodify directives add a slight wrinkle in that
frontends might want to have cat-blob directives appear after the start
of the filemodify and before the data directive contained within it. I
think it would have been better to disallow such a case (it would be
trivial to use cat-blob before the filemodify instead), but since there
is evidence this was used, for backwards compatibility let's support
that case too.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This is not a very important change, and one that I expect to have no
performance impact whatsoever, but reading the code bothered me. The
parsing of command types in cmd_main() mostly runs in order of most
common to least common commands; sure, it's hard to say for sure what
the most common are without some type of study, but it seems fairly
clear to mark the original four ("blob", "commit", "tag", "reset") as
the most prominent. Indeed, the parsing for most other commands were
added to later in the list. However, when "ls" was added, it was stuck
near the top of the list, with no rationale for that particular
location. Move it down to later to appease my Tourette's-like internal
twitching that its former location was causing.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The docs claimed `ls` commands could appear almost anywhere, but the
code told a different story. Modify the docs to match the code.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|