gitweb.git
Merge branch 'tb/trace2-va-list-fix'Junio C Hamano Tue, 16 Apr 2019 10:28:10 +0000 (19:28 +0900)

Merge branch 'tb/trace2-va-list-fix'

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

Merge branch 'tz/completion'Junio C Hamano Tue, 16 Apr 2019 10:28:09 +0000 (19:28 +0900)

Merge branch 'tz/completion'

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

Merge branch 'ma/doc-diff-doc-vs-doctor-comparison'Junio C Hamano Tue, 16 Apr 2019 10:28:09 +0000 (19:28 +0900)

Merge branch 'ma/doc-diff-doc-vs-doctor-comparison'

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

Merge branch 'tz/t4038-bash-redirect-target-workaround'Junio C Hamano Tue, 16 Apr 2019 10:28:09 +0000 (19:28 +0900)

Merge branch 'tz/t4038-bash-redirect-target-workaround'

Work-around extra warning from bash in our tests.

* tz/t4038-bash-redirect-target-workaround:
t4038-diff-combined: quote paths with whitespace

Merge branch 'ab/drop-scripted-rebase'Junio C Hamano Tue, 16 Apr 2019 10:28:09 +0000 (19:28 +0900)

Merge branch 'ab/drop-scripted-rebase'

Retire scripted "git rebase" implementation.

* ab/drop-scripted-rebase:
rebase: remove the rebase.useBuiltin setting

Merge branch 'jk/perf-lib-tee'Junio C Hamano Tue, 16 Apr 2019 10:28:08 +0000 (19:28 +0900)

Merge branch 'jk/perf-lib-tee'

Code cleanup in the test framework.

* jk/perf-lib-tee:
perf-lib.sh: rely on test-lib.sh for --tee handling

Merge branch 'ab/doc-misc-typofixes'Junio C Hamano Tue, 16 Apr 2019 10:28:08 +0000 (19:28 +0900)

Merge branch 'ab/doc-misc-typofixes'

Typofixes.

* ab/doc-misc-typofixes:
doc: fix typos in man pages

Merge branch 'mh/pack-protocol-doc-fix'Junio C Hamano Tue, 16 Apr 2019 10:28:08 +0000 (19:28 +0900)

Merge branch 'mh/pack-protocol-doc-fix'

Docfix.

* mh/pack-protocol-doc-fix:
fix pack protocol example client/server communication

Merge branch 'tg/glossary-overlay'Junio C Hamano Tue, 16 Apr 2019 10:28:07 +0000 (19:28 +0900)

Merge branch 'tg/glossary-overlay'

Doc update.

* tg/glossary-overlay:
glossary: add definition for overlay

Merge branch 'nd/checkout-f-while-conflicted-fix'Junio C Hamano Tue, 16 Apr 2019 10:28:07 +0000 (19:28 +0900)

Merge branch 'nd/checkout-f-while-conflicted-fix'

"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

Merge branch 'pw/rerere-autoupdate'Junio C Hamano Tue, 16 Apr 2019 10:28:07 +0000 (19:28 +0900)

Merge branch 'pw/rerere-autoupdate'

Doc updates.

* pw/rerere-autoupdate:
merge: tweak --rerere-autoupdate documentation
am/cherry-pick/rebase/revert: document --rerere-autoupdate

Merge branch 'jc/format-patch-error-check'Junio C Hamano Tue, 16 Apr 2019 10:28:06 +0000 (19:28 +0900)

Merge branch 'jc/format-patch-error-check'

"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

Merge branch 'js/get-short-oid-drop-cache'Junio C Hamano Tue, 16 Apr 2019 10:28:06 +0000 (19:28 +0900)

Merge branch 'js/get-short-oid-drop-cache'

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

Merge branch 'dl/subtree-limit-to-one-rev'Junio C Hamano Tue, 16 Apr 2019 10:28:06 +0000 (19:28 +0900)

Merge branch 'dl/subtree-limit-to-one-rev'

"git subtree" (in contrib/) update.

* dl/subtree-limit-to-one-rev:
contrib/subtree: ensure only one rev is provided

Merge branch 'js/init-db-update-for-mingw'Junio C Hamano Tue, 16 Apr 2019 10:28:05 +0000 (19:28 +0900)

Merge branch 'js/init-db-update-for-mingw'

"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

Merge branch 'js/remote-curl-i18n'Junio C Hamano Tue, 16 Apr 2019 10:28:05 +0000 (19:28 +0900)

Merge branch 'js/remote-curl-i18n'

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

Merge branch 'js/anonymize-remote-curl-diag'Junio C Hamano Tue, 16 Apr 2019 10:28:04 +0000 (19:28 +0900)

Merge branch 'js/anonymize-remote-curl-diag'

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

Merge branch 'ma/asciidoctor-fixes-more'Junio C Hamano Tue, 16 Apr 2019 10:28:04 +0000 (19:28 +0900)

Merge branch 'ma/asciidoctor-fixes-more'

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

Merge branch 'ma/asciidoctor-fixes'Junio C Hamano Tue, 16 Apr 2019 10:28:03 +0000 (19:28 +0900)

Merge branch 'ma/asciidoctor-fixes'

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

Merge branch 'jt/test-protocol-version'Junio C Hamano Tue, 16 Apr 2019 10:28:03 +0000 (19:28 +0900)

Merge branch 'jt/test-protocol-version'

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

Merge branch 'nd/diff-parseopt-3'Junio C Hamano Tue, 16 Apr 2019 10:28:03 +0000 (19:28 +0900)

Merge branch 'nd/diff-parseopt-3'

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

sha1-name.c: remove the_repo from get_oid_mb()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:41 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_oid_mb()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from other get_oid_*Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:40 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from other get_oid_*

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from maybe_die_on_misspelt... Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:39 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from maybe_die_on_misspelt_object_name

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule-config.c: use repo_get_oid for reading .gitmo... Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:38 +0000 (16:33 +0700)

submodule-config.c: use repo_get_oid for reading .gitmodules

Since 76e9bdc437 (submodule: support reading .gitmodules when it's not
in the working tree - 2018-10-25), every time you do

git grep --recurse-submodules

you are likely to see one warning line per submodule (unless all those
submodules also have submodules). On a superproject with plenty of
submodules (I've seen one with 67) this is really annoying.

The warning was there because we could not resolve extended SHA-1
syntax on a submodule. We can now. Make use of the new API and get rid
of the warning.

It would be even better if config_with_options() supports multiple
repositories too. But one step at a time.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: add repo_get_oid()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:37 +0000 (16:33 +0700)

sha1-name.c: add repo_get_oid()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_oid_with_context_1()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:36 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_oid_with_context_1()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from resolve_relative_path()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:35 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from resolve_relative_path()

"remove" is not entirely correct. But at least the function is aware
that if the given repo is not the_repository, then $CWD and
is_inside_work_tree() means nothing.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from diagnose_invalid_inde... Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:34 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from diagnose_invalid_index_path()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from handle_one_ref()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:33 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from handle_one_ref()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_oid_1()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:32 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_oid_1()

There is a cyclic dependency between one of these functions so they
cannot be converted one by one, so all related functions are converted
at once.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_oid_basic()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:31 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_oid_basic()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_describe_name()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:30 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_describe_name()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_oid_oneline()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:29 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_oid_oneline()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: add repo_interpret_branch_name()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:28 +0000 (16:33 +0700)

sha1-name.c: add repo_interpret_branch_name()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from interpret_branch_mark()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:27 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from interpret_branch_mark()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from interpret_nth_prior_c... Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:26 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from interpret_nth_prior_checkout()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from get_short_oid()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:25 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from get_short_oid()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: add repo_for_each_abbrev()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:24 +0000 (16:33 +0700)

sha1-name.c: add repo_for_each_abbrev()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: store and use repo in struct disambiguate_... Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:23 +0000 (16:33 +0700)

sha1-name.c: store and use repo in struct disambiguate_state

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: add repo_find_unique_abbrev_r()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:22 +0000 (16:33 +0700)

sha1-name.c: add repo_find_unique_abbrev_r()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from find_abbrev_len_packed()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:21 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from find_abbrev_len_packed()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1-name.c: remove the_repo from sort_ambiguous()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:20 +0000 (16:33 +0700)

sha1-name.c: remove the_repo from sort_ambiguous()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit.c: add repo_get_commit_tree()Nguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:19 +0000 (16:33 +0700)

commit.c: add repo_get_commit_tree()

Remove the implicit dependency on the_repository in this function.
It will be used in sha1-name.c functions when they are updated to take
any 'struct repository'. get_commit_tree() remains as a compat wrapper,
to be slowly replaced later.

Any access to "maybe_tree" field directly will result in _broken_ code
after running through commit.cocci because we can't know what is the
right repository to use.

the_repository would be correct most of the time. But we're relying less
and less on the_repository and that assumption may no longer be
true. The transformation now is more of a poor man replacement for a C++
compiler catching access to private fields.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit.cocci: refactor code, avoid double rewriteNguyễn Thái Ngọc Duy Tue, 16 Apr 2019 09:33:18 +0000 (16:33 +0700)

commit.cocci: refactor code, avoid double rewrite

"maybe" pointer in 'struct commit' is tricky because it can be lazily
initialized to take advantage of commit-graph if available. This makes
it not safe to access directly.

This leads to a rule in commit.cocci to rewrite 'x->maybe_tree' to
'get_commit_tree(x)'. But that rule alone could lead to incorrectly
rewrite assignments, e.g. from

x->maybe_tree = yes

to

get_commit_tree(x) = yes

Because of this we have a second rule to revert this effect. Szeder
found out that we could do better by performing the assignment rewrite
rule first, then the remaining is read-only access and handled by the
current first rule.

For this to work, we need to transform "x->maybe_tree = y" to something
that does NOT contain "x->maybe_tree" to avoid the original first
rule. This is where set_commit_tree() comes in.

Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

update_info_refs(): drop unused force parameterJeff King Fri, 5 Apr 2019 18:14:07 +0000 (14:14 -0400)

update_info_refs(): drop unused force parameter

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>

server-info: drop objdirlen pointer arithmeticJeff King Fri, 5 Apr 2019 18:14:04 +0000 (14:14 -0400)

server-info: drop objdirlen pointer arithmetic

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>

server-info: drop nr_alloc struct memberJeff King Fri, 5 Apr 2019 18:14:00 +0000 (14:14 -0400)

server-info: drop nr_alloc struct member

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>

server-info: use strbuf to read old info/packs fileJeff King Fri, 5 Apr 2019 18:13:56 +0000 (14:13 -0400)

server-info: use strbuf to read old info/packs file

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>

server-info: simplify cleanup in parse_pack_def()Jeff King Fri, 5 Apr 2019 18:13:14 +0000 (14:13 -0400)

server-info: simplify cleanup in parse_pack_def()

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>

server-info: fix blind pointer arithmeticJeff King Fri, 5 Apr 2019 18:13:10 +0000 (14:13 -0400)

server-info: fix blind pointer arithmetic

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>

http: simplify parsing of remote objects/info/packsJeff King Fri, 5 Apr 2019 18:12:55 +0000 (14:12 -0400)

http: simplify parsing of remote objects/info/packs

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>

packfile: fix pack basename computationJeff King Fri, 5 Apr 2019 18:06:22 +0000 (14:06 -0400)

packfile: fix pack basename computation

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>

midx: check both pack and index names for containmentJeff King Fri, 5 Apr 2019 18:06:04 +0000 (14:06 -0400)

midx: check both pack and index names for containment

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>

t5319: drop useless --buffer from cat-fileJeff King Fri, 5 Apr 2019 18:05:03 +0000 (14:05 -0400)

t5319: drop useless --buffer from cat-file

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>

t5319: fix bogus cat-file argumentJeff King Fri, 5 Apr 2019 18:04:56 +0000 (14:04 -0400)

t5319: fix bogus cat-file argument

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>

pack-revindex: open index if necessaryJeff King Fri, 5 Apr 2019 18:04:24 +0000 (14:04 -0400)

pack-revindex: open index if necessary

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>

packfile.h: drop extern from function declarationsJeff King Fri, 5 Apr 2019 18:03:41 +0000 (14:03 -0400)

packfile.h: drop extern from function declarations

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>

remote-testgit: move it into the support directory... Johannes Schindelin Fri, 12 Apr 2019 12:00:31 +0000 (05:00 -0700)

remote-testgit: move it into the support directory for t5801

The `git-remote-testgit` script is really only used in
`t5801-remote-helpers.sh`. It does not even contain any `@@<MAGIC>@@`
placeholders that would need to be interpolated via `make
git-remote-testgit`.

Let's just move it to a new home, decluttering the top-level directory
and clarifying that this is just a test helper, not an official Git
command that we would want to ever support.

Suggested by Ævar Arnfjörð Bjarmason.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch: do not consider peeled tags as advertised tipsJeff King Sat, 13 Apr 2019 05:57:37 +0000 (01:57 -0400)

fetch: do not consider peeled tags as advertised tips

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>

remote.c: make singular free_ref() publicJeff King Sat, 13 Apr 2019 05:54:31 +0000 (01:54 -0400)

remote.c: make singular free_ref() public

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>

fetch: use free_refs()Jeff King Sat, 13 Apr 2019 05:54:09 +0000 (01:54 -0400)

fetch: use free_refs()

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>

pkt-line: prepare buffer before handling ERR packetsJeff King Sat, 13 Apr 2019 05:54:02 +0000 (01:54 -0400)

pkt-line: prepare buffer before handling ERR packets

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>

upload-pack: send ERR packet for non-tip objectsJeff King Sat, 13 Apr 2019 05:53:34 +0000 (01:53 -0400)

upload-pack: send ERR packet for non-tip objects

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>

t5530: check protocol response for "not our ref"Jeff King Sat, 13 Apr 2019 05:53:09 +0000 (01:53 -0400)

t5530: check protocol response for "not our ref"

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>

t5516: drop ok=sigpipe from unreachable-want testsJeff King Sat, 13 Apr 2019 05:52:18 +0000 (01:52 -0400)

t5516: drop ok=sigpipe from unreachable-want tests

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>

gettext tests: export the restored GIT_TEST_GETTEXT_POISONJunio C Hamano Mon, 15 Apr 2019 04:48:31 +0000 (13:48 +0900)

gettext tests: export the restored GIT_TEST_GETTEXT_POISON

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>

progress: break too long progress bar linesSZEDER Gábor Fri, 12 Apr 2019 19:45:15 +0000 (21:45 +0200)

progress: break too long progress bar lines

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>

progress: clear previous progress update dynamicallySZEDER Gábor Fri, 12 Apr 2019 19:45:14 +0000 (21:45 +0200)

progress: clear previous progress update dynamically

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>

macOS: make sure that gettext is foundJohannes Schindelin Sun, 14 Apr 2019 21:19:29 +0000 (14:19 -0700)

macOS: make sure that gettext is found

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>

t9822: skip tests if file names cannot be ISO-8859... Johannes Schindelin Sun, 14 Apr 2019 21:19:29 +0000 (14:19 -0700)

t9822: skip tests if file names cannot be ISO-8859-1 encoded

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>

submodule foreach: fix "<command> --quiet" not being... Nguyễn Thái Ngọc Duy Fri, 12 Apr 2019 10:08:19 +0000 (17:08 +0700)

submodule foreach: fix "<command> --quiet" not being respected

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>

tests: disallow the use of abbreviated options (by... Johannes Schindelin Fri, 12 Apr 2019 09:37:24 +0000 (02:37 -0700)

tests: disallow the use of abbreviated options (by default)

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>

replace: peel tag when passing a tag first to --graftChristian Couder Sun, 31 Mar 2019 13:46:59 +0000 (15:46 +0200)

replace: peel tag when passing a tag first to --graft

When passing a tag as the first argument to `git replace --graft`,
it can be useful to accept it and use the underlying commit as a
the commit that will be replaced.

This already works for lightweight tags, but unfortunately
for annotated tags we have been using the hash of the tag object
instead of the hash of the underlying commit.

Especially we would pass the hash of the tag object to
replace_object_oid() where we would likely fail with an error
like:

"error: Objects must be of the same type.
'annotated_replaced_object' points to a replaced object of type 'tag'
while 'replacement' points to a replacement object of type 'commit'."

This patch fixes that by using the hash of the underlying commit
when an annotated tag is passed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

replace: peel tag when passing a tag as parent to ... Christian Couder Sun, 31 Mar 2019 13:46:58 +0000 (15:46 +0200)

replace: peel tag when passing a tag as parent to --graft

When passing a tag as a parent argument to `git replace --graft`,
it can be useful to accept it and use the underlying commit as a
parent.

This already works for lightweight tags, but unfortunately
for annotated tags we have been using the hash of the tag object
instead of the hash of the underlying commit as a parent in the
replacement object we create.

This created invalid objects, but the replace succeeded even if
it showed an error like:

error: object A is a tag, not a commit

This patch fixes that by using the hash of the underlying commit
when an annotated tag is passed.

While at it, let's also update an error message to make it
clearer.

Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-email: default to quoted-printable when CR is... brian m. carlson Sat, 13 Apr 2019 22:45:51 +0000 (22:45 +0000)

send-email: default to quoted-printable when CR is present

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>

pack-objects: write objects packed to trace2Jonathan Tan Thu, 11 Apr 2019 17:36:26 +0000 (10:36 -0700)

pack-objects: write objects packed to trace2

This is useful when investigating performance of pushes, and other times
when no progress information is written (because the pack is written to
stdout).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

progress: use xmalloc/xcallocJeff King Thu, 11 Apr 2019 13:49:57 +0000 (09:49 -0400)

progress: use xmalloc/xcalloc

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>

xdiff: use xmalloc/xreallocJeff King Thu, 11 Apr 2019 13:49:25 +0000 (09:49 -0400)

xdiff: use xmalloc/xrealloc

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>

xdiff: use git-compat-utilJeff King Thu, 11 Apr 2019 13:48:33 +0000 (09:48 -0400)

xdiff: use git-compat-util

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: use xmallocJeff King Thu, 11 Apr 2019 13:48:14 +0000 (09:48 -0400)

test-prio-queue: use xmalloc

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>

tag: advise on nested tagsDenton Liu Thu, 4 Apr 2019 18:25:15 +0000 (11:25 -0700)

tag: advise on nested tags

Robert Dailey reported confusion on the mailing list about a nested
tag which was most likely created by mistake. Jeff King noted that
this isn't a very common case and creating a tag-to-a-tag can be a
user-error.

Suggest that it may be a mistake with an advice message when
creating such a tag. Those who do want to create a tag that point
at another tag regularly can turn it off with the usual advice
mechanism.

Reported-by: Robert Dailey <rcdailey.lists@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
[jc: fixed test style and tweaked the log message]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3000 (ls-files -o): widen description to reflect curre... Kyle Meyer Wed, 10 Apr 2019 15:28:57 +0000 (11:28 -0400)

t3000 (ls-files -o): widen description to reflect current tests

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>

untracked cache: fix off-by-oneJohannes Schindelin Wed, 10 Apr 2019 12:56:48 +0000 (05:56 -0700)

untracked cache: fix off-by-one

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>

rev-list: detect broken root treesJeff King Wed, 10 Apr 2019 02:13:25 +0000 (19:13 -0700)

rev-list: detect broken root trees

When the traversal machinery sees a commit without a root tree, it
assumes that the tree was part of a BOUNDARY commit, and quietly ignores
the tree. But it could also be caused by a commit whose root tree is
broken or missing.

Instead, let's die() when we see a NULL root tree. We can differentiate
it from the BOUNDARY case by seeing if the commit was actually parsed.
This covers that case, plus future-proofs us against any others where we
might try to show an unparsed commit.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

rev-list: let traversal die when --missing is not in useJeff King Wed, 10 Apr 2019 02:13:23 +0000 (19:13 -0700)

rev-list: let traversal die when --missing is not in use

Commit 7c0fe330d5 (rev-list: handle missing tree objects properly,
2018-10-05) taught the traversal machinery used by git-rev-list to
ignore missing trees, so that rev-list could handle them itself.

However, it does so only by checking via oid_object_info_extended() that
the object exists at all. This can miss several classes of errors that
were previously detected by rev-list:

- type mismatches (e.g., we expected a tree but got a blob)

- failure to read the object data (e.g., due to bitrot on disk)

This is especially important because we use "rev-list --objects" as our
connectivity check to admit new objects to the repository, and it will
now miss these cases (though the bitrot one is less important here,
because we'd typically have just hashed and stored the object).

There are a few options to fix this:

1. we could check these properties in rev-list when we do the existence
check. This is probably too expensive in practice (perhaps even for
a type check, but definitely for checking the whole content again,
which implies loading each object into memory twice).

2. teach the traversal machinery to differentiate between a missing
object, and one that could not be loaded as expected. This probably
wouldn't be too hard to detect type mismatches, but detecting bitrot
versus a truly missing object would require deep changes to the
object-loading code.

3. have the traversal machinery communicate the failure to the caller,
so that it can decide how to proceed without re-evaluting the object
itself.

Of those, I think (3) is probably the best path forward. However, this
patch does none of them. In the name of expediently fixing the
regression to a normal "rev-list --objects" that we use for connectivity
checks, this simply restores the pre-7c0fe330d5 behavior of having the
traversal die as soon as it fails to load a tree (when --missing is set
to MA_ERROR, which is the default).

Note that we can't get rid of the object-existence check in
finish_object(), because this also handles blobs (which are not
otherwise checked at all by the traversal code).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

get_commit_tree(): return NULL for broken treeJeff King Wed, 10 Apr 2019 02:13:20 +0000 (19:13 -0700)

get_commit_tree(): return NULL for broken tree

Return NULL from 'get_commit_tree()' when a commit's root tree is
corrupt, doesn't exist, or points to an object which is not a tree.

In [1], this situation became a BUG(), but it can certainly occur in
cases which are not a bug in Git, for e.g., if a caller manually crafts
a commit whose tree is corrupt in any of the above ways.

Note that the expect_failure test in t6102 triggers this BUG(), but we
can't flip it to expect_success yet. Solving this problem actually
reveals a second bug.

[1]: 7b8a21dba1 (commit-graph: lazy-load trees for commits, 2018-04-06)

Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

list-objects.c: handle unexpected non-tree entriesTaylor Blau Wed, 10 Apr 2019 02:13:19 +0000 (19:13 -0700)

list-objects.c: handle unexpected non-tree entries

Apply similar treatment as the previous commit for non-tree entries,
too.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

list-objects.c: handle unexpected non-blob entriesTaylor Blau Wed, 10 Apr 2019 02:13:17 +0000 (19:13 -0700)

list-objects.c: handle unexpected non-blob entries

Fix one of the cases described in the previous commit where a tree-entry
that is promised to a blob is in fact a non-blob.

When 'lookup_blob()' returns NULL, it is because Git has cached the
requested object as a non-blob. In this case, prevent a SIGSEGV by
'die()'-ing immediately before attempting to dereference the result.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t: introduce tests for unexpected object typesTaylor Blau Wed, 10 Apr 2019 02:13:14 +0000 (19:13 -0700)

t: introduce tests for unexpected object types

Call an object's type "unexpected" when the actual type of an object
does not match Git's contextual expectation. For example, a tree entry
whose mode differs from the object's actual type, or a commit's parent
which is not another commit, and so on.

This can manifest itself in various unfortunate ways, including Git
SIGSEGV-ing under specific conditions. Consider the following example:
Git traverses a blob (say, via `git rev-list`), and then tries to read
out a tree-entry which lists that object as something other than a blob.
In this case, `lookup_blob()` will return NULL, and the subsequent
dereference will result in a SIGSEGV.

Introduce tests that present objects of "unexpected" type in the above
fashion to 'git rev-list'. Mark as failures the combinations that are
already broken (i.e., they exhibit the segfault described above). In the
cases that are not broken (i.e., they have NULL-ness checks or similar),
mark these as expecting success.

We might hit an unexpected type in two different ways (imagine we have a
tree entry that claims to be a tree but actually points to a blob):

- when we call lookup_tree(), we might find that we've already seen
the object referenced as a blob, in which case we'd get NULL. We
can exercise this with "git rev-list --objects $blob $tree", which
guarantees that the blob will have been parsed before we look in
the tree. These tests are marked as "seen" in the test script.

- we call lookup_tree() successfully, but when we try to read the
object, we find out it's something else. We construct our tests
such that $blob is not otherwise mentioned in $tree. These tests
are marked as "lone" in the script.

We should check that we behave sensibly in both cases (especially
because it is easy for a malicious actor to provoke one case or the
other).

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

add: error appropriately on repository with no commitsKyle Meyer Tue, 9 Apr 2019 23:07:37 +0000 (19:07 -0400)

add: error appropriately on repository with no commits

The previous commit made 'git add' abort when given a repository that
doesn't have a commit checked out. However, the output upon failure
isn't appropriate:

% git add repo
warning: adding embedded git repository: repo
hint: You've added another git repository inside your current repository.
hint: [...]
error: unable to index file 'repo/'
fatal: adding files failed

The hint doesn't apply in this case, and the error message doesn't
tell the user why 'repo' couldn't be added to the index.

Provide better output by teaching add_to_index() to error when given a
git directory where HEAD can't be resolved. To avoid the embedded
repository warning and hint, call check_embedded_repo() only after
add_file_to_index() succeeds because, in general, its output doesn't
make sense if adding to the index fails.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

dir: do not traverse repositories with no commitsKyle Meyer Tue, 9 Apr 2019 23:07:36 +0000 (19:07 -0400)

dir: do not traverse repositories with no commits

When treat_directory() encounters a directory that is not in the index
and DIR_NO_GITLINKS is unset, it calls resolve_gitlink_ref() to decide
if a directory looks like a repository, in which case the directory
won't be traversed. As a result, 'status -uall' and 'ls-files -o'
will show only the directory, even when there are untracked files
within the directory.

For the unusual case where a repository doesn't have a commit checked
out, resolve_gitlink_ref() returns -1 because HEAD cannot be resolved,
and the directory is treated as a normal directory (i.e. traversal
does not stop at the repository boundary). The status and ls-files
commands above list untracked files within the repository rather than
showing only the top-level directory. And if 'git add' is called on a
repository with no commit checked out, any untracked files under the
repository are added as blobs in the top-level project, a behavior
that is unlikely to be what the caller intended.

The above case is a corner case in an already unusual situation of the
working tree containing a repository that is not a tracked submodule,
but we might as well treat anything that looks like a repository
consistently. Loosen the "looks like a repository" criteria in
treat_directory() by replacing resolve_gitlink_ref() with
is_nonbare_repository_dir(), one of the checks that is performed
downstream when resolve_gitlink_ref() is called.

As the required update to t3700-add shows, calling 'git add' on a
repository with no commit checked out will now raise an error. While
this is the desired behavior, note that the output isn't yet
appropriate. The next commit will improve this output.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: refuse to add repository with no commitsKyle Meyer Tue, 9 Apr 2019 23:07:35 +0000 (19:07 -0400)

submodule: refuse to add repository with no commits

When the path given to 'git submodule add' is an existing repository
that is not in the index, the repository is passed to 'git add'. If
this repository doesn't have a commit checked out, we don't get a
useful result: there is no subproject OID to track, and any untracked
files in the sub-repository are added as blobs in the top-level
repository.

To avoid getting into this state, abort if the path is a repository
that doesn't have a commit checked out. Note that this check must
come before the 'git add --dry-run' check because the next commit will
make 'git add' fail when given a repository that doesn't have a commit
checked out.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: teach set-branch subcommandDenton Liu Fri, 8 Feb 2019 11:21:34 +0000 (03:21 -0800)

submodule: teach set-branch subcommand

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>

Documentation/git-show-branch: avoid literal {apostrophe}Todd Zullinger Wed, 10 Apr 2019 00:37:33 +0000 (20:37 -0400)

Documentation/git-show-branch: avoid literal {apostrophe}

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>

Documentation/git-svn: improve asciidoctor compatibilityTodd Zullinger Wed, 10 Apr 2019 00:37:34 +0000 (20:37 -0400)

Documentation/git-svn: improve asciidoctor compatibility

The second paragraph in the CONFIGURATION section intends to emphasize
the word 'must' with bold type. It does so by writing it as *must*, and
this works fine with AsciiDoc. It usually works great with Asciidoctor,
too, but in this particular instance, we have another "*" earlier in the
paragraph. We do escape it, and it is rendered literally just like we
want it to, but Asciidoctor then ends up tripping on the second (or
third) of the asterisks in this paragraph.

Since that asterisk is (part of) a literal example, we can set it in
monospace, by giving it as `*`. Adjust the whole paragraph in this way.
There's lots more monospacing to be done in this document, but since our
main motivation is addressing AsciiDoc/Asciidoctor discrepancies like
this one, let's just convert this one paragraph.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The fourth batchJunio C Hamano Tue, 9 Apr 2019 17:19:09 +0000 (02:19 +0900)

The fourth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jt/submodule-fetch-errmsg'Junio C Hamano Tue, 9 Apr 2019 17:14:26 +0000 (02:14 +0900)

Merge branch 'jt/submodule-fetch-errmsg'

Error message update.

* jt/submodule-fetch-errmsg:
submodule: explain first attempt failure clearly

Merge branch 'jk/sha1dc'Junio C Hamano Tue, 9 Apr 2019 17:14:26 +0000 (02:14 +0900)

Merge branch 'jk/sha1dc'

Build update for SHA-1 with collision detection.

* jk/sha1dc:
Makefile: fix unaligned loads in sha1dc with UBSan

Merge branch 'jk/promote-ggg'Junio C Hamano Tue, 9 Apr 2019 17:14:25 +0000 (02:14 +0900)

Merge branch 'jk/promote-ggg'

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