Git development
 help / color / mirror / Atom feed
* Re: What's the recommendation for forgetting all rerere's records?
From: Junio C Hamano @ 2023-12-08 23:19 UTC (permalink / raw)
  To: Sean Allred; +Cc: git
In-Reply-To: <m0y1e7kkft.fsf@epic96565.epic.com>

Sean Allred <allred.sean@gmail.com> writes:

> When outside the context of a conflict (no rebase/merge in progress),
> what's the intended workflow to clear out the contents of
> $GIT_DIR/rr-cache?

You could "rm -fr .git/rr-cache/??*" if you want.

The "intended" workflow is there will no need to "clear out" at all;
you may notice mistaken resolution, and you will remove it when you
notice one, but the bulk of the remaining database entries ought to
be still correct.

^ permalink raw reply

* Re: [PATCH v2 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
From: Eric Sunshine @ 2023-12-08 23:46 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Patrick Steinhardt, git, Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <ZXOML2pcqVnVo0oX@nand.local>

On Fri, Dec 8, 2023 at 4:35 PM Taylor Blau <me@ttaylorr.com> wrote:
> On Fri, Dec 08, 2023 at 03:53:10PM +0100, Patrick Steinhardt wrote:
> > +static void test_reftable_stack_add_performs_auto_compaction(void)
> > +{
> > +             char name[100];
> > +             snprintf(name, sizeof(name), "branch%04d", i);
> > +             ref.refname = name;
>
> Is there a reason that we have to use snprintf() here and not a strbuf?
>
> I would have expected to see something like:
>
>     struct strbuf buf = STRBUF_INIT;
>     /* ... */
>     strbuf_addf(&buf, "branch%04d", i);
>     ref.refname = strbuf_detach(&buf, NULL);

If I'm reading the code correctly, this use of strbuf would leak each
time through the loop.

> I guess it doesn't matter too much, but I think if we can avoid using
> snprintf(), it's worth doing. If we must use snprintf() here, we should
> probably use Git's xsnprintf() instead.

xstrfmt() from strbuf.h would be even simpler if the intention is to
allocate a new string which will be freed later.

In this case, though, assuming I understand the intent, I think the
more common and safe idiom in this codebase is something like this:

    struct strbuf name = STRBUF_INIT;
    strbuf_addstr(&name, "branch");
    size_t len = name.len;
    for (...) {
        strbuf_setlen(&name, len);
        strbuf_addf(&name, "%04d", i);
        ref.refname = name.buf;
        ...
    }
    strbuf_release(&name);

^ permalink raw reply

* Re: [PATCH 05/24] midx: implement `DISP` chunk
From: Junio C Hamano @ 2023-12-09  1:40 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Patrick Steinhardt
In-Reply-To: <ZW95WSErCXvkfrAG@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

> Hopefully you're satisfied with the way things are split up and
> organized currently, but if you have suggestions on other ways I could
> slice or dice this, please let me know.

I did wonder how expensive to recompute and validate the "distinct"
information (in other words, is it too expensive for the consumers
of an existing midx file to see which packs are distinct on demand
before they stream contents out of the underlying packs?), as the
way the packs are marked as distinct looked rather error prone (you
can very easily list packfiles with overlaps with "+" prefix and the
DISK chunk writer does not even notice that you lied to it).  As long
as "git fsck" catches when two packs that are marked as distinct share
an object, that is OK, but the arrangement did look rather brittle
to me.

^ permalink raw reply

* What's cooking in git.git (Dec 2023, #01; Sat, 9)
From: Junio C Hamano @ 2023-12-09  2:02 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking in my tree.  Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release).  Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive.  A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).

Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors.  Some
repositories have only a subset of branches.

With maint, master, next, seen, todo:

	git://git.kernel.org/pub/scm/git/git.git/
	git://repo.or.cz/alt-git.git/
	https://kernel.googlesource.com/pub/scm/git/git/
	https://github.com/git/git/
	https://gitlab.com/git-vcs/git/

With all the integration branches and topics broken out:

	https://github.com/gitster/git/

Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):

	git://git.kernel.org/pub/scm/git/git-htmldocs.git/
	https://github.com/gitster/git-htmldocs.git/

Release tarballs are available at:

	https://www.kernel.org/pub/software/scm/git/

--------------------------------------------------
[New Topics]

* jp/use-diff-index-in-pre-commit-sample (2023-12-03) 1 commit
 - hooks--pre-commit: detect non-ASCII when renaming

 The sample pre-commit hook that tries to catch introduction of new
 paths that use potentially non-portable characters did not notice
 an existing path getting renamed to such a problematic path, when
 rename detection was enabled.

 Will merge to 'next'.
 source: <pull.1291.v2.git.git.1701360836307.gitgitgadget@gmail.com>


* mk/doc-gitfile-more (2023-12-03) 1 commit
 - doc: make the gitfile syntax easier to discover

 Doc update.

 Will merge to 'next'.
 source: <20231128065558.1061206-1-mk+copyleft@pimpmybyte.de>


* ps/ref-tests-update-more (2023-12-03) 10 commits
 - t6301: write invalid object ID via `test-tool ref-store`
 - t5551: stop writing packed-refs directly
 - t5401: speed up creation of many branches
 - t4013: simplify magic parsing and drop "failure"
 - t3310: stop checking for reference existence via `test -f`
 - t1417: make `reflog --updateref` tests backend agnostic
 - t1410: use test-tool to create empty reflog
 - t1401: stop treating FETCH_HEAD as real reference
 - t1400: split up generic reflog tests from the reffile-specific ones
 - t0410: mark tests to require the reffiles backend

 Tests update.

 Will merge to 'next'.
 source: <cover.1701242407.git.ps@pks.im>


* sh/completion-with-reftable (2023-12-03) 2 commits
 - completion: stop checking for reference existence via `test -f`
 - completion: refactor existence checks for special refs

 Command line completion script (in contrib/) learned to work better
 with the reftable backend.

 Expecting a reroll.
 source: <20231130202404.89791-1-stanhu@gmail.com>


* en/header-cleanup (2023-12-03) 12 commits
 - treewide: remove unnecessary includes in source files
 - treewide: add direct includes currently only pulled in transitively
 - trace2/tr2_tls.h: remove unnecessary include
 - submodule-config.h: remove unnecessary include
 - pkt-line.h: remove unnecessary include
 - line-log.h: remove unnecessary include
 - http.h: remove unnecessary include
 - fsmonitor--daemon.h: remove unnecessary includes
 - blame.h: remove unnecessary includes
 - archive.h: remove unnecessary include
 - treewide: remove unnecessary includes in source files
 - treewide: remove unnecessary includes from header files

 Remove unused header "#include".

 Has a few interactions with topics in flight.
 source: <pull.1617.git.1701585682.gitgitgadget@gmail.com>


* jc/revision-parse-int (2023-12-09) 1 commit
 - revision: parse integer arguments to --max-count, --skip, etc., more carefully

 The command line parser for the "log" family of commands was too
 loose when parsing certain numbers, e.g., silently ignoring the
 extra 'q' in "git log -n 1q" without complaining, which has been
 tightened up.

 Will merge to 'next'.
 source: <xmqq5y181fx0.fsf_-_@gitster.g>


* jk/bisect-reset-fix (2023-12-09) 1 commit
 - bisect: always clean on reset

 "git bisect reset" has been taught to clean up state files and refs
 even when BISECT_START file is gone.

 Will merge to 'next'.
 source: <20231207065341.GA778781@coredump.intra.peff.net>


* jk/implicit-true (2023-12-09) 7 commits
 - fsck: handle NULL value when parsing message config
 - trailer: handle NULL value when parsing trailer-specific config
 - submodule: handle NULL value when parsing submodule.*.branch
 - help: handle NULL value for alias.* config
 - trace2: handle NULL values in tr2_sysenv config callback
 - setup: handle NULL value when parsing extensions
 - config: handle NULL value when parsing non-bools
 (this branch is used by jk/config-cleanup.)

 Some codepaths did not correctly parse configuration variables
 specified with valueless "true", which has been corrected.

 Will merge to 'next'.
 source: <20231207071030.GA1275835@coredump.intra.peff.net>


* jk/config-cleanup (2023-12-09) 9 commits
 - sequencer: simplify away extra git_config_string() call
 - gpg-interface: drop pointless config_error_nonbool() checks
 - push: drop confusing configset/callback redundancy
 - config: use git_config_string() for core.checkRoundTripEncoding
 - diff: give more detailed messages for bogus diff.* config
 - config: use config_error_nonbool() instead of custom messages
 - imap-send: don't use git_die_config() inside callback
 - git_xmerge_config(): prefer error() to die()
 - config: reject bogus values for core.checkstat
 (this branch uses jk/implicit-true.)

 Code clean-up around use of configuration variables.

 Will merge to 'next'.
 source: <20231207071030.GA1275835@coredump.intra.peff.net>
 source: <20231207072338.GA1277727@coredump.intra.peff.net>


* jk/end-of-options (2023-12-09) 1 commit
 - parse-options: decouple "--end-of-options" and "--"

 "git log --end-of-options --rev -- --path" learned to interpret
 "--rev" as a rev, and "--path" as a path, as expected.

 Will merge to 'next'.
 source: <20231206222145.GA136253@coredump.intra.peff.net>


* ps/clone-into-reftable-repository (2023-12-09) 7 commits
 - builtin/clone: create the refdb with the correct object format
 - builtin/clone: skip reading HEAD when retrieving remote
 - builtin/clone: set up sparse checkout later
 - builtin/clone: fix bundle URIs with mismatching object formats
 - remote-curl: rediscover repository when fetching refs
 - setup: allow skipping creation of the refdb
 - setup: extract function to create the refdb

 "git clone" has been prepared to allow cloning a repository with
 non-default hash function into a repository that uses the reftable
 backend.

 Will merge to 'next'?
 source: <cover.1701863960.git.ps@pks.im>


* rs/incompatible-options-messages (2023-12-09) 7 commits
 - worktree: simplify incompatibility message for --orphan and commit-ish
 - worktree: standardize incompatibility messages
 - clean: factorize incompatibility message
 - revision, rev-parse: factorize incompatibility messages about - -exclude-hidden
 - revision: use die_for_incompatible_opt3() for - -graph/--reverse/--walk-reflogs
 - repack: use die_for_incompatible_opt3() for -A/-k/--cruft
 - push: use die_for_incompatible_opt4() for - -delete/--tags/--all/--mirror

 Clean-up code that handles combinations of incompatible options.

 Will merge to 'next'.
 source: <20231206115215.94467-1-l.s.r@web.de>

--------------------------------------------------
[Stalled]

* pw/rebase-sigint (2023-09-07) 1 commit
 - rebase -i: ignore signals when forking subprocesses

 If the commit log editor or other external programs (spawned via
 "exec" insn in the todo list) receive internactive signal during
 "git rebase -i", it caused not just the spawned program but the
 "Git" process that spawned them, which is often not what the end
 user intended.  "git" learned to ignore SIGINT and SIGQUIT while
 waiting for these subprocesses.

 Expecting a reroll.
 cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
 source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>


* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
 - cherry-pick: refuse cherry-pick sequence if index is dirty

 "git cherry-pick A" that replays a single commit stopped before
 clobbering local modification, but "git cherry-pick A..B" did not,
 which has been corrected.

 Expecting a reroll.
 cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
 source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>


* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
 - diff-lib: fix check_removed() when fsmonitor is active
 - Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
 - Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
 (this branch uses jc/fake-lstat.)

 The optimization based on fsmonitor in the "diff --cached"
 codepath is resurrected with the "fake-lstat" introduced earlier.

 It is unknown if the optimization is worth resurrecting, but in case...
 source: <xmqqr0n0h0tw.fsf@gitster.g>

--------------------------------------------------
[Cooking]

* ad/merge-file-diff-algo (2023-11-22) 1 commit
 - merge-file: add --diff-algorithm option

 "git merge-file" learned to take the "--diff-algorithm" option to
 use algorithm different from the default "myers" diff.

 Will merge to 'next'.
 source: <pull.1606.v2.git.git.1700507932937.gitgitgadget@gmail.com>


* ak/p4-initial-empty-commits (2023-11-23) 1 commit
 - git-p4: fix fast import when empty commit is first

 Expecting a reroll.
 source: <pull.1609.git.git.1700639764041.gitgitgadget@gmail.com>


* jc/checkout-B-branch-in-use (2023-12-09) 3 commits
 - fixup! checkout: forbid "-B <branch>" from touching a branch used elsewhere
 - checkout: forbid "-B <branch>" from touching a branch used elsewhere
 - checkout: refactor die_if_checked_out() caller

 "git checkout -B <branch> [<start-point>]" allowed a branch that is
 in use in another worktree to be updated and checked out, which
 might be a bit unexpected.  The rule has been tightened, which is a
 breaking change.  "--ignore-other-worktrees" option is required to
 unbreak you, if you are used to the current behaviour that "-B"
 overrides the safety.

 Needs review.
 source: <xmqqjzq9cl70.fsf@gitster.g>


* jh/trace2-redact-auth (2023-11-23) 4 commits
 - t0212: test URL redacting in EVENT format
 - t0211: test URL redacting in PERF format
 - trace2: redact passwords from https:// URLs by default
 - trace2: fix signature of trace2_def_param() macro

 trace2 streams used to record the URLs that potentially embed
 authentication material, which has been corrected.

 Will merge to 'next'.
 source: <pull.1616.git.1700680717.gitgitgadget@gmail.com>


* ps/commit-graph-less-paranoid (2023-11-26) 1 commit
 - commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default

 Earlier we stopped relying on commit-graph that (still) records
 information about commits that are lost from the object store,
 which has negative performance implications.  The default has been
 flipped to disable this pessimization.

 Will merge to 'next'.
 source: <17e08289cd59d20de0de9b4e18f5e6bf77987351.1700823746.git.ps@pks.im>


* ps/reftable-fixes (2023-11-22) 8 commits
 - reftable/stack: fix stale lock when dying
 - reftable/merged: reuse buffer to compute record keys
 - reftable/stack: reuse buffers when reloading stack
 - reftable/stack: perform auto-compaction with transactional interface
 - reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
 - reftable: handle interrupted writes
 - reftable: handle interrupted reads
 - reftable: wrap EXPECT macros in do/while

 Bunch of small fix-ups to the reftable code.

 Needs review.
 source: <cover.1700549493.git.ps@pks.im>


* en/complete-sparse-checkout (2023-12-03) 4 commits
 - completion: avoid user confusion in non-cone mode
 - completion: avoid misleading completions in cone mode
 - completion: fix logic for determining whether cone mode is active
 - completion: squelch stray errors in sparse-checkout completion

 Command line completion (in contrib/) learned to complete path
 arguments to the "add/set" subcommands of "git sparse-checkout"
 better.

 Will merge to 'next'.
 source: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>


* jb/reflog-expire-delete-dry-run-options (2023-11-26) 1 commit
 - builtin/reflog.c: fix dry-run option short name

 Command line parsing fix for "git reflog".

 Will merge to 'next'.
 source: <20231126000514.85509-1-josh@brob.st>


* jc/orphan-unborn (2023-11-24) 2 commits
 - orphan/unborn: fix use of 'orphan' in end-user facing messages
 - orphan/unborn: add to the glossary and use them consistently

 Doc updates to clarify what an "unborn branch" means.

 Comments?
 source: <xmqq4jhb977x.fsf@gitster.g>


* rs/column-leakfix (2023-11-27) 1 commit
 - column: release strbuf and string_list after use

 Leakfix.

 Will merge to 'next'.
 source: <f087137d-a5aa-487e-a1cb-0ad7117b38ed@web.de>


* rs/i18n-cannot-be-used-together (2023-11-27) 1 commit
 - i18n: factorize even more 'incompatible options' messages

 Clean-up code that handles combinations of incompatible options.

 Will merge to 'next'.
 source: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>


* ac/fuzz-show-date (2023-11-20) 1 commit
 - fuzz: add new oss-fuzz fuzzer for date.c / date.h

 Subject approxidate() and show_date() machinery to OSS-Fuzz.

 Will merge to 'next'.
 source: <pull.1612.v4.git.1700243267653.gitgitgadget@gmail.com>


* js/packfile-h-typofix (2023-11-20) 1 commit
 - packfile.c: fix a typo in `each_file_in_pack_dir_fn()`'s declaration

 Typofix.

 Will merge to 'next'.
 source: <pull.1614.git.1700226915859.gitgitgadget@gmail.com>


* jw/builtin-objectmode-attr (2023-11-16) 2 commits
 - SQUASH???
 - attr: add builtin objectmode values support

 The builtin_objectmode attribute is populated for each path
 without adding anything in .gitattributes files, which would be
 useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
 to limit to executables.

 Will merge to 'next' after squashing the fix-up in?
 source: <20231116054437.2343549-1-jojwang@google.com>


* ps/ref-deletion-updates (2023-11-17) 4 commits
 - refs: remove `delete_refs` callback from backends
 - refs: deduplicate code to delete references
 - refs/files: use transactions to delete references
 - t5510: ensure that the packed-refs file needs locking

 Simplify API implementation to delete references by eliminating
 duplication.

 Will merge to 'next'.
 source: <cover.1699951815.git.ps@pks.im>


* tz/send-email-negatable-options (2023-11-17) 2 commits
  (merged to 'next' on 2023-11-17 at f09e533e43)
 + send-email: avoid duplicate specification warnings
 + perl: bump the required Perl version to 5.8.1 from 5.8.0

 Newer versions of Getopt::Long started giving warnings against our
 (ab)use of it in "git send-email".  Bump the minimum version
 requirement for Perl to 5.8.1 (from September 2002) to allow
 simplifying our implementation.

 Will merge to 'master'.
 source: <20231116193014.470420-1-tmz@pobox.com>


* js/ci-discard-prove-state (2023-11-14) 1 commit
  (merged to 'next' on 2023-11-14 at fade3ba143)
 + ci: avoid running the test suite _twice_
 (this branch uses ps/ci-gitlab.)

 The way CI testing used "prove" could lead to running the test
 suite twice needlessly, which has been corrected.

 Will merge to 'master'.
 source: <pull.1613.git.1699894837844.gitgitgadget@gmail.com>


* jk/chunk-bounds-more (2023-11-09) 9 commits
  (merged to 'next' on 2023-11-13 at 3df4b18bea)
 + commit-graph: mark chunk error messages for translation
 + commit-graph: drop verify_commit_graph_lite()
 + commit-graph: check order while reading fanout chunk
 + commit-graph: use fanout value for graph size
 + commit-graph: abort as soon as we see a bogus chunk
 + commit-graph: clarify missing-chunk error messages
 + commit-graph: drop redundant call to "lite" verification
 + midx: check consistency of fanout table
 + commit-graph: handle overflow in chunk_size checks
 (this branch is used by tb/pair-chunk-expect.)

 Code clean-up for jk/chunk-bounds topic.

 Will merge to 'master'.
 source: <20231109070310.GA2697602@coredump.intra.peff.net>


* ps/httpd-tests-on-nixos (2023-11-11) 3 commits
  (merged to 'next' on 2023-11-13 at 81bd6f5334)
 + t9164: fix inability to find basename(1) in Subversion hooks
 + t/lib-httpd: stop using legacy crypt(3) for authentication
 + t/lib-httpd: dynamically detect httpd and modules path

 Portability tweak.

 Will merge to 'master'.
 source: <cover.1699596457.git.ps@pks.im>


* ss/format-patch-use-encode-headers-for-cover-letter (2023-11-10) 1 commit
  (merged to 'next' on 2023-11-14 at 1a4bd59e15)
 + format-patch: fix ignored encode_email_headers for cover letter

 "git format-patch --encode-email-headers" ignored the option when
 preparing the cover letter, which has been corrected.

 Will merge to 'master'.
 source: <20231109111950.387219-1-contact@emersion.fr>


* ps/ban-a-or-o-operator-with-test (2023-11-11) 4 commits
  (merged to 'next' on 2023-11-14 at d84471baab)
 + Makefile: stop using `test -o` when unlinking duplicate executables
 + contrib/subtree: convert subtree type check to use case statement
 + contrib/subtree: stop using `-o` to test for number of args
 + global: convert trivial usages of `test <expr> -a/-o <expr>`

 Test and shell scripts clean-up.

 Will merge to 'master'.
 source: <cover.1699609940.git.ps@pks.im>


* ak/rebase-autosquash (2023-11-16) 3 commits
  (merged to 'next' on 2023-11-17 at 3ed6e79445)
 + rebase: rewrite --(no-)autosquash documentation
 + rebase: support --autosquash without -i
 + rebase: fully ignore rebase.autoSquash without -i

 "git rebase --autosquash" is now enabled for non-interactive rebase,
 but it is still incompatible with the apply backend.

 Will merge to 'master'.
 source: <20231114214339.10925-1-andy.koppe@gmail.com>


* vd/for-each-ref-unsorted-optimization (2023-11-16) 10 commits
  (merged to 'next' on 2023-11-17 at ff99420bf6)
 + t/perf: add perf tests for for-each-ref
 + ref-filter.c: use peeled tag for '*' format fields
 + for-each-ref: clean up documentation of --format
 + ref-filter.c: filter & format refs in the same callback
 + ref-filter.c: refactor to create common helper functions
 + ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
 + ref-filter.h: add functions for filter/format & format-only
 + ref-filter.h: move contains caches into filter
 + ref-filter.h: add max_count and omit_empty to ref_format
 + ref-filter.c: really don't sort when using --no-sort

 "git for-each-ref --no-sort" still sorted the refs alphabetically
 which paid non-trivial cost.  It has been redefined to show output
 in an unspecified order, to allow certain optimizations to take
 advantage of.

 Will merge to 'master'.
 source: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>


* jw/git-add-attr-pathspec (2023-11-04) 1 commit
  (merged to 'next' on 2023-11-13 at b61be94e4d)
 + attr: enable attr pathspec magic for git-add and git-stash

 "git add" and "git stash" learned to support the ":(attr:...)"
 magic pathspec.

 Will merge to 'master'.
 source: <20231103163449.1578841-1-jojwang@google.com>


* ps/ci-gitlab (2023-11-09) 8 commits
  (merged to 'next' on 2023-11-10 at ea7ed67945)
 + ci: add support for GitLab CI
 + ci: install test dependencies for linux-musl
 + ci: squelch warnings when testing with unusable Git repo
 + ci: unify setup of some environment variables
 + ci: split out logic to set up failed test artifacts
 + ci: group installation of Docker dependencies
 + ci: make grouping setup more generic
 + ci: reorder definitions for grouping functions
 (this branch is used by js/ci-discard-prove-state.)

 Add support for GitLab CI.

 Will merge to 'master'.
 source: <cover.1699514143.git.ps@pks.im>


* ps/ref-tests-update (2023-11-03) 10 commits
  (merged to 'next' on 2023-11-13 at dc26e55d6f)
 + t: mark several tests that assume the files backend with REFFILES
 + t7900: assert the absence of refs via git-for-each-ref(1)
 + t7300: assert exact states of repo
 + t4207: delete replace references via git-update-ref(1)
 + t1450: convert tests to remove worktrees via git-worktree(1)
 + t: convert tests to not access reflog via the filesystem
 + t: convert tests to not access symrefs via the filesystem
 + t: convert tests to not write references via the filesystem
 + t: allow skipping expected object ID in `ref-store update-ref`
 + Merge branch 'ps/show-ref' into ps/ref-tests-update

 Update ref-related tests.

 Will merge to 'master'.
 source: <cover.1698914571.git.ps@pks.im>


* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
 - fetch: no redundant error message for atomic fetch
 - t5574: test porcelain output of atomic fetch

 "git fetch --atomic" issued an unnecessary empty error message,
 which has been corrected.

 Expecting an update.
 cf. <ZTjQIrCgSANAT8wR@tanuki>
 source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>


* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
 - bugreport: include +i in outfile suffix as needed

 Instead of auto-generating a filename that is already in use for
 output and fail the command, `git bugreport` learned to fuzz the
 filename to avoid collisions with existing files.

 Expecting a reroll.
 cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
 source: <20231016214045.146862-2-jacob@initialcommit.io>


* kh/t7900-cleanup (2023-10-17) 9 commits
 - t7900: fix register dependency
 - t7900: factor out packfile dependency
 - t7900: fix `print-args` dependency
 - t7900: fix `pfx` dependency
 - t7900: factor out common schedule setup
 - t7900: factor out inheritance test dependency
 - t7900: create commit so that branch is born
 - t7900: setup and tear down clones
 - t7900: remove register dependency

 Test clean-up.

 Perhaps discard?
 cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
 source: <cover.1697319294.git.code@khaugsbakk.name>


* tb/merge-tree-write-pack (2023-10-23) 5 commits
 - builtin/merge-tree.c: implement support for `--write-pack`
 - bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
 - bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
 - bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
 - bulk-checkin: extract abstract `bulk_checkin_source`

 "git merge-tree" learned "--write-pack" to record its result
 without creating loose objects.

 Broken when an object created during a merge is needed to continue merge
 cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
 source: <cover.1698101088.git.me@ttaylorr.com>


* tb/pair-chunk-expect (2023-11-10) 8 commits
 - midx: read `OOFF` chunk with `pair_chunk_expect()`
 - midx: read `OIDL` chunk with `pair_chunk_expect()`
 - commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
 - commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
 - commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
 - commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
 - chunk-format: introduce `pair_chunk_expect()` helper
 - Merge branch 'jk/chunk-bounds-more' into HEAD
 (this branch uses jk/chunk-bounds-more.)

 Further code clean-up.

 Needs review.
 source: <cover.1699569246.git.me@ttaylorr.com>


* tb/path-filter-fix (2023-10-18) 17 commits
 - bloom: introduce `deinit_bloom_filters()`
 - commit-graph: reuse existing Bloom filters where possible
 - object.h: fix mis-aligned flag bits table
 - commit-graph: drop unnecessary `graph_read_bloom_data_context`
 - commit-graph.c: unconditionally load Bloom filters
 - bloom: prepare to discard incompatible Bloom filters
 - bloom: annotate filters with hash version
 - commit-graph: new filter ver. that fixes murmur3
 - repo-settings: introduce commitgraph.changedPathsVersion
 - t4216: test changed path filters with high bit paths
 - t/helper/test-read-graph: implement `bloom-filters` mode
 - bloom.h: make `load_bloom_filter_from_graph()` public
 - t/helper/test-read-graph.c: extract `dump_graph_info()`
 - gitformat-commit-graph: describe version 2 of BDAT
 - commit-graph: ensure Bloom filters are read with consistent settings
 - revision.c: consult Bloom filters for root commits
 - t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`

 The Bloom filter used for path limited history traversal was broken
 on systems whose "char" is unsigned; update the implementation and
 bump the format version to 2.

 Needs (hopefully final and quick) review.
 source: <cover.1697653929.git.me@ttaylorr.com>


* cc/git-replay (2023-11-26) 14 commits
 - replay: stop assuming replayed branches do not diverge
 - replay: add --contained to rebase contained branches
 - replay: add --advance or 'cherry-pick' mode
 - replay: use standard revision ranges
 - replay: make it a minimal server side command
 - replay: remove HEAD related sanity check
 - replay: remove progress and info output
 - replay: add an important FIXME comment about gpg signing
 - replay: change rev walking options
 - replay: introduce pick_regular_commit()
 - replay: die() instead of failing assert()
 - replay: start using parse_options API
 - replay: introduce new builtin
 - t6429: remove switching aspects of fast-rebase

 Introduce "git replay", a tool meant on the server side without
 working tree to recreate a history.

 Will merge to 'next'.
 cf. <6bfe1541-54dd-ca6b-e930-94d3038060f1@gmx.de>
 source: <20231124111044.3426007-1-christian.couder@gmail.com>


* ak/color-decorate-symbols (2023-10-23) 7 commits
 - log: add color.decorate.pseudoref config variable
 - refs: exempt pseudorefs from pattern prefixing
 - refs: add pseudorefs array and iteration functions
 - log: add color.decorate.ref config variable
 - log: add color.decorate.symbol config variable
 - log: use designated inits for decoration_colors
 - config: restructure color.decorate documentation

 A new config for coloring.

 Needs review.
 source: <20231023221143.72489-1-andy.koppe@gmail.com>


* js/update-urls-in-doc-and-comment (2023-11-26) 4 commits
 - doc: refer to internet archive
 - doc: update links for andre-simon.de
 - doc: switch links to https
 - doc: update links to current pages

 Stale URLs have been updated to their current counterparts (or
 archive.org) and HTTP links are replaced with working HTTPS links.

 Will merge to 'next'.
 source: <pull.1589.v3.git.1700796916.gitgitgadget@gmail.com>


* la/trailer-cleanups (2023-10-20) 3 commits
 - trailer: use offsets for trailer_start/trailer_end
 - trailer: find the end of the log message
 - commit: ignore_non_trailer computes number of bytes to ignore

 Code clean-up.

 Comments?
 source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>


* eb/hash-transition (2023-10-02) 30 commits
 - t1016-compatObjectFormat: add tests to verify the conversion between objects
 - t1006: test oid compatibility with cat-file
 - t1006: rename sha1 to oid
 - test-lib: compute the compatibility hash so tests may use it
 - builtin/ls-tree: let the oid determine the output algorithm
 - object-file: handle compat objects in check_object_signature
 - tree-walk: init_tree_desc take an oid to get the hash algorithm
 - builtin/cat-file: let the oid determine the output algorithm
 - rev-parse: add an --output-object-format parameter
 - repository: implement extensions.compatObjectFormat
 - object-file: update object_info_extended to reencode objects
 - object-file-convert: convert commits that embed signed tags
 - object-file-convert: convert commit objects when writing
 - object-file-convert: don't leak when converting tag objects
 - object-file-convert: convert tag objects when writing
 - object-file-convert: add a function to convert trees between algorithms
 - object: factor out parse_mode out of fast-import and tree-walk into in object.h
 - cache: add a function to read an OID of a specific algorithm
 - tag: sign both hashes
 - commit: export add_header_signature to support handling signatures on tags
 - commit: convert mergetag before computing the signature of a commit
 - commit: write commits for both hashes
 - object-file: add a compat_oid_in parameter to write_object_file_flags
 - object-file: update the loose object map when writing loose objects
 - loose: compatibilty short name support
 - loose: add a mapping between SHA-1 and SHA-256 for loose objects
 - repository: add a compatibility hash algorithm
 - object-names: support input of oids in any supported hash
 - oid-array: teach oid-array to handle multiple kinds of oids
 - object-file-convert: stubs for converting from one object format to another

 Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.

 Needs review.
 source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>


* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
 - archive: support remote archive from stateless transport
 - transport-helper: call do_take_over() in connect_helper
 - transport-helper: call do_take_over() in process_connect
 - transport-helper: no connection restriction in connect_helper

 "git archive --remote=<remote>" learned to talk over the smart
 http (aka stateless) transport.

 Needs review.
 source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>


* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
 - pkt-line: do not chomp newlines for sideband messages
 - pkt-line: memorize sideband fragment in reader
 - test-pkt-line: add option parser for unpack-sideband

 Sideband demultiplexer fixes.

 Needs review.
 source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>


* js/config-parse (2023-09-21) 5 commits
 - config-parse: split library out of config.[c|h]
 - config.c: accept config_parse_options in git_config_from_stdin
 - config: report config parse errors using cb
 - config: split do_event() into start and flush operations
 - config: split out config_parse_options

 The parsing routines for the configuration files have been split
 into a separate file.

 Needs review.
 source: <cover.1695330852.git.steadmon@google.com>


* jc/fake-lstat (2023-09-15) 1 commit
 - cache: add fake_lstat()
 (this branch is used by jc/diff-cached-fsmonitor-fix.)

 A new helper to let us pretend that we called lstat() when we know
 our cache_entry is up-to-date via fsmonitor.

 Needs review.
 source: <xmqqcyykig1l.fsf@gitster.g>


* js/doc-unit-tests (2023-11-10) 3 commits
  (merged to 'next' on 2023-11-10 at 7d00ffd06b)
 + ci: run unit tests in CI
 + unit tests: add TAP unit test framework
 + unit tests: add a project plan document
 (this branch is used by js/doc-unit-tests-with-cmake.)

 Process to add some form of low-level unit tests has started.

 Will merge to 'master'.
 source: <cover.1699555664.git.steadmon@google.com>


* js/doc-unit-tests-with-cmake (2023-11-10) 7 commits
  (merged to 'next' on 2023-11-10 at b4503c9c8c)
 + cmake: handle also unit tests
 + cmake: use test names instead of full paths
 + cmake: fix typo in variable name
 + artifacts-tar: when including `.dll` files, don't forget the unit-tests
 + unit-tests: do show relative file paths
 + unit-tests: do not mistake `.pdb` files for being executable
 + cmake: also build unit tests
 (this branch uses js/doc-unit-tests.)

 Update the base topic to work with CMake builds.

 Will merge to 'master'.
 source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>


* jc/rerere-cleanup (2023-08-25) 4 commits
 - rerere: modernize use of empty strbuf
 - rerere: try_merge() should use LL_MERGE_ERROR when it means an error
 - rerere: fix comment on handle_file() helper
 - rerere: simplify check_one_conflict() helper function

 Code clean-up.

 Not ready to be reviewed yet.
 source: <20230824205456.1231371-1-gitster@pobox.com>


* rj/status-bisect-while-rebase (2023-10-16) 1 commit
 - status: fix branch shown when not only bisecting

 "git status" is taught to show both the branch being bisected and
 being rebased when both are in effect at the same time.

 Needs review.
 source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>

--------------------------------------------------
[Discarded]

* jc/strbuf-comment-line-char (2023-11-01) 4 commits
 . strbuf: move env-using functions to environment.c
 . strbuf: make add_lines() public
 . strbuf_add_commented_lines(): drop the comment_line_char parameter
 . strbuf_commented_addf(): drop the comment_line_char parameter

 Code simplification that goes directly against a past libification
 topic.  It is hard to judge because the "libification" is done
 piecewise without seemingly clear design principle.

 Will discard.
 source: <cover.1698791220.git.jonathantanmy@google.com>

^ permalink raw reply

* Re: [PATCH 05/24] midx: implement `DISP` chunk
From: Taylor Blau @ 2023-12-09  2:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Patrick Steinhardt
In-Reply-To: <xmqqlea4nofm.fsf@gitster.g>

On Fri, Dec 08, 2023 at 05:40:29PM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > Hopefully you're satisfied with the way things are split up and
> > organized currently, but if you have suggestions on other ways I could
> > slice or dice this, please let me know.
>
> I did wonder how expensive to recompute and validate the "distinct"
> information (in other words, is it too expensive for the consumers
> of an existing midx file to see which packs are distinct on demand
> before they stream contents out of the underlying packs?), as the
> way the packs are marked as distinct looked rather error prone (you
> can very easily list packfiles with overlaps with "+" prefix and the
> DISK chunk writer does not even notice that you lied to it).  As long
> as "git fsck" catches when two packs that are marked as distinct share
> an object, that is OK, but the arrangement did look rather brittle
> to me.

It's likely too expensive to do on the reading side for every
pack-objects operation or MIDX load. But we do check this property when
we write the MIDX, see these lines from midx.c::get_sorted_entries():

    for (cur_object = 0; cur_object < fanout.nr; cur_object++) {
      struct pack_midx_entry *ours = &fanout.entries[cur_object];
      if (cur_object) {
        struct pack_midx_entry *prev = &fanout.entries[cur_object - 1];
        if (oideq(&prev->oid, &ours->oid)) {
          if (prev->disjoint && ours->disjoint)
            die(_("duplicate object '%s' among disjoint packs '%s', '%s'"),
                oid_to_hex(&prev->oid),
                info[prev->pack_int_id].pack_name,
                info[ours->pack_int_id].pack_name);
          continue;
        }
      }

This series doesn't yet have a corresponding step in the fsck builtin,
but I will investigate adding one.

I'm happy to include it in a subsequent round here, but I worry that
this series is already on the verge of being too complex as-is, so it
may be nice as a follow-up, too.

Thanks,
Taylor

^ permalink raw reply

* [BUG] git-bisect man page description of terms command doesn't mention old/new support
From: Britton Kerin @ 2023-12-09 11:58 UTC (permalink / raw)
  To: git

It's a very small issue but it seems that git bisect terms does
support --term-old and --term-new options, however the man page says:

    git bisect terms [--term-good | --term-bad]

The description for the start subcommand does document the support for
the more general terms correctly:

    git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]

so maybe it's worth fixing the git bisect terms documentation.

Britton

^ permalink raw reply

* New attempt to export test-lib from Git, maybe Sharness2?
From: Jiang Xin @ 2023-12-09 13:35 UTC (permalink / raw)
  To: Git List, Felipe Contreras, Junio C Hamano
  Cc: Jiang Xin, Mathias Lafeldt, Christian Couder
In-Reply-To: <CAMP44s3qa_CoM_4--UmwYQTgO-5dHh6=jogH-rxF7OXEWr53Lw@mail.gmail.com>

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

On Mon, 20 Mar 2023 18:15:17 -0600, Felipe Contreras wrote:
> Hello,
> 
> Version 1.2.0 of Sharness [1] -- the test harness library derived from
> Git's test lib -- is released. 

I like Sharness which make it possible to reuse the test framework of
Git. I try to use sharness in my projects and I'm having fun writing
test cases using familiar shell scripts.

But the test framework used by Sharpness is from Git v1.7.9 and is
quite old. The absence of test helper API and command options such as
"--run=..." is quite inconvenient for me. Furthermore, the lack of
lint tools results in many errors in my test cases (e.g., missing
"&&" between statements).

It's not easy to upgrade sharness to the latest test framework of Git.
So I decide to start a new project. The new project is named test-lib,
see:

  * https://github.com/jiangxin/test-lib

Some of my projects have upgraded the test framework from sharkness to
test-lib:

 * git-po-helper: https://github.com/git-l10n/git-po-helper/tree/main/test
 * git-repo-go: https://github.com/alibaba/git-repo-go/tree/master/test

I wonder if we can start Sharness2 based on this solution. See the
README of the test-lib project for details:


diff --git a/README.md b/README.md
new file mode 100644
index 0000000000..4cc529ab73
--- /dev/null
+++ b/README.md
@@ -0,0 +1,159 @@
+Test Lib From Git Core
+======================
+
+Test-lib is a test framework developed by Junio and is specifically
+designed for the Git project. It allows us to write a test suite
+using shell script, which contains a collection of test cases. The
+output of each test suite is presented in TAP ([Test Anything
+Protocol]) format. We can use test-lib or any other TAP harness
+programs (e.g., prove) to run and analyze the output of the test
+suites.
+
+In order to reuse the git test framework into other projects,
+the [sharness project] made a successful attempt. However, it is
+based on an outdated version of git (v1.7.9), which results in
+bugs and missing new features. For example:
+
+ * Commit d88785e424 (test-lib: set `BASH_XTRACEFD` automatically,
+   2016-05-11) and commit a5bf824f3b4d (t: prevent '-x' tracing
+   from interfering with test helpers' stderr, 2018-02-25) of the Git
+   project addressed bugs when we run test suites with "-x" option.
+
+ * Commit 0445e6f0a1 (test-lib: '--run' to run only specific tests,
+   2014-04-30) provided better control of the set of tests to run.
+
+ * Commit 92b269f5c5 (test-lib: turn on `GIT_TEST_CHAIN_LINT`
+   by default, 2015-04-22) turned on chain-lint by default to
+   prevent the accidental omission of "&&" between statements in
+   test cases. Additionally, Git offers more linter tools such as
+   "chainlint.pl" to help write correct test cases.
+
+ * The latest version of the Git project includes numerous test
+   helpers that are not present in sharness. These helpers
+   provide a more comprehensive and efficient testing developing
+   tools. E.g.: `test_bool_env`, `test_cmp_bin`, `test_commit`,
+   `test_config`, `test_env`, `test_file_not_empty`, `test_file_size`,
+   `test_line_count`, `test_oid`, `test_path_exists`,
+   `test_path_is_executable`, `test_path_is_missing`, `test_tick`,
+   `write_script`, etc.
+
+In order to reuse the latest test framework of the Git project and
+easy to maintain, use the following strategies:
+
+1. Use [git-filter-repo] to export test-lib related files and their
+   commit histories from the Git project. The generated tailored
+   commits are saved in the branch named "git-test-lib".
+
+2. The test-lib test framework relies on a helper program named
+   "test-tool", which is written in C. To use test-lib without the
+   need for C compilation, re-implemented part of the "test-tool"
+   subcommands in Python.
+
+3. Apply a series of patches to test-lib so that it can be easily
+   reused in other projects. Some patches are borrowed from Sharness.
+
+4. This project is a by-product of the Git project. For each Git
+   release, we will re-run step 1 to export the latest test
+   framework from Git projects which may include new files we
+   missed in older versions. The generated commit history will
+   overwrite the "git-test-lib" branch, and the master branch will
+   rebase on it.
+
+
+Install test-lib
+----------------
+
+To create test suites in shell scripts powered by test-lib, you can
+follow these steps:
+
+1. Set up a directory (such as "test") to save test suites and
+   files of test-lib.
+
+        $ mkdir test
+
+2. Clone or copy the test-lib repository inside the test directory.
+
+        $ cd test
+        $ git clone https://github.com/jiangixn/test-lib lib
+
+4. Copy files from the example test directory.
+
+        $ cp lib/test-example/.gitignore .
+        $ cp lib/test-example/.gitattributes .
+        $ cp lib/test-example/Makefile .
+
+4. Start writing a test suite powered by test-lib, make sure it
+   sources the "test-lib.sh" file from test-lib. Refer to the example
+   test suite (e.g. "test/t0001-test-tool-chmtime.sh") to write
+   your own test suite:
+
+        test_description='My first test suite'
+
+        . lib/test-lib.sh
+
+
+Usage of test-lib
+-----------------
+
+As for how to use test-lib to write, run and manage your test suites,
+please see the documentation [README.git] for reference.
+
+
+Filtering test-lib from the Git project
+---------------------------------------
+
+"test-lib" is part of the Git project and is stored in the "t/"
+subdirectory. We use "git-filter-repo" to export test-lib related
+files to the root directory of this project. We repeat this
+periodically to save historical commits of test-lib in the
+"git-test-lib" branch of this project. The master branch of this
+project will merge with or rebase onto the "git-test-lib" branch.
+As how to filter and export test-lib from the Git project, see the
+steps below.
+
+1. Make a fresh clone of the Git project before filtering.
+
+        $ git clone --single-branch --no-tags \
+          https://github.com/git/git.git \
+          git-test-lib
+
+        $ cd git-test-lib
+
+2. Use git-filter-repo to filter and export test-lib.
+
+        $ git filter-repo \
+          --preserve-commit-encoding \
+          --prune-degenerate always \
+          --path COPYING \
+          --path shared.mak \
+          --path t/.gitattributes \
+          --path t/.gitignore \
+          --path t/Makefile \
+          --path t/README \
+          --path t/aggregate-results.sh \
+          --path t/chainlint/ \
+          --path t/chainlint.pl \
+          --path t/check-non-portable-shell.pl \
+          --path t/oid-info/ \
+          --path t/perf/.gitignore \
+          --path t/perf/Makefile \
+          --path t/perf/README \
+          --path t/perf/aggregate.perl \
+          --path t/perf/config \
+          --path t/perf/min_time.perl \
+          --path t/perf/perf-lib.sh \
+          --path t/perf/p0000-perf-lib-sanity.sh \
+          --path t/perf/run \
+          --path t/test-lib-functions.sh \
+          --path t/test-lib-github-workflow-markup.sh \
+          --path t/test-lib-junit.sh \
+          --path t/test-lib.sh \
+          --path t/lib-subtest.sh \
+          --path t/t0000-basic.sh \
+          --path-rename t/:
+
+
+[Test Anything Protocol]: http://testanything.org/
+[sharness project]: https://github.com/felipec/sharness
+[git-filter-repo]: https://github.com/newren/git-filter-repo
+[README.git]: ./README.git
-- 
2.42.0.rc2.21.gd1f87c2148


^ permalink raw reply related

* Re* [BUG] git-bisect man page description of terms command doesn't mention old/new support
From: Junio C Hamano @ 2023-12-09 16:13 UTC (permalink / raw)
  To: Britton Kerin; +Cc: git, Christian Couder, Matthieu Moy
In-Reply-To: <CAC4O8c9ieZC4SBJf54ZuTfAvnkhGuDaibBQ-m9Zw_n5VhUFPag@mail.gmail.com>

Britton Kerin <britton.kerin@gmail.com> writes:

> It's a very small issue but it seems that git bisect terms does
> support --term-old and --term-new options, however the man page says:
>
>     git bisect terms [--term-good | --term-bad]
>
> The description for the start subcommand does document the support for
> the more general terms correctly:
>
>     git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
>
> so maybe it's worth fixing the git bisect terms documentation.

In the description, we see

    To get a reminder of the currently used terms, use

    ------------------------------------------------
    git bisect terms
    ------------------------------------------------

    You can get just the old (respectively new) term with `git bisect terms
    --term-old` or `git bisect terms --term-good`.

so you could read that

	git bisect terms --term-good
	git bisect terms --term-old

are the same thing, and when you squint your eyes, you can probably
guess that

	git bisect terms --term-bad
	git bisect terms --term-new

are the same.  But I agree that the documentation should not force
you to guess.  This dates back to 21b55e33 (bisect: add 'git bisect
terms' to view the current terms, 2015-06-29).

------------ >8 ------------ >8 ------------ >8 ------------
Subject: [PATCH] bisect: document "terms" subcommand more fully

The documentation for "git bisect terms", although it did not hide
any information, was a bit incomplete and forced readers to fill in
the blanks to get the complete picture.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-bisect.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git c/Documentation/git-bisect.txt w/Documentation/git-bisect.txt
index 191b4a42b6..16daa09c78 100644
--- c/Documentation/git-bisect.txt
+++ w/Documentation/git-bisect.txt
@@ -20,7 +20,7 @@ on the subcommand:
 		  [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
  git bisect (bad|new|<term-new>) [<rev>]
  git bisect (good|old|<term-old>) [<rev>...]
- git bisect terms [--term-good | --term-bad]
+ git bisect terms [--term-(good|old) | --term-(bad|new)]
  git bisect skip [(<rev>|<range>)...]
  git bisect reset [<commit>]
  git bisect (visualize|view)
@@ -165,8 +165,10 @@ To get a reminder of the currently used terms, use
 git bisect terms
 ------------------------------------------------
 
-You can get just the old (respectively new) term with `git bisect terms
---term-old` or `git bisect terms --term-good`.
+You can get just the old term with `git bisect terms --term-old`
+or `git bisect terms --term-good`; `git bisect terms --term-new`
+and `git bisect terms --term-bad` can be used to learn how to call
+the commits more recent than the sought change.
 
 If you would like to use your own terms instead of "bad"/"good" or
 "new"/"old", you can choose any names you like (except existing bisect


^ permalink raw reply related

* [BUG] description of bit bisect start wrongly mentions <paths>
From: Britton Kerin @ 2023-12-09 16:20 UTC (permalink / raw)
  To: git

It should be <path>... and not <paths>... in this:

     git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
                            [--no-checkout] [--first-parent] [<bad>
[<good>...]] [--] [<paths>...]

This is worth fixing not only for consistency but because path is
mentioned in the subsequent text and paths is not.

Britton

^ permalink raw reply

* Problems with Windows + schannel + http.sslCert
From: Ragesh Krishna @ 2023-12-09 17:37 UTC (permalink / raw)
  To: git@vger.kernel.org

Hi folks,

I'm trying to use SSL client auth on Windows. My git installation currently lists only schannel as the supported backend. The problem is that this always gives me "fatal: refusing to work with credential missing host field". I can see from my server at the other end that a connection was attempted and then terminated unexpectedly. However, I'm not sure what git is expecting me to do to make this work. I suspect it is an schannel vs. openssl difference, because the same credentials work fine on a linux machine. Is there something I can do to dig deeper and understand why this is failing?

Thanks,
-- Ragesh.

^ permalink raw reply

* RE: Problems with Windows + schannel + http.sslCert
From: Ragesh Krishna @ 2023-12-09 17:53 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <TY0PR06MB544239787E909DD4EAC1CA42D189A@TY0PR06MB5442.apcprd06.prod.outlook.com>

Please ignore. Further research suggests that this is an issue with curl on Windows not yet supporting client auth with the schannel backend. Sorry for the noise.

-- Ragesh.

-----Original Message-----
From: Ragesh Krishna 
Sent: Saturday, December 9, 2023 11:08 PM
To: git@vger.kernel.org
Subject: Problems with Windows + schannel + http.sslCert

Hi folks,

I'm trying to use SSL client auth on Windows. My git installation currently lists only schannel as the supported backend. The problem is that this always gives me "fatal: refusing to work with credential missing host field". I can see from my server at the other end that a connection was attempted and then terminated unexpectedly. However, I'm not sure what git is expecting me to do to make this work. I suspect it is an schannel vs. openssl difference, because the same credentials work fine on a linux machine. Is there something I can do to dig deeper and understand why this is failing?

Thanks,
-- Ragesh.

^ permalink raw reply

* Re: New attempt to export test-lib from Git, maybe Sharness2?
From: Junio C Hamano @ 2023-12-09 18:59 UTC (permalink / raw)
  To: Jiang Xin; +Cc: git, Jiang Xin, Mathias Lafeldt, Christian Couder
In-Reply-To: <802ca62b9d9672e9553ab064452d46e0d72dfc76.1702116416.git.zhiyou.jx@alibaba-inc.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> It's not easy to upgrade sharness to the latest test framework of Git.

So?

> So I decide to start a new project. The new project is named test-lib,
> see:
>
>   * https://github.com/jiangxin/test-lib
>
> Some of my projects have upgraded the test framework from sharkness to
> test-lib:
>
>  * git-po-helper: https://github.com/git-l10n/git-po-helper/tree/main/test
>  * git-repo-go: https://github.com/alibaba/git-repo-go/tree/master/test
>
> I wonder if we can start Sharness2 based on this solution. See the
> README of the test-lib project for details:

Is it a viable option to stick to the name "test-lib" (or possibly,
"git-test-lib" to make it more prominent to say where it came from)?

If you do not plan to coordinate with those who work on (the remnant
of) the original sharness based on an ancient version of our test
framework, and do not plan to actively transition its users to your
version, it is less confusing if you named yours differently, as it
avoids hinting that your version is a successor of theirs.

I am not sure if reusing the history of our project verbatim using
filter-repo is really a good way to help those who are interested in
the test framework, by the way.  We make changes for our own purpose
and as a part of such an effort we may touch the test framework to
make it easier to test the changes we made, e.g.,

https://github.com/jiangxin/test-lib/commit/0d5db66ef2b9d8ed5bcee9a0167672dc88b1b026

and unedited filter-repo result will describe such a commit
primarily to explain why the changes in the commit was made on Git
side.  Most of the changes described in the resulting commit message
are discarded by filter-repo and the resulting history becomes hard
to follow.

^ permalink raw reply

* Re: [PATCH 0/7] clone: fix init of refdb with wrong object format
From: Junio C Hamano @ 2023-12-10  3:16 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1701863960.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> While at it I noticed that this actually fixes a bug with bundle URIs
> when the object formats diverge in this way.
> ...
> This patch series is actually the last incompatibility for the reftable
> backend that I have found. All tests except for the files-backend
> specific ones pass now with the current state I have at [1], which is
> currently at e6f2f592b7 (t: skip tests which are incompatible with
> reftable, 2023-11-24)

An existing test

    $ make && cd t && GIT_TEST_DEFAULT_HASH=sha256 sh t5550-http-fetch-dumb.sh

passes with vanilla Git 2.43, but with these patches applied, it
fails the "7 - empty dumb HTTP" step.

Thanks.


^ permalink raw reply

* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Liam Beguin @ 2023-12-10 21:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Kousik Sanagavarapu, git
In-Reply-To: <xmqqwmucjhuw.fsf@gitster.g>

Hi Junio,

Apologies for the late reply.

On Tue, Nov 21, 2023 at 05:21:43AM +0900, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Another line of thought is perhaps it is potentially useful to teach
> > the --format= machinery to be a bit more programmable, e.g. allowing
> > to compute a substring of an existing field %{%aE#*@} without having
> > to waste a letter each for the local part and domain part.  But as I
> > already said, we are now talking about "postprocessing", and adding
> > complexity to our codebase only to have incomplete flexibility may
> > not be worth it.  A more specific %(authoremail:localpart) and its
> > domain counterpart may be easier to explain and understand.
> >
> > In any case, it is a bit too late to say "let's not waste the
> > precious single letter namespace to add useless features", as we
> > have come way too far, so I do not mind too much using a currently
> > unused letter $X for yet another author and committer trait.
> 
> When I wrote the above, I somehow forgot the existing work in the
> ref-filter (aka "for-each-ref") placeholders, where we have support
> to a lot more flexible way to customize these things.

I looked into this a little, after your first email. I'll try to make
time to have another look.

> For example, "%(authoremail:mailmap,localpart)" can be used to say,
> instead of wasting two letters 'l' and 'L' out of precious 52, that
> we want e-mail address honoring the mailmap, and take only the local
> part.  And the support for the host part of the address that this
> topic discussed should be implementable fairly easily (just adding
> EO_HOSTPART bit to the email_option structure would be sufficient)
> on the ref-filter side.
> 
> We saw efforts from time to time to give "log --pretty=format:" more
> of the good things from the "for-each-ref --format=" placeholders
> (and vice versa), and it may give us a good way forward.

This definitely sounds like a better approach than wasting two more
letters.

Liam

^ permalink raw reply

* [BUG] git bisect start gives incorrect error message when good/bad swapped
From: Britton Kerin @ 2023-12-11  6:20 UTC (permalink / raw)
  To: git

When good/bad are entirely swapped git bisect says:

     $ git bisect start initial_commit latest_master_commit
     Some good revs are not ancestors of the bad rev.
     git bisect cannot work properly in this case.
     Maybe you mistook good and bad revs?

But that's not true because when good is a non-ancestor from a branch
a common ancestor is automatically selected:

     $ git bisect start latest_master_commit latest_unmerged_branch_commit
     Bisecting: a merge base must be tested
     [b93212577c2e8603ed7285b55a0931dcf552c628] I'm yet another test commit

In this case latest_unmerged_branch_commit is not an ancestor of
latest_master_commit but a common ancestor is selected automatically
with a message, and when the branch point is marked as bad git
correctly indicates that the problem has been fixed on the branch (and
bisection stops):

     $ git bisect bad
     The merge base b93212577c2e8603ed7285b55a0931dcf552c628 is bad.
     This means the bug has been fixed between
b93212577c2e8603ed7285b55a0931dcf552c628 and
[1b4470e66cb26244be9aa5f68cca042a0ef4270e].

I suspect what the message at the top should be saying is that the bad
rev *is* an ancestor of some good rev but I haven't thought/tested
carefully enough to feel sure that's the case.

I think this is worth fixing somehow because the wrong message gives a
wrong idea of git capabilities and might make the case when a common
ancestor is automatically selected more unexpected and so more
confusing.

On a more minor note the use of 'merge base' to refer to the common
ancestor is unfortunate since these messages are produced when working
on branches that aren't actually merged.  It would be better to use
'common ancestor' (as the merge-base man page does).

Britton

^ permalink raw reply

* Re: [PATCH 3/9] imap-send: don't use git_die_config() inside callback
From: Patrick Steinhardt @ 2023-12-11  7:43 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Jeff King, git
In-Reply-To: <ZXOfrKYsmOjOHGmj@nand.local>

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Fri, Dec 08, 2023 at 05:58:52PM -0500, Taylor Blau wrote:
> On Thu, Dec 07, 2023 at 09:57:55AM +0100, Patrick Steinhardt wrote:
> > On Thu, Dec 07, 2023 at 02:24:58AM -0500, Jeff King wrote:
> > [snip]
> > > diff --git a/imap-send.c b/imap-send.c
> > > index 996651e4f8..5b0fe4f95a 100644
> > > --- a/imap-send.c
> > > +++ b/imap-send.c
> > > @@ -1346,7 +1346,7 @@ static int git_imap_config(const char *var, const char *val,
> > >  		server.port = git_config_int(var, val, ctx->kvi);
> > >  	else if (!strcmp("imap.host", var)) {
> > >  		if (!val) {
> > > -			git_die_config("imap.host", "Missing value for 'imap.host'");
> > > +			return error("Missing value for 'imap.host'");
> >
> > Nit: while at it we might also mark this error for translation. Not
> > worth a reroll on its own though.
> 
> This string goes away entirely in the next patch, so I don't think we
> need to mark it here.
> 
> Thanks,
> Taylor

Ah, true. Never mind in that case.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH][RESEND] show-ref: use die_for_incompatible_opt3()
From: René Scharfe @ 2023-12-11  8:09 UTC (permalink / raw)
  To: Git List
  Cc: Jean-Noël Avila, Patrick Steinhardt, Junio C Hamano,
	Eric Sunshine
In-Reply-To: <CAPig+cR5PKkyC24LkOU4+yzng1xeBOBbADTBHXH61xkAR7kymw@mail.gmail.com>

Use the standard message for reporting the use of multiple mutually
exclusive options by calling die_for_incompatible_opt3() instead of
rolling our own.  This has the benefits of showing only the actually
given options, reducing the number of strings to translate and making
the UI slightly more consistent.

Adjust the test to no longer insist on a specific order of the
reported options, as this implementation detail does not affect the
usefulness of the error message.

Reported-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
Original submission:
https://lore.kernel.org/git/d1f28272-635d-4638-b0f4-76d64013b0d5@web.de/

 builtin/show-ref.c  |  6 +++---
 t/t1403-show-ref.sh | 16 +++++++++-------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7aac525a87..59d2291cbf 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -315,9 +315,9 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, show_ref_options,
 			     show_ref_usage, 0);

-	if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
-		die(_("only one of '%s', '%s' or '%s' can be given"),
-		    "--exclude-existing", "--verify", "--exists");
+	die_for_incompatible_opt3(exclude_existing_opts.enabled, "--exclude-existing",
+				  verify, "--verify",
+				  exists, "--exists");

 	if (exclude_existing_opts.enabled)
 		return cmd_show_ref__exclude_existing(&exclude_existing_opts);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index b50ae6fcf1..d477689e33 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -197,18 +197,20 @@ test_expect_success 'show-ref --verify with dangling ref' '
 '

 test_expect_success 'show-ref sub-modes are mutually exclusive' '
-	cat >expect <<-EOF &&
-	fatal: only one of ${SQ}--exclude-existing${SQ}, ${SQ}--verify${SQ} or ${SQ}--exists${SQ} can be given
-	EOF
-
 	test_must_fail git show-ref --verify --exclude-existing 2>err &&
-	test_cmp expect err &&
+	grep "verify" err &&
+	grep "exclude-existing" err &&
+	grep "cannot be used together" err &&

 	test_must_fail git show-ref --verify --exists 2>err &&
-	test_cmp expect err &&
+	grep "verify" err &&
+	grep "exists" err &&
+	grep "cannot be used together" err &&

 	test_must_fail git show-ref --exclude-existing --exists 2>err &&
-	test_cmp expect err
+	grep "exclude-existing" err &&
+	grep "exists" err &&
+	grep "cannot be used together" err
 '

 test_expect_success '--exists with existing reference' '
--
2.43.0

^ permalink raw reply related

* Re: [PATCH 09/24] repack: implement `--extend-disjoint` mode
From: Patrick Steinhardt @ 2023-12-11  8:18 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZXOdPLotcS5daNke@nand.local>

[-- Attachment #1: Type: text/plain, Size: 4189 bytes --]

On Fri, Dec 08, 2023 at 05:48:28PM -0500, Taylor Blau wrote:
> On Fri, Dec 08, 2023 at 09:19:25AM +0100, Patrick Steinhardt wrote:
> > > > One thing I wondered: do we need to consider the `-l` flag? When using
> > > > an alternate object directory it is totally feasible that the alternate
> > > > may be creating new disjoint packages without us knowing, and thus we
> > > > may not be able to guarantee the disjoint property anymore.
> > >
> > > I don't think so. We'd only care about one direction of this (that
> > > alternates do not create disjoint packs which overlap with ours, instead
> > > of the other way around), but since we don't put non-local packs in the
> > > MIDX, I think we're OK.
> > >
> > > I suppose that you might run into trouble if you use the chained MIDX
> > > thing (via its `->next` pointer). I haven't used that feature myself, so
> > > I'd have to play around with it.
> >
> > We do use this feature at GitLab for forks, where forks connect to a
> > common alternate object directory to deduplicate objects. As both the
> > fork repository and the alternate object directory use an MIDX I think
> > they would be set up exactly like that.
> 
> Yep, that's right. I wasn't sure whether or not this feature had been
> used extensively in production or not (we don't use it at GitHub, since
> objects only live in their fork repositories for a short while before
> moving to the fork network repository).
> 
> > I guess the only really viable solution here is to ignore disjoint packs
> > in the main repo that connects to the alternate in the case where the
> > alternate has any disjoint packs itself.
> 
> I think the behavior you'd get here is that we'd only look for disjoint
> packs in the first MIDX in the chain (which is always the local one),
> and we'd only recognizes packs from that MIDX as being potentially
> disjoint.
> 
> If you have the bulk of your repositories in the alternate, then I think
> you might want to consider how we combine the two. 

Yes, in the general case the bulk of objects is indeed contained in the
alternate.

> My sense is that you'd want to be disjoint with respect to anything
> downstream of you.

Ideally yes, but this is unfortunately not easily achievable in the
general case. It's one of the many painpoints that alternates bring with
them.

Suppose two forks A and B are connected to the same alternate. Both A
and B now gain the same set of objects via whatever means. At this point
these objects can be stored in disjoint packs in each of the repos as
they are not yet deduplicated via the alternate. But if you were to pull
objects from either A or B into the alternate then you cannot ensure
disjointedness at all anymore because you would first have to repack
objects in both A and B.

For two forks this might still seem manageable. But as soon as your fork
network grows larger it's clear that this becomes almost impossible to
do. So ultimately, I don't see an alternative to ignoring disjointedness
bits in either of the two linked-together repos.

> Whether or not this is a feature that you/others need, I definitely
> think we should leave it out of this series, since I am (a) fairly
> certain that this is possible to do, and (b) already feel like this
> series on its own is complicated enough.

I'm perfectly fine if we say that the benefits of your patch series
cannot yet be applied to repositories with alternates. But from my point
of view it's a requirement that this patch series does not silently
break this usecase due to Git starting to generate disjointed packs
where it cannot ensure that the disjointedness property holds.

As I haven't yet read through this whole patch series, the question is
boils down to whether the end result is opt-in or opt-out. If it was
opt-out then I could see the above usecase breaking silently. If it was
opt-in then things should be fine and we can address this ommission in a
follow up patch series. We at GitLab would definitely be interested in
helping out with this given that it directly affects us and that the
demonstrated savings seem very promising.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v3 01/11] reftable: wrap EXPECT macros in do/while
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 4427 bytes --]

The `EXPECT` macros used by the reftable test framework are all using a
single `if` statement with the actual condition. This results in weird
syntax when using them in if/else statements like the following:

```
if (foo)
	EXPECT(foo == 2)
else
	EXPECT(bar == 2)
```

Note that there need not be a trailing semicolon. Furthermore, it is not
immediately obvious whether the else now belongs to the `if (foo)` or
whether it belongs to the expanded `if (foo == 2)` from the macro.

Fix this by wrapping the macros in a do/while loop.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/test_framework.h | 58 +++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/reftable/test_framework.h b/reftable/test_framework.h
index 774cb275bf..ee44f735ae 100644
--- a/reftable/test_framework.h
+++ b/reftable/test_framework.h
@@ -12,32 +12,38 @@ license that can be found in the LICENSE file or at
 #include "system.h"
 #include "reftable-error.h"
 
-#define EXPECT_ERR(c)                                                  \
-	if (c != 0) {                                                  \
-		fflush(stderr);                                        \
-		fflush(stdout);                                        \
-		fprintf(stderr, "%s: %d: error == %d (%s), want 0\n",  \
-			__FILE__, __LINE__, c, reftable_error_str(c)); \
-		abort();                                               \
-	}
-
-#define EXPECT_STREQ(a, b)                                               \
-	if (strcmp(a, b)) {                                              \
-		fflush(stderr);                                          \
-		fflush(stdout);                                          \
-		fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \
-			__LINE__, #a, a, #b, b);                         \
-		abort();                                                 \
-	}
-
-#define EXPECT(c)                                                          \
-	if (!(c)) {                                                        \
-		fflush(stderr);                                            \
-		fflush(stdout);                                            \
-		fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \
-			__LINE__, #c);                                     \
-		abort();                                                   \
-	}
+#define EXPECT_ERR(c)                                                          \
+	do {                                                                   \
+		if (c != 0) {                                                  \
+			fflush(stderr);                                        \
+			fflush(stdout);                                        \
+			fprintf(stderr, "%s: %d: error == %d (%s), want 0\n",  \
+				__FILE__, __LINE__, c, reftable_error_str(c)); \
+			abort();                                               \
+		}                                                              \
+	} while (0)
+
+#define EXPECT_STREQ(a, b)                                                       \
+	do {                                                                     \
+		if (strcmp(a, b)) {                                              \
+			fflush(stderr);                                          \
+			fflush(stdout);                                          \
+			fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \
+				__LINE__, #a, a, #b, b);                         \
+			abort();                                                 \
+		}                                                                \
+	} while (0)
+
+#define EXPECT(c)                                                                  \
+	do {                                                                       \
+		if (!(c)) {                                                        \
+			fflush(stderr);                                            \
+			fflush(stdout);                                            \
+			fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \
+				__LINE__, #c);                                     \
+			abort();                                                   \
+		}                                                                  \
+	} while (0)
 
 #define RUN_TEST(f)                          \
 	fprintf(stderr, "running %s\n", #f); \
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 00/11] reftable: small set of fixes
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1700549493.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 5933 bytes --]

Hi,

this is the third version of my patch series that addresses several
smallish issues in the reftable backend.

There's only a small set of changes compared to v2:

  - Patch 4: convert to use a `struct strbuf` instead of `snprintf()`.

  - Patch 5: improve commit message.

  - Patch 6: note that `stack_filename()` resets the `struct strbuf` in
    the commit message.

  - Patch 7: use the `struct filelock`'s lock path instead of the
    temporary buffer.

Thanks for your suggestions, Taylor and Eric!

Patrick

Patrick Steinhardt (11):
  reftable: wrap EXPECT macros in do/while
  reftable: handle interrupted reads
  reftable: handle interrupted writes
  reftable/stack: verify that `reftable_stack_add()` uses
    auto-compaction
  reftable/stack: perform auto-compaction with transactional interface
  reftable/stack: reuse buffers when reloading stack
  reftable/stack: fix stale lock when dying
  reftable/stack: fix use of unseeded randomness
  reftable/merged: reuse buffer to compute record keys
  reftable/block: introduce macro to initialize `struct block_iter`
  reftable/block: reuse buffer to compute record keys

 reftable/block.c          |  23 ++++----
 reftable/block.h          |   6 +++
 reftable/block_test.c     |   4 +-
 reftable/blocksource.c    |   2 +-
 reftable/iter.h           |   8 +--
 reftable/merged.c         |  31 +++++------
 reftable/merged.h         |   2 +
 reftable/reader.c         |   7 ++-
 reftable/readwrite_test.c |   6 +--
 reftable/stack.c          |  73 +++++++++++---------------
 reftable/stack_test.c     | 107 +++++++++++++++++++++++++++++++++++++-
 reftable/test_framework.h |  58 ++++++++++++---------
 12 files changed, 213 insertions(+), 114 deletions(-)

Range-diff against v2:
 1:  0ebbb02d32 =  1:  5b2a64ca9f reftable: wrap EXPECT macros in do/while
 2:  b404fdf066 =  2:  3e8e63ece5 reftable: handle interrupted reads
 3:  8c1d78b12b =  3:  1700d00d1c reftable: handle interrupted writes
 4:  8061b9d2fc !  4:  5e27d0a556 reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +{
     +	struct reftable_write_options cfg = { 0 };
     +	struct reftable_stack *st = NULL;
    ++	struct strbuf refname = STRBUF_INIT;
     +	char *dir = get_tmp_dir(__LINE__);
     +	int err, i, n = 20;
     +
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +			.value_type = REFTABLE_REF_SYMREF,
     +			.value.symref = "master",
     +		};
    -+		char name[100];
     +
     +		/*
     +		 * Disable auto-compaction for all but the last runs. Like this
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +		 */
     +		st->disable_auto_compact = i != n;
     +
    -+		snprintf(name, sizeof(name), "branch%04d", i);
    -+		ref.refname = name;
    ++		strbuf_reset(&refname);
    ++		strbuf_addf(&refname, "branch-%04d", i);
    ++		ref.refname = refname.buf;
     +
     +		err = reftable_stack_add(st, &write_test_ref, &ref);
     +		EXPECT_ERR(err);
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +	}
     +
     +	reftable_stack_destroy(st);
    ++	strbuf_release(&refname);
     +	clear_dir(dir);
     +}
     +
 5:  77b9ae8aa6 !  5:  dd180eba40 reftable/stack: perform auto-compaction with transactional interface
    @@ Commit message
     
         Whenever updating references or reflog entries in the reftable stack, we
         need to add a new table to the stack, thus growing the stack's length by
    -    one. It can thus happen quite fast that the stack grows very long, which
    -    results in performance issues when trying to read records. But besides
    -    performance issues, this can also lead to exhaustion of file descriptors
    -    very rapidly as every single table requires a separate descriptor when
    +    one. The stack can grow to become quite long rather quickly, leading to
    +    performance issues when trying to read records. But besides performance
    +    issues, this can also lead to exhaustion of file descriptors very
    +    rapidly as every single table requires a separate descriptor when
         opening the stack.
     
         While git-pack-refs(1) fixes this issue for us by merging the tables, it
 6:  f797feff8d !  6:  6ed9ba60db reftable/stack: reuse buffers when reloading stack
    @@ Commit message
         of the allocated buffer outside of the loop.
     
         Refactor the code to instead reuse the buffers to reduce the number of
    -    allocations we need to do.
    +    allocations we need to do. Note that we do not have to manually reset
    +    the buffer because `stack_filename()` does this for us already.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
 7:  e82a68aecd !  7:  fbd9efa56d reftable/stack: fix stale lock when dying
    @@ reftable/stack.c: static int reftable_stack_init_addition(struct reftable_additi
      	}
      	if (st->config.default_permissions) {
     -		if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
    -+		if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
    ++		if (chmod(add->lock_file->filename.buf, st->config.default_permissions) < 0) {
      			err = REFTABLE_IO_ERROR;
      			goto done;
      		}
 8:  bab4fb93df =  8:  5598460b81 reftable/stack: fix use of unseeded randomness
 9:  cbf77ec45a =  9:  79e0382603 reftable/merged: reuse buffer to compute record keys
10:  c9a1405a9a = 10:  8574ad7635 reftable/block: introduce macro to initialize `struct block_iter`
11:  02b11f3a80 = 11:  eeb6c35823 reftable/block: reuse buffer to compute record keys

base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v3 02/11] reftable: handle interrupted reads
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

There are calls to pread(3P) and read(3P) where we don't properly handle
interrupts. Convert them to use `pread_in_full()` and `read_in_full()`,
respectively.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/blocksource.c | 2 +-
 reftable/stack.c       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 8331b34e82..a1ea304429 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -109,7 +109,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
 	struct file_block_source *b = v;
 	assert(off + size <= b->size);
 	dest->data = reftable_malloc(size);
-	if (pread(b->fd, dest->data, size, off) != size)
+	if (pread_in_full(b->fd, dest->data, size, off) != size)
 		return -1;
 	dest->len = size;
 	return size;
diff --git a/reftable/stack.c b/reftable/stack.c
index ddbdf1b9c8..ed108a929b 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -92,7 +92,7 @@ static int fd_read_lines(int fd, char ***namesp)
 	}
 
 	buf = reftable_malloc(size + 1);
-	if (read(fd, buf, size) != size) {
+	if (read_in_full(fd, buf, size) != size) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 03/11] reftable: handle interrupted writes
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]

There are calls to write(3P) where we don't properly handle interrupts.
Convert them to use `write_in_full()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c      | 6 +++---
 reftable/stack_test.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index ed108a929b..f0cadad490 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -42,7 +42,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st,
 static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
 {
 	int *fdp = (int *)arg;
-	return write(*fdp, data, sz);
+	return write_in_full(*fdp, data, sz);
 }
 
 int reftable_new_stack(struct reftable_stack **dest, const char *dir,
@@ -554,7 +554,7 @@ int reftable_addition_commit(struct reftable_addition *add)
 		strbuf_addstr(&table_list, "\n");
 	}
 
-	err = write(add->lock_file_fd, table_list.buf, table_list.len);
+	err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len);
 	strbuf_release(&table_list);
 	if (err < 0) {
 		err = REFTABLE_IO_ERROR;
@@ -1024,7 +1024,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 		strbuf_addstr(&ref_list_contents, "\n");
 	}
 
-	err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
+	err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
 	if (err < 0) {
 		err = REFTABLE_IO_ERROR;
 		unlink(new_table_path.buf);
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index d0b717510f..0644c8ad2e 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -78,7 +78,7 @@ static void test_read_file(void)
 	int i = 0;
 
 	EXPECT(fd > 0);
-	n = write(fd, out, strlen(out));
+	n = write_in_full(fd, out, strlen(out));
 	EXPECT(n == strlen(out));
 	err = close(fd);
 	EXPECT(err >= 0);
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]

While we have several tests that check whether we correctly perform
auto-compaction when manually calling `reftable_stack_auto_compact()`,
we don't have any tests that verify whether `reftable_stack_add()` does
call it automatically. Add one.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack_test.c | 49 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 0644c8ad2e..52b4dc3b14 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -850,6 +850,54 @@ static void test_reftable_stack_auto_compaction(void)
 	clear_dir(dir);
 }
 
+static void test_reftable_stack_add_performs_auto_compaction(void)
+{
+	struct reftable_write_options cfg = { 0 };
+	struct reftable_stack *st = NULL;
+	struct strbuf refname = STRBUF_INIT;
+	char *dir = get_tmp_dir(__LINE__);
+	int err, i, n = 20;
+
+	err = reftable_new_stack(&st, dir, cfg);
+	EXPECT_ERR(err);
+
+	for (i = 0; i <= n; i++) {
+		struct reftable_ref_record ref = {
+			.update_index = reftable_stack_next_update_index(st),
+			.value_type = REFTABLE_REF_SYMREF,
+			.value.symref = "master",
+		};
+
+		/*
+		 * Disable auto-compaction for all but the last runs. Like this
+		 * we can ensure that we indeed honor this setting and have
+		 * better control over when exactly auto compaction runs.
+		 */
+		st->disable_auto_compact = i != n;
+
+		strbuf_reset(&refname);
+		strbuf_addf(&refname, "branch-%04d", i);
+		ref.refname = refname.buf;
+
+		err = reftable_stack_add(st, &write_test_ref, &ref);
+		EXPECT_ERR(err);
+
+		/*
+		 * The stack length should grow continuously for all runs where
+		 * auto compaction is disabled. When enabled, we should merge
+		 * all tables in the stack.
+		 */
+		if (i != n)
+			EXPECT(st->merged->stack_len == i + 1);
+		else
+			EXPECT(st->merged->stack_len == 1);
+	}
+
+	reftable_stack_destroy(st);
+	strbuf_release(&refname);
+	clear_dir(dir);
+}
+
 static void test_reftable_stack_compaction_concurrent(void)
 {
 	struct reftable_write_options cfg = { 0 };
@@ -960,6 +1008,7 @@ int stack_test_main(int argc, const char *argv[])
 	RUN_TEST(test_reftable_stack_add);
 	RUN_TEST(test_reftable_stack_add_one);
 	RUN_TEST(test_reftable_stack_auto_compaction);
+	RUN_TEST(test_reftable_stack_add_performs_auto_compaction);
 	RUN_TEST(test_reftable_stack_compaction_concurrent);
 	RUN_TEST(test_reftable_stack_compaction_concurrent_clean);
 	RUN_TEST(test_reftable_stack_hash_id);
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 05/11] reftable/stack: perform auto-compaction with transactional interface
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 4283 bytes --]

Whenever updating references or reflog entries in the reftable stack, we
need to add a new table to the stack, thus growing the stack's length by
one. The stack can grow to become quite long rather quickly, leading to
performance issues when trying to read records. But besides performance
issues, this can also lead to exhaustion of file descriptors very
rapidly as every single table requires a separate descriptor when
opening the stack.

While git-pack-refs(1) fixes this issue for us by merging the tables, it
runs too irregularly to keep the length of the stack within reasonable
limits. This is why the reftable stack has an auto-compaction mechanism:
`reftable_stack_add()` will call `reftable_stack_auto_compact()` after
its added the new table, which will auto-compact the stack as required.

But while this logic works alright for `reftable_stack_add()`, we do not
do the same in `reftable_addition_commit()`, which is the transactional
equivalent to the former function that allows us to write multiple
updates to the stack atomically. Consequentially, we will easily run
into file descriptor exhaustion in code paths that use many separate
transactions like e.g. non-atomic fetches.

Fix this issue by calling `reftable_stack_auto_compact()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c      |  6 +++++
 reftable/stack_test.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/reftable/stack.c b/reftable/stack.c
index f0cadad490..f5d18a842a 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -584,6 +584,12 @@ int reftable_addition_commit(struct reftable_addition *add)
 	add->new_tables_len = 0;
 
 	err = reftable_stack_reload(add->stack);
+	if (err)
+		goto done;
+
+	if (!add->stack->disable_auto_compact)
+		err = reftable_stack_auto_compact(add->stack);
+
 done:
 	reftable_addition_close(add);
 	return err;
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 52b4dc3b14..14a3fc11ee 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -289,6 +289,61 @@ static void test_reftable_stack_transaction_api(void)
 	clear_dir(dir);
 }
 
+static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
+{
+	char *dir = get_tmp_dir(__LINE__);
+	struct reftable_write_options cfg = {0};
+	struct reftable_addition *add = NULL;
+	struct reftable_stack *st = NULL;
+	int i, n = 20, err;
+
+	err = reftable_new_stack(&st, dir, cfg);
+	EXPECT_ERR(err);
+
+	for (i = 0; i <= n; i++) {
+		struct reftable_ref_record ref = {
+			.update_index = reftable_stack_next_update_index(st),
+			.value_type = REFTABLE_REF_SYMREF,
+			.value.symref = "master",
+		};
+		char name[100];
+
+		snprintf(name, sizeof(name), "branch%04d", i);
+		ref.refname = name;
+
+		/*
+		 * Disable auto-compaction for all but the last runs. Like this
+		 * we can ensure that we indeed honor this setting and have
+		 * better control over when exactly auto compaction runs.
+		 */
+		st->disable_auto_compact = i != n;
+
+		err = reftable_stack_new_addition(&add, st);
+		EXPECT_ERR(err);
+
+		err = reftable_addition_add(add, &write_test_ref, &ref);
+		EXPECT_ERR(err);
+
+		err = reftable_addition_commit(add);
+		EXPECT_ERR(err);
+
+		reftable_addition_destroy(add);
+
+		/*
+		 * The stack length should grow continuously for all runs where
+		 * auto compaction is disabled. When enabled, we should merge
+		 * all tables in the stack.
+		 */
+		if (i != n)
+			EXPECT(st->merged->stack_len == i + 1);
+		else
+			EXPECT(st->merged->stack_len == 1);
+	}
+
+	reftable_stack_destroy(st);
+	clear_dir(dir);
+}
+
 static void test_reftable_stack_validate_refname(void)
 {
 	struct reftable_write_options cfg = { 0 };
@@ -1016,6 +1071,7 @@ int stack_test_main(int argc, const char *argv[])
 	RUN_TEST(test_reftable_stack_log_normalize);
 	RUN_TEST(test_reftable_stack_tombstone);
 	RUN_TEST(test_reftable_stack_transaction_api);
+	RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction);
 	RUN_TEST(test_reftable_stack_update_index_check);
 	RUN_TEST(test_reftable_stack_uptodate);
 	RUN_TEST(test_reftable_stack_validate_refname);
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 06/11] reftable/stack: reuse buffers when reloading stack
From: Patrick Steinhardt @ 2023-12-11  9:07 UTC (permalink / raw)
  To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Taylor Blau, Eric Sunshine
In-Reply-To: <cover.1702285387.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

In `reftable_stack_reload_once()` we iterate over all the tables added
to the stack in order to figure out whether any of the tables needs to
be reloaded. We use a set of buffers in this context to compute the
paths of these tables, but discard those buffers on every iteration.
This is quite wasteful given that we do not need to transfer ownership
of the allocated buffer outside of the loop.

Refactor the code to instead reuse the buffers to reduce the number of
allocations we need to do. Note that we do not have to manually reset
the buffer because `stack_filename()` does this for us already.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index f5d18a842a..2dd2373360 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -204,6 +204,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 		reftable_calloc(sizeof(struct reftable_table) * names_len);
 	int new_readers_len = 0;
 	struct reftable_merged_table *new_merged = NULL;
+	struct strbuf table_path = STRBUF_INIT;
 	int i;
 
 	while (*names) {
@@ -223,13 +224,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 
 		if (!rd) {
 			struct reftable_block_source src = { NULL };
-			struct strbuf table_path = STRBUF_INIT;
 			stack_filename(&table_path, st, name);
 
 			err = reftable_block_source_from_file(&src,
 							      table_path.buf);
-			strbuf_release(&table_path);
-
 			if (err < 0)
 				goto done;
 
@@ -267,16 +265,13 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 	for (i = 0; i < cur_len; i++) {
 		if (cur[i]) {
 			const char *name = reader_name(cur[i]);
-			struct strbuf filename = STRBUF_INIT;
-			stack_filename(&filename, st, name);
+			stack_filename(&table_path, st, name);
 
 			reader_close(cur[i]);
 			reftable_reader_free(cur[i]);
 
 			/* On Windows, can only unlink after closing. */
-			unlink(filename.buf);
-
-			strbuf_release(&filename);
+			unlink(table_path.buf);
 		}
 	}
 
@@ -288,6 +283,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 	reftable_free(new_readers);
 	reftable_free(new_tables);
 	reftable_free(cur);
+	strbuf_release(&table_path);
 	return err;
 }
 
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox