git/git.git
22 months agot7604: clean up style
Denton Liu [Wed, 17 Apr 2019 10:23:23 +0000 (11:23 +0100)]
t7604: clean up style

Before, we had some Git commands which were upstream of the pipe. This
meant that if it produced an error, it would've gone unnoticed. Refactor
to place Git commands on their own.

Also, while we're at it, remove spaces after redirection operators.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agot3507: clean up style
Denton Liu [Wed, 17 Apr 2019 10:23:22 +0000 (11:23 +0100)]
t3507: clean up style

Remove space after redirection operators for style. Also, remove a git
command which was upstream of a pipe. Finally, let grep and sed open
their own input instead of letting the shell redirect the input.

Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agot7600: clean up style
Denton Liu [Wed, 17 Apr 2019 10:23:21 +0000 (11:23 +0100)]
t7600: clean up style

Clean up the 'merge --squash c3 with c7' test by removing some
unnecessary braces and removing a pipe.

Also, generally cleanup style by unindenting a here-doc, removing stray
spaces after a redirection operator and allowing sed to open its own
input instead of redirecting input from the shell.

Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-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>
22 months agosequencer: fix cleanup with --signoff and -x
Phillip Wood [Fri, 29 Mar 2019 11:08:42 +0000 (11:08 +0000)]
sequencer: fix cleanup with --signoff and -x

Before commit 356ee4659b ("sequencer: try to commit without forking 'git
commit'", 2017-11-24) when --signoff or -x were given on the command
line the commit message was cleaned up with --cleanup=space or
commit.cleanup if it was set. Unfortunately this behavior was lost when
I implemented committing without forking. Fix this and add some tests to
catch future regressions.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agoclone: send server options when using protocol v2
Jonathan Tan [Fri, 12 Apr 2019 19:51:22 +0000 (12:51 -0700)]
clone: send server options when using protocol v2

Commit 5e3548ef16 ("fetch: send server options when using protocol v2",
2018-04-24) taught "fetch" the ability to send server options when using
protocol v2, but not "clone". This ability is triggered by "-o" or
"--server-option".

Teach "clone" the same ability, except that because "clone" already
has "-o" for another parameter, teach "clone" only to receive
"--server-option".

Explain in the documentation, both for clone and for fetch, that server
handling of server options are server-specific. This is similar to
receive-pack's handling of push options - currently, they are just sent
to hooks to interpret as they see fit.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agotransport: die if server options are unsupported
Jonathan Tan [Fri, 12 Apr 2019 19:51:21 +0000 (12:51 -0700)]
transport: die if server options are unsupported

Server options were added in commit 5e3548ef16 ("fetch: send server
options when using protocol v2", 2018-04-24), supported only for
protocol version 2. But if the user specifies server options, and the
protocol version being used doesn't support them, the server options are
silently ignored.

Teach any transport users to die instead in this situation, just like
how "push" dies if push options are provided when the server doesn't
support them.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agoref-filter: use correct branch for %(push:track)
Damien Robert [Tue, 16 Apr 2019 12:16:46 +0000 (14:16 +0200)]
ref-filter: use correct branch for %(push:track)

In ref-filter.c, when processing the atom %(push:track), the
ahead/behind values are computed using `stat_tracking_info` which refers
to the upstream branch.

Fix that by introducing a new flag `for_push` in `stat_tracking_info`
in remote.c, which does the same thing but for the push branch.
Update the few callers of `stat_tracking_info` to handle this flag. This
ensure that whenever we use this function in the future, we are careful
to specify is this should apply to the upstream or the push branch.

This bug was not detected in t/t6300-for-each-ref.sh because in the test
for push:track, both the upstream and the push branches were behind by 1
from the local branch. Change the test so that the upstream branch is
behind by 1 while the push branch is ahead by 1. This allows us to test
that %(push:track) refers to the correct branch.

This changes the expected value of some following tests (by introducing
new references), so update them too.

Signed-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agoThe fifth batch
Junio C Hamano [Tue, 16 Apr 2019 10:33:35 +0000 (19:33 +0900)]
The fifth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
22 months agoMerge branch 'sg/t5318-cleanup'
Junio C Hamano [Tue, 16 Apr 2019 10:28:12 +0000 (19:28 +0900)]
Merge branch 'sg/t5318-cleanup'

Code cleanup.

* sg/t5318-cleanup:
  t5318-commit-graph: remove unused variable

22 months agoMerge branch 'jt/t5551-protocol-v2-does-not-have-half-auth'
Junio C Hamano [Tue, 16 Apr 2019 10:28:12 +0000 (19:28 +0900)]
Merge branch 'jt/t5551-protocol-v2-does-not-have-half-auth'

Test update.

* jt/t5551-protocol-v2-does-not-have-half-auth:
  t5551: mark half-auth no-op fetch test as v0-only

22 months agoMerge branch 'jk/http-walker-status-fix'
Junio C Hamano [Tue, 16 Apr 2019 10:28:11 +0000 (19:28 +0900)]
Merge branch 'jk/http-walker-status-fix'

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

22 months agoMerge branch 'jh/midx-verify-too-many-packs'
Junio C Hamano [Tue, 16 Apr 2019 10:28:11 +0000 (19:28 +0900)]
Merge branch 'jh/midx-verify-too-many-packs'

"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

22 months agoMerge branch 'bb/unicode-12'
Junio C Hamano [Tue, 16 Apr 2019 10:28:11 +0000 (19:28 +0900)]
Merge branch 'bb/unicode-12'

Unicode update.

* bb/unicode-12:
  unicode: update the width tables to Unicode 12

22 months agoMerge branch 'jk/refs-double-abort'
Junio C Hamano [Tue, 16 Apr 2019 10:28:11 +0000 (19:28 +0900)]
Merge branch 'jk/refs-double-abort'

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

22 months agoMerge branch 'nd/checkout-m-doc-update'
Junio C Hamano [Tue, 16 Apr 2019 10:28:10 +0000 (19:28 +0900)]
Merge branch 'nd/checkout-m-doc-update'

Doc about the above.

* nd/checkout-m-doc-update:
  checkout.txt: note about losing staged changes with --merge

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agoMerge 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

22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-name.c: remove the_repo from maybe_die_on_misspelt_object_name
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>
22 months agosubmodule-config.c: use repo_get_oid for reading .gitmodules
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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-name.c: remove the_repo from diagnose_invalid_index_path()
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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-name.c: remove the_repo from interpret_nth_prior_checkout()
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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-name.c: store and use repo in struct disambiguate_state
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>
22 months agosha1-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>
22 months agosha1-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>
22 months agosha1-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>
22 months agocommit.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>
22 months agocommit.cocci: refactor code, avoid double rewrite
Nguyễ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>
22 months agoupdate_info_refs(): drop unused force parameter
Jeff 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>
22 months agoserver-info: drop objdirlen pointer arithmetic
Jeff 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>
22 months agoserver-info: drop nr_alloc struct member
Jeff 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>
22 months agoserver-info: use strbuf to read old info/packs file
Jeff 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>
22 months agoserver-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>
22 months agoserver-info: fix blind pointer arithmetic
Jeff 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>
22 months agohttp: simplify parsing of remote objects/info/packs
Jeff 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>
22 months agopackfile: fix pack basename computation
Jeff 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>
22 months agomidx: check both pack and index names for containment
Jeff 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>
22 months agot5319: drop useless --buffer from cat-file
Jeff 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>
22 months agot5319: fix bogus cat-file argument
Jeff 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>
22 months agopack-revindex: open index if necessary
Jeff 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>
22 months agopackfile.h: drop extern from function declarations
Jeff 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>
22 months agoremote-testgit: move it into the support directory for t5801
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>
22 months agofetch: do not consider peeled tags as advertised tips
Jeff 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>
22 months agoremote.c: make singular free_ref() public
Jeff 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>
22 months agofetch: 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>
22 months agopkt-line: prepare buffer before handling ERR packets
Jeff 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>
22 months agoupload-pack: send ERR packet for non-tip objects
Jeff 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>
22 months agot5530: 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>
22 months agot5516: drop ok=sigpipe from unreachable-want tests
Jeff 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>
22 months agogettext tests: export the restored GIT_TEST_GETTEXT_POISON
Junio 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>
22 months agoprogress: break too long progress bar lines
SZEDER 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>
22 months agoprogress: clear previous progress update dynamically
SZEDER 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>
22 months agomacOS: make sure that gettext is found
Johannes 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>
22 months agot9822: skip tests if file names cannot be ISO-8859-1 encoded
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>
22 months agosubmodule foreach: fix "<command> --quiet" not being respected
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>
22 months agotests: disallow the use of abbreviated options (by default)
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>
22 months agoreplace: peel tag when passing a tag first to --graft
Christian 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>
22 months agoreplace: peel tag when passing a tag as parent to --graft
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>
22 months agosend-email: default to quoted-printable when CR is present
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>
22 months agopack-objects: write objects packed to trace2
Jonathan 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>
22 months agoprogress: use xmalloc/xcalloc
Jeff 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>
22 months agoxdiff: use xmalloc/xrealloc
Jeff 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>
22 months agoxdiff: use git-compat-util
Jeff 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>
22 months agotest-prio-queue: use xmalloc
Jeff 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>
22 months agotag: advise on nested tags
Denton 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>
22 months agot3000 (ls-files -o): widen description to reflect current tests
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>
22 months agountracked cache: fix off-by-one
Johannes 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>
22 months agorev-list: detect broken root trees
Jeff 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>