* Re: [PATCH 3/4] ci: use more recent linux32 image
From: Jeff King @ 2024-09-13 4:55 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, git
In-Reply-To: <ZuLi4x664v8dbm2i@pks.im>
On Thu, Sep 12, 2024 at 02:47:38PM +0200, Patrick Steinhardt wrote:
> On Thu, Sep 12, 2024 at 01:53:00PM +0200, Patrick Steinhardt wrote:
> > On Thu, Sep 12, 2024 at 07:22:42AM -0400, Jeff King wrote:
> > > On Thu, Sep 12, 2024 at 12:41:03PM +0200, Patrick Steinhardt wrote:
> > And with that the [fixed] pipeline builds and executes our tests just
> > fine. I didn't wait for tests to finish though.
> >
> > Patrick
> >
> > [here]: https://gitlab.com/gitlab-org/git/-/merge_requests/210
> > [first]: https://gitlab.com/gitlab-org/git/-/jobs/7808775485
> > [fixed]: https://gitlab.com/gitlab-org/git/-/jobs/7808836999
>
> Most of the tests pass, except for t5559. Seems like it doesn't have
> mod_http2. Maybe its Apache version is too old, or it needs to be
> installed separately.
Yeah, according to "apt-file", there's no package which contains
mod_http2.so. t5559 is supposed to notice that during webserver setup
and just skip the script. Poking at it myself in a xenial container, I
think it does do so correctly. So that's good.
But the CI environment switches GIT_TEST_HTTPD from "auto" to "true",
turning a setup failure into an error. This is overall a good thing
(since we'd notice if our apache setup breaks), but obviously is wrong
here. Unfortunately we don't have a knob just for http2. So the best we
can do is something like (might be whitespace-damaged, I just pasted it
out of a container session):
diff --git a/ci/lib.sh b/ci/lib.sh
index 51f8f59..0514f6a 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -336,7 +336,15 @@ ubuntu-*)
fi
MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
- export GIT_TEST_HTTPD=true
+ case "$distro" in
+ ubuntu-16.04)
+ # too old for http/2
+ export GIT_TEST_HTTPD=auto
+ ;;
+ *)
+ export GIT_TEST_HTTPD=yes
+ ;;
+ esac
# The Linux build installs the defined dependency versions below.
# The OS X build installs much more recent versions, whichever
That would still run the regular tests, and just turn the http2 failure
into a "skip". But it does make me nervous that we'd break something for
the non-http2 tests on that old platform and never realize it. So maybe
we need a GIT_TEST_HTTP2 knob that defaults to the value of
GIT_TEST_HTTPD. And then we can turn it off for 16.04, leave the regular
one as "yes".
I assume you're collecting a few patches to make your new xenial job
work. I think what I suggested above should be pretty easy to implement,
but let me know if you'd like me to come up with something concrete.
-Peff
^ permalink raw reply related
* Re: [PATCH 2/4] remote: print an error if refspec cannot be removed
From: Patrick Steinhardt @ 2024-09-13 3:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood via GitGitGadget, git, Han Jiang, Phillip Wood
In-Reply-To: <xmqqplp84zbe.fsf@gitster.g>
On Thu, Sep 12, 2024 at 09:22:13AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > I don't think we want to return the error code from `error()`, do we?
> > `set_branches()` is wired up as a subcommand, so we'd ultimately do
> > `exit(-1)` instead of `exit(1)` if we returned the `error()` code here.
>
> Hmph, I thought there was somebody doing !! to canonicalize the
> return value to exit status in the call chain.
>
> ... goes and looks again ...
>
> After finding the subcommand in fn, cmd_remote() ends with
>
> if (fn) {
> return !!fn(argc, argv, prefix);
> } else {
> ...
> return !!show_all();
> }
Ah, never mind in that case. I didn't look far enough indeed. Thanks for
correcting my claim!
Patrick
^ permalink raw reply
* What's cooking in git.git (Sep 2024, #04; Thu, 12)
From: Junio C Hamano @ 2024-09-12 23:52 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-scm/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/
--------------------------------------------------
[Graduated to 'master']
* ah/mergetols-vscode (2024-09-01) 1 commit
(merged to 'next' on 2024-09-04 at 425c5c83e2)
+ mergetools: vscode: new tool
"git mergetool" learned to use VSCode as a merge backend.
source: <20240902025918.99657-1-alexhenrie24@gmail.com>
* gt/unit-test-oid-array (2024-09-01) 1 commit
(merged to 'next' on 2024-09-05 at 92d0881bb0)
+ t: port helper/test-oid-array.c to unit-tests/t-oid-array.c
Another unit-test.
source: <20240901212649.4910-1-shyamthakkar001@gmail.com>
* jc/mailinfo-header-cleanup (2024-08-20) 1 commit
(merged to 'next' on 2024-09-05 at 9a30adb035)
+ mailinfo: we parse fixed headers
Code clean-up.
cf. <Zsb1rGQbglHMiBHI@tanuki>
source: <xmqq1q2i6gw7.fsf@gitster.g>
* jk/free-commit-buffer-of-skipped-commits (2024-08-30) 1 commit
(merged to 'next' on 2024-09-03 at a8fb72a4d5)
+ revision: free commit buffers for skipped commits
The code forgot to discard unnecessary in-core commit buffer data
for commits that "git log --skip=<number>" traversed but omitted
from the output, which has been corrected.
source: <20240830205331.GA1038751@coredump.intra.peff.net>
* jk/messages-with-excess-lf-fix (2024-09-05) 1 commit
(merged to 'next' on 2024-09-06 at edb0958483)
+ drop trailing newline from warning/error/die messages
One-line messages to "die" and other helper functions will get LF
added by these helper functions, but many existing messages had an
unnecessary LF at the end, which have been corrected.
source: <20240905085149.GA2340826@coredump.intra.peff.net>
* kl/cat-file-on-sparse-index (2024-09-04) 2 commits
(merged to 'next' on 2024-09-06 at a3c78e9398)
+ builtin/cat-file: mark 'git cat-file' sparse-index compatible
+ t1092: allow run_on_* functions to use standard input
"git cat-file" works well with the sparse-index, and gets marked as
such.
source: <pull.1770.v4.git.git.1725401207.gitgitgadget@gmail.com>
* ps/declare-pack-redundamt-dead (2024-09-03) 1 commit
(merged to 'next' on 2024-09-04 at 6a97b07329)
+ Documentation/BreakingChanges: announce removal of git-pack-redundant(1)
"git pack-redundant" has been marked for removal in Git 3.0.
source: <a6be9f5e9eb1f426b1a17b89e3db1bc7532758b5.1725264748.git.ps@pks.im>
* ps/index-pack-outside-repo-fix (2024-09-04) 1 commit
(merged to 'next' on 2024-09-05 at d7ff867595)
+ builtin/index-pack: fix segfaults when running outside of a repo
"git verify-pack" and "git index-pack" started dying outside a
repository, which has been corrected.
source: <9a4267b8854312351f82286b6025d0a3d0e66743.1725429169.git.ps@pks.im>
* ps/pack-refs-auto-heuristics (2024-09-04) 3 commits
(merged to 'next' on 2024-09-06 at 068ed2f7ae)
+ refs/files: use heuristic to decide whether to repack with `--auto`
+ t0601: merge tests for auto-packing of refs
+ wrapper: introduce `log2u()`
"git pack-refs --auto" for the files backend was too aggressive,
which has been a bit tamed.
source: <cover.1725439407.git.ps@pks.im>
* rj/compat-terminal-unused-fix (2024-09-01) 1 commit
(merged to 'next' on 2024-09-04 at 4ad97be799)
+ compat/terminal: mark parameter of git_terminal_prompt() UNUSED
Build fix.
source: <ce1c1d66-e0eb-4143-b334-1a83c0492415@ramsayjones.plus.com>
* sp/mailmap (2024-09-06) 1 commit
(merged to 'next' on 2024-09-07 at aa952cf271)
+ .mailmap document current address.
Update to a mailmap entry.
source: <20240906153003.110200-2-ischis2@cox.net>
* tb/multi-pack-reuse-fix (2024-08-27) 5 commits
(merged to 'next' on 2024-09-06 at 552494ec2f)
+ builtin/pack-objects.c: do not open-code `MAX_PACK_OBJECT_HEADER`
+ pack-bitmap.c: avoid repeated `pack_pos_to_offset()` during reuse
+ builtin/pack-objects.c: translate bit positions during pack-reuse
+ pack-bitmap: tag bitmapped packs with their corresponding MIDX
+ t/t5332-multi-pack-reuse.sh: verify pack generation with --strict
A data corruption bug when multi-pack-index is used and the same
objects are stored in multiple packfiles has been corrected.
cf. <20240905091043.GB2556395@coredump.intra.peff.net>
source: <cover.1724793201.git.me@ttaylorr.com>
--------------------------------------------------
[New Topics]
* jc/ci-upload-artifact-and-linux32 (2024-09-09) 1 commit
(merged to 'next' on 2024-09-11 at 62991bef5b)
+ ci: remove 'Upload failed tests' directories' step from linux32 jobs
(this branch is used by jk/ci-linux32-update.)
CI started failing completely for linux32 jobs, as the step to
upload failed test directory uses GitHub actions that is deprecated
and is now disabled. Remove the step so at least we will know if
the tests are passing.
Will merge to 'master'.
source: <xmqqy140o2kb.fsf@gitster.g>
* jk/ref-filter-trailer-fixes (2024-09-10) 10 commits
(merged to 'next' on 2024-09-10 at ce7299fe2e)
+ ref-filter: fix leak with unterminated %(if) atoms
+ ref-filter: add ref_format_clear() function
+ ref-filter: fix leak when formatting %(push:remoteref)
+ ref-filter: fix leak with %(describe) arguments
+ ref-filter: fix leak of %(trailers) "argbuf"
+ ref-filter: store ref_trailer_buf data per-atom
+ ref-filter: drop useless cast in trailers_atom_parser()
+ ref-filter: strip signature when parsing tag trailers
+ ref-filter: avoid extra copies of payload/signature
+ t6300: drop newline from wrapped test title
Bugfixes and leak plugging in "git for-each-ref --format=..." code
paths.
Will merge to 'master'.
source: <20240909230758.GA921697@coredump.intra.peff.net>
* jk/ci-linux32-update (2024-09-12) 4 commits
- ci: use regular action versions for linux32 job
- ci: use more recent linux32 image
- ci: unify ubuntu and ubuntu32 dependencies
- ci: drop run-docker scripts
(this branch uses jc/ci-upload-artifact-and-linux32.)
CI updates
Will merge to 'next'.
source: <20240912094238.GA589050@coredump.intra.peff.net>
* jk/interop-test-build-options (2024-09-12) 1 commit
- t/interop: allow per-version make options
The support to customize build options to adjust for older versions
and/or older systems for the interop tests has been improved.
Will merge to 'next'.
source: <20240911061009.GA1538383@coredump.intra.peff.net>
* jk/no-openssl-with-openssl-sha1 (2024-09-12) 1 commit
- imap-send: handle NO_OPENSSL even when openssl exists
The "imap-send" now allows to be compiled with NO_OPENSSL and
OPENSSL_SHA1 defined together.
Will merge to 'next'.
source: <20240911061257.GA1538490@coredump.intra.peff.net>
* ma/test-libcurl-prereq (2024-09-11) 2 commits
- t0211: add missing LIBCURL prereq
- t1517: add missing LIBCURL prereq
Test portability fix.
Will merge to 'next'.
source: <cover.1726049108.git.martin.agren@gmail.com>
--------------------------------------------------
[Cooking]
* cc/promisor-remote-capability (2024-09-10) 4 commits
- promisor-remote: check advertised name or URL
- Add 'promisor-remote' capability to protocol v2
- strbuf: refactor strbuf_trim_trailing_ch()
- version: refactor strbuf_sanitize()
The v2 protocol learned to allow the server to advertise possible
promisor remotes, and the client to respond with what promissor
remotes it uses, so that the server side can omit objects that the
client can lazily obtain from these other promissor remotes.
Comments?
source: <20240910163000.1985723-1-christian.couder@gmail.com>
* rj/cygwin-has-dev-tty (2024-09-08) 1 commit
(merged to 'next' on 2024-09-09 at 5c5726050f)
+ config.mak.uname: add HAVE_DEV_TTY to cygwin config section
Cygwin does have /dev/tty support that is needed by things like
single-key input mode.
Will merge to 'master'.
source: <e3339b4d-dab1-4247-b70e-d3224bab1b6b@ramsayjones.plus.com>
* ah/apply-3way-ours (2024-09-09) 1 commit
(merged to 'next' on 2024-09-10 at 989ba9708b)
+ apply: support --ours, --theirs, and --union for three-way merges
"git apply --3way" learned to take "--ours" and other options.
Will merge to 'master'.
source: <20240909141109.3102-2-alexhenrie24@gmail.com>
* ds/pack-name-hash-tweak (2024-09-09) 4 commits
- p5313: add size comparison test
- p5314: add a size test for name-hash collisions
- git-repack: update usage to match docs
- pack-objects: add --full-name-hash option
In a repository with too many (more than --window size) similarly
named files, "git repack" would not find good delta-base candidate
and worse, it may not use a blob from exactly the same path as a
good delta-base candidate. Optionally replace the name hash so
that only blobs at the same path and nothing else are used as
delta-base candidate.
Needs review.
source: <pull.1785.git.1725890210.gitgitgadget@gmail.com>
* ps/reftable-exclude (2024-09-09) 7 commits
- refs/reftable: wire up support for exclude patterns
- reftable/reader: make table iterator reseekable
- t/unit-tests: introduce reftable library
- Makefile: stop listing test library objects twice
- builtin/receive-pack: fix exclude patterns when announcing refs
- refs: properly apply exclude patterns to namespaced refs
- Merge branch 'cp/unit-test-reftable-stack' into ps/reftable-exclude
(this branch uses cp/unit-test-reftable-stack.)
The reftable backend learned to more efficiently handle exclude
patterns while enumerating the refs.
Needs review.
source: <cover.1725881266.git.ps@pks.im>
* ds/doc-wholesale-disabling-advice-messages (2024-09-06) 1 commit
(merged to 'next' on 2024-09-07 at a52a31f161)
+ advice: recommend GIT_ADVICE=0 for tools
The environment GIT_ADVICE has been intentionally kept undocumented
to discourage its use by interactive users. Add documentation to
help tool writers.
Will merge to 'master'.
source: <pull.1781.v2.git.1725654155162.gitgitgadget@gmail.com>
* jk/sparse-fdleak-fix (2024-09-06) 3 commits
(merged to 'next' on 2024-09-07 at 2551aeee9e)
+ sparse-checkout: use fdopen_lock_file() instead of xfdopen()
+ sparse-checkout: check commit_lock_file when writing patterns
+ sparse-checkout: consolidate cleanup when writing patterns
A file descriptor left open is now properly closed when "git
sparse-checkout" updates the sparse patterns.
Will merge to 'master'.
source: <20240906034557.GA3693911@coredump.intra.peff.net>
* bl/trailers-and-incomplete-last-line-fix (2024-09-06) 1 commit
(merged to 'next' on 2024-09-09 at a09f0889bb)
+ interpret-trailers: handle message without trailing newline
The interpret-trailers command failed to recognise the end of the
message when the commit log ends in an incomplete line.
Will merge to 'master'.
source: <20240906145743.2059405-1-brianmlyles@gmail.com>
* jc/doc-skip-fetch-all-and-prefetch (2024-09-09) 1 commit
(merged to 'next' on 2024-09-09 at a2bf302636)
+ doc: remote.*.skip{DefaultUpdate,FetchAll} stops prefetch
Doc updates.
Will merge to 'master'.
source: <xmqqseu8u8m0.fsf@gitster.g>
* rs/diff-exit-code-fix (2024-09-08) 2 commits
(merged to 'next' on 2024-09-09 at f52bb4afb2)
+ diff: report dirty submodules as changes in builtin_diff()
+ diff: report copies and renames as changes in run_diff_cmd()
In a few corner cases "git diff --exit-code" failed to report
"changes" (e.g., renamed without any content change), which has
been corrected.
Will merge to 'master'.
source: <0864c86a-5562-4780-92c5-59d6c1a35aad@web.de>
* pw/rebase-autostash-fix (2024-09-03) 1 commit
- rebase: apply and cleanup autostash when rebase fails to start
"git rebase --autostash" failed to resurrect the autostashed
changes when the command gets aborted after giving back control
asking for hlep in conflict resolution.
Will merge to 'next'.
source: <pull.1772.v2.git.1725289979450.gitgitgadget@gmail.com>
* cp/unit-test-reftable-stack (2024-09-09) 6 commits
(merged to 'next' on 2024-09-09 at 0dddbbb60d)
+ t-reftable-stack: add test for stack iterators
+ t-reftable-stack: add test for non-default compaction factor
+ t-reftable-stack: use reftable_ref_record_equal() to compare ref records
+ t-reftable-stack: use Git's tempfile API instead of mkstemp()
+ t: harmonize t-reftable-stack.c with coding guidelines
+ t: move reftable/stack_test.c to the unit testing framework
(this branch is used by ps/reftable-exclude.)
Another reftable test migrated to the unit-test framework.
Will merge to 'master'.
source: <20240908041632.4948-1-chandrapratap3519@gmail.com>
* ds/scalar-no-tags (2024-09-06) 1 commit
(merged to 'next' on 2024-09-07 at fc06d19cfb)
+ scalar: add --no-tags option to 'scalar clone'
The "scalar clone" command learned the "--no-tags" option.
Will merge to 'master'.
source: <pull.1780.v2.git.1725654102035.gitgitgadget@gmail.com>
* jc/pass-repo-to-builtins (2024-09-11) 3 commits
- add: pass in repo variable instead of global the_repository
- builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h
- builtin: add a repository parameter for builtin functions
The convention to calling into built-in command implementation has
been updated to pass the repository, if known, together with the
prefix value.
Will merge to 'next'?
source: <pull.1778.v2.git.git.1726001960.gitgitgadget@gmail.com>
* tb/weak-sha1-for-tail-sum (2024-09-06) 9 commits
- csum-file.c: use fast SHA-1 implementation when available
- Makefile: allow specifying a SHA-1 for non-cryptographic uses
- hash.h: scaffolding for _fast hashing variants
- sha1: do not redefine `platform_SHA_CTX` and friends
- i5500-git-daemon.sh: use compile-able version of Git without OpenSSL
- pack-objects: use finalize_object_file() to rename pack/idx/etc
- finalize_object_file(): implement collision check
- finalize_object_file(): refactor unlink_or_warn() placement
- finalize_object_file(): check for name collision before renaming
The checksum at the tail of files are now computed without
collision detection protection.
Will merge to 'next'?
source: <cover.1725651952.git.me@ttaylorr.com>
* es/chainlint-message-updates (2024-09-10) 3 commits
(merged to 'next' on 2024-09-11 at a3fd02a009)
+ chainlint: reduce annotation noise-factor
+ chainlint: make error messages self-explanatory
+ chainlint: don't be fooled by "?!...?!" in test body
The error messages from the test script checker have been improved.
Will merge to 'master'.
source: <20240910041013.68948-1-ericsunshine@charter.net>
* ps/environ-wo-the-repository (2024-09-12) 21 commits
- environment: stop storing "core.notesRef" globally
- environment: stop storing "core.warnAmbiguousRefs" globally
- environment: stop storing "core.preferSymlinkRefs" globally
- environment: stop storing "core.logAllRefUpdates" globally
- refs: stop modifying global `log_all_ref_updates` variable
- branch: stop modifying `log_all_ref_updates` variable
- repo-settings: track defaults close to `struct repo_settings`
- repo-settings: split out declarations into a standalone header
- environment: guard state depending on a repository
- environment: reorder header to split out `the_repository`-free section
- environment: move `set_git_dir()` and related into setup layer
- environment: make `get_git_namespace()` self-contained
- environment: move object database functions into object layer
- config: make dependency on repo in `read_early_config()` explicit
- config: document `read_early_config()` and `read_very_early_config()`
- environment: make `get_git_work_tree()` accept a repository
- environment: make `get_graft_file()` accept a repository
- environment: make `get_index_file()` accept a repository
- environment: make `get_object_directory()` accept a repository
- environment: make `get_git_common_dir()` accept a repository
- environment: make `get_git_dir()` accept a repository
Code clean-up.
Will merge to 'next'.
source: <cover.1726139990.git.ps@pks.im>
* gt/unit-test-oidset (2024-08-25) 1 commit
- unit-tests: add tests for oidset.h
Another unit-test.
Expecting a reroll.
source: <20240824172028.39419-1-shyamthakkar001@gmail.com>
* ps/leakfixes-part-6 (2024-09-05) 22 commits
- builtin/repack: fix leaking keep-pack list
- merge-ort: fix two leaks when handling directory rename modifications
- match-trees: fix leaking prefixes in `shift_tree()`
- builtin/fmt-merge-msg: fix leaking buffers
- builtin/grep: fix leaking object context
- builtin/pack-objects: plug leaking list of keep-packs
- builtin/repack: fix leaking line buffer when packing promisors
- negotiator/skipping: fix leaking commit entries
- shallow: fix leaking members of `struct shallow_info`
- shallow: free grafts when unregistering them
- object: clear grafts when clearing parsed object pool
- gpg-interface: fix misdesigned signing key interfaces
- send-pack: fix leaking push cert nonce
- remote: fix leak in reachability check of a remote-tracking ref
- remote: fix leaking tracking refs
- builtin/submodule--helper: fix leaking refs on push-check
- submodule: fix leaking fetch task data
- upload-pack: fix leaking child process data on reachability checks
- builtin/push: fix leaking refspec query result
- send-pack: fix leaking common object IDs
- fetch-pack: fix memory leaks on fetch negotiation
- t/test-lib: allow skipping leak checks for passing tests
More leakfixes.
Will merge to 'next'.
source: <cover.1725530720.git.ps@pks.im>
* sj/ref-contents-check (2024-09-03) 4 commits
- ref: add symlink ref content check for files backend
- ref: add symref content check for files backend
- ref: add regular ref content check for files backend
- ref: initialize "fsck_ref_report" with zero
"git fsck" learned to issue warnings on "curiously formatted" ref
contents that have always been taken valid but something Git
wouldn't have written itself (e.g., missing terminating end-of-line
after the full object name).
Expecting a reroll.
source: <Ztb-mgl50cwGVO8A@ArchLinux>
* tb/incremental-midx-part-2 (2024-08-28) 16 commits
- fixup! midx: implement writing incremental MIDX bitmaps
- midx: implement writing incremental MIDX bitmaps
- pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators
- pack-bitmap.c: keep track of each layer's type bitmaps
- ewah: implement `struct ewah_or_iterator`
- pack-bitmap.c: apply pseudo-merge commits with incremental MIDXs
- pack-bitmap.c: compute disk-usage with incremental MIDXs
- pack-bitmap.c: teach `rev-list --test-bitmap` about incremental MIDXs
- pack-bitmap.c: support bitmap pack-reuse with incremental MIDXs
- pack-bitmap.c: teach `show_objects_for_type()` about incremental MIDXs
- pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs
- pack-bitmap.c: open and store incremental bitmap layers
- pack-revindex: prepare for incremental MIDX bitmaps
- Documentation: describe incremental MIDX bitmaps
- Merge branch 'tb/pseudo-merge-bitmap-fixes' into tb/incremental-midx-part-2
- Merge branch 'tb/incremental-midx-part-1' into tb/incremental-midx-part-2
Incremental updates of multi-pack index files.
Needs review.
source: <cover.1723760847.git.me@ttaylorr.com>
* ps/clar-unit-test (2024-09-10) 15 commits
(merged to 'next' on 2024-09-11 at ccc0289490)
+ Makefile: rename clar-related variables to avoid confusion
(merged to 'next' on 2024-09-05 at 87fb0a399a)
+ clar: add CMake support
+ t/unit-tests: convert ctype tests to use clar
+ t/unit-tests: convert strvec tests to use clar
+ t/unit-tests: implement test driver
+ Makefile: wire up the clar unit testing framework
+ Makefile: do not use sparse on third-party sources
+ Makefile: make hdr-check depend on generated headers
+ Makefile: fix sparse dependency on GENERATED_H
+ clar: stop including `shellapi.h` unnecessarily
+ clar(win32): avoid compile error due to unused `fs_copy()`
+ clar: avoid compile error with mingw-w64
+ t/clar: fix compatibility with NonStop
+ t: import the clar unit testing framework
+ t: do not pass GIT_TEST_OPTS to unit tests with prove
Import clar unit tests framework libgit2 folks invented for our
use.
Will merge to 'master'.
cf. <d5b1c95b-cbdc-4711-849e-c2cfc67787ee@gmail.com>
source: <cover.1725459142.git.ps@pks.im>
* js/libgit-rust (2024-09-09) 7 commits
. SQUASH???
. Makefile: add option to build and test libgit-rs and libgit-rs-sys
. libgit: add higher-level libgit crate
. config: add git_configset_alloc() and git_configset_clear_and_free()
. libgit-sys: add repo initialization and config access
. libgit-sys: introduce Rust wrapper for libgit.a
. common-main: split init and exit code into new files
An rust binding to libgit.a functions has been introduced.
Expecting a reroll.
cf. <xmqqv7z8tjd7.fsf@gitster.g>
source: <20240906221853.257984-1-calvinwan@google.com>
* jc/range-diff-lazy-setup (2024-08-09) 2 commits
(merged to 'next' on 2024-09-10 at 2e04a06b22)
+ remerge-diff: clean up temporary objdir at a central place
+ remerge-diff: lazily prepare temporary objdir on demand
Code clean-up.
Will merge to 'master'.
source: <xmqqr0ax9vlk.fsf@gitster.g>
* jc/too-many-arguments (2024-08-06) 4 commits
- miscellaneous: avoid "too many arguments"
- notes: avoid "too many arguments"
- cat-file: avoid "too many arguments"
- refs: avoid "too many arguments"
Error message clarification.
On hold.
source: <20240806003539.3292562-1-gitster@pobox.com>
* ja/doc-synopsis-markup (2024-09-05) 3 commits
- doc: apply synopsis simplification on git-clone and git-init
- doc: update the guidelines to reflect the current formatting rules
- doc: introduce a synopsis typesetting
The way AsciiDoc is used for SYNOPSIS part of the manual pages has
been revamped. The sources, at least for the simple cases, got
vastly pleasant to work with.
Waiting for comments.
source: <pull.1766.v4.git.1725573126.gitgitgadget@gmail.com>
* ew/cat-file-optim (2024-08-25) 10 commits
- cat-file: use writev(2) if available
- cat-file: batch_write: use size_t for length
- cat-file: batch-command uses content_limit
- object_info: content_limit only applies to blobs
- packfile: packed_object_info avoids packed_to_object_type
- cat-file: use delta_base_cache entries directly
- packfile: inline cache_or_unpack_entry
- packfile: fix off-by-one in content_limit comparison
- packfile: allow content-limit for cat-file
- packfile: move sizep computation
"git cat-file --batch" has been optimized.
Waiting for review responses.
source: <20240823224630.1180772-1-e@80x24.org>
--------------------------------------------------
[Discarded]
* tc/fetch-bundle-uri (2024-07-24) 3 commits
. fetch: use bundle URIs when having creationToken heuristic
. transport: introduce transport_has_remote_bundle_uri()
. clone: remove double bundle list clear code
Allow "git fetch" take advantage of bundleURI feature.
Has been expecting a reroll for too long.
source: <ZqObobw8FsDMkllm@tanuki>
^ permalink raw reply
* [PATCH 2/2] Git.pm: use "rev-parse --absolute-git-dir" rather than perl code
From: Jeff King @ 2024-09-12 22:37 UTC (permalink / raw)
To: Rodrigo; +Cc: git
In-Reply-To: <20240912223413.GA649897@coredump.intra.peff.net>
When we open a repository with the "Directory" option, we use "rev-parse
--git-dir" to get the path relative to that directory, and then use
Cwd::abs_path() to make it absolute (since our process working directory
may not be the same).
These days we can just ask for "--absolute-git-dir" instead, which saves
us a little code. That option was added in Git v2.13.0 via a2f5a87626
(rev-parse: add '--absolute-git-dir' option, 2017-02-03). I don't think
we make any promises about running mismatched versions of git and
Git.pm, but even if somebody tries it, that's sufficiently old that it
should be OK.
Signed-off-by: Jeff King <peff@peff.net>
---
I retained the "require Cwd" here since we use it in the conditional
(but moved it closer to the point of use). It's not strictly necessary,
as earlier code will have required it as a side effect, but it's
probably best not to rely on that.
perl/Git.pm | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index cf1ef0b32a..667152c6c6 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -187,7 +187,7 @@ sub repository {
try {
# Note that "--is-bare-repository" must come first, as
# --git-dir output could contain newlines.
- $out = $search->command([qw(rev-parse --is-bare-repository --git-dir)],
+ $out = $search->command([qw(rev-parse --is-bare-repository --absolute-git-dir)],
STDERR => 0);
} catch Git::Error::Command with {
throw Error::Simple("fatal: not a git repository: $opts{Directory}");
@@ -196,12 +196,12 @@ sub repository {
chomp $out;
my ($bare, $dir) = split /\n/, $out, 2;
- require Cwd;
- require File::Spec;
- File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
- $opts{Repository} = Cwd::abs_path($dir);
+ # We know this is an absolute path, because we used
+ # --absolute-git-dir above.
+ $opts{Repository} = $dir;
if ($bare ne 'true') {
+ require Cwd;
# If --git-dir went ok, this shouldn't die either.
my $prefix = $search->command_oneline('rev-parse', '--show-prefix');
$dir = Cwd::abs_path($opts{Directory}) . '/';
--
2.46.0.918.gab30941bff
^ permalink raw reply related
* [PATCH 1/2] Git.pm: fix bare repository search with Directory option
From: Jeff King @ 2024-09-12 22:36 UTC (permalink / raw)
To: Rodrigo; +Cc: git
In-Reply-To: <20240912223413.GA649897@coredump.intra.peff.net>
When opening a bare repository like:
Git->repository(Directory => '/path/to/bare.git');
we will incorrectly point the repository object at the _current_
directory, not the one specified by the option.
The bug was introduced by 20da61f25f (Git.pm: trust rev-parse to find
bare repositories, 2022-10-22). Before then, we'd ask "rev-parse
--git-dir" if it was a Git repo, and if it returned anything, we'd
correctly convert that result to an absolute path using File::Spec and
Cwd::abs_path(). If it didn't, we'd guess it might be a bare repository
and find it ourselves, which was wrong (rev-parse should find even a
bare repo, and our search circumvented some of its rules).
That commit dropped most of the custom bare-repo search code in favor of
using "rev-parse --is-bare-repository" and trusting the "--git-dir" it
returned. But it mistakenly left some of the bare-repo code path in
place, which was now broken. That code calls Cwd::abs_path($dir); prior
to 20da61f25f $dir contained the "Directory" option the user passed in.
But afterwards, it contains the output of "rev-parse --git-dir". And
since our tentative rev-parse command is invoked after changing
directory, it will always be the relative path "."! So we'll end up with
the absolute path of the process's current directory, not the Directory
option the caller asked for.
So the non-bare case is correct, but the bare one is broken. Our tests
only check the non-bare one, so we didn't notice. We can fix this by
running the same absolute-path fixup code for both sides.
Helped-by: Rodrigo <rodrigolive@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
perl/Git.pm | 10 ++++------
t/t9700-perl-git.sh | 3 ++-
t/t9700/test.pl | 5 +++++
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index aebfe0c6e0..cf1ef0b32a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -197,11 +197,11 @@ sub repository {
my ($bare, $dir) = split /\n/, $out, 2;
require Cwd;
- if ($bare ne 'true') {
- require File::Spec;
- File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
- $opts{Repository} = Cwd::abs_path($dir);
+ require File::Spec;
+ File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
+ $opts{Repository} = Cwd::abs_path($dir);
+ if ($bare ne 'true') {
# If --git-dir went ok, this shouldn't die either.
my $prefix = $search->command_oneline('rev-parse', '--show-prefix');
$dir = Cwd::abs_path($opts{Directory}) . '/';
@@ -214,8 +214,6 @@ sub repository {
$opts{WorkingCopy} = $dir;
$opts{WorkingSubdir} = $prefix;
- } else {
- $opts{Repository} = Cwd::abs_path($dir);
}
delete $opts{Directory};
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index ccc8212d73..4431697122 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -45,7 +45,8 @@ test_expect_success 'set up test repository' '
'
test_expect_success 'set up bare repository' '
- git init --bare bare.git
+ git init --bare bare.git &&
+ git -C bare.git --work-tree=. commit --allow-empty -m "bare commit"
'
test_expect_success 'use t9700/test.pl to test Git.pm' '
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index d8e85482ab..2e1d50d4d1 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -147,6 +147,11 @@ sub adjust_dirsep {
unlink $tmpfile3;
chdir($abs_repo_dir);
+# open alternate bare repo
+my $r4 = Git->repository(Directory => "$abs_repo_dir/bare.git");
+is($r4->command_oneline(qw(log --format=%s)), "bare commit",
+ "log of bare repo works");
+
# unquoting paths
is(Git::unquote_path('abc'), 'abc', 'unquote unquoted path');
is(Git::unquote_path('"abc def"'), 'abc def', 'unquote simple quoted path');
--
2.46.0.918.gab30941bff
^ permalink raw reply related
* [PATCH 0/2] fix bare repositories with Git.pm
From: Jeff King @ 2024-09-12 22:34 UTC (permalink / raw)
To: Rodrigo; +Cc: git
In-Reply-To: <CAGUZU_JZd_+8y19=kGif6u1+4n_+iOcVWV4p-kC0Uo=8Ev=aBA@mail.gmail.com>
On Thu, Sep 12, 2024 at 06:32:00PM +0200, Rodrigo wrote:
> We're having an issue migrating from 2.31.1 to 2.46.0. The following
> Perl code does not work in 2.46.0 as it did in 2.31.1 (tested linux
> and darwin, did not check Windows):
>
> # test.pl
> use Git;
> my $repo = Git->repository( Directory => '/repo/bare.git' );
> my ($fh, $ctx) = $repo->command_output_pipe('rev-list', "2acf3456");
> print do { local $/; <$fh> };
>
> Run:
>
> $ cd /home/rodrigo
> $ perl test.pl
>
> Fails with the error:
>
> fatal: not a git repository: '/home/rodrigo'
Yikes, good catch. That's a pretty bad bug. I'm surprised we didn't
cover this in the tests, but it's specific to bare repositories.
> Bug hunting through the Git.pm code and skimming through the Git SCM
> repo, there's a significant change (commit 20da61f25) that makes the
> recent Git.pm rely on:
>
> git rev-parse --is-bare-repository --git-dir
Yep, I confirmed via bisection that that commit is the culprit. Your
analysis is mostly good, though...
> for locating the correct (maybe a parent) repo directory. The change
> was understandably done for security (and other many) reasons. It
> started using --is-bare-repository to detect if it's a bare repository
> we're dealing with, instead of relying on the old Git.pm redundant
> code for bare repo detection, which was a sound decision. But some
> crucial code was taken out.
...the problem is actually that not enough code was taken out. ;) I left
the code in the bare-only code to turn the relative path to absolute,
but it used the wrong path (the one returned by rev-parse, rather than
the original Directory option that was passed in). That bare-only path
should just go away entirely, and both should use the same (correct)
code to get the absolute path.
> SOLUTION: I propose using "--absolute-git-dir" instead of "--git-dir":
>
> git rev-parse --is-bare-repository --absolute-git-dir
>
> Which will replace the `.` rev-parse response with a full path
> resolved by git itself (and not Perl). This means the change to the
> Perl code is minimal. I don't know if this will resolve all possible
> cases, but it does fix our issue.
It does fix all cases, but it leaves some redundant code in place.
Here are two patches. The first does the minimal fix within the code
(what 20da61f25 should have done!) and corrects the problem. The second
switches to --absolute-git-dir and drops the now-redundant code.
Thank you for a very thorough bug report!
[1/2]: Git.pm: fix bare repository search with Directory option
[2/2]: Git.pm: use "rev-parse --absolute-git-dir" rather than perl code
perl/Git.pm | 14 ++++++--------
t/t9700-perl-git.sh | 3 ++-
t/t9700/test.pl | 5 +++++
3 files changed, 13 insertions(+), 9 deletions(-)
-Peff
^ permalink raw reply
* Re: [PATCH 0/2] Simplify "commented" API functions
From: Junio C Hamano @ 2024-09-12 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <20240912205301.1809355-1-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> [earlier] https://lore.kernel.org/git/xmqq7cn4g3nx.fsf@gitster.g/
>
> Junio C Hamano (2):
> strbuf: retire strbuf_commented_addf()
> strbuf: retire strbuf_commented_lines()
This of course has a semantic conflict with the change that hides
not just "the_repository" and functions that implicitly use that
variable but other unrelated globals from environment.h behind a
compatibility macro.
A trivial merge-fix to be used to annotate the merge to make it an
evil merge whose result compiles looks like this:
merge-fix/jc/strbuf-commented-something
diff --git a/strbuf.c b/strbuf.c
index 05a6dc944b..cb972b07f7 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -7,6 +7,14 @@
#include "utf8.h"
#include "date.h"
+/*
+ * We do not need the_repository at all, but the comment_line_str is
+ * hidden behind USE_THE_REPOSITORY_VARIABLE CPP macro. We could
+ * #define it before including "envirnoment.h". Or we can make an
+ * external declaration ourselves here.
+ */
+extern const char *comment_line_str;
+
int starts_with(const char *str, const char *prefix)
{
for (; ; str++, prefix++)
^ permalink raw reply related
* [PATCH 2/2] strbuf: retire strbuf_commented_lines()
From: Junio C Hamano @ 2024-09-12 20:53 UTC (permalink / raw)
To: git
In-Reply-To: <20240912205301.1809355-1-gitster@pobox.com>
The API allows the caller to pass any string as the comment prefix,
but in reality, everybody passes the comment_line_str from the
environment.
Replace this overly flexible API with strbuf_comment_lines() that
does not allow customization of the comment_line_str (usually '#'
but can be configured via the core.commentChar).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/notes.c | 10 +++++-----
builtin/stripspace.c | 2 +-
fmt-merge-msg.c | 17 +++++++----------
rebase-interactive.c | 6 +++---
sequencer.c | 12 +++++-------
strbuf.c | 7 +++----
strbuf.h | 7 ++-----
wt-status.c | 4 ++--
8 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/builtin/notes.c b/builtin/notes.c
index 4cc5bfedc3..8c4d7f6f89 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -178,7 +178,7 @@ static void write_commented_object(int fd, const struct object_id *object)
if (strbuf_read(&buf, show.out, 0) < 0)
die_errno(_("could not read 'show' output"));
- strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);
+ strbuf_add_comment_lines(&cbuf, buf.buf, buf.len);
write_or_die(fd, cbuf.buf, cbuf.len);
strbuf_release(&cbuf);
@@ -206,10 +206,10 @@ static void prepare_note_data(const struct object_id *object, struct note_data *
copy_obj_to_fd(fd, old_note);
strbuf_addch(&buf, '\n');
- strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_str);
- strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)),
- comment_line_str);
- strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_str);
+ strbuf_add_comment_lines(&buf, "\n", strlen("\n"));
+ strbuf_add_comment_lines(&buf, _(note_template),
+ strlen(_(note_template)));
+ strbuf_add_comment_lines(&buf, "\n", strlen("\n"));
write_or_die(fd, buf.buf, buf.len);
write_commented_object(fd, object);
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index e5626e5126..7be99a2ec0 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf)
size_t len;
msg = strbuf_detach(buf, &len);
- strbuf_add_commented_lines(buf, msg, len, comment_line_str);
+ strbuf_add_comment_lines(buf, msg, len);
free(msg);
}
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 6acb37b480..19d7dd10a0 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -511,8 +511,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,
strbuf_complete_line(tagbuf);
if (sig->len) {
strbuf_addch(tagbuf, '\n');
- strbuf_add_commented_lines(tagbuf, sig->buf, sig->len,
- comment_line_str);
+ strbuf_add_comment_lines(tagbuf, sig->buf, sig->len);
}
}
@@ -556,19 +555,17 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
if (tag_number == 2) {
struct strbuf tagline = STRBUF_INIT;
strbuf_addch(&tagline, '\n');
- strbuf_add_commented_lines(&tagline,
- origins.items[first_tag].string,
- strlen(origins.items[first_tag].string),
- comment_line_str);
+ strbuf_add_comment_lines(&tagline,
+ origins.items[first_tag].string,
+ strlen(origins.items[first_tag].string));
strbuf_insert(&tagbuf, 0, tagline.buf,
tagline.len);
strbuf_release(&tagline);
}
strbuf_addch(&tagbuf, '\n');
- strbuf_add_commented_lines(&tagbuf,
- origins.items[i].string,
- strlen(origins.items[i].string),
- comment_line_str);
+ strbuf_add_comment_lines(&tagbuf,
+ origins.items[i].string,
+ strlen(origins.items[i].string));
fmt_tag_signature(&tagbuf, &sig, buf, len);
}
strbuf_release(&payload);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index dd2ec363d8..1670c51fef 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -80,7 +80,7 @@ void append_todo_help(int command_count,
shortrevisions, shortonto, command_count);
}
- strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str);
+ strbuf_add_comment_lines(buf, msg, strlen(msg));
if (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR)
msg = _("\nDo not remove any line. Use 'drop' "
@@ -89,7 +89,7 @@ void append_todo_help(int command_count,
msg = _("\nIf you remove a line here "
"THAT COMMIT WILL BE LOST.\n");
- strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str);
+ strbuf_add_comment_lines(buf, msg, strlen(msg));
if (edit_todo)
msg = _("\nYou are editing the todo file "
@@ -100,7 +100,7 @@ void append_todo_help(int command_count,
msg = _("\nHowever, if you remove everything, "
"the rebase will be aborted.\n\n");
- strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str);
+ strbuf_add_comment_lines(buf, msg, strlen(msg));
}
int edit_todo_list(struct repository *r, struct replay_opts *opts,
diff --git a/sequencer.c b/sequencer.c
index bf844ce98e..77872d02f4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1911,7 +1911,7 @@ static int is_fixup_flag(enum todo_command command, unsigned flag)
}
/*
- * Wrapper around strbuf_add_commented_lines() which avoids double
+ * Wrapper around strbuf_add_comment_lines() which avoids double
* commenting commit subjects.
*/
static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)
@@ -1928,7 +1928,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)
s += count;
len -= count;
}
- strbuf_add_commented_lines(buf, s, len, comment_line_str);
+ strbuf_add_comment_lines(buf, s, len);
}
/* Does the current fixup chain contain a squash command? */
@@ -2028,7 +2028,7 @@ static int append_squash_message(struct strbuf *buf, const char *body,
strbuf_addf(buf, _(nth_commit_msg_fmt),
++ctx->current_fixup_count + 1);
strbuf_addstr(buf, "\n\n");
- strbuf_add_commented_lines(buf, body, commented_len, comment_line_str);
+ strbuf_add_comment_lines(buf, body, commented_len);
/* buf->buf may be reallocated so store an offset into the buffer */
fixup_off = buf->len;
strbuf_addstr(buf, body + commented_len);
@@ -2119,8 +2119,7 @@ static int update_squash_messages(struct repository *r,
_(first_commit_msg_str));
strbuf_addstr(&buf, "\n\n");
if (is_fixup_flag(command, flag))
- strbuf_add_commented_lines(&buf, body, strlen(body),
- comment_line_str);
+ strbuf_add_comment_lines(&buf, body, strlen(body));
else
strbuf_addstr(&buf, body);
@@ -2139,8 +2138,7 @@ static int update_squash_messages(struct repository *r,
strbuf_addf(&buf, _(skip_nth_commit_msg_fmt),
++ctx->current_fixup_count + 1);
strbuf_addstr(&buf, "\n\n");
- strbuf_add_commented_lines(&buf, body, strlen(body),
- comment_line_str);
+ strbuf_add_comment_lines(&buf, body, strlen(body));
} else
return error(_("unknown command: %d"), command);
repo_unuse_commit_buffer(r, commit, message);
diff --git a/strbuf.c b/strbuf.c
index 6c525da7a6..3dfd25d94d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -379,10 +379,9 @@ static void add_lines(struct strbuf *out,
strbuf_complete_line(out);
}
-void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
- size_t size, const char *comment_prefix)
+void strbuf_add_comment_lines(struct strbuf *out, const char *buf, size_t size)
{
- add_lines(out, comment_prefix, buf, size, 1);
+ add_lines(out, comment_line_str, buf, size, 1);
}
void strbuf_comment_addf(struct strbuf *sb, const char *fmt, ...)
@@ -395,7 +394,7 @@ void strbuf_comment_addf(struct strbuf *sb, const char *fmt, ...)
strbuf_vaddf(&buf, fmt, params);
va_end(params);
- strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_str);
+ strbuf_add_comment_lines(sb, buf.buf, buf.len);
if (incomplete_line)
sb->buf[--sb->len] = '\0';
diff --git a/strbuf.h b/strbuf.h
index aebc8020ae..fcb38c3905 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -284,12 +284,9 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
/**
* Add a NUL-terminated string to the buffer. Each line will be prepended
- * by a comment character and a blank.
+ * by the comment character and a blank.
*/
-void strbuf_add_commented_lines(struct strbuf *out,
- const char *buf, size_t size,
- const char *comment_prefix);
-
+void strbuf_add_comment_lines(struct strbuf *out, const char *buf, size_t size);
/**
* Add data of given length to the buffer.
diff --git a/wt-status.c b/wt-status.c
index 44d29b721f..b94ee1f727 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1033,7 +1033,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom
if (s->display_comment_prefix) {
size_t len;
summary_content = strbuf_detach(&summary, &len);
- strbuf_add_commented_lines(&summary, summary_content, len, comment_line_str);
+ strbuf_add_comment_lines(&summary, summary_content, len);
free(summary_content);
}
@@ -1112,7 +1112,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
strbuf_comment_addf(buf, "%s", cut_line);
- strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_str);
+ strbuf_add_comment_lines(buf, explanation, strlen(explanation));
}
void wt_status_add_cut_line(struct wt_status *s)
--
2.46.0-717-g3841ff3f09
^ permalink raw reply related
* [PATCH 1/2] strbuf: retire strbuf_commented_addf()
From: Junio C Hamano @ 2024-09-12 20:53 UTC (permalink / raw)
To: git
In-Reply-To: <20240912205301.1809355-1-gitster@pobox.com>
The API allows the caller to pass any string as the comment prefix,
but in reality, everybody passes the comment_line_str from the
environment.
Replace this overly flexible API with strbuf_comment_addf() that
does not allow customization of the comment_line_str (usually '#'
but can be configured via the core.commentChar).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
add-patch.c | 8 ++++----
builtin/branch.c | 2 +-
builtin/merge.c | 8 ++++----
builtin/tag.c | 4 ++--
rebase-interactive.c | 2 +-
sequencer.c | 4 ++--
strbuf.c | 6 +++---
strbuf.h | 6 +++---
wt-status.c | 2 +-
9 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 46f6bddfe5..32c990cc18 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1114,11 +1114,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
size_t i;
strbuf_reset(&s->buf);
- strbuf_commented_addf(&s->buf, comment_line_str,
+ strbuf_comment_addf(&s->buf,
_("Manual hunk edit mode -- see bottom for "
"a quick guide.\n"));
render_hunk(s, hunk, 0, 0, &s->buf);
- strbuf_commented_addf(&s->buf, comment_line_str,
+ strbuf_comment_addf(&s->buf,
_("---\n"
"To remove '%c' lines, make them ' ' lines "
"(context).\n"
@@ -1127,13 +1127,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
s->mode->is_reverse ? '+' : '-',
s->mode->is_reverse ? '-' : '+',
comment_line_str);
- strbuf_commented_addf(&s->buf, comment_line_str, "%s",
+ strbuf_comment_addf(&s->buf, "%s",
_(s->mode->edit_hunk_hint));
/*
* TRANSLATORS: 'it' refers to the patch mentioned in the previous
* messages.
*/
- strbuf_commented_addf(&s->buf, comment_line_str,
+ strbuf_comment_addf(&s->buf,
_("If it does not apply cleanly, you will be "
"given an opportunity to\n"
"edit again. If all lines of the hunk are "
diff --git a/builtin/branch.c b/builtin/branch.c
index 48cac74f97..b2a4206e1b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -681,7 +681,7 @@ static int edit_branch_description(const char *branch_name)
exists = !read_branch_desc(&buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(&buf, '\n');
- strbuf_commented_addf(&buf, comment_line_str,
+ strbuf_comment_addf(&buf,
_("Please edit the description for the branch\n"
" %s\n"
"Lines starting with '%s' will be stripped.\n"),
diff --git a/builtin/merge.c b/builtin/merge.c
index 9fba27d85d..8794fea28f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -863,15 +863,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)
strbuf_addch(&msg, '\n');
if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
wt_status_append_cut_line(&msg);
- strbuf_commented_addf(&msg, comment_line_str, "\n");
+ strbuf_comment_addf(&msg, "\n");
}
- strbuf_commented_addf(&msg, comment_line_str,
+ strbuf_comment_addf(&msg,
_(merge_editor_comment));
if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
- strbuf_commented_addf(&msg, comment_line_str,
+ strbuf_comment_addf(&msg,
_(scissors_editor_comment));
else
- strbuf_commented_addf(&msg, comment_line_str,
+ strbuf_comment_addf(&msg,
_(no_scissors_editor_comment), comment_line_str);
}
if (signoff)
diff --git a/builtin/tag.c b/builtin/tag.c
index a1fb218512..0929cfc158 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -334,10 +334,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,
struct strbuf buf = STRBUF_INIT;
strbuf_addch(&buf, '\n');
if (opt->cleanup_mode == CLEANUP_ALL)
- strbuf_commented_addf(&buf, comment_line_str,
+ strbuf_comment_addf(&buf,
_(tag_template), tag, comment_line_str);
else
- strbuf_commented_addf(&buf, comment_line_str,
+ strbuf_comment_addf(&buf,
_(tag_template_nocleanup), tag, comment_line_str);
write_or_die(fd, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index cbeb864147..dd2ec363d8 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -73,7 +73,7 @@ void append_todo_help(int command_count,
if (!edit_todo) {
strbuf_addch(buf, '\n');
- strbuf_commented_addf(buf, comment_line_str,
+ strbuf_comment_addf(buf,
Q_("Rebase %s onto %s (%d command)",
"Rebase %s onto %s (%d commands)",
command_count),
diff --git a/sequencer.c b/sequencer.c
index a2284ac9e9..bf844ce98e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -731,11 +731,11 @@ void append_conflicts_hint(struct index_state *istate,
}
strbuf_addch(msgbuf, '\n');
- strbuf_commented_addf(msgbuf, comment_line_str, "Conflicts:\n");
+ strbuf_comment_addf(msgbuf, "Conflicts:\n");
for (i = 0; i < istate->cache_nr;) {
const struct cache_entry *ce = istate->cache[i++];
if (ce_stage(ce)) {
- strbuf_commented_addf(msgbuf, comment_line_str,
+ strbuf_comment_addf(msgbuf,
"\t%s\n", ce->name);
while (i < istate->cache_nr &&
!strcmp(ce->name, istate->cache[i]->name))
diff --git a/strbuf.c b/strbuf.c
index 3d2189a7f6..6c525da7a6 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,4 +1,5 @@
#include "git-compat-util.h"
+#include "environment.h"
#include "gettext.h"
#include "hex-ll.h"
#include "strbuf.h"
@@ -384,8 +385,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
add_lines(out, comment_prefix, buf, size, 1);
}
-void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix,
- const char *fmt, ...)
+void strbuf_comment_addf(struct strbuf *sb, const char *fmt, ...)
{
va_list params;
struct strbuf buf = STRBUF_INIT;
@@ -395,7 +395,7 @@ void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix,
strbuf_vaddf(&buf, fmt, params);
va_end(params);
- strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix);
+ strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_str);
if (incomplete_line)
sb->buf[--sb->len] = '\0';
diff --git a/strbuf.h b/strbuf.h
index 003f880ff7..aebc8020ae 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -385,11 +385,11 @@ __attribute__((format (printf,2,3)))
void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
/**
- * Add a formatted string prepended by a comment character and a
+ * Add a formatted string prepended by the comment character and a
* blank to the buffer.
*/
-__attribute__((format (printf, 3, 4)))
-void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix, const char *fmt, ...);
+__attribute__((format (printf,2,3)))
+void strbuf_comment_addf(struct strbuf *sb, const char *fmt, ...);
__attribute__((format (printf,2,0)))
void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
diff --git a/wt-status.c b/wt-status.c
index b778eef989..44d29b721f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1111,7 +1111,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
{
const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
- strbuf_commented_addf(buf, comment_line_str, "%s", cut_line);
+ strbuf_comment_addf(buf, "%s", cut_line);
strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_str);
}
--
2.46.0-717-g3841ff3f09
^ permalink raw reply related
* [PATCH 0/2] Simplify "commented" API functions
From: Junio C Hamano @ 2024-09-12 20:52 UTC (permalink / raw)
To: git
We [earlier] noticed that a few helper functions that format strings
into a strbuf, prefixed with an arbitrary comment leading character,
are forcing their callers to always pass the same argument. Instead
of keeping this excess flexibility nobody wants in practice, narrow
the API so that all callers of these functions will get the same
comment leading character.
Superficially it may appear that this goes against one of the recent
trend, "war on global variables", but I doubt it matters much in the
longer run.
These call sites all have already been relying on the global
"comment_line_str" anyway, and passing the variable from the top of
arbitrary deep call chain, which may not care specifically about
this single variable comment_line_str, would not scale as a
solution. If we were to make it a convention to pass something from
the very top of the call chain everywhere, it should not be an
individual variable that is overly specific, like comment_line_str.
Rather, it has to be something that allows access to a bag of all
the global settings (possibly wider than "the_repository" struct).
We can also limit our reliance to the globals by allowing a single
global function call to obtaion such a bag of all global settings,
i.e. "get_the_environment()", and allow everybody, including
functions at the leaf level, to ask "what is the comment_line_str in
the environment I am working in?". As part of the libification, we
can plug in an appropriate shim for get_the_environment().
[earlier] https://lore.kernel.org/git/xmqq7cn4g3nx.fsf@gitster.g/
Junio C Hamano (2):
strbuf: retire strbuf_commented_addf()
strbuf: retire strbuf_commented_lines()
add-patch.c | 8 ++++----
builtin/branch.c | 2 +-
builtin/merge.c | 8 ++++----
builtin/notes.c | 10 +++++-----
builtin/stripspace.c | 2 +-
builtin/tag.c | 4 ++--
fmt-merge-msg.c | 17 +++++++----------
rebase-interactive.c | 8 ++++----
sequencer.c | 16 +++++++---------
strbuf.c | 11 +++++------
strbuf.h | 13 +++++--------
wt-status.c | 6 +++---
12 files changed, 48 insertions(+), 57 deletions(-)
--
2.46.0-717-g3841ff3f09
^ permalink raw reply
* [PATCH] unicode: update the width tables to Unicode 16
From: Beat Bolli @ 2024-09-12 20:40 UTC (permalink / raw)
To: git; +Cc: gitster, Beat Bolli
Unicode 16 has been announced on 2024-09-10 [0], so update the character
width tables to the new version.
[0] https://blog.unicode.org/2024/09/announcing-unicode-standard-version-160.html
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
unicode-width.h | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/unicode-width.h b/unicode-width.h
index be5bf8c4f2..3ffee123a0 100644
--- a/unicode-width.h
+++ b/unicode-width.h
@@ -27,7 +27,7 @@ static const struct interval zero_width[] = {
{ 0x0829, 0x082D },
{ 0x0859, 0x085B },
{ 0x0890, 0x0891 },
-{ 0x0898, 0x089F },
+{ 0x0897, 0x089F },
{ 0x08CA, 0x0902 },
{ 0x093A, 0x093A },
{ 0x093C, 0x093C },
@@ -227,8 +227,9 @@ static const struct interval zero_width[] = {
{ 0x10A3F, 0x10A3F },
{ 0x10AE5, 0x10AE6 },
{ 0x10D24, 0x10D27 },
+{ 0x10D69, 0x10D6D },
{ 0x10EAB, 0x10EAC },
-{ 0x10EFD, 0x10EFF },
+{ 0x10EFC, 0x10EFF },
{ 0x10F46, 0x10F50 },
{ 0x10F82, 0x10F85 },
{ 0x11001, 0x11001 },
@@ -261,6 +262,11 @@ static const struct interval zero_width[] = {
{ 0x11340, 0x11340 },
{ 0x11366, 0x1136C },
{ 0x11370, 0x11374 },
+{ 0x113BB, 0x113C0 },
+{ 0x113CE, 0x113CE },
+{ 0x113D0, 0x113D0 },
+{ 0x113D2, 0x113D2 },
+{ 0x113E1, 0x113E2 },
{ 0x11438, 0x1143F },
{ 0x11442, 0x11444 },
{ 0x11446, 0x11446 },
@@ -280,7 +286,8 @@ static const struct interval zero_width[] = {
{ 0x116AD, 0x116AD },
{ 0x116B0, 0x116B5 },
{ 0x116B7, 0x116B7 },
-{ 0x1171D, 0x1171F },
+{ 0x1171D, 0x1171D },
+{ 0x1171F, 0x1171F },
{ 0x11722, 0x11725 },
{ 0x11727, 0x1172B },
{ 0x1182F, 0x11837 },
@@ -319,8 +326,11 @@ static const struct interval zero_width[] = {
{ 0x11F36, 0x11F3A },
{ 0x11F40, 0x11F40 },
{ 0x11F42, 0x11F42 },
+{ 0x11F5A, 0x11F5A },
{ 0x13430, 0x13440 },
{ 0x13447, 0x13455 },
+{ 0x1611E, 0x16129 },
+{ 0x1612D, 0x1612F },
{ 0x16AF0, 0x16AF4 },
{ 0x16B30, 0x16B36 },
{ 0x16F4F, 0x16F4F },
@@ -351,6 +361,7 @@ static const struct interval zero_width[] = {
{ 0x1E2AE, 0x1E2AE },
{ 0x1E2EC, 0x1E2EF },
{ 0x1E4EC, 0x1E4EF },
+{ 0x1E5EE, 0x1E5EF },
{ 0x1E8D0, 0x1E8D6 },
{ 0x1E944, 0x1E94A },
{ 0xE0001, 0xE0001 },
@@ -366,8 +377,10 @@ static const struct interval double_width[] = {
{ 0x23F3, 0x23F3 },
{ 0x25FD, 0x25FE },
{ 0x2614, 0x2615 },
+{ 0x2630, 0x2637 },
{ 0x2648, 0x2653 },
{ 0x267F, 0x267F },
+{ 0x268A, 0x268F },
{ 0x2693, 0x2693 },
{ 0x26A1, 0x26A1 },
{ 0x26AA, 0x26AB },
@@ -401,11 +414,10 @@ static const struct interval double_width[] = {
{ 0x3099, 0x30FF },
{ 0x3105, 0x312F },
{ 0x3131, 0x318E },
-{ 0x3190, 0x31E3 },
+{ 0x3190, 0x31E5 },
{ 0x31EF, 0x321E },
{ 0x3220, 0x3247 },
-{ 0x3250, 0x4DBF },
-{ 0x4E00, 0xA48C },
+{ 0x3250, 0xA48C },
{ 0xA490, 0xA4C6 },
{ 0xA960, 0xA97C },
{ 0xAC00, 0xD7A3 },
@@ -420,7 +432,7 @@ static const struct interval double_width[] = {
{ 0x16FF0, 0x16FF1 },
{ 0x17000, 0x187F7 },
{ 0x18800, 0x18CD5 },
-{ 0x18D00, 0x18D08 },
+{ 0x18CFF, 0x18D08 },
{ 0x1AFF0, 0x1AFF3 },
{ 0x1AFF5, 0x1AFFB },
{ 0x1AFFD, 0x1AFFE },
@@ -430,6 +442,8 @@ static const struct interval double_width[] = {
{ 0x1B155, 0x1B155 },
{ 0x1B164, 0x1B167 },
{ 0x1B170, 0x1B2FB },
+{ 0x1D300, 0x1D356 },
+{ 0x1D360, 0x1D376 },
{ 0x1F004, 0x1F004 },
{ 0x1F0CF, 0x1F0CF },
{ 0x1F18E, 0x1F18E },
@@ -470,11 +484,10 @@ static const struct interval double_width[] = {
{ 0x1F93C, 0x1F945 },
{ 0x1F947, 0x1F9FF },
{ 0x1FA70, 0x1FA7C },
-{ 0x1FA80, 0x1FA88 },
-{ 0x1FA90, 0x1FABD },
-{ 0x1FABF, 0x1FAC5 },
-{ 0x1FACE, 0x1FADB },
-{ 0x1FAE0, 0x1FAE8 },
+{ 0x1FA80, 0x1FA89 },
+{ 0x1FA8F, 0x1FAC6 },
+{ 0x1FACE, 0x1FADC },
+{ 0x1FADF, 0x1FAE9 },
{ 0x1FAF0, 0x1FAF8 },
{ 0x20000, 0x2FFFD },
{ 0x30000, 0x3FFFD }
--
2.45.2
^ permalink raw reply related
* Re: [PATCH v2] rebase: apply and cleanup autostash when rebase fails to start
From: Junio C Hamano @ 2024-09-12 20:44 UTC (permalink / raw)
To: Phillip Wood via GitGitGadget
Cc: git, Brian Lyles, Patrick Steinhardt, Phillip Wood, Phillip Wood
In-Reply-To: <xmqqr09tvmi5.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> Thanks to Junio and Patrick for their comments on V1. I've updated the
>> commit message to correct the typos found by Patrick and added an
>> explanation of why it is safe to remove the state directory.
>
> Any other comments, or are we all happy with this iteration?
Let me mark the topic for 'next'. Thanks for reporting, fixing and
reviewing.
^ permalink raw reply
* Re: [PATCH v3 00/21] environment: guard reliance on `the_repository`
From: Junio C Hamano @ 2024-09-12 20:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Calvin Wan, Justin Tobler, karthik nayak
In-Reply-To: <cover.1726139990.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> this is the third version of my patch series which guards functions and
> variables in the environment subsystem that rely on `the_repository`
> with the `USE_THE_REPOSITORY_VARIABLE` define.
>
> Changes compared to v2:
>
> - Adapt BUG messages in the first 5 commits to better match the new
> semantics of this function.
>
> - Adapt commit message to mention that we don't only move over
> `odb_mkstemp()`, but also `odb_pack_keep()`.
>
> - Explain why setting REF_FORCE_CREATE_REFLOG is equivalent to setting
> LOG_REFS_NORMAL.
Looking good. Let me mark the topic for 'next' soonish.
Thanks, all.
^ permalink raw reply
* Re: [PATCH v2 00/22] Memory leak fixes (pt.6)
From: Junio C Hamano @ 2024-09-12 20:29 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Calvin Wan, Josh Steadmon, Elijah Newren, Toon claes
In-Reply-To: <cover.1725530720.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this is the second version of this round of memory leak fixes.
>
> There are only some smallish changes compared to v1:
>
> - Explain leak checking a bit more carefully and document the new
> `GIT_TEST_PASSING_SANITIZE_LEAK=check-failing` value in t/README.
>
> - Some more trivial commit message improvements.
>
> Thanks!
Looking good. Let me mark the topic for 'next' soonish.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 5/9] i5500-git-daemon.sh: use compile-able version of Git without OpenSSL
From: Junio C Hamano @ 2024-09-12 20:28 UTC (permalink / raw)
To: Jeff King
Cc: Taylor Blau, git, brian m. carlson, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <20240911061257.GA1538490@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Subject: [PATCH] imap-send: handle NO_OPENSSL even when openssl exists
> ...
> But we can observe that we only need this definition in one spot: the
> struct which holds the variable. So rather than play around with macros
> that may cause unexpected effects, we can just directly use the correct
> type in that struct.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> imap-send.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Neat. Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH 10/9] ref-filter: fix leak with unterminated %(if) atoms
From: Junio C Hamano @ 2024-09-12 20:24 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git, Brooke Kuhlmann
In-Reply-To: <20240912111858.GA617985@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Sep 12, 2024 at 12:22:16PM +0200, Patrick Steinhardt wrote:
>
>> > diff --git c/ref-filter.c w/ref-filter.c
>> > index b06e18a569..d2040f5047 100644
>> > --- c/ref-filter.c
>> > +++ w/ref-filter.c
>> > @@ -3471,7 +3471,8 @@ int format_ref_array_item(struct ref_array_item *info,
>> > }
>> > }
>> > if (state.stack->prev) {
>> > - pop_stack_element(&state.stack);
>> > + while (state.stack->prev)
>> > + pop_stack_element(&state.stack);
>> > return strbuf_addf_ret(error_buf, -1, _("format: %%(end) atom missing"));
>> > }
>> > strbuf_addbuf(final_buf, &state.stack->output);
>>
>> Hm. It certainly feels like we should do that. I couldn't construct a
>> test case that fails with the leak sanitizer though. If it's a leak I'm
>> sure I'll eventually hit it when I continue down the road headed towards
>> leak-free-ness.
>
> Hmm. I think just:
>
> ./git for-each-ref --format='%(if)%(then)%(if)%(then)%(if)%(then)'
>
> should trigger it, and running it in the debugger I can see that we exit
> the function with multiple entries.
>
> Valgrind claims the memory is still reachable, but I don't see how. The
> "state" variable is accessible only inside that function. The only thing
> we do after returning is die(). I wonder if it is a false negative
> because the stack is left undisturbed (especially because the compiler
> knows that die() does not return).
Yup, the reason why I didn't add any test was because the leak
checker failed to notice the apparent leak.
> At any rate, I think the same would apply to the earlier error returns:
> ...
> All that said, I am content to leave it for now. Even if it's a real
> leak, it's one that happens once per program right before exiting with
> an error. Most of the value in cleaning up trivial leaks like that are
> to reduce the noise from analyzers so that we can find the much more
> important leaks that scale with the input. If the analyzers aren't
> complaining and we think it's trivial, it may not be worth spending a
> lot of time on.
That is good to me, too.
^ permalink raw reply
* Re: [PATCH 0/4] make linux32 ci job work with recent actions
From: Junio C Hamano @ 2024-09-12 19:41 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20240912094238.GA589050@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> OK, here's what I came up with. I built it on top of what you have
> queued in jc/ci-upload-artifact-and-linux32, but it could be applied
> separately (and it should merge OK).
Will queue.
Let's target to merge this down to 'maint'; sooner the better.
Thanks.
^ permalink raw reply
* Can't use bare repositories with Git.pm
From: Rodrigo @ 2024-09-12 16:32 UTC (permalink / raw)
To: git
We're having an issue migrating from 2.31.1 to 2.46.0. The following
Perl code does not work in 2.46.0 as it did in 2.31.1 (tested linux
and darwin, did not check Windows):
# test.pl
use Git;
my $repo = Git->repository( Directory => '/repo/bare.git' );
my ($fh, $ctx) = $repo->command_output_pipe('rev-list', "2acf3456");
print do { local $/; <$fh> };
Run:
$ cd /home/rodrigo
$ perl test.pl
Fails with the error:
fatal: not a git repository: '/home/rodrigo'
If the repository it points to is a *bare repository* outside of the
current working directory, it only works if the directory is a child
directory to the bare (/repo/bare.git/info). The code above does work
for non-bare repos, and also works if we set `Repository =>
"/repo/bare.git"` instead of `Directory => ...`, but the Git.pm
documentation states `Directory => ...` should work for both bare and
non-bare alike, like it did in 2.31.1 (and other versions).
Bug hunting through the Git.pm code and skimming through the Git SCM
repo, there's a significant change (commit 20da61f25) that makes the
recent Git.pm rely on:
git rev-parse --is-bare-repository --git-dir
for locating the correct (maybe a parent) repo directory. The change
was understandably done for security (and other many) reasons. It
started using --is-bare-repository to detect if it's a bare repository
we're dealing with, instead of relying on the old Git.pm redundant
code for bare repo detection, which was a sound decision. But some
crucial code was taken out.
Now if the directory path we're passing to `Directory => ...` is a
bare repo this new code will fail because git rev-parse --git-dir will
return a dot `.` (internally Git.pm will chdir() to the directory
before running git rev-parse, hence the result). The issue is that the
dot is now is being taken as is by Git.pm as the intended directory,
tricking it to think my cwd /home/rodrigo is the bare repository,
leading finally to the fatal error.
This could even become a security issue if the Perl program runs from
another working dir which happens to be a sensitive repo. Git.pm
commands would take action on the wrong repo. This hypothetical Perl
program, if run as, ie., a server program, could reveal / change
sensitive information from/on a server.
SOLUTION: I propose using "--absolute-git-dir" instead of "--git-dir":
git rev-parse --is-bare-repository --absolute-git-dir
Which will replace the `.` rev-parse response with a full path
resolved by git itself (and not Perl). This means the change to the
Perl code is minimal. I don't know if this will resolve all possible
cases, but it does fix our issue.
Here's a diff on my proposal -- sorry, noob in this neck of the woods,
didn't know if this is the correct way to contribute, but the change
is tiny and better if parsed by seasoned eyes anyway:
diff --git a/perl/Git.pm b/perl/Git.pm
index aebfe0c..63d0f92 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -187,7 +187,7 @@ sub repository {
try {
# Note that "--is-bare-repository" must come first, as
# --git-dir output could contain newlines.
- $out = $search->command([qw(rev-parse
--is-bare-repository --git-dir)],
+ $out = $search->command([qw(rev-parse
--is-bare-repository --absolute-git-dir)],
STDERR => 0);
} catch Git::Error::Command with {
throw Error::Simple("fatal: not a git
repository: $opts{Directory}");
thanks and best regards,
Rod
gihub.com/rodrigolive
^ permalink raw reply related
* Re: [PATCH 4/4] remote: check branch names
From: Junio C Hamano @ 2024-09-12 16:32 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Phillip Wood via GitGitGadget, git, Han Jiang, Phillip Wood
In-Reply-To: <ZuK80YvPSo8WUpp2@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Agreed. It's also kind of curious that the function lives in
> "object-name.c" and not in "refs.c".
Because the helper groks things like "-" (aka "@{-1}"), it does a
bit more than "is this a reasonable name for a ref" and "please give
me the current value of this ref". Also "refs/remotes/origin/HEAD"
may be valid as a refname, but forbidding "refs/heads/HEAD" is done
conceptually one level closer to the end-users. Eventually, I think
it should move next to branch.c:validate_branchname() as a common
helper between "git branch" and "git remote" (possibly also with
"git switch/checkout", if they need to do validation themselves, but
I suspect they just call into branch.c at a bit higher "here is a
name, create it and you are free to complain---I do not care about
the details of why you decide the name is bad" interface).
Thanks.
^ permalink raw reply
* Re: [PATCH 2/4] remote: print an error if refspec cannot be removed
From: Junio C Hamano @ 2024-09-12 16:22 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Phillip Wood via GitGitGadget, git, Han Jiang, Phillip Wood
In-Reply-To: <ZuK8yihccDjgQGZV@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> I don't think we want to return the error code from `error()`, do we?
> `set_branches()` is wired up as a subcommand, so we'd ultimately do
> `exit(-1)` instead of `exit(1)` if we returned the `error()` code here.
Hmph, I thought there was somebody doing !! to canonicalize the
return value to exit status in the call chain.
... goes and looks again ...
After finding the subcommand in fn, cmd_remote() ends with
if (fn) {
return !!fn(argc, argv, prefix);
} else {
...
return !!show_all();
}
^ permalink raw reply
* Re: [PATCH 3/4] ci: use more recent linux32 image
From: Patrick Steinhardt @ 2024-09-12 12:47 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <ZuLWHBvrT31KVd9W@pks.im>
On Thu, Sep 12, 2024 at 01:53:00PM +0200, Patrick Steinhardt wrote:
> On Thu, Sep 12, 2024 at 07:22:42AM -0400, Jeff King wrote:
> > On Thu, Sep 12, 2024 at 12:41:03PM +0200, Patrick Steinhardt wrote:
> And with that the [fixed] pipeline builds and executes our tests just
> fine. I didn't wait for tests to finish though.
>
> Patrick
>
> [here]: https://gitlab.com/gitlab-org/git/-/merge_requests/210
> [first]: https://gitlab.com/gitlab-org/git/-/jobs/7808775485
> [fixed]: https://gitlab.com/gitlab-org/git/-/jobs/7808836999
Most of the tests pass, except for t5559. Seems like it doesn't have
mod_http2. Maybe its Apache version is too old, or it needs to be
installed separately.
Patrick
^ permalink raw reply
* Re: [PATCH 3/4] ci: use more recent linux32 image
From: Patrick Steinhardt @ 2024-09-12 11:53 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20240912112242.GA622312@coredump.intra.peff.net>
On Thu, Sep 12, 2024 at 07:22:42AM -0400, Jeff King wrote:
> On Thu, Sep 12, 2024 at 12:41:03PM +0200, Patrick Steinhardt wrote:
>
> > On Thu, Sep 12, 2024 at 05:47:30AM -0400, Jeff King wrote:
> > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > > index 97f9b06310..db8e8f75a4 100644
> > > --- a/.github/workflows/main.yml
> > > +++ b/.github/workflows/main.yml
> > > @@ -339,8 +339,8 @@ jobs:
> > > image: alpine
> > > distro: alpine-latest
> > > - jobname: linux32
> > > - image: daald/ubuntu32:xenial
> > > - distro: ubuntu32-16.04
> > > + image: i386/ubuntu:focal
> > > + distro: ubuntu32-20.04
> > > - jobname: pedantic
> > > image: fedora
> > > distro: fedora-latest
> >
> > We could counteract the loss of testing against Ubuntu 16.04 by adding
> > it to GitLab CI instead:
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 2589098eff7..80b1668ebeb 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -25,6 +25,9 @@ test:linux:
> > fi
> > parallel:
> > matrix:
> > + - jobname: linux-old
> > + image: ubuntu:16.04
> > + CC: gcc
> > - jobname: linux-sha256
> > image: ubuntu:latest
> > CC: clang
> >
> > I didn't test it, but it should work alright. GitLab doesn't put any
> > additional executables into the container, so it is entirely self
> > contained. Let me know in case you think this is a good idea and I'll
> > run a CI pipeline against this change.
>
> That seems like a good thing to do to mitigate the loss. In a perfect
> world we'd have all platforms running all the tests, just because it
> helps align the work between finding and fixing (i.e., I might introduce
> a bug and not even know it is failing, and you have to spend time
> reporting it to me). But the world isn't perfect, so finding out about
> my bug _eventually_ is OK. :)
For reference, [here] is the merge request. The [first] pipeline failed
because Java is too old, as you also mention in one of the preceding
commits:
JGit Version
Exception in thread "main" java.lang.UnsupportedClassVersionError: org/eclipse/jgit/pgm/Main has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
at java.net.URLClassLoader.defineClass(URLClassLoader.java:468)
at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
at org.springframework.boot.loader.LaunchedURLClassLoader.loadClass(LaunchedURLClassLoader.java:151)
at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:348)
at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:46)
at org.springframework.boot.loader.Launcher.launch(Launcher.java:108)
at org.springframework.boot.loader.Launcher.launch(Launcher.java:58)
at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:65)
I've thus added the following hunk:
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 735ee6f4639..c85b1f55700 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -55,6 +55,11 @@ ubuntu-*|ubuntu32-*)
${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
case "$distro" in
+ ubuntu-16.04)
+ # Does not support JGit, but we also don't really care about
+ # the others. We rather care whether Git still compiles and
+ # runs fine overall.
+ ;;
ubuntu-*)
mkdir --parents "$CUSTOM_PATH"
And with that the [fixed] pipeline builds and executes our tests just
fine. I didn't wait for tests to finish though.
Patrick
[here]: https://gitlab.com/gitlab-org/git/-/merge_requests/210
[first]: https://gitlab.com/gitlab-org/git/-/jobs/7808775485
[fixed]: https://gitlab.com/gitlab-org/git/-/jobs/7808836999
^ permalink raw reply
* Re: [PATCH 10/9] ref-filter: fix leak with unterminated %(if) atoms
From: Patrick Steinhardt @ 2024-09-12 11:32 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Brooke Kuhlmann
In-Reply-To: <20240912111858.GA617985@coredump.intra.peff.net>
On Thu, Sep 12, 2024 at 07:18:58AM -0400, Jeff King wrote:
> All that said, I am content to leave it for now. Even if it's a real
> leak, it's one that happens once per program right before exiting with
> an error. Most of the value in cleaning up trivial leaks like that are
> to reduce the noise from analyzers so that we can find the much more
> important leaks that scale with the input. If the analyzers aren't
> complaining and we think it's trivial, it may not be worth spending a
> lot of time on.
Agreed. Thanks for digging!
Patrick
^ permalink raw reply
* [PATCH v3 21/21] environment: stop storing "core.notesRef" globally
From: Patrick Steinhardt @ 2024-09-12 11:30 UTC (permalink / raw)
To: git; +Cc: Calvin Wan, Justin Tobler, Junio C Hamano, karthik nayak
In-Reply-To: <cover.1726139990.git.ps@pks.im>
Stop storing the "core.notesRef" config value globally. Instead,
retrieve the value in `default_notes_ref()`. The code is never called in
a hot loop anyway, so doing this on every invocation should be perfectly
fine.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/notes.c | 22 ++++++++++++++--------
config.c | 8 --------
environment.c | 1 -
environment.h | 2 --
notes.c | 21 +++++++++++++--------
notes.h | 3 ++-
6 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/builtin/notes.c b/builtin/notes.c
index 04f9dfb7fbd..5d594a07240 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -897,6 +897,7 @@ static int merge(int argc, const char **argv, const char *prefix)
1, PARSE_OPT_NONEG),
OPT_END()
};
+ char *notes_ref;
argc = parse_options(argc, argv, prefix, options,
git_notes_merge_usage, 0);
@@ -924,7 +925,8 @@ static int merge(int argc, const char **argv, const char *prefix)
if (do_commit)
return merge_commit(&o);
- o.local_ref = default_notes_ref();
+ notes_ref = default_notes_ref(the_repository);
+ o.local_ref = notes_ref;
strbuf_addstr(&remote_ref, argv[0]);
expand_loose_notes_ref(&remote_ref);
o.remote_ref = remote_ref.buf;
@@ -953,7 +955,7 @@ static int merge(int argc, const char **argv, const char *prefix)
}
strbuf_addf(&msg, "notes: Merged notes from %s into %s",
- remote_ref.buf, default_notes_ref());
+ remote_ref.buf, notes_ref);
strbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip "notes: " */
result = notes_merge(&o, t, &result_oid);
@@ -961,7 +963,7 @@ static int merge(int argc, const char **argv, const char *prefix)
if (result >= 0) /* Merge resulted (trivially) in result_oid */
/* Update default notes ref with new commit */
refs_update_ref(get_main_ref_store(the_repository), msg.buf,
- default_notes_ref(), &result_oid, NULL, 0,
+ notes_ref, &result_oid, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
struct worktree **worktrees;
@@ -973,14 +975,14 @@ static int merge(int argc, const char **argv, const char *prefix)
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
worktrees = get_worktrees();
wt = find_shared_symref(worktrees, "NOTES_MERGE_REF",
- default_notes_ref());
+ notes_ref);
if (wt)
die(_("a notes merge into %s is already in-progress at %s"),
- default_notes_ref(), wt->path);
+ notes_ref, wt->path);
free_worktrees(worktrees);
- if (refs_update_symref(get_main_ref_store(the_repository), "NOTES_MERGE_REF", default_notes_ref(), NULL))
+ if (refs_update_symref(get_main_ref_store(the_repository), "NOTES_MERGE_REF", notes_ref, NULL))
die(_("failed to store link to current notes ref (%s)"),
- default_notes_ref());
+ notes_ref);
fprintf(stderr, _("Automatic notes merge failed. Fix conflicts in %s "
"and commit the result with 'git notes merge --commit', "
"or abort the merge with 'git notes merge --abort'.\n"),
@@ -988,6 +990,7 @@ static int merge(int argc, const char **argv, const char *prefix)
}
free_notes(t);
+ free(notes_ref);
strbuf_release(&remote_ref);
strbuf_release(&msg);
return result < 0; /* return non-zero on conflicts */
@@ -1084,6 +1087,7 @@ static int prune(int argc, const char **argv, const char *prefix)
static int get_ref(int argc, const char **argv, const char *prefix)
{
struct option options[] = { OPT_END() };
+ char *notes_ref;
argc = parse_options(argc, argv, prefix, options,
git_notes_get_ref_usage, 0);
@@ -1092,7 +1096,9 @@ static int get_ref(int argc, const char **argv, const char *prefix)
usage_with_options(git_notes_get_ref_usage, options);
}
- puts(default_notes_ref());
+ notes_ref = default_notes_ref(the_repository);
+ puts(notes_ref);
+ free(notes_ref);
return 0;
}
diff --git a/config.c b/config.c
index 53c68f3da61..1266eab0860 100644
--- a/config.c
+++ b/config.c
@@ -1555,14 +1555,6 @@ static int git_default_core_config(const char *var, const char *value,
return git_config_string(&check_roundtrip_encoding, var, value);
}
- if (!strcmp(var, "core.notesref")) {
- if (!value)
- return config_error_nonbool(var);
- free(notes_ref_name);
- notes_ref_name = xstrdup(value);
- return 0;
- }
-
if (!strcmp(var, "core.editor")) {
FREE_AND_NULL(editor_program);
return git_config_string(&editor_program, var, value);
diff --git a/environment.c b/environment.c
index 9dd000cda36..a2ce9980818 100644
--- a/environment.c
+++ b/environment.c
@@ -67,7 +67,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
-char *notes_ref_name;
int grafts_keep_true_parents;
int core_apply_sparse_checkout;
int core_sparse_checkout_cone;
diff --git a/environment.h b/environment.h
index aa38133da9c..923e12661e1 100644
--- a/environment.h
+++ b/environment.h
@@ -203,8 +203,6 @@ enum object_creation_mode {
};
extern enum object_creation_mode object_creation_mode;
-extern char *notes_ref_name;
-
extern int grafts_keep_true_parents;
extern int repository_format_precious_objects;
diff --git a/notes.c b/notes.c
index da42df282d5..f4f18daf07e 100644
--- a/notes.c
+++ b/notes.c
@@ -992,15 +992,16 @@ static int notes_display_config(const char *k, const char *v,
return 0;
}
-const char *default_notes_ref(void)
+char *default_notes_ref(struct repository *repo)
{
- const char *notes_ref = NULL;
+ char *notes_ref = NULL;
+
if (!notes_ref)
- notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
+ notes_ref = xstrdup_or_null(getenv(GIT_NOTES_REF_ENVIRONMENT));
if (!notes_ref)
- notes_ref = notes_ref_name; /* value of core.notesRef config */
+ repo_config_get_string(repo, "core.notesref", ¬es_ref);
if (!notes_ref)
- notes_ref = GIT_NOTES_DEFAULT_REF;
+ notes_ref = xstrdup(GIT_NOTES_DEFAULT_REF);
return notes_ref;
}
@@ -1010,13 +1011,14 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
struct object_id oid, object_oid;
unsigned short mode;
struct leaf_node root_tree;
+ char *to_free = NULL;
if (!t)
t = &default_notes_tree;
assert(!t->initialized);
if (!notes_ref)
- notes_ref = default_notes_ref();
+ notes_ref = to_free = default_notes_ref(the_repository);
update_ref_namespace(NAMESPACE_NOTES, xstrdup(notes_ref));
if (!combine_notes)
@@ -1033,7 +1035,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
if (flags & NOTES_INIT_EMPTY ||
repo_get_oid_treeish(the_repository, notes_ref, &object_oid))
- return;
+ goto out;
if (flags & NOTES_INIT_WRITABLE && refs_read_ref(get_main_ref_store(the_repository), notes_ref, &object_oid))
die("Cannot use notes ref %s", notes_ref);
if (get_tree_entry(the_repository, &object_oid, "", &oid, &mode))
@@ -1043,6 +1045,9 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
oidclr(&root_tree.key_oid, the_repository->hash_algo);
oidcpy(&root_tree.val_oid, &oid);
load_subtree(t, &root_tree, t->root, 0);
+
+out:
+ free(to_free);
}
struct notes_tree **load_notes_trees(struct string_list *refs, int flags)
@@ -1105,7 +1110,7 @@ void load_display_notes(struct display_notes_opt *opt)
if (!opt || opt->use_default_notes > 0 ||
(opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {
- string_list_append(&display_notes_refs, default_notes_ref());
+ string_list_append_nodup(&display_notes_refs, default_notes_ref(the_repository));
display_ref_env = getenv(GIT_NOTES_DISPLAY_REF_ENVIRONMENT);
if (display_ref_env) {
string_list_add_refs_from_colon_sep(&display_notes_refs,
diff --git a/notes.h b/notes.h
index 235216944bc..6dc6d7b2654 100644
--- a/notes.h
+++ b/notes.h
@@ -4,6 +4,7 @@
#include "string-list.h"
struct object_id;
+struct repository;
struct strbuf;
/*
@@ -70,7 +71,7 @@ extern struct notes_tree {
* 3. The value of the core.notesRef config variable, if set
* 4. GIT_NOTES_DEFAULT_REF (i.e. "refs/notes/commits")
*/
-const char *default_notes_ref(void);
+char *default_notes_ref(struct repository *repo);
/*
* Flags controlling behaviour of notes tree initialization
--
2.46.0.551.gc5ee8f2d1c.dirty
^ permalink raw reply related
* [PATCH v3 20/21] environment: stop storing "core.warnAmbiguousRefs" globally
From: Patrick Steinhardt @ 2024-09-12 11:30 UTC (permalink / raw)
To: git; +Cc: Calvin Wan, Justin Tobler, Junio C Hamano, karthik nayak
In-Reply-To: <cover.1726139990.git.ps@pks.im>
Same as the preceding commits, storing the "core.warnAmbiguousRefs"
value globally is misdesigned as this setting may be set per repository.
Move the logic into the repo-settings subsystem. The usual pattern here
is that users are expected to call `prepare_repo_settings()` before they
access the settings themselves. This seems somewhat fragile though, as
it is easy to miss and leads to somewhat ugly code patterns at the call
sites.
Instead, introduce a new function that encapsulates this logic for us.
This also allows us to change how exactly the lazy initialization works
in the future, e.g. by only partially initializing values as requested
by the caller.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rev-parse.c | 4 +++-
config.c | 5 -----
environment.c | 1 -
environment.h | 1 -
object-name.c | 5 +++--
ref-filter.c | 3 ++-
refs.c | 4 ++--
repo-settings.c | 9 +++++++++
repo-settings.h | 4 ++++
9 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index a5108266daf..34b46754426 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -19,6 +19,7 @@
#include "path.h"
#include "diff.h"
#include "read-cache-ll.h"
+#include "repo-settings.h"
#include "repository.h"
#include "revision.h"
#include "setup.h"
@@ -899,7 +900,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
}
if (opt_with_value(arg, "--abbrev-ref", &arg)) {
abbrev_ref = 1;
- abbrev_ref_strict = warn_ambiguous_refs;
+ abbrev_ref_strict =
+ repo_settings_get_warn_ambiguous_refs(the_repository);
if (arg) {
if (!strcmp(arg, "strict"))
abbrev_ref_strict = 1;
diff --git a/config.c b/config.c
index a59890180a3..53c68f3da61 100644
--- a/config.c
+++ b/config.c
@@ -1447,11 +1447,6 @@ static int git_default_core_config(const char *var, const char *value,
return 0;
}
- if (!strcmp(var, "core.warnambiguousrefs")) {
- warn_ambiguous_refs = git_config_bool(var, value);
- return 0;
- }
-
if (!strcmp(var, "core.abbrev")) {
if (!value)
return config_error_nonbool(var);
diff --git a/environment.c b/environment.c
index 6805c7b01df..9dd000cda36 100644
--- a/environment.c
+++ b/environment.c
@@ -35,7 +35,6 @@ int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
int assume_unchanged;
int is_bare_repository_cfg = -1; /* unspecified */
-int warn_ambiguous_refs = 1;
int warn_on_object_refname_ambiguity = 1;
int repository_format_precious_objects;
char *git_commit_encoding;
diff --git a/environment.h b/environment.h
index 0cab644e2d3..aa38133da9c 100644
--- a/environment.h
+++ b/environment.h
@@ -156,7 +156,6 @@ extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
-extern int warn_ambiguous_refs;
extern int warn_on_object_refname_ambiguity;
extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
diff --git a/object-name.c b/object-name.c
index 09c1bd93a31..c892fbe80aa 100644
--- a/object-name.c
+++ b/object-name.c
@@ -20,6 +20,7 @@
#include "pretty.h"
#include "object-store-ll.h"
#include "read-cache-ll.h"
+#include "repo-settings.h"
#include "repository.h"
#include "setup.h"
#include "midx.h"
@@ -959,7 +960,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
int fatal = !(flags & GET_OID_QUIETLY);
if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {
- if (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {
+ if (repo_settings_get_warn_ambiguous_refs(r) && warn_on_object_refname_ambiguity) {
refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);
if (refs_found > 0) {
warning(warn_msg, len, str);
@@ -1020,7 +1021,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
if (!refs_found)
return -1;
- if (warn_ambiguous_refs && !(flags & GET_OID_QUIETLY) &&
+ if (repo_settings_get_warn_ambiguous_refs(r) && !(flags & GET_OID_QUIETLY) &&
(refs_found > 1 ||
!get_short_oid(r, str, len, &tmp_oid, GET_OID_QUIETLY)))
warning(warn_msg, len, str);
diff --git a/ref-filter.c b/ref-filter.c
index b6c6c101276..7f5cf5a1269 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
#include "object-name.h"
#include "object-store-ll.h"
#include "oid-array.h"
+#include "repo-settings.h"
#include "repository.h"
#include "commit.h"
#include "mailmap.h"
@@ -2160,7 +2161,7 @@ static const char *show_ref(struct refname_atom *atom, const char *refname)
if (atom->option == R_SHORT)
return refs_shorten_unambiguous_ref(get_main_ref_store(the_repository),
refname,
- warn_ambiguous_refs);
+ repo_settings_get_warn_ambiguous_refs(the_repository));
else if (atom->option == R_LSTRIP)
return lstrip_ref_components(refname, atom->lstrip);
else if (atom->option == R_RSTRIP)
diff --git a/refs.c b/refs.c
index d7402bcd196..3bee3e78299 100644
--- a/refs.c
+++ b/refs.c
@@ -730,7 +730,7 @@ int expand_ref(struct repository *repo, const char *str, int len,
if (r) {
if (!refs_found++)
*ref = xstrdup(r);
- if (!warn_ambiguous_refs)
+ if (!repo_settings_get_warn_ambiguous_refs(repo))
break;
} else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
warning(_("ignoring dangling symref %s"), fullref.buf);
@@ -775,7 +775,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
if (oid)
oidcpy(oid, &hash);
}
- if (!warn_ambiguous_refs)
+ if (!repo_settings_get_warn_ambiguous_refs(r))
break;
}
strbuf_release(&path);
diff --git a/repo-settings.c b/repo-settings.c
index 1322fd2f972..4699b4b3650 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -140,3 +140,12 @@ enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *re
return LOG_REFS_UNSET;
}
+
+int repo_settings_get_warn_ambiguous_refs(struct repository *repo)
+{
+ prepare_repo_settings(repo);
+ if (repo->settings.warn_ambiguous_refs < 0)
+ repo_cfg_bool(repo, "core.warnambiguousrefs",
+ &repo->settings.warn_ambiguous_refs, 1);
+ return repo->settings.warn_ambiguous_refs;
+}
diff --git a/repo-settings.h b/repo-settings.h
index 76adb96a669..51d6156a117 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -56,16 +56,20 @@ struct repo_settings {
enum fetch_negotiation_setting fetch_negotiation_algorithm;
int core_multi_pack_index;
+ int warn_ambiguous_refs; /* lazily loaded via accessor */
};
#define REPO_SETTINGS_INIT { \
.index_version = -1, \
.core_untracked_cache = UNTRACKED_CACHE_KEEP, \
.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE, \
+ .warn_ambiguous_refs = -1, \
}
void prepare_repo_settings(struct repository *r);
/* Read the value for "core.logAllRefUpdates". */
enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo);
+/* Read the value for "core.warnAmbiguousRefs". */
+int repo_settings_get_warn_ambiguous_refs(struct repository *repo);
#endif /* REPO_SETTINGS_H */
--
2.46.0.551.gc5ee8f2d1c.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox