Merge branch 'jk/fsck-tree-mode-bits-fix' into maint

"git fsck" reads mode from tree objects but canonicalizes the mode
before passing it to the logic to check object sanity, which has
hid broken tree objects from the checking logic.  This has been
corrected, but to help exiting projects with broken tree objects
that they cannot fix retroactively, the severity of anomalies this
code detects has been demoted to "info" for now.
source: <YvQcNpizy9uOZiAz@coredump.intra.peff.net>

* jk/fsck-tree-mode-bits-fix:
  fsck: downgrade tree badFilemode to "info"
  fsck: actually detect bad file modes in trees
  tree-walk: add a mechanism for getting non-canonicalized modes

Merge branch 'fc/vimdiff-layout-vimdiff3-fix' into maint

"vimdiff3" regression fix.
source: <20220810154618.307275-1-felipe.contreras@gmail.com>

* fc/vimdiff-layout-vimdiff3-fix:
  mergetools: vimdiff: simplify tabfirst
  mergetools: vimdiff: fix single window layouts
  mergetools: vimdiff: rework tab logic
  mergetools: vimdiff: fix for diffopt
  mergetools: vimdiff: silence annoying messages
  mergetools: vimdiff: make vimdiff3 actually work
  mergetools: vimdiff: fix comment

Merge branch 'js/safe-directory-plus' into maint

Platform-specific code that determines if a directory is OK to use
as a repository has been taught to report more details, especially
on Windows.
source: <pull.1286.v2.git.1659965270.gitgitgadget@gmail.com>

* js/safe-directory-plus:
  mingw: handle a file owned by the Administrators group correctly
  mingw: be more informative when ownership check fails on FAT32
  mingw: provide details about unsafe directories' ownership
  setup: prepare for more detailed "dubious ownership" messages
  setup: fix some formatting

Merge branch 'pw/use-glibc-tunable-for-malloc-optim' into maint

Avoid repeatedly running getconf to ask libc version in the test
suite, and instead just as it once per script.
source: <pull.1311.git.1659620305757.gitgitgadget@gmail.com>

* pw/use-glibc-tunable-for-malloc-optim:
  tests: cache glibc version check

Merge branch 'ab/hooks-regression-fix' into maint

A follow-up fix to a fix for a regression in 2.36.
source: <patch-1.1-2450e3e65cf-20220805T141402Z-avarab@gmail.com>

* ab/hooks-regression-fix:
  hook API: don't segfault on strbuf_addf() to NULL "out"

Merge branch 'gc/git-reflog-doc-markup' into maint

Doc mark-up fix.
source: <pull.1304.git.git.1659387885711.gitgitgadget@gmail.com>

* gc/git-reflog-doc-markup:
  Documentation/git-reflog: remove unneeded \ from \{

Merge branch 'js/ort-clean-up-after-failed-merge' into maint

Plug memory leaks in the failure code path in the "merge-ort" merge
strategy backend.
source: <pull.1307.v2.git.1659114727.gitgitgadget@gmail.com>

* js/ort-clean-up-after-failed-merge:
  merge-ort: do leave trace2 region even if checkout fails
  merge-ort: clean up after failed merge

Merge branch 'jk/struct-zero-init-with-older-gcc' into maint

Older gcc with -Wall complains about the universal zero initializer
"struct s = { 0 };" idiom, which makes developers' lives
inconvenient (as -Werror is enabled by DEVELOPER=YesPlease).  The
build procedure has been tweaked to help these compilers.
source: <YuQ60ZUPBHAVETD7@coredump.intra.peff.net>

* jk/struct-zero-init-with-older-gcc:
  config.mak.dev: squelch -Wno-missing-braces for older gcc

Merge branch 'js/lstat-mingw-enotdir-fix' into maint

Fix to lstat() emulation on Windows.
source: <pull.1291.v3.git.1659089152877.gitgitgadget@gmail.com>

* js/lstat-mingw-enotdir-fix:
  lstat(mingw): correctly detect ENOTDIR scenarios

Merge branch 'js/mingw-with-python' into maint

Conditionally allow building Python interpreter on Windows
source: <pull.1306.v2.git.1659109272.gitgitgadget@gmail.com>

* js/mingw-with-python:
  mingw: remove unneeded `NO_CURL` directive
  mingw: remove unneeded `NO_GETTEXT` directive
  windows: include the Python bits when building Git for Windows

Merge branch 'ca/unignore-local-installation-on-windows' into maint

Fix build procedure for Windows that uses CMake so that it can pick
up the shell interpreter from local installation location.
source: <pull.1304.git.1658912756815.gitgitgadget@gmail.com>

* ca/unignore-local-installation-on-windows:
  cmake: support local installations of git

Git 2.37.2

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

Merge branch 'jc/string-list-cleanup' into maint

Code clean-up.
source: <xmqq7d471dns.fsf@gitster.g>

* jc/string-list-cleanup:
  builtin/remote.c: use the right kind of STRING_LIST_INIT

Merge branch 'mt/pkt-line-comment-tweak' into maint

In-code comment clarification.
source: <6a14443c101fa132498297af6d7a483520688d75.1658488203.git.matheus.bernardino@usp.br>

* mt/pkt-line-comment-tweak:
  pkt-line.h: move comment closer to the associated code

Merge branch 'ma/t4200-update' into maint

Test fix.
source: <20220718154322.2177166-1-martin.agren@gmail.com>

* ma/t4200-update:
  t4200: drop irrelevant code

Merge branch 'tb/commit-graph-genv2-upgrade-fix' into maint

There was a bug in the codepath to upgrade generation information
in commit-graph from v1 to v2 format, which has been corrected.
source: <cover.1657667404.git.me@ttaylorr.com>

* tb/commit-graph-genv2-upgrade-fix:
  commit-graph: fix corrupt upgrade from generation v1 to v2
  commit-graph: introduce `repo_find_commit_pos_in_graph()`
  t5318: demonstrate commit-graph generation v2 corruption

Merge branch 'tk/untracked-cache-with-uall' into maint

Fix for a bug that makes write-tree to fail to write out a
non-existent index as a tree, introduced in 2.37.
source: <20220722212232.833188-1-martin.agren@gmail.com>

* tk/untracked-cache-with-uall:
  read-cache: make `do_read_index()` always set up `istate->repo`

Merge branch 'mt/checkout-count-fix' into maint

"git checkout" miscounted the paths it updated, which has been
corrected.
source: <cover.1657799213.git.matheus.bernardino@usp.br>

* mt/checkout-count-fix:
  checkout: fix two bugs on the final count of updated entries
  checkout: show bug about failed entries being included in final report
  checkout: document bug where delayed checkout counts entries twice

Merge branch 'cl/rerere-train-with-no-sign' into maint

"rerere-train" script (in contrib/) used to honor commit.gpgSign
while recreating the throw-away merges.
source: <PH7PR14MB5594A27B9295E95ACA4D6A69CE8F9@PH7PR14MB5594.namprd14.prod.outlook.com>

* cl/rerere-train-with-no-sign:
  contrib/rerere-train: avoid useless gpg sign in training

Merge branch 'kk/p4-client-name-encoding-fix' into maint

"git p4" did not handle non-ASCII client name well, which has been
corrected.
source: <pull.1285.v3.git.git.1658394440.gitgitgadget@gmail.com>

* kk/p4-client-name-encoding-fix:
  git-p4: refactoring of p4CmdList()
  git-p4: fix bug with encoding of p4 client name

Merge branch 'mb/p4-utf16-crlf' into maint

"git p4" working on UTF-16 files on Windows did not implement
CRLF-to-LF conversion correctly, which has been corrected.
source: <pull.1294.v2.git.git.1658341065221.gitgitgadget@gmail.com>

* mb/p4-utf16-crlf:
  git-p4: fix CR LF handling for utf16 files

Merge branch 'hx/lookup-commit-in-graph-fix' into maint

A corner case bug where lazily fetching objects from a promisor
remote resulted in infinite recursion has been corrected.
source: <cover.1656593279.git.hanxin.hx@bytedance.com>

* hx/lookup-commit-in-graph-fix:
  t5330: remove run_with_limited_processses()
  commit-graph.c: no lazy fetch in lookup_commit_in_graph()

Merge branch 'jc/resolve-undo' into maint

The resolve-undo information in the index was not protected against
GC, which has been corrected.
source: <xmqq35f7kzad.fsf@gitster.g>

* jc/resolve-undo:
  fsck: do not dereference NULL while checking resolve-undo data
  revision: mark blobs needed for resolve-undo as reachable

fsck: downgrade tree badFilemode to "info"

The previous commit un-broke the "badFileMode" check; before then it was
literally testing nothing. And as far as I can tell, it has been so
since the very initial version of fsck.

The current severity of "badFileMode" is just "warning". But in the
--strict mode used by transfer.fsckObjects, that is elevated to an
error. This will potentially cause hassle for users, because historical
objects with bad modes will suddenly start causing pushes to many server
operators to be rejected.

At the same time, these bogus modes aren't actually a big risk. Because
we canonicalize them everywhere besides fsck, they can't cause too much
mischief in the real world. The worst thing you can do is end up with
two almost-identical trees that have different hashes but are
interpreted the same. That will generally cause things to be inefficient
rather than wrong, and is a bug somebody working on a Git implementation
would want to fix, but probably not worth inconveniencing users by
refusing to push or fetch.

So let's downgrade this to "info" by default, which is our setting for
"mention this when fscking, but don't ever reject, even under strict
mode". If somebody really wants to be paranoid, they can still adjust
the level using config.

Suggested-by: Xavier Morel <xavier.morel@masklinn.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fsck: actually detect bad file modes in trees

We use the normal tree_desc code to iterate over trees in fsck, meaning
we only see the canonicalized modes it returns. And hence we'd never see
anything unexpected, since it will coerce literally any garbage into one
of our normal and accepted modes.

We can use the new RAW_MODES flag to see the real modes, and then use
the existing code to actually analyze them. The existing code is written
as allow-known-good, so there's not much point in testing a variety of
breakages. The one tested here should be S_IFREG but with nonsense
permissions.

Do note that the error-reporting here isn't great. We don't mention the
specific bad mode, but just that the tree has one or more broken modes.
But when you go to look at it with "git ls-tree", we'll report the
canonicalized mode! This isn't ideal, but given that this should come up
rarely, and that any number of other tree corruptions might force you
into looking at the binary bytes via "cat-file", it's not the end of the
world. And it's something we can improve on top later if we choose.

Reported-by: Xavier Morel <xavier.morel@masklinn.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tree-walk: add a mechanism for getting non-canonicalized modes

When using init_tree_desc() and tree_entry() to iterate over a tree, we
always canonicalize the modes coming out of the tree. This is a good
thing to prevent bugs or oddities in normal code paths, but it's
counter-productive for tools like fsck that want to see the exact
contents.

We can address this by adding an option to avoid the extra
canonicalization. A few notes on the implementation:

  - I've attached the new option to the tree_desc struct itself. The
    actual code change is in decode_tree_entry(), which is in turn
    called by the public update_tree_entry(), tree_entry(), and
    init_tree_desc() functions, plus their "gently" counterparts.

    By letting it ride along in the struct, we can avoid changing the
    signature of those functions, which are called many times. Plus it's
    conceptually simpler: you really want a particular iteration of a
    tree to be "raw" or not, rather than individual calls.

  - We still have to set the new option somewhere. The struct is
    initialized by init_tree_desc(). I added the new flags field only to
    the "gently" version. That avoids disturbing the much more numerous
    non-gentle callers, and it makes sense that anybody being careful
    about looking at raw modes would also be careful about bogus trees
    (i.e., the caller will be something like fsck in the first place).

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

mergetools: vimdiff: simplify tabfirst

If we wrap the tabdo command there's no need for a separate command
call.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: fix single window layouts

Layouts with a single window other than "MERGED" do not work (e.g.
"LOCAL" or "MERGED+LOCAL").

This is because as the documentation of bufdo says:

    The last buffer (or where an error occurred) becomes the current
    buffer.

And we do always do bufdo the end.

Additionally, we do it only once, when it should be per tab.

Fix this by doing it once per tab right after it's created and before
any buffer is switched.

Cc: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: rework tab logic

If we treat tabs especially, the logic becomes much simpler.

Cc: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: fix for diffopt

When diffopt has hiddenoff set and there's only one window (as is the
case in the single window mode) the diff mode is turned off.

We don't want that, so turn that option off.

Cc: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: silence annoying messages

When using the single window mode we are greeted with the following
warning:

  "./content_LOCAL_8975" 6L, 28B
  "./content_BASE_8975" 6 lines, 29 bytes
  "./content_REMOTE_8975" 6 lines, 29 bytes
  "content" 16 lines, 115 bytes
  Press ENTER or type command to continue

every time.

Silence that.

Suggested-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: make vimdiff3 actually work

When vimdiff3 was added in 7c147b77d3 (mergetools: add vimdiff3 mode,
2014-04-20), the description made clear the intention:

    It's similar to the default, except that the other windows are
    hidden.  This ensures that removed/added colors are still visible on
    the main merge window, but the other windows not visible.

However, in 0041797449 (vimdiff: new implementation with layout support,
2022-03-30) this was broken by generating a command that never creates
windows, and therefore vim never shows the diff.

The layout support implementation broke the whole purpose of vimdiff3,
and simply shows MERGED, which is no different from simply opening the
file with vim.

In order to show the diff, the windows need to be created first, and
then when they are hidden the diff remains (if hidenoff isn't set), but
by setting the `hidden` option the initial buffers are marked as hidden
thus making the feature work.

Suggested-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mergetools: vimdiff: fix comment

The name of the variable is wrong, and it can be set to anything, like
1.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Fernando Ramos <greenfoo@u92.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mingw: handle a file owned by the Administrators group correctly

When an Administrator creates a file or directory, the created
file/directory is owned not by the Administrator SID, but by the
_Administrators Group_ SID. The reason is that users with administrator
privileges usually run in unprivileged ("non-elevated") mode, and their
user SID does not change when running in elevated mode.

This is is relevant e.g. when running a GitHub workflow on a build
agent, which runs in elevated mode: cloning a Git repository in a script
step will cause the worktree to be owned by the Administrators Group
SID, for example.

Let's handle this case as following: if the current user is an
administrator, Git should consider a worktree owned by the
Administrators Group as if it were owned by said user.

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

mingw: be more informative when ownership check fails on FAT32

The FAT file system has no concept of ACLs. Therefore, it cannot store
any ownership information anyway, and the `GetNamedSecurityInfoW()` call
pretends that everything is owned "by the world".

Let's special-case that scenario and tell the user what's going on.

This addresses https://github.com/git-for-windows/git/issues/3886

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

mingw: provide details about unsafe directories' ownership

When Git refuses to use an existing repository because it is owned by
someone else than the current user, it can be a bit tricky on Windows to
figure out what is going on.

Let's help with that by providing more detailed information.

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

setup: prepare for more detailed "dubious ownership" messages

When verifying the ownership of the Git directory, we sometimes would
like to say a bit more about it, e.g. when using a platform-dependent
code path (think: Windows has the permission model that is so different
from Unix'), but only when it is a appropriate to actually say
something.

To allow for that, collect that information and hand it back to the
caller (whose responsibility it is to show it or not).

Note: We do not actually fill in any platform-dependent information yet,
this commit just adds the infrastructure to be able to do so.

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

setup: fix some formatting

In preparation for touching code that was introduced in 3b0bf2704980
(setup: tighten ownership checks post CVE-2022-24765, 2022-05-10) and
that was formatted differently than preferred in the Git project, fix
the indentation before actually modifying the code.

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

Downmerge a bit more for 2.37.x

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

Merge branch 'sg/index-format-doc-update' into maint

Docfix.
source: <20220718085640.7395-1-szeder.dev@gmail.com>

* sg/index-format-doc-update:
  index-format.txt: remove outdated list of supported extensions

Merge branch 'ma/sparse-checkout-cone-doc-fix' into maint

Docfix.
source: <20220718100530.2068354-1-martin.agren@gmail.com>

* ma/sparse-checkout-cone-doc-fix:
  config/core.txt: fix minor issues for `core.sparseCheckoutCone`

Merge branch 'ds/win-syslog-compiler-fix' into maint

Workaround for a false positive compiler warning.
source: <pull.1294.git.1658256354725.gitgitgadget@gmail.com>

* ds/win-syslog-compiler-fix:
  compat/win32: correct for incorrect compiler warning

Merge branch 'ld/osx-keychain-usage-fix' into maint

Workaround for a compiler warning against use of die() in
osx-keychain (in contrib/).
source: <pull.1293.git.1658251503775.gitgitgadget@gmail.com>

* ld/osx-keychain-usage-fix:
  osx-keychain: fix compiler warning

Merge branch 'ds/doc-wo-whitelist' into maint

Avoid "white/black-list" in documentation and code comments.
source: <pull.1274.v3.git.1658255537.gitgitgadget@gmail.com>

* ds/doc-wo-whitelist:
  transport.c: avoid "whitelist"
  t: avoid "whitelist"
  git.txt: remove redundant language
  git-cvsserver: clarify directory list
  daemon: clarify directory arguments

Merge branch 'mb/config-document-include' into maint

Add missing documentation for "include" and "includeIf" features in
"git config" file format, which incidentally teaches the command
line completion to include them in its offerings.
source: <pull.1285.v2.git.1658002423864.gitgitgadget@gmail.com>

* mb/config-document-include:
  config.txt: document include, includeIf

Merge branch 'rs/mingw-tighten-mkstemp' into maint

mkstemp() emulation on Windows has been improved.
source: <7265e37f-fd29-3579-b840-19a1df52a59f@web.de>

* rs/mingw-tighten-mkstemp:
  mingw: avoid mktemp() in mkstemp() implementation

Merge branch 'jk/clone-unborn-confusion' into maint

"git clone" from a repository with some ref whose HEAD is unborn
did not set the HEAD in the resulting repository correctly, which
has been corrected.
source: <YsdyLS4UFzj0j/wB@coredump.intra.peff.net>

* jk/clone-unborn-confusion:
  clone: move unborn head creation to update_head()
  clone: use remote branch if it matches default HEAD
  clone: propagate empty remote HEAD even with other branches
  clone: drop extra newline from warning message

hook API: don't segfault on strbuf_addf() to NULL "out"

Fix a logic error in a082345372e (hook API: fix v2.36.0 regression:
hooks should be connected to a TTY, 2022-06-07). When it started using
the "ungroup" API added in fd3aaf53f71 (run-command: add an "ungroup"
option to run_process_parallel(), 2022-06-07) it should have made the
same sort of change that fd3aaf53f71 itself made in
"t/helper/test-run-command.c".

The correct way to emit this "Couldn't start" output with "ungroup"
would be:

fprintf(stderr, _("Couldn't start hook '%s'\n"), hook_path);

But we should instead remove the emitting of this output. As the added
test shows we already emit output when we can't run the child. The
"cannot run" output here is emitted by run-command.c's
child_err_spew().

So the addition of the "Couldn't start hook" output here in
96e7225b310 (hook: add 'run' subcommand, 2021-12-22) was always
redundant. For the pre-commit hook we'll now emit exactly the same
output as we did before f443246b9f2 (commit: convert
{pre-commit,prepare-commit-msg} hook to hook.h, 2021-12-22) (and
likewise for others).

We could at this point add this to the pick_next_hook() callbacks in
hook.c:

assert(!out);
assert(!*pp_task_cb);

And this to notify_start_failure() and notify_hook_finished() (in the
latter case the parameter is called "pp_task_cp"):

assert(!out);
assert(!pp_task_cb);

But let's leave any such instrumentation for some eventual cleanup of
the "ungroup" API.

Reported-by: Ilya K <me@0upti.me>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
Reviewed-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: cache glibc version check

131b94a10a ("test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_
on glibc >= 2.34", 2022-03-04) introduced a check for the version of
glibc that is in use. This check is performed as part of
setup_malloc_check() which is called at least once for each test. As
the test involves forking `getconf` and `expr` cache the result and
use that within setup_malloc_check() to avoid forking these extra
processes for each test.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Documentation/git-reflog: remove unneeded \ from \{

There are some inconsistencies with how different asciidoc environments
handle different combinations of "\{<>}", e.g. these results were
observed with asciidoc on two different environments:

  | Input     | Output (env A) | Output (env B)   | same/different |
  |-----------+----------------+------------------+----------------|
  | \{<foo>\} | {&lt;foo&gt;}  | \{&lt;foo&gt;}^M | different      |
  | {<foo>}   | {&lt;foo&gt;}  | {&lt;foo&gt;}    | same           |
  | \{<foo>}  | {&lt;foo&gt;}  | \{&lt;foo&gt;}^M | different      |
  | \{foo\}   | {foo}          | {foo}            | same           |
  | \{\}      | {}             | \{}^M            | different      |
  | \{}       | {}             | {}               | same           |
  | {\}       | {}             | {}               | same           |

The only instance of this biting us is "@\{<specifier>\}" in
Documentation/git-reflog.txt; all other combinations of "\{<>}" (e.g. in
Documentation/revisions.txt) seem to render consistently.

Fix this inconsistent rendering by removing the unnecessary "\" in
Documentation/git-reflog.txt.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

merge-ort: do leave trace2 region even if checkout fails

In 557ac0350d9 (merge-ort: begin performance work; instrument with
trace2_region_* calls, 2021-01-23), we added Trace2 instrumentation, but
in the error path that returns early, we forgot to tell Trace2 that
we're leaving the region. Let's fix that.

Pointed-out-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

merge-ort: clean up after failed merge

In 9fefce68dc8 (merge-ort: basic outline for merge_switch_to_result(),
2020-12-13), we added functionality to lay down the result of a merge on
disk. But we forgot to release the data structures in case
`unpack_trees()` failed to run properly.

This was pointed out by the `linux-leaks` job in our CI runs.

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

config.mak.dev: squelch -Wno-missing-braces for older gcc

Versions of gcc prior to 4.9 complain about an initialization like:

  struct inner { int x; };
  struct outer { struct inner; };
  struct outer foo = { 0 };

and insist on:

  struct outer foo = { { 0 } };

Newer compilers handle this just fine. And ignoring the window even on
older compilers is fine; the resulting code is correct, but we just get
caught by -Werror.

Let's relax this for older compilers to make developer lives easier (we
don't care much about non-developers on old compilers; they may see a
warning, but it won't stop compilation).

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

lstat(mingw): correctly detect ENOTDIR scenarios

Files' attributes can indicate more than just whether they are files or
directories. It was reported in Git for Windows that on certain network
shares, this led to a nasty problem trying to create tags:

$ git tag -a -m "automatic tag creation"  test_dir/test_tag
fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory

Note: This does not necessarily happen with all types of network shares.
One setup where it _did_ happen is a Windows Server 2019 VM, and as
hinted in

http://woshub.com/slow-network-shared-folder-refresh-windows-server/

in the indicated instance the following commands worked around the bug:

Set-SmbClientConfiguration -DirectoryCacheLifetime 0
Set-SmbClientConfiguration -FileInfoCacheLifetime 0
Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0

This would impact performance negatively, though, as it essentially
turns off all caching, therefore we do not want to require users to do
that just to be able to use Git on Windows.

The underlying bug is in the code added in 4b0abd5c695 (mingw: let
lstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that
emulates the POSIX behavior where `lstat()` should return `ENOENT` if
the file or directory simply does not exist but could be created, and
`ENOTDIR` if there is no file or directory nor could there be because a
leading path already exists and is not a directory.

In that code, the return value of `GetFileAttributesW()` is interpreted
as an enum value, not as a bit field, so that a perfectly fine leading
directory can be misdetected as "not a directory".

As a consequence, the `read_refs_internal()` function would return
`ENOTDIR`, suggesting not only that the tag in the `git tag` invocation
above does not exist, but that it cannot even be created.

Let's fix the code so that it interprets the return value of the
`GetFileAttributesW()` call correctly.

This fixes https://github.com/git-for-windows/git/issues/3727

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

mingw: remove unneeded `NO_CURL` directive

In df5218b4c30 (config.mak.uname: support MSys2, 2016-01-13), we
introduced support for building Git for Windows in the then-brand new
Git for Windows v2.x build environment that was based off of MSYS2.

To do that, we split the non-msysGit part (that targeted MSys1) in two,
and instead of sharing the `NO_CURL = YesPlease` setting with MSys1, we
overrode it for MSYS2 with the empty value because we very much want to
build Git for Windows with libcurl.

But that was unnecessary: we never set that variable beforehand,
therefore there is no need to override it.

Let's just remove that unnecessary line.

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

mingw: remove unneeded `NO_GETTEXT` directive

In f9206ce2681 (mingw: let's use gettext with MSYS2, 2016-01-26), we
flipped the switch to build Git for Windows with support for gettext.

However, the way we flipped the switch was by changing the value of the
`NO_GETTEXT` variable from a non-empty string to the empty string, as if
there was any `NO_GETTEXT` definition we needed to override.

But that was a mistake: while there _is_ a definition, it is in the
`THIS_IS_MSYSGIT` section, i.e. it does not affect the Git for Windows
part at all.

Let's just remove that unnecessary line.

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

windows: include the Python bits when building Git for Windows

While Git for Windows does not _ship_ Python (in order to save on
bandwidth), MSYS2 provides very fine Python interpreters that users can
easily take advantage of, by using Git for Windows within its SDK.

Previously, we excluded the Python bits, mostly due to historical
reasons: In the Git for Windows v1.x days, we built Git using
MSys/MinGW, without support for any Python scripts.

Therefore, let's move out the `NO_PYTHON` definition from the generic
part of the MINGW section (which includes special handling for MSYS2/Git
for Windows, for the long-superseded msysGit environment, as well as for
the setup of probably just one developer remaining with their MSys1)
into the two sections that cover different environments than Git for
Windows' SDK.

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

Downmerge a handful of fixes for 2.37.x maintenance track

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

Merge branch 'tk/rev-parse-doc-clarify-at-u' into maint

Doc update.
source: <pull.1265.v2.git.1655960512385.gitgitgadget@gmail.com>

* tk/rev-parse-doc-clarify-at-u:
  rev-parse: documentation adjustment - mention remote tracking with @{u}

Merge branch 'll/ls-files-tests-update' into maint

Test update.
source: <pull.1269.v6.git.1656863349926.gitgitgadget@gmail.com>

* ll/ls-files-tests-update:
  ls-files: update test style

Merge branch 'ds/t5510-brokequote' into maint

Test fix.
source: <484a330e-0902-6e1b-8189-63c72dcea494@github.com>

* ds/t5510-brokequote:
  t5510: replace 'origin' with URL more carefully

Merge branch 'tb/pack-objects-remove-pahole-comment' into maint

Comment fix.
source: <1379af2e9d271b501ef3942398e7f159a9c77973.1656440978.git.me@ttaylorr.com>

* tb/pack-objects-remove-pahole-comment:
  pack-objects.h: remove outdated pahole results

Merge branch 'en/t6429-test-must-be-empty-fix' into maint

A test fix.
source: <pull.1276.git.1656652799863.gitgitgadget@gmail.com>

* en/t6429-test-must-be-empty-fix:
  t6429: fix use of non-existent function

Merge branch 'ds/vscode-settings' into maint

* ds/vscode-settings:
  vscode: improve tab size and wrapping

Merge branch 'cr/setup-bug-typo' into maint

Typofix in a BUG() message.
source: <pull.1255.git.1654782920256.gitgitgadget@gmail.com>

* cr/setup-bug-typo:
  setup: fix function name in a BUG() message

Merge branch 'pb/diff-doc-raw-format' into maint

Update "git diff/log --raw" format documentation.
source: <pull.1259.git.1655123383.gitgitgadget@gmail.com>

* pb/diff-doc-raw-format:
  diff-index.txt: update raw output format in examples
  diff-format.txt: correct misleading wording
  diff-format.txt: dst can be 0* SHA-1 when path is deleted, too

Merge branch 'jk/revisions-doc-markup-fix' into maint

Documentation mark-up fix.
source: <YrOmsA04FZae89be@coredump.intra.peff.net>

* jk/revisions-doc-markup-fix:
  revisions.txt: escape "..." to avoid asciidoc horizontal ellipsis

Merge branch 'rs/combine-diff-with-incompatible-options' into maint

Certain diff options are currently ignored when combined-diff is
shown; mark them as incompatible with the feature.
source: <220524.86v8tuvfl1.gmgdl@evledraar.gmail.com>

* rs/combine-diff-with-incompatible-options:
  combine-diff: abort if --output is given
  combine-diff: abort if --ignore-matching-lines is given

Merge branch 'ac/bitmap-format-doc' into maint

Adjust technical/bitmap-format to be formatted by AsciiDoc, and
add some missing information to the documentation.
source: <pull.1246.v4.git.1655355834.gitgitgadget@gmail.com>

* ac/bitmap-format-doc:
  bitmap-format.txt: add information for trailing checksum
  bitmap-format.txt: fix some formatting issues
  bitmap-format.txt: feed the file to asciidoc to generate html

Merge branch 'ab/test-quoting-fix' into maint

Fixes for tests when the source directory has unusual characters in
its path, e.g. whitespaces, double-quotes, etc.
source: <cover-v2-0.3-00000000000-20220630T101646Z-avarab@gmail.com>

* ab/test-quoting-fix:
  config tests: fix harmless but broken "rm -r" cleanup
  test-lib.sh: fix prepend_var() quoting issue
  tests: add missing double quotes to included library paths

Merge branch 'ro/mktree-allow-missing-fix' into maint

"git mktree --missing" lazily fetched objects that are missing from
the local object store, which was totally unnecessary for the purpose
of creating the tree object(s) from its input.
source: <748f39a9-65aa-2110-cf92-7ddf81b5f507@roku.com>

* ro/mktree-allow-missing-fix:
  mktree: do not check type of remote objects

Merge branch 'dr/i18n-die-warn-error-usage' into maint

Give _() markings to fatal/warning/usage: labels that are shown in
front of these messages.
source: <pull.1279.v2.git.git.1655819877758.gitgitgadget@gmail.com>

* dr/i18n-die-warn-error-usage:
  i18n: mark message helpers prefix for translation

Merge branch 'ds/git-rebase-doc-markup' into maint

References to commands-to-be-typed-literally in "git rebase"
documentation mark-up have been corrected.
source: <pull.1270.v3.git.1656508868146.gitgitgadget@gmail.com>

* ds/git-rebase-doc-markup:
  git-rebase.txt: use back-ticks consistently

Merge branch 'gg/worktree-from-the-above' into maint

In a non-bare repository, the behavior of Git when the
core.worktree configuration variable points at a directory that has
a repository as its subdirectory, regressed in Git 2.27 days.
source: <20220616234433.225-1-gg.oss@outlook.com>
source: <20220616231956.154-1-gg.oss@outlook.com>

* gg/worktree-from-the-above:
  dir: minor refactoring / clean-up
  dir: traverse into repository

Merge branch 'fr/vimdiff-layout-fix' into maint

Recent update to vimdiff layout code has been made more robust
against different end-user vim settings.
source: <20220708181024.45839-1-greenfoo@u92.eu>

* fr/vimdiff-layout-fix:
  vimdiff: make layout engine more robust against user vim settings

Merge branch 'en/merge-dual-dir-renames-fix' into maint

Fixes a long-standing corner case bug around directory renames in
the merge-ort strategy.
source: <pull.1268.v4.git.1656984823.gitgitgadget@gmail.com>

* en/merge-dual-dir-renames-fix:
  merge-ort: fix issue with dual rename and add/add conflict
  merge-ort: shuffle the computation and cleanup of potential collisions
  merge-ort: make a separate function for freeing struct collisions
  merge-ort: small cleanups of check_for_directory_rename
  t6423: add tests of dual directory rename plus add/add conflict

Merge branch 'jk/diff-files-cleanup-fix' into maint

An earlier attempt to plug leaks placed a clean-up label to jump to
at a bogus place, which as been corrected.
source: <Ys0c0ePxPOqZ/5ck@coredump.intra.peff.net>

* jk/diff-files-cleanup-fix:
  diff-files: move misplaced cleanup label

Merge branch 'js/vimdiff-quotepath-fix' into maint

Variable quoting fix in the vimdiff driver of "git mergetool"
source: <pull.1287.v2.git.1657809063728.gitgitgadget@gmail.com>

* js/vimdiff-quotepath-fix:
  mergetool(vimdiff): allow paths to contain spaces again

Merge branch 'js/shortlog-sort-stably' into maint

"git shortlog -n" relied on the underlying qsort() to be stable,
which shouldn't have.  Fixed.
source: <pull.1290.git.1657813429221.gitgitgadget@gmail.com>

* js/shortlog-sort-stably:
  shortlog: use a stable sort

Merge branch 'js/ci-github-workflow-markup' into maint

A fix for a regression in test framework.
source: <pull.1288.git.1657789234416.gitgitgadget@gmail.com>

* js/ci-github-workflow-markup:
  tests: fix incorrect --write-junit-xml code

cmake: support local installations of git

At least in systems where the user is local and not an administrator
git will install in a subdirectory of %APPDATALOCAL%, so it makes
sense to also look there for the shell needed by the cmake integration
with Visual Studio.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read-cache: make `do_read_index()` always set up `istate->repo`

If there is no index file, e.g., because the repository has just been
created, we return zero early (unless `must_exist` makes us die
instead.)

This early return means we do not set up `istate->repo`. With
`core.untrackedCache=true`, the recent e6a653554b ("untracked-cache:
support '--untracked-files=all' if configured", 2022-03-31) will
eventually pass down `istate->repo` as a null pointer to
`repo_config_get_string()`, causing a segmentation fault.

If we do hit this early return, set up `istate->repo` similar to when we
actually read the index.

Reported-by: Joey Hess <id@joeyh.name>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

pkt-line.h: move comment closer to the associated code

ec9a37d ("pkt-line.[ch]: remove unused packet_read_line_buf()",
2021-10-14) removed the "src_buffer" and "src_len" parameters from
packet_read(), only leaving them at packet_read_with_status(). Let's
also update the function documentation by moving the comment about these
parameters from the former to the latter.

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-p4: refactoring of p4CmdList()

The function p4CmdList executes a Perforce command and
decodes the marshalled python dictionary. Special care has to be
taken for certain dictionary values which contain non-unicode characters.
The old handling contained separate hacks for each of the corresponding
dictionary keys. This commit tries to refactor the coding to handle the
special cases uniformely.

Signed-off-by: Kilian Kilger <kkilger@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/remote.c: use the right kind of STRING_LIST_INIT

Since 4a4b4cda (builtin-remote: Make "remote -v" display push urls,
2009-06-13), the string_list that was initialized with 0 in its
strdup_string member is immediately made to strdup its key strings
by flipping the strdup_string member to true.  When 183113a5
(string_list: Add STRING_LIST_INIT macro and make use of it.,
2010-07-04) has introduced STRING_LIST_INIT macros, it mechanically
replaced the initialization to STRING_LIST_INIT_NODUP.

Instead, just use the other initialization macro to make it strdup
the key from the beginning.

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

git-p4: fix CR LF handling for utf16 files

Perforce silently replaces LF with CR LF for "utf16" files if the client
is a native Windows client. Since git's autocrlf logic does not undo
this transformation for UTF-16 encoded files, git-p4 replaces CR LF with
LF during the sync if the file type "utf16" is detected and the Perforce
client platform indicates that this conversion is performed.

Windows only runs on little-endian architectures, therefore the encoding
of the byte stream received from the Perforce client is UTF-16-LE and
the relevant byte sequence is 0D 00 0A 00.

Signed-off-by: Moritz Baumann <moritz.baumann@sap.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

compat/win32: correct for incorrect compiler warning

The 'win build' job of our CI build is failing with the following error:

compat/win32/syslog.c: In function 'syslog':
compat/win32/syslog.c:53:17: error: pointer 'pos' may be used after \
    'realloc' [-Werror=use-after-free]
   53 |                 memmove(pos + 2, pos + 1, strlen(pos));
    CC compat/poll/poll.o
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compat/win32/syslog.c:47:23: note: call to 'realloc' here
   47 |                 str = realloc(str, st_add(++str_len, 1));
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

However, between this realloc() and the use we have a line that resets
the value of 'pos'. Thus, this error is incorrect. It is likely due to a
new version of the compiler on the CI machines.

Instead of waiting for a new compiler, create a new variable to avoid
this error.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

transport.c: avoid "whitelist"

The word "whitelist" has cultural implications that are not inclusive.
Thankfully, it is not difficult to reword and avoid its use.

The GIT_ALLOW_PROTOCOL environment variable was referred to as a
"whitelist", but the word "allow" is already part of the variable.
Replace "whitelist" with "allow_list" in these cases to demonstrate that
we are processing a list of allowed protocols.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t: avoid "whitelist"

The word "whitelist" has cultural implications that are not inclusive.
Thankfully, it is not difficult to reword and avoid its use.

Focus on changes in the test scripts, since most of the changes are in
comments and test names. The renamed test_allow_var helper is only used
once inside the widely-used test_proto helper.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git.txt: remove redundant language

The documentation for GIT_ALLOW_PROTOCOL has a sentence that adds no
value, since it repeats the meaning from the previous sentence (twice!).

The word "whitelist" has cultural implications that are not inclusive,
which brought attention to this sentence.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-cvsserver: clarify directory list

The documentation and error messages for git-cvsserver include some
references to a "whitelist" that is not otherwise included in the
documentation. When different parts of the documentation do not use
common language, this can lead to confusion as to how things are meant
to operate.

Further, the word "whitelist" has cultural implications that make its
use non-inclusive. Thankfully, we can remove it while increasing
clarity.

Update Documentation/git-cvsserver.txt in a similar way to the previous
change to Documentation/git-daemon.txt. The optional '<directory>...'
list can specify a list of allowed directories. We refer to that list
directly inside of the documentation for the GIT_CVSSERVER_ROOT
environment variable.

While modifying this documentation, update the environment variables to
use a list format. We use the modern way of tabbing the description of
each variable in this section. We do _not_ update the description of
'<directory>...' to use tabs this way since the rest of the items in the
OPTIONS list do not use this modern formatting.

A single error message in the actual git-cvsserver.perl code refers to
the whitelist during argument parsing. Instead, refer to the directory
list that has been clarified in the documentation.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

daemon: clarify directory arguments

The undecorated arguments to the 'git-daemon' command provide a list of
directories. When at least one directory is specified, then 'git-daemon'
only serves requests that are within that directory list. The boolean
'--strict-paths' option makes the list more explicit in that
subdirectories are no longer included.

The existing documentation and error messages around this directory list
refer to it and its behavior as a "whitelist". The word "whitelist" has
cultural implications that are not inclusive.  Thankfully, it is not
difficult to reword and avoid its use. In the process, we can define the
purpose of this directory list directly.

In Documentation/git-daemon.txt, rewrite the OPTIONS section around the
'<directory>' option. Add additional clarity to the other options that
refer to these directories.

Some error messages can also be improved in daemon.c. The
'--strict-paths' option requires '<directory>' arguments, so refer to
that section of the documentation directly. A logerror() call points out
that a requested directory is not in the specified directory list. We
can use "list" here without any loss of information.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

osx-keychain: fix compiler warning

Update git-credential-osxkeychain.c to remove 'format string is not a string
literal (potentially insecure)' compiler warning by treating the string as
an argument.

Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

contrib/rerere-train: avoid useless gpg sign in training

Users may have configured "git merge" to always require GPG
signing the resulting commits. We are not running "git merge" to
re-create merge commits, but merely to replay merge conflicts,
and we will immediately discard the resulting commits; there
is no point in signing them.

Override such configuration that forces useless signing from the
command line with the "--no-gpg-sign" option.

Signed-off-by: Celeste Liu <coelacanthus@outlook.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4200: drop irrelevant code

While setting up an unresolved merge for `git rerere`, we run `git
rev-parse` and `git fmt-merge-msg` to create a variable `$fifth` and a
commit-message file `msg`, which we then never actually use. This has
been like that since these tests were added in 672d1b789b ("rerere:
migrate to parse-options API", 2010-08-05). This does exercise `git
rev-parse` and `git fmt-merge-msg`, but doesn't contribute to testing
`git rerere`. Drop these lines.

Reported-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config/core.txt: fix minor issues for `core.sparseCheckoutCone`

The sparse checkout feature can be used in "cone mode" or "non-cone
mode". In this one instance in the documentation, we refer to the latter
as "non cone mode" with whitespace rather than a hyphen. Align this with
the rest of our documentation.

A few words later in the same paragraph, there's mention of "a more
flexible patterns". Drop that leading "a" to fix the grammar.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

index-format.txt: remove outdated list of supported extensions

The first section of 'Documentation/technical/index-format.txt'
mentions that "Git currently supports cache tree and resolve undo
extensions", but then goes on, and in the "Extensions" section
describes not only these two, but six other extensions [1].

Remove this sentence, as it's misleading about the status of all those
other extensions.

Alternatively we could keep that sentence and update the list of
extensions, but that might well lead to a recurring issue, because
apparently this list is never updated when a new index extension is
added.

[1] Split index, untracked cache, FS monitor cache, end of index
    entry, index entry offset table and sparse directory entries.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config.txt: document include, includeIf

Git config's tab completion does not yet know about the "include"
and "includeIf" sections, nor the related "path" variable.

Add a description for these two sections in
'Documentation/config/includeif.txt', which points to git-config's
documentation, specifically the "Includes" and "Conditional Includes"
subsections.

As a side effect, tab completion can successfully complete the
'include', 'includeIf', and 'include.add' expressions.
This effect is tested by two new ad-hoc tests.
Variable completion only works for "include" for now.

Credit for the ideas behind this patch goes to
Ævar Arnfjörð Bjarmason.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Manuel Boni <ziosombrero@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: fix corrupt upgrade from generation v1 to v2

The previous commit demonstrates a bug where a commit-graph using
generation v2 could enter a state where one of the GDA2 values has its
most-significant bit set (indicating that its value should be read from
the extended offset table in the GDO2 chunk) without having a GDO2 chunk
to read from.

This results in the following error message being displayed to the
caller:

    fatal: commit-graph requires overflow generation data but has none

This bug arises in the following scenario:

  - We decide to write a commit-graph using generation number v2, and
    decide (correctly) that no GDO2 chunk is necessary (e.g., because
    all of the commiter date offsets are no larger than 2^31-1).

  - The v2 generation numbers are stored in the `->generation` member of
    the commit slab holding `struct commit_graph_data`'s.

  - Later on, `load_commit_graph_info()` is called, overwriting the
    v2 generation data in the aforementioned slab with any existing v1
    generation data.

Then, when the commit-graph code goes to write the GDA2 chunk via
`write_graph_chunk_generation_data()`, we use the overwritten generation
v1 data in a place where we expect to use a v2 generation number:

    offset = commit_graph_data_at(c)->generation - c->date;

...because `commit_graph_data_at(c)->generation` used to hold the v2
generation data, but it was overwritten to contain the v1 generation
number via `load_commit_graph_info()`.

If the `offset` computation above overflows the v2 generation number
max, then `write_graph_chunk_generation_data()` will update its count of
large offsets and write the marker accordingly:

    if (offset > GENERATION_NUMBER_V2_OFFSET_MAX) {
        offset = CORRECTED_COMMIT_DATE_OFFSET_OVERFLOW | num_generation_data_overflows;
        num_generation_data_overflows++;
    }

and reads will look for the GDO2 chunk containing the overflowing v2
generation number, *after* the commit-graph code decided that no such
chunk was necessary.

The main problem is that the slab containing `struct commit_graph_data`
has a dual purpose. It is used to hold data that we are about to write
to disk while generating a commit-graph, as well as hold data that was
read from an existing commit-graph.

When the two mix, namely when the result of reading the commit-graph has
a side-effect that mixes poorly with an in-progress commit-graph write,
we end up with corrupt data.

A complete fix might be to introduce a new slab that is used exclusively
for writing, and gate access between the two slabs based on context
provided by the caller (e.g., whether this computation is part of a
"read" or "write" operation).

But a more minimal fix addresses the only known path which overwrites
the slab data, which is `compute_bloom_filters()` ->
`get_or_compute_bloom_filter()` -> `load_commit_graph_info()` ->
`fill_commit_graph_info()` by avoiding the last call which clobbers the
data altogether.

This path only needs to learn the graph position of a given commit so
that it can be used in `load_bloom_filter_from_graph()`. By replacing
the last steps of the above with one that records the graph position
into a temporary variable which is then used to load the existing Bloom
data, we eliminate the clobbering, removing the corruption.

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

commit-graph: introduce `repo_find_commit_pos_in_graph()`

Low-level callers in systems that are adjacent to the commit-graph (like
the changed-path Bloom filter code) could benefit from being able to
call a function like `parse_commit_in_graph()` without modifying the
corresponding commit slab data.

This is useful in contexts where that slab data is being used to prepare
for an upcoming commit-graph write, where Git must be careful to avoid
clobbering any of that data during a read operation.

Introduce a low-level variant of `parse_commit_in_graph()` which returns
the graph position of a given commit only, without modifying any of the
slab data.

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