t-reftable-block: mark unused argv/argc

This is conceptually the same as the cases in df9d638c24 (unit-tests:
ignore unused argc/argv, 2024-08-17), but this unit test was migrated
from the reftable tests in a parallel branch.

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

t-reftable-block: add tests for index blocks

In the current testing setup, block operations are left unexercised
for index blocks. Add a test that exercises these operations for
index blocks.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: add tests for obj blocks

In the current testing setup, block operations are left unexercised
for obj blocks. Add a test that exercises these operations for obj
blocks.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: add tests for log blocks

In the current testing setup, block operations are only exercised
for ref blocks. Add another test that exercises these operations
for log blocks as well.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: remove unnecessary variable 'j'

Currently, there are two variables for array indices, 'i' and 'j'.
The variable 'j' is used only once and can be easily replaced with
'i'. Get rid of 'j' and replace its occurence with 'i'.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: use xstrfmt() instead of xstrdup()

Use xstrfmt() to assign a formatted string to a ref record's
refname instead of xstrdup(). This helps save the overhead of
a local 'char' buffer as well as makes the test more compact.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: use block_iter_reset() instead of block_iter_close()

block_iter_reset() restores a block iterator to its state at the time
of initialization without freeing any memory while block_iter_close()
deallocates the memory for the iterator.

In the current testing setup, a block iterator is allocated and
deallocated for every iteration of a loop, which hurts performance.
Improve upon this by using block_iter_reset() at the start of each
iteration instead. This has the added benifit of testing
block_iter_reset(), which currently remains untested.

Similarly, remove reftable_record_release() for a reftable record
that is still in use.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: use reftable_record_key() instead of strbuf_addstr()

In the current testing setup, the record key required for many block
iterator functions is manually stored in a strbuf struct and then
passed to these functions. This is not ideal when there exists a
dedicated function to encode a record's key into a strbuf, namely
reftable_record_key(). Use this function instead of manual encoding.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: use reftable_record_equal() instead of check_str()

In the current testing setup, operations like read and write for
reftable blocks as defined by reftable/block.{c, h} are verified by
comparing only the keys of input and output reftable records. This is
not ideal because there can exist inequal reftable records with the
same key. Use the dedicated function for record comparison,
reftable_record_equal(), instead of key-based comparison.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t-reftable-block: release used block reader

Used block readers must be released using block_reader_release() to
prevent the occurence of a memory leak. Make test_block_read_write()
conform to this statement.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t: harmonize t-reftable-block.c with coding guidelines

Harmonize the newly ported test unit-tests/t-reftable-block.c
with the following guidelines:
- Single line 'for' statements must omit curly braces.
- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.
- Array sizes and indices should preferably be of type 'size_t'and
  not 'int'.
- Return code variable should preferably be named 'ret', not 'n'.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t: move reftable/block_test.c to the unit testing framework

reftable/block_test.c exercises the functions defined in
reftable/block.{c, h}. Migrate reftable/block_test.c to the unit
testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework and renaming the tests to follow the unit-tests'
naming conventions.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Sync with 'maint' for Windows+VS build jobs used at CI

Merge branch 'jk/midx-unused-fix'

Code clean-up in the base topic.

* jk/midx-unused-fix:
  midx: drop unused parameters from add_midx_to_chain()

Merge branch 'js/ci-win-vs-build' into maint-2.46

Sync with Windows+VS build jobs used at CI.

* js/ci-win-vs-build:
  ci(win+VS): download the vcpkg artifacts using a dedicated GitHub Action
  ci: bump microsoft/setup-msbuild from v1 to v2

ci(win+VS): download the vcpkg artifacts using a dedicated GitHub Action

The Git for Windows project provides a GitHub Action to download and
cache Azure Pipelines artifacts (such as the `vcpkg` artifacts), hiding
gnarly internals, and also providing some robustness against network
glitches. Let's use it.

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

ci: bump microsoft/setup-msbuild from v1 to v2

The main benefit: The new version uses a node.js version that is not yet
deprecated.

Links:
- [Release notes](https://github.com/microsoft/setup-msbuild/releases)
- [Changelog](https://github.com/microsoft/setup-msbuild/blob/main/building-release.md)
- [Commits](https://github.com/microsoft/setup-msbuild/compare/v1...v2)

This patch was originally by GitHub's Dependabot, but I cannot attribute
that bot properly because it has no dedicated email address. Probably
because it hasn't reached legal age yet, or something.

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

The sixth batch

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

Merge branch 'ps/transport-leakfix-test-updates'

Test updates.

* ps/transport-leakfix-test-updates:
  transport: mark more tests leak-free

Merge branch 'tb/incremental-midx-part-1'

Incremental updates of multi-pack index files.

* tb/incremental-midx-part-1:
  midx: implement support for writing incremental MIDX chains
  t/t5313-pack-bounds-checks.sh: prepare for sub-directories
  t: retire 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP'
  midx: implement verification support for incremental MIDXs
  midx: support reading incremental MIDX chains
  midx: teach `midx_fanout_add_midx_fanout()` about incremental MIDXs
  midx: teach `midx_preferred_pack()` about incremental MIDXs
  midx: teach `midx_contains_pack()` about incremental MIDXs
  midx: remove unused `midx_locate_pack()`
  midx: teach `fill_midx_entry()` about incremental MIDXs
  midx: teach `nth_midxed_offset()` about incremental MIDXs
  midx: teach `bsearch_midx()` about incremental MIDXs
  midx: introduce `bsearch_one_midx()`
  midx: teach `nth_bitmapped_pack()` about incremental MIDXs
  midx: teach `nth_midxed_object_oid()` about incremental MIDXs
  midx: teach `prepare_midx_pack()` about incremental MIDXs
  midx: teach `nth_midxed_pack_int_id()` about incremental MIDXs
  midx: add new fields for incremental MIDX chains
  Documentation: describe incremental MIDX format

Merge branch 'jc/tests-no-useless-tee'

Test fixes.

* jc/tests-no-useless-tee:
  tests: drop use of 'tee' that hides exit status

Merge branch 'rs/unit-tests-test-run'

Unit-test framework has learned a simple control structure to allow
embedding test statements in-line instead of having to create a new
function to contain them.

* rs/unit-tests-test-run:
  t-strvec: use if_test
  t-reftable-basics: use if_test
  t-ctype: use if_test
  unit-tests: add if_test
  unit-tests: show location of checks outside of tests
  t0080: use here-doc test body

Sync with 'maint'

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

Prepare for 2.46.1

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

Merge branch 'sj/ref-fsck'

"git fsck" infrastructure has been taught to also check the sanity
of the ref database, in addition to the object database.

* sj/ref-fsck:
  fsck: add ref name check for files backend
  files-backend: add unified interface for refs scanning
  builtin/refs: add verify subcommand
  refs: set up ref consistency check infrastructure
  fsck: add refs report function
  fsck: add a unified interface for reporting fsck messages
  fsck: make "fsck_error" callback generic
  fsck: rename objects-related fsck error functions
  fsck: rename "skiplist" to "skip_oids"

Merge branch 'ps/p4-tests-updates' into maint-2.46

Perforce tests have been updated.
cf. <na5mwletzpnacietbc7pzqcgb622mvrwgrkjgjosysz3gvjcso@gzxxi7d7icr7>

* ps/p4-tests-updates:
  t98xx: mark Perforce tests as memory-leak free
  ci: update Perforce version to r23.2
  t98xx: fix Perforce tests with p4d r23 and newer

Merge branch 'ks/unit-test-comment-typofix' into maint-2.46

Typofix.

* ks/unit-test-comment-typofix:
  unit-tests/test-lib: fix typo in check_pointer_eq() description

Merge branch 'dh/encoding-trace-optim' into maint-2.46

An expensive operation to prepare tracing was done in re-encoding
code path even when the tracing was not requested, which has been
corrected.

* dh/encoding-trace-optim:
  convert: return early when not tracing

Merge branch 'dd/notes-empty-no-edit-by-default' into maint-2.46

"git notes add -m '' --allow-empty" and friends that take prepared
data to create notes should not invoke an editor, but it started
doing so since Git 2.42, which has been corrected.

* dd/notes-empty-no-edit-by-default:
  notes: do not trigger editor when adding an empty note

Merge branch 'jc/doc-rebase-fuzz-vs-offset-fix' into maint-2.46

"git rebase --help" referred to "offset" (the difference between
the location a change was taken from and the change gets replaced)
incorrectly and called it "fuzz", which has been corrected.

* jc/doc-rebase-fuzz-vs-offset-fix:
  doc: difference in location to apply is "offset", not "fuzz"

Merge branch 'tn/doc-commit-fix' into maint-2.46

Docfix.

* tn/doc-commit-fix:
  doc: remove dangling closing parenthesis

Merge branch 'pw/add-patch-with-suppress-blank-empty' into maint-2.46

"git add -p" by users with diff.suppressBlankEmpty set to true
failed to parse the patch that represents an unmodified empty line
with an empty line (not a line with a single space on it), which
has been corrected.

* pw/add-patch-with-suppress-blank-empty:
  add-patch: use normalize_marker() when recounting edited hunk
  add-patch: handle splitting hunks with diff.suppressBlankEmpty

Merge branch 'jt/doc-post-receive-hook-update' into maint-2.46

Doc update.

* jt/doc-post-receive-hook-update:
  doc: clarify post-receive hook behavior

Merge branch 'jc/how-to-maintain-updates' (early part) into maint-2.46

* 'jc/how-to-maintain-updates' (early part):
  howto-maintain: update daily tasks
  howto-maintain: cover a whole development cycle

Merge branch 'jc/doc-one-shot-export-with-shell-func' into maint-2.46

It has been documented that we avoid "VAR=VAL shell_func" and why.

* jc/doc-one-shot-export-with-shell-func:
  CodingGuidelines: document a shell that "fails" "VAR=VAL shell_func"

Merge branch 'jc/checkout-no-op-switch-errors' into maint-2.46

"git checkout --ours" (no other arguments) complained that the
option is incompatible with branch switching, which is technically
correct, but found confusing by some users.  It now says that the
user needs to give pathspec to specify what paths to checkout.

* jc/checkout-no-op-switch-errors:
  checkout: special case error messages during noop switching

The fifth batch

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

Merge branch 'xx/diff-tree-remerge-diff-fix'

"git rev-list ... | git diff-tree -p --remerge-diff --stdin" should
behave more or less like "git log -p --remerge-diff" but instead it
crashed, forgetting to prepare a temporary object store needed.

* xx/diff-tree-remerge-diff-fix:
  diff-tree: fix crash when used with --remerge-diff

Merge branch 'jc/refs-symref-referent'

The refs API has been taught to give symref target information to
the users of ref iterators, allowing for-each-ref and friends to
avoid an extra ref_resolve_* API call per a symbolic ref.

* jc/refs-symref-referent:
  ref-filter: populate symref from iterator
  refs: add referent to each_ref_fn
  refs: keep track of unresolved reference value in iterators

Merge branch 'ps/submodule-ref-format'

Support to specify ref backend for submodules has been enhanced.

* ps/submodule-ref-format:
  object: fix leaking packfiles when closing object store
  submodule: fix leaking seen submodule names
  submodule: fix leaking fetch tasks
  builtin/submodule: allow "add" to use different ref storage format
  refs: fix ref storage format for submodule ref stores
  builtin/clone: propagate ref storage format to submodules
  builtin/submodule: allow cloning with different ref storage format
  git-submodule.sh: break overly long command lines

Merge branch 'ag/t7004-modernize'

Coding style fixes to a test script.

* ag/t7004-modernize:
  t7004: make use of write_script
  t7004: use single quotes instead of double quotes
  t7004: begin the test body on the same line as test_expect_success
  t7004: description on the same line as test_expect_success
  t7004: do not prepare things outside test_expect_success
  t7004: use indented here-doc
  t7004: one command per line
  t7004: remove space after redirect operators

Merge branch 'ps/reftable-stack-compaction'

The code paths to compact multiple reftable files have been updated
to correctly deal with multiple compaction triggering at the same
time.

* ps/reftable-stack-compaction:
  reftable/stack: handle locked tables during auto-compaction
  reftable/stack: fix corruption on concurrent compaction
  reftable/stack: use lock_file when adding table to "tables.list"
  reftable/stack: do not die when fsyncing lock file files
  reftable/stack: simplify tracking of table locks
  reftable/stack: update stats on failed full compaction
  reftable/stack: test compaction with already-locked tables
  reftable/stack: extract function to setup stack with N tables
  reftable/stack: refactor function to gather table sizes

Merge branch 'es/doc-platform-support-policy'

A policy document that describes platform support levels and
expectation on platform stakeholders has been introduced.

* es/doc-platform-support-policy:
  Documentation: add platform support policy

Merge branch 'gt/unit-test-hashmap'

An existing test of hashmap API has been rewritten with the
unit-test framework.

* gt/unit-test-hashmap:
  t: port helper/test-hashmap.c to unit-tests/t-hashmap.c

Merge branch 'jc/t3206-test-when-finished-fix'

Test clean-up.

* jc/t3206-test-when-finished-fix:
  t3206: test_when_finished before dirtying operations, not after

Merge branch 'rs/t-example-simplify'

Unit test simplification.

* rs/t-example-simplify:
  t-example-decorate: remove test messages

Merge branch 'jc/safe-directory'

Follow-up on 2.45.1 regression fix.

* jc/safe-directory:
  safe.directory: setting safe.directory="." allows the "current" directory
  safe.directory: normalize the configured path
  safe.directory: normalize the checked path
  safe.directory: preliminary clean-up

The fourth batch

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

Merge branch 'tb/t7704-deflake'

A test that fails on an unusually slow machine was found, and made
less likely to cause trouble by lengthening the expiry value it
uses.

* tb/t7704-deflake:
  t/t7704-repack-cruft.sh: avoid failures during long-running tests

Merge branch 'jc/document-use-of-local'

Doc update.

* jc/document-use-of-local:
  doc: note that AT&T ksh does not work with our test suite

Merge branch 'rs/use-decimal-width'

Code clean-up.

* rs/use-decimal-width:
  log-tree: use decimal_width()

Merge branch 'ss/packed-ref-store-leakfix'

Leakfix.

* ss/packed-ref-store-leakfix:
  refs/files: prevent memory leak by freeing packed_ref_store

Merge branch 'cp/unit-test-reftable-tree'

A test in reftable library has been rewritten using the unit test
framework.

* cp/unit-test-reftable-tree:
  t-reftable-tree: improve the test for infix_walk()
  t-reftable-tree: add test for non-existent key
  t-reftable-tree: split test_tree() into two sub-test functions
  t: move reftable/tree_test.c to the unit testing framework
  reftable: remove unnecessary curly braces in reftable/tree.c

Merge branch 'kl/test-fixes'

A flakey test and incorrect calls to strtoX() functions have been
fixed.

* kl/test-fixes:
  t6421: fix test to work when repo dir contains d0
  set errno=0 before strtoX calls

Merge branch 'jc/reflog-expire-lookup-commit-fix'

"git reflog expire" failed to honor annotated tags when computing
reachable commits.

* jc/reflog-expire-lookup-commit-fix:
  Revert "reflog expire: don't use lookup_commit_reference_gently()"

Merge branch 'jr/ls-files-expand-literal-doc'

Docfix.

* jr/ls-files-expand-literal-doc:
  doc: fix hex code escapes in git-ls-files

Merge branch 'jc/leakfix-mailmap'

Leakfix.

* jc/leakfix-mailmap:
  mailmap: plug memory leak in read_mailmap_blob()

Merge branch 'jc/leakfix-hashfile'

Leakfix.

* jc/leakfix-hashfile:
  csum-file: introduce discard_hashfile()

Merge branch 'jc/patch-id'

The patch parser in "git patch-id" has been tightened to avoid
getting confused by lines that look like a patch header in the log
message.

* jc/patch-id:
  patch-id: tighten code to detect the patch header
  patch-id: rewrite code that detects the beginning of a patch
  patch-id: make get_one_patchid() more extensible
  patch-id: call flush_current_id() only when needed
  t4204: patch-id supports various input format

Merge branch 'ps/refs-wo-the-repository'

In the refs subsystem, implicit reliance of the_repository has been
eliminated; the repository associated with the ref store object is
used instead.

* ps/refs-wo-the-repository:
  refs/reftable: stop using `the_repository`
  refs/packed: stop using `the_repository`
  refs/files: stop using `the_repository`
  refs/files: stop using `the_repository` in `parse_loose_ref_contents()`
  refs: stop using `the_repository`

Merge branch 'jc/jl-git-no-advice-fix'

Remove leftover debugging cruft from a test script.

* jc/jl-git-no-advice-fix:
  t0018: remove leftover debugging cruft

Merge branch 'tb/config-fixed-value-with-valueless-true'

"git config --value=foo --fixed-value section.key newvalue" barfed
when the existing value in the configuration file used the
valueless true syntax, which has been corrected.

* tb/config-fixed-value-with-valueless-true:
  config.c: avoid segfault with --fixed-value and valueless config

Merge branch 'jk/apply-patch-mode-check-fix'

The patch parser in 'git apply' has been a bit more lenient against
unexpected mode bits, like 100664, recorded on extended header lines.

* jk/apply-patch-mode-check-fix:
  apply: canonicalize modes read from patches

Merge branch 'ps/ref-api-cleanup'

Code clean-up.

* ps/ref-api-cleanup:
  refs: drop `ref_store`-less functions

Merge branch 'ps/ls-remote-out-of-repo-fix'

A recent update broke "git ls-remote" used outside a repository,
which has been corrected.

* ps/ls-remote-out-of-repo-fix:
  builtin/ls-remote: fall back to SHA1 outside of a repo

Merge branch 'jc/transport-leakfix'

Leakfix.

* jc/transport-leakfix:
  transport: fix leak with transport helper URLs

Merge branch 'rh/http-proxy-path'

The value of http.proxy can have "path" at the end for a socks
proxy that listens to a unix-domain socket, but we started to
discard it when we taught proxy auth code path to use the
credential helpers, which has been corrected.

* rh/http-proxy-path:
  http: do not ignore proxy path

Merge branch 'cp/unit-test-reftable-pq'

The tests for "pq" part of reftable library got rewritten to use
the unit test framework.

* cp/unit-test-reftable-pq:
  t-reftable-pq: add tests for merged_iter_pqueue_top()
  t-reftable-pq: add test for index based comparison
  t-reftable-pq: make merged_iter_pqueue_check() callable by reference
  t-reftable-pq: make merged_iter_pqueue_check() static
  t: move reftable/pq_test.c to the unit testing framework
  reftable: change the type of array indices to 'size_t' in reftable/pq.c
  reftable: remove unnecessary curly braces in reftable/pq.c

Merge branch 'jk/osxkeychain-username-is-nul-terminated'

The credential helper to talk to OSX keychain sometimes sent
garbage bytes after the username, which has been corrected.

* jk/osxkeychain-username-is-nul-terminated:
  credential/osxkeychain: respect NUL terminator in username

Merge branch 'ps/leakfixes-part-3'

More leakfixes.

* ps/leakfixes-part-3: (24 commits)
  commit-reach: fix trivial memory leak when computing reachability
  convert: fix leaking config strings
  entry: fix leaking pathnames during delayed checkout
  object-name: fix leaking commit list items
  t/test-repository: fix leaking repository
  builtin/credential-cache: fix trivial leaks
  builtin/worktree: fix leaking derived branch names
  builtin/shortlog: fix various trivial memory leaks
  builtin/rerere: fix various trivial memory leaks
  builtin/credential-store: fix leaking credential
  builtin/show-branch: fix several memory leaks
  builtin/rev-parse: fix memory leak with `--parseopt`
  builtin/stash: fix various trivial memory leaks
  builtin/remote: fix various trivial memory leaks
  builtin/remote: fix leaking strings in `branch_list`
  builtin/ls-remote: fix leaking `pattern` strings
  builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`
  builtin/submodule--helper: fix leaking clone depth parameter
  builtin/name-rev: fix various trivial memory leaks
  builtin/describe: fix trivial memory leak when describing blob
  ...

midx: drop unused parameters from add_midx_to_chain()

When loading a chained midx, we build up an array of hashes, one per
layer of the chain. But since the chain is also represented by the
linked list of multi_pack_index structs, nobody actually reads this
array. We pass it to add_midx_to_chain(), but the parameters are
completely ignored.

So we can drop those unused parameters. And then we can see that its
sole caller, load_midx_chain_fd_st(), only cares about one layer hash at a
time (for parsing each line and feeding it to the single-layer midx
code). So we can replace the array with a single object_id on the stack.

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

ref-filter: populate symref from iterator

With a previous commit, the reference the symbolic ref points to is saved
in the ref iterator records. Instead of making a separate call to
resolve_refdup() each time, we can just populate the ref_array_item with
the value from the iterator.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

refs: add referent to each_ref_fn

Add a parameter to each_ref_fn so that callers to the ref APIs
that use this function as a callback can have acess to the
unresolved value of a symbolic ref.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

refs: keep track of unresolved reference value in iterators

Since ref iterators do not hold onto the direct value of a reference
without resolving it, the only way to get ahold of a direct value of a
symbolic ref is to make a separate call to refs_read_symbolic_ref.

To make accessing the direct value of a symbolic ref more efficient,
let's save the direct value of the ref in the iterators for both the
files backend and the reftable backend.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff-tree: fix crash when used with --remerge-diff

When using "git-diff-tree" to get the tree diff for merge commits with
the diff format set to `remerge`, a bug is triggered as shown below:

  $ git diff-tree -r --remerge-diff 363337e6eb
  363337e6eb812d0c0d785ed4261544f35559ff8b
  BUG: log-tree.c:1006: did a remerge diff without remerge_objdir?!?

This bug is reported by `log-tree.c:do_remerge_diff`, where a bug check
added in commit 7b90ab467a (log: clean unneeded objects during log
--remerge-diff, 2022-02-02) detects the absence of `remerge_objdir` when
attempting to clean up temporary objects generated during the remerge
process.

After some further digging, I find that the remerge-related diff options
were introduced in db757e8b8d (show, log: provide a --remerge-diff
capability, 2022-02-02), which also affect the setup of `rev_info` for
"git-diff-tree", but were not accounted for in the original
implementation (inferred from the commit message).

Elijah Newren, the author of the remerge diff feature, notes that other
callers of `log-tree.c:log_tree_commit` (the only caller of
`log-tree.c:do_remerge_diff`) also exist, but:

  `builtin/am.c`: manually sets all flags; remerge_diff is not among them
  `sequencer.c`: manually sets all flags; remerge_diff is not among them

so `builtin/diff-tree.c` really is the only caller that was overlooked
when remerge-diff functionality was added.

This commit resolves the crash by adding `remerge_objdir` setup logic to
`builtin/diff-tree.c`, mirroring `builtin/log.c:cmd_log_walk_no_free`.
It also includes the necessary cleanup for `remerge_objdir`.

Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: drop use of 'tee' that hides exit status

A few tests have "| tee output" downstream of a git command, and
then inspect the contents of the file.  The net effect is that we
use an extra process, and hide the exit status from the upstream git
command.

In any of these tests, I do not see a reason why we want to hide a
possible failure from these git commands.  Replace the use of tee
with a plain simple redirection.

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

The third batch

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

Merge branch 'ps/p4-tests-updates'

Perforce tests have been updated.

* ps/p4-tests-updates:
  t98xx: mark Perforce tests as memory-leak free
  ci: update Perforce version to r23.2
  t98xx: fix Perforce tests with p4d r23 and newer

Merge branch 'dh/encoding-trace-optim'

An expensive operation to prepare tracing was done in re-encoding
code path even when the tracing was not requested, which has been
corrected.

* dh/encoding-trace-optim:
  convert: return early when not tracing

Merge branch 'ps/doc-more-c-coding-guidelines'

Some project conventions have been added to CodingGuidelines.

* ps/doc-more-c-coding-guidelines:
  Documentation: consistently use spaces inside initializers
  Documentation: document idiomatic function names
  Documentation: document naming schema for structs and their functions
  Documentation: clarify indentation style for C preprocessor directives
  clang-format: fix indentation width for preprocessor directives

Merge branch 'rs/grep-omit-blank-lines-after-function-at-eof'

"git grep -W" omits blank lines that follow the found function at
the end of the file, just like it omits blank lines before the next
function.

* rs/grep-omit-blank-lines-after-function-at-eof:
  grep: -W: skip trailing empty lines at EOF, too

Merge branch 'dd/notes-empty-no-edit-by-default'

"git notes add -m '' --allow-empty" and friends that take prepared
data to create notes should not invoke an editor, but it started
doing so since Git 2.42, which has been corrected.

* dd/notes-empty-no-edit-by-default:
  notes: do not trigger editor when adding an empty note

Merge branch 'es/shell-check-updates'

Test script linter has been updated to catch an attempt to use
one-shot export construct "VAR=VAL func" for shell functions (which
does not work for some shells) better.

* es/shell-check-updates:
  check-non-portable-shell: improve `VAR=val shell-func` detection
  check-non-portable-shell: suggest alternative for `VAR=val shell-func`
  check-non-portable-shell: loosen one-shot assignment error message
  t4034: fix use of one-shot variable assignment with shell function
  t3430: drop unnecessary one-shot "VAR=val shell-func" invocation

Merge branch 'rj/add-p-pager'

A 'P' command to "git add -p" that passes the patch hunk to the
pager has been added.

* rj/add-p-pager:
  add-patch: render hunks through the pager
  pager: introduce wait_for_pager
  pager: do not close fd 2 unnecessarily
  add-patch: test for 'p' command

Merge branch 'ks/unit-test-comment-typofix'

Typofix.

* ks/unit-test-comment-typofix:
  unit-tests/test-lib: fix typo in check_pointer_eq() description

t7004: make use of write_script

Use write_script which takes care of emitting the `#!/bin/sh` line
and the `chmod +x`.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: use single quotes instead of double quotes

Some test bodies and test description are surrounded with double
quotes instead of single quotes, violating our coding style.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: begin the test body on the same line as test_expect_success

Test body should begin with a single quote right after the test
description instead of backslash followed by new line.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: description on the same line as test_expect_success

There are several tests in t7004 where the test description that
follows `test_expect_success` is on a separate line, violating our
coding style. Adapt these to be on the same line.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: do not prepare things outside test_expect_success

Do not prepare expect and other things outside test_expect_success.
If such code fails for some reason, we won't necessarily hear about
it in a timely fashion (or perhaps at all). By placing all code inside
`test_expect_success` it ensures that we know immediately if it fails.

Also add '\' before EOF to avoid shell interpolation and '-' to allow
indentation of the body.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: use indented here-doc

Use <<-\EOF instead of <<\EOF where the latter allows us to indent
the body of the here-doc.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: one command per line

One of the tests in t7004 has multiple commands on a single line,
which is discouraged. Adapt these by splitting up these into one
line per command.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7004: remove space after redirect operators

Modernize 't7004' by removing whitespace after redirect operators.

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: handle locked tables during auto-compaction

When compacting tables, it may happen that we want to compact a set of
tables which are already locked by a concurrent process that compacts
them. In the case where we wanted to perform a full compaction of all
tables it is sensible to bail out in this case, as we cannot fulfill the
requested action.

But when performing auto-compaction it isn't necessarily in our best
interest of us to abort the whole operation. For example, due to the
geometric compacting schema that we use, it may be that process A takes
a lot of time to compact the bulk of all tables whereas process B
appends a bunch of new tables to the stack. B would in this case also
notice that it has to compact the tables that process A is compacting
already and thus also try to compact the same range, probably including
the new tables it has appended. But because those tables are locked
already, it will fail and thus abort the complete auto-compaction. The
consequence is that the stack will grow longer and longer while A isn't
yet done with compaction, which will lead to a growing performance
impact.

Instead of aborting auto-compaction altogether, let's gracefully handle
this situation by instead compacting tables which aren't locked. To do
so, instead of locking from the beginning of the slice-to-be-compacted,
we start locking tables from the end of the slice. Once we hit the first
table that is locked already, we abort. If we succeeded to lock two or
more tables, then we simply reduce the slice of tables that we're about
to compact to those which we managed to lock.

This ensures that we can at least make some progress for compaction in
said scenario. It also helps in other scenarios, like for example when a
process died and left a stale lockfile behind. In such a case we can at
least ensure some compaction on a best-effort basis.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: fix corruption on concurrent compaction

The locking employed by compaction uses the following schema:

  1. Lock "tables.list" and verify that it matches the version we have
     loaded in core.

  2. Lock each of the tables in the user-supplied range of tables that
     we are supposed to compact. These locks prohibit any concurrent
     process to compact those tables while we are doing that.

  3. Unlock "tables.list". This enables concurrent processes to add new
     tables to the stack, but also allows them to compact tables outside
     of the range of tables that we have locked.

  4. Perform the compaction.

  5. Lock "tables.list" again.

  6. Move the compacted table into place.

  7. Write the new order of tables, including the compacted table, into
     the lockfile.

  8. Commit the lockfile into place.

Letting concurrent processes modify the "tables.list" file while we are
doing the compaction is very much part of the design and thus expected.
After all, it may take some time to compact tables in the case where we
are compacting a lot of very large tables.

But there is a bug in the code. Suppose we have two processes which are
compacting two slices of the table. Given that we lock each of the
tables before compacting them, we know that the slices must be disjunct
from each other. But regardless of that, compaction performed by one
process will always impact what the other process needs to write to the
"tables.list" file.

Right now, we do not check whether the "tables.list" has been changed
after we have locked it for the second time in (5). This has the
consequence that we will always commit the old, cached in-core tables to
disk without paying to respect what the other process has written. This
scenario would then lead to data loss and corruption.

This can even happen in the simpler case of one compacting process and
one writing process. The newly-appended table by the writing process
would get discarded by the compacting process because it never sees the
new table.

Fix this bug by re-checking whether our stack is still up to date after
locking for the second time. If it isn't, then we adjust the indices of
tables to replace in the updated stack.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: use lock_file when adding table to "tables.list"

When modifying "tables.list", we need to lock the list before updating
it to ensure that no concurrent writers modify the list at the same
point in time. While we do this via the `lock_file` subsystem when
compacting the stack, we manually handle the lock when adding a new
table to it. While not wrong, it is at least inconsistent.

Refactor the code to consistently lock "tables.list" via the `lock_file`
subsytem.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: do not die when fsyncing lock file files

We use `fsync_component_or_die()` when committing an addition to the
"tables.list" lock file, which unsurprisingly dies in case the fsync
fails. Given that this is part of the reftable library, we should never
die and instead let callers handle the error.

Adapt accordingly and use `fsync_component()` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: simplify tracking of table locks

When compacting tables, we store the locks of all tables we are about to
compact in the `table_locks` array. As we currently only ever compact
all tables in the user-provided range or none, we simply track those
locks via the indices of the respective tables in the merged stack.

This is about to change though, as we will introduce a mode where auto
compaction gracefully handles the case of already-locked files. In this
case, it may happen that we only compact a subset of the user-supplied
range of tables. In this case, the indices will not necessarily match
the lock indices anymore.

Refactor the code such that we track the number of locks via a separate
variable. The resulting code is expected to perform the same, but will
make it easier to perform the described change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: update stats on failed full compaction

When auto-compaction fails due to a locking error, we update the
statistics to indicate this failure. We're not doing the same when
performing a full compaction.

Fix this inconsistency by using `stack_compact_range_stats()`, which
handles the stat update for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reftable/stack: test compaction with already-locked tables

We're lacking test coverage for compacting tables when some of the
tables that we are about to compact are locked. Add two tests that
exercise this, one for auto-compaction and one for full compaction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>