git
3 years agoMerge branch 'cc/write-promisor-file'
Junio C Hamano [Mon, 25 Jan 2021 22:19:19 +0000 (14:19 -0800)] 
Merge branch 'cc/write-promisor-file'

A bit of code refactoring.

* cc/write-promisor-file:
  pack-write: die on error in write_promisor_file()
  fetch-pack: refactor writing promisor file
  fetch-pack: rename helper to create_promisor_file()

3 years agoMerge branch 'jx/bundle'
Junio C Hamano [Mon, 25 Jan 2021 22:19:19 +0000 (14:19 -0800)] 
Merge branch 'jx/bundle'

"git bundle" learns "--stdin" option to read its refs from the
standard input.  Also, it now does not lose refs whey they point
at the same object.

* jx/bundle:
  bundle: arguments can be read from stdin
  bundle: lost objects when removing duplicate pendings
  test: add helper functions for git-bundle

3 years agoMerge branch 'ab/mailmap'
Junio C Hamano [Mon, 25 Jan 2021 22:19:19 +0000 (14:19 -0800)] 
Merge branch 'ab/mailmap'

Clean-up docs, codepaths and tests around mailmap.

* ab/mailmap: (22 commits)
  shortlog: remove unused(?) "repo-abbrev" feature
  mailmap doc + tests: document and test for case-insensitivity
  mailmap tests: add tests for empty "<>" syntax
  mailmap tests: add tests for whitespace syntax
  mailmap tests: add a test for comment syntax
  mailmap doc + tests: add better examples & test them
  tests: refactor a few tests to use "test_commit --append"
  test-lib functions: add an --append option to test_commit
  test-lib functions: add --author support to test_commit
  test-lib functions: document arguments to test_commit
  test-lib functions: expand "test_commit" comment template
  mailmap: test for silent exiting on missing file/blob
  mailmap tests: get rid of overly complex blame fuzzing
  mailmap tests: add a test for "not a blob" error
  mailmap tests: remove redundant entry in test
  mailmap tests: improve --stdin tests
  mailmap tests: modernize syntax & test idioms
  mailmap tests: use our preferred whitespace syntax
  mailmap doc: start by mentioning the comment syntax
  check-mailmap doc: note config options
  ...

3 years agoMerge branch 'ps/fetch-atomic'
Junio C Hamano [Mon, 25 Jan 2021 22:19:19 +0000 (14:19 -0800)] 
Merge branch 'ps/fetch-atomic'

"git fetch" learns to treat ref updates atomically in all-or-none
fashion, just like "git push" does, with the new "--atomic" option.

* ps/fetch-atomic:
  fetch: implement support for atomic reference updates
  fetch: allow passing a transaction to `s_update_ref()`
  fetch: refactor `s_update_ref` to use common exit path
  fetch: use strbuf to format FETCH_HEAD updates
  fetch: extract writing to FETCH_HEAD

3 years agoMerge branch 'jk/log-cherry-pick-duplicate-patches'
Junio C Hamano [Mon, 25 Jan 2021 22:19:19 +0000 (14:19 -0800)] 
Merge branch 'jk/log-cherry-pick-duplicate-patches'

When more than one commit with the same patch ID appears on one
side, "git log --cherry-pick A...B" did not exclude them all when a
commit with the same patch ID appears on the other side.  Now it
does.

* jk/log-cherry-pick-duplicate-patches:
  patch-ids: handle duplicate hashmap entries

3 years agoMerge branch 'js/default-branch-name-tests-final-stretch'
Junio C Hamano [Mon, 25 Jan 2021 22:19:18 +0000 (14:19 -0800)] 
Merge branch 'js/default-branch-name-tests-final-stretch'

Prepare tests not to be affected by the name of the default branch
"git init" creates.

* js/default-branch-name-tests-final-stretch: (28 commits)
  tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed
  t99*: adjust the references to the default branch name "main"
  tests(git-p4): transition to the default branch name `main`
  t9[5-7]*: adjust the references to the default branch name "main"
  t9[0-4]*: adjust the references to the default branch name "main"
  t8*: adjust the references to the default branch name "main"
  t7[5-9]*: adjust the references to the default branch name "main"
  t7[0-4]*: adjust the references to the default branch name "main"
  t6[4-9]*: adjust the references to the default branch name "main"
  t64*: preemptively adjust alignment to prepare for `master` -> `main`
  t6[0-3]*: adjust the references to the default branch name "main"
  t5[6-9]*: adjust the references to the default branch name "main"
  t55[4-9]*: adjust the references to the default branch name "main"
  t55[23]*: adjust the references to the default branch name "main"
  t551*: adjust the references to the default branch name "main"
  t550*: adjust the references to the default branch name "main"
  t5503: prepare aligned comment for replacing `master` with `main`
  t5[0-4]*: adjust the references to the default branch name "main"
  t5323: prepare centered comment for `master` -> `main`
  t4*: adjust the references to the default branch name "main"
  ...

3 years agoMerge branch 'dl/reflog-with-single-entry'
Junio C Hamano [Mon, 25 Jan 2021 22:19:18 +0000 (14:19 -0800)] 
Merge branch 'dl/reflog-with-single-entry'

After expiring a reflog and making a single commit, the reflog for
the branch would record a single entry that knows both @{0} and
@{1}, but we failed to answer "what commit were we on?", i.e. @{1}

* dl/reflog-with-single-entry:
  refs: allow @{n} to work with n-sized reflog
  refs: factor out set_read_ref_cutoffs()

3 years agoMerge branch 'sj/untracked-files-in-submodule-directory-is-not-dirty'
Junio C Hamano [Mon, 25 Jan 2021 22:19:18 +0000 (14:19 -0800)] 
Merge branch 'sj/untracked-files-in-submodule-directory-is-not-dirty'

"git diff" showed a submodule working tree with untracked cruft as
"Submodule commit <objectname>-dirty", but a natural expectation is
that the "-dirty" indicator would align with "git describe --dirty",
which does not consider having untracked files in the working tree
as source of dirtiness.  The inconsistency has been fixed.

* sj/untracked-files-in-submodule-directory-is-not-dirty:
  diff: do not show submodule with untracked files as "-dirty"

3 years agoMerge branch 'jc/deprecate-pack-redundant'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'jc/deprecate-pack-redundant'

Warn loudly when the "pack-redundant" command, which has been left
stale with almost unusable performance issues, gets used, as we no
longer want to recommend its use (instead just "repack -d" instead).

* jc/deprecate-pack-redundant:
  pack-redundant: gauge the usage before proposing its removal

3 years agoMerge branch 'jk/forbid-lf-in-git-url'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'jk/forbid-lf-in-git-url'

Newline characters in the host and path part of git:// URL are
now forbidden.

* jk/forbid-lf-in-git-url:
  fsck: reject .gitmodules git:// urls with newlines
  git_connect_git(): forbid newlines in host and path

3 years agoMerge branch 'ab/branch-sort'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'ab/branch-sort'

The implementation of "git branch --sort" wrt the detached HEAD
display has always been hacky, which has been cleaned up.

* ab/branch-sort:
  branch: show "HEAD detached" first under reverse sort
  branch: sort detached HEAD based on a flag
  ref-filter: move ref_sorting flags to a bitfield
  ref-filter: move "cmp_fn" assignment into "else if" arm
  ref-filter: add braces to if/else if/else chain
  branch tests: add to --sort tests
  branch: change "--local" to "--list" in comment

3 years agoMerge branch 'en/diffcore-rename'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'en/diffcore-rename'

File-level rename detection updates.

* en/diffcore-rename:
  diffcore-rename: remove unnecessary duplicate entry checks
  diffcore-rename: accelerate rename_dst setup
  diffcore-rename: simplify and accelerate register_rename_src()
  t4058: explore duplicate tree entry handling in a bit more detail
  t4058: add more tests and documentation for duplicate tree entry handling
  diffcore-rename: reduce jumpiness in progress counters
  diffcore-rename: simplify limit check
  diffcore-rename: avoid usage of global in too_many_rename_candidates()
  diffcore-rename: rename num_create to num_destinations

3 years agoMerge branch 'ma/more-opaque-lock-file'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'ma/more-opaque-lock-file'

Code clean-up.

* ma/more-opaque-lock-file:
  read-cache: try not to peek into `struct {lock_,temp}file`
  refs/files-backend: don't peek into `struct lock_file`
  midx: don't peek into `struct lock_file`
  commit-graph: don't peek into `struct lock_file`
  builtin/gc: don't peek into `struct lock_file`

3 years agoMerge branch 'en/merge-ort-3'
Junio C Hamano [Mon, 25 Jan 2021 22:19:17 +0000 (14:19 -0800)] 
Merge branch 'en/merge-ort-3'

Rename detection is added to the "ORT" merge strategy.

* en/merge-ort-3:
  merge-ort: add implementation of type-changed rename handling
  merge-ort: add implementation of normal rename handling
  merge-ort: add implementation of rename collisions
  merge-ort: add implementation of rename/delete conflicts
  merge-ort: add implementation of both sides renaming differently
  merge-ort: add implementation of both sides renaming identically
  merge-ort: add basic outline for process_renames()
  merge-ort: implement compare_pairs() and collect_renames()
  merge-ort: implement detect_regular_renames()
  merge-ort: add initial outline for basic rename detection
  merge-ort: add basic data structures for handling renames

3 years agoMerge branch 'ab/mktag'
Junio C Hamano [Mon, 25 Jan 2021 22:19:16 +0000 (14:19 -0800)] 
Merge branch 'ab/mktag'

"git mktag" validates its input using its own rules before writing
a tag object---it has been updated to share the logic with "git
fsck".

* ab/mktag: (23 commits)
  mktag: add a --[no-]strict option
  mktag: mark strings for translation
  mktag: convert to parse-options
  mktag: allow omitting the header/body \n separator
  mktag: allow turning off fsck.extraHeaderEntry
  fsck: make fsck_config() re-usable
  mktag: use fsck instead of custom verify_tag()
  mktag: use puts(str) instead of printf("%s\n", str)
  mktag: remove redundant braces in one-line body "if"
  mktag: use default strbuf_read() hint
  mktag tests: test verify_object() with replaced objects
  mktag tests: improve verify_object() test coverage
  mktag tests: test "hash-object" compatibility
  mktag tests: stress test whitespace handling
  mktag tests: run "fsck" after creating "mytag"
  mktag tests: don't create "mytag" twice
  mktag tests: don't redirect stderr to a file needlessly
  mktag tests: remove needless SHA-1 hardcoding
  mktag tests: use "test_commit" helper
  mktag tests: don't needlessly use a subshell
  ...

3 years agoThe third batch
Junio C Hamano [Sat, 16 Jan 2021 05:48:20 +0000 (21:48 -0800)] 
The third batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'jc/macos-install-dependencies-fix'
Junio C Hamano [Sat, 16 Jan 2021 05:48:47 +0000 (21:48 -0800)] 
Merge branch 'jc/macos-install-dependencies-fix'

Fix for procedure to building CI test environment for mac.

* jc/macos-install-dependencies-fix:
  ci/install-depends: attempt to fix "brew cask" stuff

3 years agoMerge branch 'tb/local-clone-race-doc'
Junio C Hamano [Sat, 16 Jan 2021 05:48:47 +0000 (21:48 -0800)] 
Merge branch 'tb/local-clone-race-doc'

Doc update.

* tb/local-clone-race-doc:
  Documentation/git-clone.txt: document race with --local

3 years agoMerge branch 'bc/doc-status-short'
Junio C Hamano [Sat, 16 Jan 2021 05:48:47 +0000 (21:48 -0800)] 
Merge branch 'bc/doc-status-short'

Doc update.

* bc/doc-status-short:
  docs: rephrase and clarify the git status --short format

3 years agoMerge branch 'dl/p4-encode-after-kw-expansion'
Junio C Hamano [Sat, 16 Jan 2021 05:48:47 +0000 (21:48 -0800)] 
Merge branch 'dl/p4-encode-after-kw-expansion'

Text encoding fix for "git p4".

* dl/p4-encode-after-kw-expansion:
  git-p4: fix syncing file types with pattern

3 years agoMerge branch 'ab/gettext-charset-comment-fix'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'ab/gettext-charset-comment-fix'

Comments update.

* ab/gettext-charset-comment-fix:
  gettext.c: remove/reword a mostly-useless comment
  Makefile: remove a warning about old GETTEXT_POISON flag

3 years agoMerge branch 'ug/doc-lose-dircache'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'ug/doc-lose-dircache'

Doc update.

* ug/doc-lose-dircache:
  doc: remove "directory cache" from man pages

3 years agoMerge branch 'ad/t4129-setfacl-target-fix'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'ad/t4129-setfacl-target-fix'

Test fix.

* ad/t4129-setfacl-target-fix:
  t4129: fix setfacl-related permissions failure

3 years agoMerge branch 'jk/t5516-deflake'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'jk/t5516-deflake'

Test fix.

* jk/t5516-deflake:
  t5516: loosen "not our ref" error check

3 years agoMerge branch 'vv/send-email-with-less-secure-apps-access'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'vv/send-email-with-less-secure-apps-access'

Doc update.

* vv/send-email-with-less-secure-apps-access:
  git-send-email.txt: mention less secure app access with Gmail

3 years agoMerge branch 'pb/mergetool-tool-help-fix'
Junio C Hamano [Sat, 16 Jan 2021 05:48:46 +0000 (21:48 -0800)] 
Merge branch 'pb/mergetool-tool-help-fix'

Fix 2.29 regression where "git mergetool --tool-help" fails to list
all the available tools.

* pb/mergetool-tool-help-fix:
  mergetool--lib: fix '--tool-help' to correctly show available tools

3 years agoMerge branch 'ds/for-each-repo-noopfix'
Junio C Hamano [Sat, 16 Jan 2021 05:48:45 +0000 (21:48 -0800)] 
Merge branch 'ds/for-each-repo-noopfix'

"git for-each-repo --config=<var> <cmd>" should not run <cmd> for
any repository when the configuration variable <var> is not defined
even once.

* ds/for-each-repo-noopfix:
  for-each-repo: do nothing on empty config

3 years agoMerge branch 'jc/sign-off'
Junio C Hamano [Sat, 16 Jan 2021 05:48:45 +0000 (21:48 -0800)] 
Merge branch 'jc/sign-off'

Doc update.

* jc/sign-off:
  SubmittingPatches: tighten wording on "sign-off" procedure

3 years agoMerge branch 'mt/t4129-with-setgid-dir'
Junio C Hamano [Sat, 16 Jan 2021 05:48:45 +0000 (21:48 -0800)] 
Merge branch 'mt/t4129-with-setgid-dir'

Some tests expect that "ls -l" output has either '-' or 'x' for
group executable bit, but setgid bit can be inherited from parent
directory and make these fields 'S' or 's' instead, causing test
failures.

* mt/t4129-with-setgid-dir:
  t4129: don't fail if setgid is set in the test directory

3 years agoMerge branch 'ds/maintenance-part-4'
Junio C Hamano [Sat, 16 Jan 2021 05:48:45 +0000 (21:48 -0800)] 
Merge branch 'ds/maintenance-part-4'

Follow-up on the "maintenance part-3" which introduced scheduled
maintenance tasks to support platforms whose native scheduling
methods are not 'cron'.

* ds/maintenance-part-4:
  maintenance: use Windows scheduled tasks
  maintenance: use launchctl on macOS
  maintenance: include 'cron' details in docs
  maintenance: extract platform-specific scheduling

3 years agoThe second batch
Junio C Hamano [Fri, 15 Jan 2021 23:03:23 +0000 (15:03 -0800)] 
The second batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'fc/completion-aliases-support'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'fc/completion-aliases-support'

Bash completion (in contrib/) update to make it easier for
end-users to add completion for their custom "git" subcommands.

* fc/completion-aliases-support:
  completion: add proper public __git_complete
  test: completion: add tests for __git_complete
  completion: bash: improve function detection
  completion: bash: add __git_have_func helper

3 years agoMerge branch 'en/stash-apply-sparse-checkout'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'en/stash-apply-sparse-checkout'

"git stash" did not work well in a sparsely checked out working
tree.

* en/stash-apply-sparse-checkout:
  stash: fix stash application in sparse-checkouts
  stash: remove unnecessary process forking
  t7012: add a testcase demonstrating stash apply bugs in sparse checkouts

3 years agoMerge branch 'ar/t6016-modernise'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'ar/t6016-modernise'

Test update.

* ar/t6016-modernise:
  t6016: move to lib-log-graph.sh framework

3 years agoMerge branch 'zh/arg-help-format'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'zh/arg-help-format'

Clean up option descriptions in "git cmd --help".

* zh/arg-help-format:
  builtin/*: update usage format
  parse-options: format argh like error messages

3 years agoMerge branch 'nk/perf-fsmonitor-cleanup'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'nk/perf-fsmonitor-cleanup'

Test fix.

* nk/perf-fsmonitor-cleanup:
  p7519: allow running without watchman prereq

3 years agoMerge branch 'ds/trace2-topo-walk'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'ds/trace2-topo-walk'

The topological walk codepath is covered by new trace2 stats.

* ds/trace2-topo-walk:
  revision: trace topo-walk statistics

3 years agoMerge branch 'rs/rebase-commit-validation'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'rs/rebase-commit-validation'

Diagnose command line error of "git rebase" early.

* rs/rebase-commit-validation:
  rebase: verify commit parameter

3 years agoMerge branch 'ma/sha1-is-a-hash'
Junio C Hamano [Fri, 15 Jan 2021 23:20:29 +0000 (15:20 -0800)] 
Merge branch 'ma/sha1-is-a-hash'

Retire more names with "sha1" in it.

* ma/sha1-is-a-hash:
  hash-lookup: rename from sha1-lookup
  sha1-lookup: rename `sha1_pos()` as `hash_pos()`
  object-file.c: rename from sha1-file.c
  object-name.c: rename from sha1-name.c

3 years agoMerge branch 'ma/doc-pack-format-varint-for-sizes'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'ma/doc-pack-format-varint-for-sizes'

Doc update.

* ma/doc-pack-format-varint-for-sizes:
  pack-format.txt: document sizes at start of delta data

3 years agoMerge branch 'ma/t1300-cleanup'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'ma/t1300-cleanup'

Code clean-up.

* ma/t1300-cleanup:
  t1300: don't needlessly work with `core.foo` configs
  t1300: remove duplicate test for `--file no-such-file`
  t1300: remove duplicate test for `--file ../foo`

3 years agoMerge branch 'pb/doc-modules-git-work-tree-typofix'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'pb/doc-modules-git-work-tree-typofix'

Doc fix.

* pb/doc-modules-git-work-tree-typofix:
  gitmodules.txt: fix 'GIT_WORK_TREE' variable name

3 years agoMerge branch 'ta/doc-typofix'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'ta/doc-typofix'

Doc fix.

* ta/doc-typofix:
  doc: fix some typos

3 years agoMerge branch 'bc/rev-parse-path-format'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'bc/rev-parse-path-format'

"git rev-parse" can be explicitly told to give output as absolute
or relative path with the `--path-format=(absolute|relative)` option.

* bc/rev-parse-path-format:
  rev-parse: add option for absolute or relative path formatting
  abspath: add a function to resolve paths with missing components

3 years agoMerge branch 'ew/decline-core-abbrev'
Junio C Hamano [Fri, 15 Jan 2021 23:20:28 +0000 (15:20 -0800)] 
Merge branch 'ew/decline-core-abbrev'

The configuration variable 'core.abbrev' can be set to 'no' to
force no abbreviation regardless of the hash algorithm.

* ew/decline-core-abbrev:
  core.abbrev=no disables abbreviations

3 years agoci/install-depends: attempt to fix "brew cask" stuff
Junio C Hamano [Fri, 15 Jan 2021 02:51:02 +0000 (18:51 -0800)] 
ci/install-depends: attempt to fix "brew cask" stuff

We run "git pull" against "$cask_repo"; clarify that we are
expecting not to have any of our own modifications and running "git
pull" to merely update, by passing "--ff-only" on the command line.

Also, the "brew cask install" command line triggers an error message
that says:

    Error: Calling brew cask install is disabled! Use brew install
    [--cask] instead.

In addition, "brew install caskroom/cask/perforce" step triggers an
error that says:

    Error: caskroom/cask was moved. Tap homebrew/cask instead.

Attempt to see if blindly following the suggestion in these error
messages gets us into a better shape.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-write: die on error in write_promisor_file()
Christian Couder [Thu, 14 Jan 2021 15:50:16 +0000 (16:50 +0100)] 
pack-write: die on error in write_promisor_file()

write_promisor_file() already uses xfopen(), so it would die
if the file cannot be opened for writing. To be consistent
with this behavior and not overlook issues, let's also die if
there are errors when we are actually writing to the file.

Suggested-by: Jeff King <peff@peff.net>
Suggested-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch-pack: refactor writing promisor file
Christian Couder [Tue, 12 Jan 2021 08:21:59 +0000 (09:21 +0100)] 
fetch-pack: refactor writing promisor file

Let's replace the 2 different pieces of code that write a
promisor file in 'builtin/repack.c' and 'fetch-pack.c'
with a new function called 'write_promisor_file()' in
'pack-write.c' and 'pack.h'.

This might also help us in the future, if we want to put
back the ref names and associated hashes that were in
the promisor files we are repacking in 'builtin/repack.c'
as suggested by a NEEDSWORK comment just above the code
we are refactoring.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch-pack: rename helper to create_promisor_file()
Christian Couder [Tue, 12 Jan 2021 08:21:58 +0000 (09:21 +0100)] 
fetch-pack: rename helper to create_promisor_file()

As we are going to refactor the code that actually writes
the promisor file into a separate function in a following
commit, let's rename the current write_promisor_file()
function to create_promisor_file().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoshortlog: remove unused(?) "repo-abbrev" feature
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:06 +0000 (21:18 +0100)] 
shortlog: remove unused(?) "repo-abbrev" feature

Remove support for the magical "repo-abbrev" comment in .mailmap
files. This was added to .mailmap parsing in [1], as a generalized
feature of the git-shortlog Perl script added earlier in [2].

There was no documentation or tests for this feature, and I don't
think it's used in practice anymore.

What it did was to allow you to specify a single string to be
search-replaced with "/.../" in the .mailmap file. E.g. for
linux.git's current .mailmap:

    git archive --remote=git@gitlab.com:linux-kernel/linux.git \
        HEAD -- .mailmap | grep -a repo-abbrev
    # repo-abbrev: /pub/scm/linux/kernel/git/

Then when running e.g.:

    git shortlog --merges --author=Linus -1 v5.10-rc7..v5.10 | grep Merge

We'd emit (the [...] is mine):

      Merge tag [...]git://git.kernel.org/.../tip/tip

But will now emit:

      Merge tag [...]git.kernel.org/pub/scm/linux/kernel/git/tip/tip

I think at this point this is just a historical artifact we can get
rid of. It was initially meant for Linus's own use when we integrated
the Perl script[2], but since then it seems he's stopped using it.

Digging through Linus's release announcements on the LKML[3] the last
release I can find that made use of this output is Linux 2.6.25-rc6
back in March 2008[4]. Later on Linus started using --no-merges[5],
and nowadays seems to prefer some custom not-quite-shortlog format of
merges from lieutenants[6].

You will still see it on linux.git if you run "git shortlog" manually
yourself with --merges, with this removed you can still get the same
output with:

    git log --pretty=fuller v5.10-rc7..v5.10 |
    sed 's!/pub/scm/linux/kernel/git/!/.../!g' |
    git shortlog

Arguably we should do the same for the search-replacing of "[PATCH]"
at the beginning with "". That seems to be another relic of a bygone
era when linux.git patches would have their E-Mail subject lines
applied as-is by "git am" or whatever. But we documented that feature
in "git-shortlog(1)", and it seems more widely applicable than
something purely kernel-specific.

1. 7595e2ee6ef (git-shortlog: make common repository prefix
   configurable with .mailmap, 2006-11-25)
2. fa375c7f1b6 (Add git-shortlog perl script, 2005-06-04)
3. https://lore.kernel.org/lkml/
4. https://lore.kernel.org/lkml/alpine.LFD.1.00.0803161651350.3020@woody.linux-foundation.org/
5. https://lore.kernel.org/lkml/BANLkTinrbh7Xi27an3uY7pDWrNKhJRYmEA@mail.gmail.com/
6. https://lore.kernel.org/lkml/CAHk-=wg1+kf1AVzXA-RQX0zjM6t9J2Kay9xyuNqcFHWV-y5ZYw@mail.gmail.com/

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap doc + tests: document and test for case-insensitivity
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:05 +0000 (21:18 +0100)] 
mailmap doc + tests: document and test for case-insensitivity

Add documentation and more tests for case-insensitivity. The existing
test only matched on the E-Mail part, but as shown here we also match
the name with strcasecmp().

This behavior was last discussed on the mailing list in the thread
starting at [1]. It seems we're keeping it like this, so let's
document it.

1. https://lore.kernel.org/git/87czykvg19.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: add tests for empty "<>" syntax
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:04 +0000 (21:18 +0100)] 
mailmap tests: add tests for empty "<>" syntax

Add tests for mailmap's handling of "<>", which is allowed on the RHS,
but not the LHS of a "<LHS> <RHS>" pair.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: add tests for whitespace syntax
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:03 +0000 (21:18 +0100)] 
mailmap tests: add tests for whitespace syntax

Add tests for mailmap's handling of whitespace, i.e. how it trims
space within "<>" and around author names.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: add a test for comment syntax
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:02 +0000 (21:18 +0100)] 
mailmap tests: add a test for comment syntax

Add a test for mailmap comment syntax. As noted in [1] there was no
test coverage for this. Let's make sure a future change doesn't break
it.

1. https://lore.kernel.org/git/CAN0heSoKYWXqskCR=GPreSHc6twCSo1345WTmiPdrR57XSShhA@mail.gmail.com/

Reported-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap doc + tests: add better examples & test them
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:01 +0000 (21:18 +0100)] 
mailmap doc + tests: add better examples & test them

Change the mailmap documentation added in 0925ce4d49 (Add map_user()
and clear_mailmap() to mailmap, 2009-02-08) to continue discussing the
Jane/Joe example. I think this makes things a lot less confusing as
we're building up more complex examples using one set of data which
covers all the things we'd like to discuss.

Also add tests to assert that what our documentation says is what's
actually happening. This is mostly (or entirely) covered by existing
tests which I'm not deleting, but having these tests for the synopsis
makes it easier to follow-along while reading the tests & docs.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotests: refactor a few tests to use "test_commit --append"
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:18:00 +0000 (21:18 +0100)] 
tests: refactor a few tests to use "test_commit --append"

Refactor a few more tests to use the new "--append" option to
"test_commit". I added it for use in the mailmap tests, but this
demonstrates how useful it is in general.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-lib functions: add an --append option to test_commit
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:59 +0000 (21:17 +0100)] 
test-lib functions: add an --append option to test_commit

Add an --append option to test_commit to append <contents> to the
<file> we're writing to. This simplifies a lot of test setup, as shown
in some of the tests being changed here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-lib functions: add --author support to test_commit
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:58 +0000 (21:17 +0100)] 
test-lib functions: add --author support to test_commit

Add support for --author to "test_commit". This will simplify some
current and future tests, one of those is being changed here.

Let's also line-wrap the "git commit" command invocation to make diffs
that add subsequent options easier to add, as they'll only need to add
a new option line.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-lib functions: document arguments to test_commit
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:57 +0000 (21:17 +0100)] 
test-lib functions: document arguments to test_commit

The --notick argument was added in [1] and was followed by --signoff
in [2], but neither of these commits added any documentation for these
options. When -C was added in [3] a comment was added to document it,
but not the other options. Let's document all of these options.

1. 44b85e89d7 (t7003: add test to filter a branch with a commit at
   epoch, 2012-07-12),
2. 5ed75e2a3f (cherry-pick: don't forget -s on failure, 2012-09-14).
3. 6f94351b0a (test-lib-functions.sh: teach test_commit -C <dir>,
   2016-12-08)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-lib functions: expand "test_commit" comment template
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:56 +0000 (21:17 +0100)] 
test-lib functions: expand "test_commit" comment template

Expand the comment template for "test_commit" to match that of
"test_commit_bulk" added in b1c36cb849 (test-lib: introduce
test_commit_bulk, 2019-07-02). It has several undocumented options,
which won't all fit on one line. Follow-up commit(s) will document
them.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap: test for silent exiting on missing file/blob
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:55 +0000 (21:17 +0100)] 
mailmap: test for silent exiting on missing file/blob

That we silently ignore missing mailmap.file or mailmap.blob values is
intentional. See 938a60d64f (mailmap: clean up read_mailmap error
handling, 2012-12-12). However, nothing tested for this. Let's do that
by checking that stderr is empty in those cases.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: get rid of overly complex blame fuzzing
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:54 +0000 (21:17 +0100)] 
mailmap tests: get rid of overly complex blame fuzzing

Change a test that used a custom fuzzing function since
bfdfa3d414 (t4203 (mailmap): stop hardcoding commit ids and dates,
2010-10-15) to just use the "blame --porcelain" output instead.

We could use the same pattern as 0ba9c9a0fb (t8008: rely on
rev-parse'd HEAD instead of sha1 value, 2017-07-26) does to do this,
but there wouldn't be any point. We're not trying to test "blame"
output here in general, just that "blame" pays attention to the
mailmap.

So it's sufficient to get the blamed line(s) and authors from the
output, which is much easier with the "--porcelain" option.

It would still be possible for there to be a bug in "blame" such that
it uses the mailmap for its "--porcelain" output, but not the regular
output. Let's test for that simply by checking if specifying the
mailmap changes the output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: add a test for "not a blob" error
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:53 +0000 (21:17 +0100)] 
mailmap tests: add a test for "not a blob" error

Add a test for one of the error conditions added in
938a60d64f (mailmap: clean up read_mailmap error handling,
2012-12-12).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: remove redundant entry in test
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:52 +0000 (21:17 +0100)] 
mailmap tests: remove redundant entry in test

Remove a redundant line in a test added in d20d654fe8 (Change current
mailmap usage to do matching on both name and email of
author/committer., 2009-02-08).

This didn't conceivably test anything useful and is most likely a
copy/paste error.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: improve --stdin tests
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:51 +0000 (21:17 +0100)] 
mailmap tests: improve --stdin tests

The --stdin tests setup the "contact" file in the main setup, let's
instead set it up in the test that uses it.

Also refactor the first test so it's obvious that the point of it is
that "check-mailmap" will spew its input as-is when given no
argument. For that one we can just use the "expect" file as-is.

Also add tests for how other "--stdin" cases are handled, e.g. one
where we actually do a mapping.

For the rest of --stdin testing we just assume we're going to get the
same output. We could follow-up and make sure everything's
round-tripped through both --stdin and the file/blob backends, but I
don't think there's much point in that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: modernize syntax & test idioms
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:50 +0000 (21:17 +0100)] 
mailmap tests: modernize syntax & test idioms

Refactor the mailmap tests to:

 * Setup "actual" test files in the body of "test_expect_success"

 * Don't have X of "test_expect_success X Y" be an unquoted string.

 * Not to carry over test config between tests, and instead use
   "test_config".

 * Replace various "echo" a line-at-a-time patterns with here-docs.

 * Change a case of "log.mailmap=False" to use the lower-case
   "false". Both work, but this ends up in git-config's boolean
   parsing and these atypical values are tested for elsewhere. Let's
   use the lower-case to not draw the reader's attention to this
   abnormality.

 * Remove commentary asserting that things work a given way in favor
   of simply testing for it, i.e. in the case of a .mailmap file
   outside of the repository.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap tests: use our preferred whitespace syntax
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:49 +0000 (21:17 +0100)] 
mailmap tests: use our preferred whitespace syntax

Change these tests to use the preferred whitespace around ">",
"<<-EOF" etc. This is an initial step in larger and more meaningful
refactoring of the file, which makes a subsequent commit easier to
read.

I'm not changing the whitespace of "echo <str> > file" patterns to
"echo <str> >file" because all of those will be changed to here-docs
in a subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap doc: start by mentioning the comment syntax
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:48 +0000 (21:17 +0100)] 
mailmap doc: start by mentioning the comment syntax

Mentioning the comment syntax and blank line support first is in line
with how "git help config" describes its format. See
b8936cf060 (config.txt grammar, typo, and asciidoc fixes, 2006-06-08)
for the paragraph I'm copying & amending from its documentation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocheck-mailmap doc: note config options
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:47 +0000 (21:17 +0100)] 
check-mailmap doc: note config options

Add a passing mention of the mailmap.file and mailmap.blob
configuration options. Before this addition a reader of the
"check-mailmap" manpage would have no idea that a custom map could be
specified, unless they'd happen to e.g. come across it in the "config"
manpage first.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap doc: quote config variables `like.this`
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:46 +0000 (21:17 +0100)] 
mailmap doc: quote config variables `like.this`

Quote the mailmap.file and mailmap.blob configuration variables as
`mailmap.file` and `mailmap.blob`, and link to git-config(1). This is
in line with the preferred way of doing this in the rest of our
documentation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomailmap doc: create a new "gitmailmap(5)" man page
Ævar Arnfjörð Bjarmason [Tue, 12 Jan 2021 20:17:45 +0000 (21:17 +0100)] 
mailmap doc: create a new "gitmailmap(5)" man page

Create a gitmailmap(5) page similar to how .gitmodules and .gitignore
have their own pages at gitmodules(5) and gitignore(5). Now instead of
"check-mailmap", "blame" and "shortlog" documentation including the
description of the format we link to one canonical place.

This makes things easier for readers, since in our manpage or
web-based[1] output it's not clear that the "MAPPING AUTHORS" sections
aren't subtly different, as opposed to just included.

1. https://git-scm.com/docs/git-check-mailmap

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch: implement support for atomic reference updates
Patrick Steinhardt [Tue, 12 Jan 2021 12:27:52 +0000 (13:27 +0100)] 
fetch: implement support for atomic reference updates

When executing a fetch, then git will currently allocate one reference
transaction per reference update and directly commit it. This means that
fetches are non-atomic: even if some of the reference updates fail,
others may still succeed and modify local references.

This is fine in many scenarios, but this strategy has its downsides.

- The view of remote references may be inconsistent and may show a
  bastardized state of the remote repository.

- Batching together updates may improve performance in certain
  scenarios. While the impact probably isn't as pronounced with loose
  references, the upcoming reftable backend may benefit as it needs to
  write less files in case the update is batched.

- The reference-update hook is currently being executed twice per
  updated reference. While this doesn't matter when there is no such
  hook, we have seen severe performance regressions when doing a
  git-fetch(1) with reference-transaction hook when the remote
  repository has hundreds of thousands of references.

Similar to `git push --atomic`, this commit thus introduces atomic
fetches. Instead of allocating one reference transaction per updated
reference, it causes us to only allocate a single transaction and commit
it as soon as all updates were received. If locking of any reference
fails, then we abort the complete transaction and don't update any
reference, which gives us an all-or-nothing fetch.

Note that this may not completely fix the first of above downsides, as
the consistent view also depends on the server-side. If the server
doesn't have a consistent view of its own references during the
reference negotiation phase, then the client would get the same
inconsistent view the server has. This is a separate problem though and,
if it actually exists, can be fixed at a later point.

This commit also changes the way we write FETCH_HEAD in case `--atomic`
is passed. Instead of writing changes as we go, we need to accumulate
all changes first and only commit them at the end when we know that all
reference updates succeeded. Ideally, we'd just do so via a temporary
file so that we don't need to carry all updates in-memory. This isn't
trivially doable though considering the `--append` mode, where we do not
truncate the file but simply append to it. And given that we support
concurrent processes appending to FETCH_HEAD at the same time without
any loss of data, seeding the temporary file with current contents of
FETCH_HEAD initially and then doing a rename wouldn't work either. So
this commit implements the simple strategy of buffering all changes and
appending them to the file on commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch: allow passing a transaction to `s_update_ref()`
Patrick Steinhardt [Tue, 12 Jan 2021 12:27:48 +0000 (13:27 +0100)] 
fetch: allow passing a transaction to `s_update_ref()`

The handling of ref updates is completely handled by `s_update_ref()`,
which will manage the complete lifecycle of the reference transaction.
This is fine right now given that git-fetch(1) does not support atomic
fetches, so each reference gets its own transaction. It is quite
inflexible though, as `s_update_ref()` only knows about a single
reference update at a time, so it doesn't allow us to alter the
strategy.

This commit prepares `s_update_ref()` and its only caller
`update_local_ref()` to allow passing an external transaction. If none
is given, then the existing behaviour is triggered which creates a new
transaction and directly commits it. Otherwise, if the caller provides a
transaction, then we only queue the update but don't commit it. This
optionally allows the caller to manage when a transaction will be
committed.

Given that `update_local_ref()` is always called with a `NULL`
transaction for now, no change in behaviour is expected from this
change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch: refactor `s_update_ref` to use common exit path
Patrick Steinhardt [Tue, 12 Jan 2021 12:27:43 +0000 (13:27 +0100)] 
fetch: refactor `s_update_ref` to use common exit path

The cleanup code in `s_update_ref()` is currently duplicated for both
succesful and erroneous exit paths. This commit refactors the function
to have a shared exit path for both cases to remove the duplication.

Suggested-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch: use strbuf to format FETCH_HEAD updates
Patrick Steinhardt [Tue, 12 Jan 2021 12:27:39 +0000 (13:27 +0100)] 
fetch: use strbuf to format FETCH_HEAD updates

This commit refactors `append_fetch_head()` to use a `struct strbuf` for
formatting the update which we're about to append to the FETCH_HEAD
file. While the refactoring doesn't have much of a benefit right now, it
serves as a preparatory step to implement atomic fetches where we need
to buffer all updates to FETCH_HEAD and only flush them out if all
reference updates succeeded.

No change in behaviour is expected from this commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch: extract writing to FETCH_HEAD
Patrick Steinhardt [Tue, 12 Jan 2021 12:27:35 +0000 (13:27 +0100)] 
fetch: extract writing to FETCH_HEAD

When performing a fetch with the default `--write-fetch-head` option, we
write all updated references to FETCH_HEAD while the updates are
performed. Given that updates are not performed atomically, it means
that we we write to FETCH_HEAD even if some or all of the reference
updates fail.

Given that we simply update FETCH_HEAD ad-hoc with each reference, the
logic is completely contained in `store_update_refs` and thus quite hard
to extend. This can already be seen by the way we skip writing to the
FETCH_HEAD: instead of having a conditional which simply skips writing,
we instead open "/dev/null" and needlessly write all updates there.

We are about to extend git-fetch(1) to accept an `--atomic` flag which
will make the fetch an all-or-nothing operation with regards to the
reference updates. This will also require us to make the updates to
FETCH_HEAD an all-or-nothing operation, but as explained doing so is not
easy with the current layout. This commit thus refactors the wa we write
to FETCH_HEAD and pulls out the logic to open, append to, commit and
close the file. While this may seem rather over-the top at first,
pulling out this logic will make it a lot easier to update the code in a
subsequent commit. It also allows us to easily skip writing completely
in case `--no-write-fetch-head` was passed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopatch-ids: handle duplicate hashmap entries
Jeff King [Tue, 12 Jan 2021 15:52:32 +0000 (10:52 -0500)] 
patch-ids: handle duplicate hashmap entries

This fixes a bug introduced in dfb7a1b4d0 (patch-ids: stop using a
hand-rolled hashmap implementation, 2016-07-29) in which

  git rev-list --cherry-pick A...B

will fail to suppress commits reachable from A even if a commit with
matching patch-id appears in B.

Around the time of that commit, the algorithm for "--cherry-pick" looked
something like this:

  0. Traverse all of the commits, marking them as being on the left or
     right side of the symmetric difference.

  1. Iterate over the left-hand commits, inserting a patch-id struct for
     each into a hashmap, and pointing commit->util to the patch-id
     struct.

  2. Iterate over the right-hand commits, checking which are present in
     the hashmap. If so, we exclude the commit from the output _and_ we
     mark the patch-id as "seen".

  3. Iterate again over the left-hand commits, checking whether
     commit->util->seen is set; if so, exclude them from the output.

At the end, we'll have eliminated commits from both sides that have a
matching patch-id on the other side. But there's a subtle assumption
here: for any given patch-id, we must have exactly one struct
representing it. If two commits from A both have the same patch-id and
we allow duplicates in the hashmap, then we run into a problem:

  a. In step 1, we insert two patch-id structs into the hashmap.

  b. In step 2, our lookups will find only one of these structs, so only
     one "seen" flag is marked.

  c. In step 3, one of the commits in A will have its commit->util->seen
     set, but the other will not. We'll erroneously output the latter.

Prior to dfb7a1b4d0, our hashmap did not allow duplicates. Afterwards,
it used hashmap_add(), which explicitly does allow duplicates.

At that point, the solution would have been easy: when we are about to
add a duplicate, skip doing so and return the existing entry which
matches. But it gets more complicated.

In 683f17ec44 (patch-ids: replace the seen indicator with a commit
pointer, 2016-07-29), our step 3 goes away entirely. Instead, in step 2,
when the right-hand side finds a matching patch_id from the left-hand
side, we can directly mark the left-hand patch_id->commit to be omitted.
Solving that would be easy, too; there's a one-to-many relationship of
patch-ids to commits, so we just need to keep a list.

But there's more. Commit b3dfeebb92 (rebase: avoid computing unnecessary
patch IDs, 2016-07-29) built on that by lazily computing the full
patch-ids. So we don't even know when adding to the hashmap whether two
commits truly have the same id. We'd have to tentatively assign them a
list, and then possibly split them apart (possibly into N new structs)
at the moment we compute the real patch-ids. This could work, but it's
complicated and error-prone.

Instead, let's accept that we may store duplicates, and teach the lookup
side to be more clever. Rather than asking for a single matching
patch-id, it will need to iterate over all matching patch-ids. This does
mean examining every entry in a single hash bucket, but the worst-case
for a hash lookup was already doing that.

We'll keep the hashmap details out of the caller by providing a simple
iteration interface. We can retain the simple has_commit_patch_id()
interface for the other callers, but we'll simplify its return value
into an integer, rather than returning the patch_id struct. That way
they won't be tempted to look at the "commit" field of the return value
without iterating.

Reported-by: Arnaud Morin <arnaud.morin@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoDocumentation/git-clone.txt: document race with --local
Taylor Blau [Mon, 11 Jan 2021 19:25:10 +0000 (14:25 -0500)] 
Documentation/git-clone.txt: document race with --local

When running 'git clone --local', the operation may fail if another
process is modifying the source repository. Document that this race
condition is known to hopefully help anyone who may run into it.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobundle: arguments can be read from stdin
Jiang Xin [Tue, 12 Jan 2021 02:27:03 +0000 (21:27 -0500)] 
bundle: arguments can be read from stdin

In order to create an incremental bundle, we need to pass many arguments
to let git-bundle ignore some already packed commits.  It will be more
convenient to pass args via stdin.  But the current implementation does
not allow us to do this.

This is because args are parsed twice when creating bundle.  The first
time for parsing args is in `compute_and_write_prerequisites()` by
running `git-rev-list` command to write prerequisites in bundle file,
and stdin is consumed in this step if "--stdin" option is provided for
`git-bundle`.  Later nothing can be read from stdin when running
`setup_revisions()` in `create_bundle()`.

The solution is to parse args once by removing the entire function
`compute_and_write_prerequisites()` and then calling function
`setup_revisions()`.  In order to write prerequisites for bundle, will
call `prepare_revision_walk()` and `traverse_commit_list()`.  But after
calling `prepare_revision_walk()`, the object array `revs.pending` is
left empty, and the following steps could not work properly with the
empty object array (`revs.pending`).  Therefore, make a copy of `revs`
to `revs_copy` for later use right after calling `setup_revisions()`.

The copy of `revs_copy` is not a deep copy, it shares the same objects
with `revs`. The object array of `revs` has been cleared, but objects
themselves are still kept.  Flags of objects may change after calling
`prepare_revision_walk()`, we can use these changed flags without
calling the `git rev-list` command and parsing its output like the
former implementation.

Also add testcases for git bundle in t6020, which read args from stdin.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobundle: lost objects when removing duplicate pendings
Jiang Xin [Tue, 12 Jan 2021 02:27:02 +0000 (21:27 -0500)] 
bundle: lost objects when removing duplicate pendings

`git rev-list` will list one commit for the following command:

    $ git rev-list 'main^!'
    <tip-commit-of-main-branch>

But providing the same rev-list args to `git bundle`, fail to create
a bundle file.

    $ git bundle create - 'main^!'
    # v2 git bundle
    -<OID> <one-line-message>

    fatal: Refusing to create empty bundle.

This is because when removing duplicate objects in function
`object_array_remove_duplicates()`, one unique pending object which has
the same name is deleted by mistake.  The revision arg 'main^!' in the
above example is parsed by `handle_revision_arg()`, and at lease two
different objects will be appended to `revs.pending`, one points to the
parent commit of the "main" branch, and the other points to the tip
commit of the "main" branch.  These two objects have the same name
"main".  Only one object is left with the name "main" after calling the
function `object_array_remove_duplicates()`.

And what's worse, when adding boundary commits into pending list, we use
one-line commit message as names, and the arbitory names may surprise
git-bundle.

Only comparing objects themselves (".item") is also not good enough,
because user may want to create a bundle with two identical objects but
with different reference names, such as: "HEAD" and "refs/heads/main".

Add new function `contains_object()` which compare both the address and
the name of the object.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest: add helper functions for git-bundle
Jiang Xin [Tue, 12 Jan 2021 02:27:01 +0000 (21:27 -0500)] 
test: add helper functions for git-bundle

Move git-bundle related functions from t5510 to a library, and this lib
will be shared with a new testcase t6020 which finds a known breakage of
"git-bundle".

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorefs: allow @{n} to work with n-sized reflog
Denton Liu [Thu, 7 Jan 2021 10:36:59 +0000 (02:36 -0800)] 
refs: allow @{n} to work with n-sized reflog

This sequence works

$ git checkout -b newbranch
$ git commit --allow-empty -m one
$ git show -s newbranch@{1}

and shows the state that was immediately after the newbranch was
created.

But then if you do

$ git reflog expire --expire=now refs/heads/newbranch
$ git commit --allow=empty -m two
$ git show -s newbranch@{1}

you'd be scolded with

fatal: log for 'newbranch' only has 1 entries

While it is true that it has only 1 entry, we have enough
information in that single entry that records the transition between
the state in which the tip of the branch was pointing at commit
'one' to the new commit 'two' built on it, so we should be able to
answer "what object newbranch was pointing at?". But we refuse to
do so.

Make @{0} the special case where we use the new side to look up that
entry. Otherwise, look up @{n} using the old side of the (n-1)th entry
of the reflog.

Suggested-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>
3 years agogettext.c: remove/reword a mostly-useless comment
Ævar Arnfjörð Bjarmason [Mon, 11 Jan 2021 13:14:51 +0000 (14:14 +0100)] 
gettext.c: remove/reword a mostly-useless comment

Mostly remove the comment I added 5e9637c6297 (i18n: add
infrastructure for translating Git with gettext, 2011-11-18). Since
then we had a fix in 9c0495d23e6 (gettext.c: detect the vsnprintf bug
at runtime, 2013-12-01) so we're not running with the "set back to C
locale" hack on any modern system.

So having more than 1/4 of the file taken up by a digression about a
glibc bug that mostly doesn't happen to anyone anymore is just a
needless distraction. Shorten the comment to make a brief mention of
the bug, and where to find more info by looking at the git history for
this now-removed comment.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMakefile: remove a warning about old GETTEXT_POISON flag
Ævar Arnfjörð Bjarmason [Mon, 11 Jan 2021 13:14:50 +0000 (14:14 +0100)] 
Makefile: remove a warning about old GETTEXT_POISON flag

Remove a migratory warning I added in 6cdccfce1e0 (i18n: make
GETTEXT_POISON a runtime option, 2018-11-08) to give anyone using that
option in their builds a heads-up about the change from compile-time
to runtime introduced in that commit.

It's been more than 2 years since then, anyone who ran into this is
likely to have made a change as a result, so removing this is long
overdue.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodocs: rephrase and clarify the git status --short format
brian m. carlson [Sun, 10 Jan 2021 19:04:48 +0000 (19:04 +0000)] 
docs: rephrase and clarify the git status --short format

The table describing the porcelain format in git-status(1) is helpful,
but it's not completely clear what the three sections mean, even to
some contributors.  As a result, users are unable to find how to detect
common cases like merge conflicts programmatically.

Let's improve this situation by rephrasing to be more explicit about
what each of the sections in the table means, to tell users in plain
language which cases are occurring, and to describe what "unmerged"
means.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorefs: factor out set_read_ref_cutoffs()
Denton Liu [Wed, 6 Jan 2021 09:01:53 +0000 (01:01 -0800)] 
refs: factor out set_read_ref_cutoffs()

This block of code is duplicated twice. In a future commit, it will be
duplicated for a third time. Factor out the common functionality into
set_read_ref_cutoffs().

In the case of read_ref_at_ent(), we are incrementing `cb->reccnt` at the
beginning of the function. Move these to right before the return so that
the `cb->reccnt - 1` is changed to `cb->reccnt` and it can be cleanly
factored out into set_read_ref_cutoffs(). The duplication of the
increment statements will be removed in a future patch.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodoc: remove "directory cache" from man pages
Utku Gultopu [Fri, 8 Jan 2021 16:54:56 +0000 (16:54 +0000)] 
doc: remove "directory cache" from man pages

"directory cache" (or "directory cache index", "cache") are obsolete
terms which have been superseded by "index". Keeping them in the
documentation may be a source of confusion. This commit replaces
them with the current term, "index", on man pages.

Signed-off-by: Utku Gultopu <ugultopu@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot5516: loosen "not our ref" error check
Jeff King [Sun, 10 Jan 2021 03:23:09 +0000 (22:23 -0500)] 
t5516: loosen "not our ref" error check

Commit 014ade7484 (upload-pack: send ERR packet for non-tip objects,
2019-04-13) added a test that greps the output of a failed fetch to make
sure that upload-pack sent us the ERR packet we expected. But checking
this is racy; despite the argument in that commit, the client may still
be sending a "done" line after the server exits, causing it to die() on
a failed write() and never see the ERR packet at all.

This fails quite rarely on Linux, but more often on macOS. However, it
can be triggered reliably with:

diff --git a/fetch-pack.c b/fetch-pack.c
index 876f90c759..cf40de9092 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -489,6 +489,7 @@ static int find_common(struct fetch_negotiator *negotiator,
 done:
  trace2_region_leave("fetch-pack", "negotiation_v0_v1", the_repository);
  if (!got_ready || !no_done) {
+ sleep(1);
  packet_buf_write(&req_buf, "done\n");
  send_request(args, fd[1], &req_buf);
  }

This is a real user-visible race that it would be nice to fix, but it's
tricky to do so: the client would have to speculatively try to read an
ERR packet after hitting a write() error. And at least for this error,
it's specific to v0 (since v2 does not enforce reachability at all).

So let's loosen the test to avoid annoying racy failures. If we
eventually do the read-after-failed-write thing, we can tighten it. And
if not, v0 will grow increasingly obsolete as servers support v2, so the
utility of this test will decrease over time anyway.

Note that we can still check stderr to make sure upload-pack bailed for
the reason we expected. It writes a similar message to stderr, and
because the server side is just another process connected by pipes,
we'll reliably see it. This would not be the case for git://, or for
ssh servers that do not relay stderr (e.g., GitHub's custom endpoint
does not).

Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot4129: fix setfacl-related permissions failure
Adam Dinwoodie [Wed, 23 Dec 2020 11:44:31 +0000 (11:44 +0000)] 
t4129: fix setfacl-related permissions failure

When running this test in Cygwin, it's necessary to remove the inherited
access control lists from the Git working directory in order for later
permissions tests to work as expected.

As such, fix an error in the test script so that the ACLs are set for
the working directory, not a nonexistent subdirectory.

Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit-send-email.txt: mention less secure app access with Gmail
Vasyl Vavrychuk [Fri, 8 Jan 2021 04:17:17 +0000 (20:17 -0800)] 
git-send-email.txt: mention less secure app access with Gmail

Google may have changed Gmail security and now less secure app access
needs to be explicitly enabled if two-factor authentication is not in
place, otherwise send-email fails with:

5.7.8 Username and Password not accepted. Learn more at
5.7.8  https://support.google.com/mail/?p=BadCredentials

Document steps required to make this work.

Signed-off-by: Vasyl Vavrychuk <vvavrychuk@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
[dl: Clean up commit message and incorporate suggestions into patch.]
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofor-each-repo: do nothing on empty config
Derrick Stolee [Fri, 8 Jan 2021 02:30:46 +0000 (02:30 +0000)] 
for-each-repo: do nothing on empty config

'git for-each-repo --config=X' should return success without calling any
subcommands when the config key 'X' has no value. The current
implementation instead segfaults.

A user could run into this issue if they used 'git maintenance start' to
initialize their cron schedule using 'git for-each-repo
--config=maintenance.repo ...' but then using 'git maintenance
unregister' to remove the config option. (Note: 'git maintenance stop'
would remove the config _and_ remove the cron schedule.)

Add a simple test to ensure this works. Use 'git help --no-such-option'
as the potential subcommand to ensure that we will hit a failure if the
subcommand is ever run.

Reported-by: Andreas Bühmann <dev@uuml.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoSubmittingPatches: tighten wording on "sign-off" procedure
Junio C Hamano [Thu, 7 Jan 2021 23:38:09 +0000 (15:38 -0800)] 
SubmittingPatches: tighten wording on "sign-off" procedure

The text says "if you can certify DCO then you add a Signed-off-by
trailer".  But it does not say anything about people who cannot or
do not want to certify.  A natural reading may be that if you do not
certify, you must not add the trailer, but it shouldn't hurt to be
overly explicit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobranch: show "HEAD detached" first under reverse sort
Ævar Arnfjörð Bjarmason [Thu, 7 Jan 2021 09:51:53 +0000 (10:51 +0100)] 
branch: show "HEAD detached" first under reverse sort

Change the output of the likes of "git branch -l --sort=-objectsize"
to show the "(HEAD detached at <hash>)" message at the start of the
output. Before the compare_detached_head() function added in a
preceding commit we'd emit this output as an emergent effect.

It doesn't make any sense to consider the objectsize, type or other
non-attribute of the "(HEAD detached at <hash>)" message for the
purposes of sorting. Let's always emit it at the top instead. The only
reason it was sorted in the first place is because we're injecting it
into the ref-filter machinery so builtin/branch.c doesn't need to do
its own "am I detached?" detection.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobranch: sort detached HEAD based on a flag
Ævar Arnfjörð Bjarmason [Thu, 7 Jan 2021 09:51:52 +0000 (10:51 +0100)] 
branch: sort detached HEAD based on a flag

Change the ref-filter sorting of detached HEAD to check the
FILTER_REFS_DETACHED_HEAD flag, instead of relying on the ref
description filled-in by get_head_description() to start with "(",
which in turn we expect to ASCII-sort before any other reference.

For context, we'd like the detached line to appear first at the start
of "git branch -l", e.g.:

    $ git branch -l
    * (HEAD detached at <hash>)
      master

This doesn't change that, but improves on a fix made in
28438e84e04 (ref-filter: sort detached HEAD lines firstly, 2019-06-18)
and gives the Chinese translation the ability to use its preferred
punctuation marks again.

In Chinese the fullwidth versions of punctuation like "()" are
typically written as (U+FF08 fullwidth left parenthesis), (U+FF09
fullwidth right parenthesis) instead[1]. This form is used in both
po/zh_{CN,TW}.po in most cases where "()" is translated in a string.

Aside from that improvement to the Chinese translation, it also just
makes for cleaner code that we mark any special cases in the ref_array
we're sorting with flags and make the sort function aware of them,
instead of piggy-backing on the general-case of strcmp() doing the
right thing.

As seen in the amended tests this made reverse sorting a bit more
consistent. Before this we'd sometimes sort this message in the
middle, now it's consistently at the beginning or end, depending on
whether we're doing a normal or reverse sort. Having it at the end
doesn't make much sense either, but at least it behaves consistently
now. A follow-up commit will make this behavior under reverse sorting
even better.

I'm removing the "TRANSLATORS" comments that were in the old code
while I'm at it. Those were added in d4919bb288e (ref-filter: move
get_head_description() from branch.c, 2017-01-10). I think it's
obvious from context, string and translation memory in typical
translation tools that these are the same or similar string.

1. https://en.wikipedia.org/wiki/Chinese_punctuation#Marks_similar_to_European_punctuation

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoref-filter: move ref_sorting flags to a bitfield
Ævar Arnfjörð Bjarmason [Thu, 7 Jan 2021 09:51:51 +0000 (10:51 +0100)] 
ref-filter: move ref_sorting flags to a bitfield

Change the reverse/ignore_case/version sort flags in the ref_sorting
struct into a bitfield. Having three of them was already a bit
unwieldy, but it would be even more so if another flag needed a
function like ref_sorting_icase_all() introduced in
76f9e569adb (ref-filter: apply --ignore-case to all sorting keys,
2020-05-03).

A follow-up change will introduce such a flag, so let's move this over
to a bitfield. Instead of using the usual '#define' pattern I'm using
the "enum" pattern from builtin/rebase.c's b4c8eb024af (builtin
rebase: support --quiet, 2018-09-04).

Perhaps there's a more idiomatic way of doing the "for each in list
amend mask" pattern than this "mask/on" variable combo. This function
doesn't allow us to e.g. do any arbitrary changes to the bitfield for
multiple flags, but I think in this case that's fine. The common case
is that we're calling this with a list of one.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoref-filter: move "cmp_fn" assignment into "else if" arm
Ævar Arnfjörð Bjarmason [Thu, 7 Jan 2021 09:51:50 +0000 (10:51 +0100)] 
ref-filter: move "cmp_fn" assignment into "else if" arm

Further amend code changed in 7c5045fc180 (ref-filter: apply fallback
refname sort only after all user sorts, 2020-05-03) to move an
assignment only used in the "else if" arm to happen there. Before that
commit the cmp_fn would be used outside of it.

We could also just skip the "cmp_fn" assignment and use
strcasecmp/strcmp directly in a ternary statement here, but this is
probably more readable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoref-filter: add braces to if/else if/else chain
Ævar Arnfjörð Bjarmason [Thu, 7 Jan 2021 09:51:49 +0000 (10:51 +0100)] 
ref-filter: add braces to if/else if/else chain

Per the CodingGuidelines add braces to an if/else if/else chain where
only the "else" had braces. This is in preparation for a subsequent
change where the "else if" will have lines added to it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck: reject .gitmodules git:// urls with newlines
Jeff King [Thu, 7 Jan 2021 09:44:17 +0000 (04:44 -0500)] 
fsck: reject .gitmodules git:// urls with newlines

The previous commit taught the clone/fetch client side to reject a
git:// URL with a newline in it. Let's also catch these when fscking a
.gitmodules file, which will give an earlier warning.

Note that it would be simpler to just complain about newline in _any_
URL, but an earlier tightening for http/ftp made sure we kept allowing
newlines for unknown protocols (and this is covered in the tests). So
we'll stick to that precedent.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit_connect_git(): forbid newlines in host and path
Jeff King [Thu, 7 Jan 2021 09:43:58 +0000 (04:43 -0500)] 
git_connect_git(): forbid newlines in host and path

When we connect to a git:// server, we send an initial request that
looks something like:

  002dgit-upload-pack repo.git\0host=example.com

If the repo path contains a newline, then it's included literally, and
we get:

  002egit-upload-pack repo
  .git\0host=example.com

This works fine if you really do have a newline in your repository name;
the server side uses the pktline framing to parse the string, not
newlines. However, there are many _other_ protocols in the wild that do
parse on newlines, such as HTTP. So a carefully constructed git:// URL
can actually turn into a valid HTTP request. For example:

  git://localhost:1234/%0d%0a%0d%0aGET%20/%20HTTP/1.1 %0d%0aHost:localhost%0d%0a%0d%0a

becomes:

  0050git-upload-pack /
  GET / HTTP/1.1
  Host:localhost

  host=localhost:1234

on the wire. Again, this isn't a problem for a real Git server, but it
does mean that feeding a malicious URL to Git (e.g., through a
submodule) can cause it to make unexpected cross-protocol requests.
Since repository names with newlines are presumably quite rare (and
indeed, we already disallow them in git-over-http), let's just disallow
them over this protocol.

Hostnames could likewise inject a newline, but this is unlikely a
problem in practice; we'd try resolving the hostname with a newline in
it, which wouldn't work. Still, it doesn't hurt to err on the side of
caution there, since we would not expect them to work in the first
place.

The ssh and local code paths are unaffected by this patch. In both cases
we're trying to run upload-pack via a shell, and will quote the newline
so that it makes it intact. An attacker can point an ssh url at an
arbitrary port, of course, but unless there's an actual ssh server
there, we'd never get as far as sending our shell command anyway.  We
_could_ similarly restrict newlines in those protocols out of caution,
but there seems little benefit to doing so.

The new test here is run alongside the git-daemon tests, which cover the
same protocol, but it shouldn't actually contact the daemon at all.  In
theory we could make the test more robust by setting up an actual
repository with a newline in it (so that our clone would succeed if our
new check didn't kick in). But a repo directory with newline in it is
likely not portable across all filesystems. Likewise, we could check
git-daemon's log that it was not contacted at all, but we do not
currently record the log (and anyway, it would make the test racy with
the daemon's log write). We'll just check the client-side stderr to make
sure we hit the expected code path.

Reported-by: Harold Kim <h.kim@flatt.tech>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe first batch in 2.31 cycle
Junio C Hamano [Thu, 7 Jan 2021 07:22:15 +0000 (23:22 -0800)] 
The first batch in 2.31 cycle

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