* [PATCH v2 0/2] doc: bisect: change plural paths to singular pathspec
From: Britton Leo Kerin @ 2024-01-03 4:02 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <a5a8c257-8550-492e-a6fa-e88ee59d4d66@smtp-relay.sendinblue.com>
Britton Leo Kerin (2):
doc: use singular form of repeatable path arg
doc: refer to pathspec instead of path
Documentation/git-bisect.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Range-diff against v1:
1: 90c081dcab ! 1: da40e4736b doc: use singular form of repeatable path arg
@@ Commit message
later document text mentions 'path' arguments, while it doesn't mention
'paths'.
- Signed-off-by: Britton L Kerin <britton.kergin@gmail.com>
+ Signed-off-by: Britton Leo Kerin <britton.kergin@gmail.com>
## Documentation/git-bisect.txt ##
@@ Documentation/git-bisect.txt: The command takes various subcommands, and different options depending
-: ---------- > 2: d932b6d501 doc: refer to pathspec instead of path
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/2] doc: use singular form of repeatable path arg
From: Britton Leo Kerin @ 2024-01-03 4:02 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin, Britton Leo Kerin
In-Reply-To: <20240103040207.661413-1-britton.kerin@gmail.com>
This is more correct because the <path>... doc syntax already indicates
that the arg is "array-type". It's how other tools do it. Finally, the
later document text mentions 'path' arguments, while it doesn't mention
'paths'.
Signed-off-by: Britton Leo Kerin <britton.kergin@gmail.com>
---
Documentation/git-bisect.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index aa02e46224..b798282788 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
on the subcommand:
git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
+ [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<path>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-good | --term-bad]
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/2] doc: refer to pathspec instead of path
From: Britton Leo Kerin @ 2024-01-03 4:02 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240103040207.661413-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
Documentation/git-bisect.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index b798282788..8e01f1d618 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
on the subcommand:
git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<path>...]
+ [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-good | --term-bad]
@@ -299,7 +299,7 @@ Cutting down bisection by giving more parameters to bisect start
You can further cut down the number of trials, if you know what part of
the tree is involved in the problem you are tracking down, by specifying
-path parameters when issuing the `bisect start` command:
+pathspec parameters when issuing the `bisect start` command:
------------
$ git bisect start -- arch/i386 include/asm-i386
--
2.43.0
^ permalink raw reply related
* What's cooking in git.git (Jan 2024, #01; Tue, 2)
From: Junio C Hamano @ 2024-01-03 1:02 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
As the past couple of weeks have been slow, topics that have been
merged to 'next' (and to a lessor extent, to 'master') this season
may have seen less scrutinizing eyes than they deserve, which might
lead bugs and regressions caused by them discovered later, but that
is a regular part of life. Let's see what happens.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* jc/orphan-unborn (2023-11-24) 2 commits
(merged to 'next' on 2023-12-21 at 030300487a)
+ orphan/unborn: fix use of 'orphan' in end-user facing messages
+ orphan/unborn: add to the glossary and use them consistently
Doc updates to clarify what an "unborn branch" means.
source: <xmqq4jhb977x.fsf@gitster.g>
* jc/retire-cas-opt-name-constant (2023-12-19) 1 commit
(merged to 'next' on 2023-12-21 at 39ef057c8b)
+ remote.h: retire CAS_OPT_NAME
Code clean-up.
source: <xmqq5y0uc7tq.fsf@gitster.g>
* la/trailer-cleanups (2023-12-20) 3 commits
(merged to 'next' on 2023-12-21 at e26ede5f55)
+ trailer: use offsets for trailer_start/trailer_end
+ trailer: find the end of the log message
+ commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* ps/pseudo-refs (2023-12-14) 4 commits
(merged to 'next' on 2023-12-21 at 3460e3d667)
+ bisect: consistently write BISECT_EXPECTED_REV via the refdb
+ refs: complete list of special refs
+ refs: propagate errno when reading special refs fails
+ wt-status: read HEAD and ORIG_HEAD via the refdb
Assorted changes around pseudoref handling.
source: <cover.1702560829.git.ps@pks.im>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
(merged to 'next' on 2023-12-21 at 929a027fb7)
+ status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
cf. <xmqqil76kyov.fsf@gitster.g>
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
* rs/rebase-use-strvec-pushf (2023-12-20) 1 commit
(merged to 'next' on 2023-12-20 at ecb190973c)
+ rebase: use strvec_pushf() for format-patch revisions
Code clean-up.
source: <4ab7431c-6c1b-448c-b4d2-e8b9be0e4eef@web.de>
* sh/completion-with-reftable (2023-12-19) 2 commits
(merged to 'next' on 2023-12-20 at 7957d4aa5b)
+ completion: support pseudoref existence checks for reftables
+ completion: refactor existence checks for pseudorefs
Command line completion script (in contrib/) learned to work better
with the reftable backend.
source: <cover.1703022850.git.stanhu@gmail.com>
--------------------------------------------------
[New Topics]
* cp/sideband-array-index-comment-fix (2023-12-28) 1 commit
- sideband.c: remove redundant 'NEEDSWORK' tag
In-code comment fix.
source: <pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com>
* ps/worktree-refdb-initialization (2023-12-28) 6 commits
- builtin/worktree: create refdb via ref backend
- worktree: expose interface to look up worktree by name
- builtin/worktree: move setup of commondir file earlier
- refs/files: skip creation of "refs/{heads,tags}" for worktrees
- setup: move creation of "refs/" into the files backend
- refs: prepare `refs_init_db()` for initializing worktree refs
Instead of manually creating refs/ hierarchy on disk upon a
creation of a secondary worktree, which is only usable via the
files backend, use the refs API to populate it.
May want to wait until other topics solidify a bit more.
cf. <xmqqedf6gpt8.fsf@gitster.g>
source: <cover.1703754513.git.ps@pks.im>
--------------------------------------------------
[Cooking]
* ml/doc-merge-updates (2023-12-20) 2 commits
(merged to 'next' on 2023-12-28 at 7a6329a219)
+ Documentation/git-merge.txt: use backticks for command wrapping
+ Documentation/git-merge.txt: fix reference to synopsis
Doc update.
Will merge to 'master'.
source: <20231220195342.17590-1-mi.al.lohmann@gmail.com>
* cp/apply-core-filemode (2023-12-26) 3 commits
- apply: code simplification
- apply: correctly reverse patch's pre- and post-image mode bits
- apply: ignore working tree filemode when !core.filemode
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
Needs review.
source: <20231226233218.472054-1-gitster@pobox.com>
* jc/archive-list-with-extra-args (2023-12-21) 1 commit
(merged to 'next' on 2023-12-28 at 2d5c20e67f)
+ archive: "--list" does not take further options
"git archive --list extra garbage" silently ignored excess command
line parameters, which has been corrected.
Will merge to 'master'.
source: <xmqqmsu3mnix.fsf@gitster.g>
* jk/t1006-cat-file-objectsize-disk (2023-12-21) 1 commit
(merged to 'next' on 2023-12-28 at d82812e636)
+ t1006: add tests for %(objectsize:disk)
Test update.
Will merge to 'master'.
source: <20231221094722.GA570888@coredump.intra.peff.net>
* js/contributor-docs-updates (2023-12-27) 9 commits
(merged to 'next' on 2024-01-02 at 0e072117cd)
+ SubmittingPatches: hyphenate non-ASCII
+ SubmittingPatches: clarify GitHub artifact format
+ SubmittingPatches: clarify GitHub visual
+ SubmittingPatches: provide tag naming advice
+ SubmittingPatches: update extra tags list
+ SubmittingPatches: discourage new trailers
+ SubmittingPatches: drop ref to "What's in git.git"
+ CodingGuidelines: write punctuation marks
+ CodingGuidelines: move period inside parentheses
Doc update.
Will merge to 'master'.
source: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
* al/unit-test-ctype (2024-01-02) 1 commit
- unit-tests: rewrite t/helper/test-ctype.c as a unit test
Move test-ctype helper to the unit-test framework.
source: <20240101104017.9452-2-ach.lumap@gmail.com>
* bk/bisect-doc-fix (2023-12-27) 1 commit
- doc: use singular form of repeatable path arg
Synopsis fix.
Expecting a reroll.
source: <3d46bca1-96d4-43ba-a912-1f7c76942287@smtp-relay.sendinblue.com>
* en/sparse-checkout-eoo (2023-12-26) 2 commits
(merged to 'next' on 2023-12-28 at 3ddf2ebab6)
+ sparse-checkout: be consistent with end of options markers
+ Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Will merge to 'master'.
source: <pull.1625.v2.git.git.1703619562639.gitgitgadget@gmail.com>
* ja/doc-placeholders-fix (2023-12-26) 2 commits
- doc: enforce placeholders in documentation
- doc: enforce dashes in placeholders
Docfix.
Needs review.
source: <pull.1626.git.1703539287.gitgitgadget@gmail.com>
* jc/sparse-checkout-set-default-fix (2023-12-26) 1 commit
(merged to 'next' on 2023-12-28 at a558eccf8e)
+ sparse-checkout: use default patterns for 'set' only !stdin
"git sparse-checkout set" added default patterns even when the
patterns are being fed from the standard input, which has been
corrected.
Will merge to 'master'.
source: <20231221065925.3234048-3-gitster@pobox.com>
* rs/fast-import-simplify-mempool-allocation (2023-12-26) 1 commit
(merged to 'next' on 2023-12-28 at 16e6dd2a69)
+ fast-import: use mem_pool_calloc()
Code simplification.
Will merge to 'master'.
source: <50c1f410-ca37-4c1c-a28b-3e9fad49f2b4@web.de>
* rs/mem-pool-improvements (2023-12-28) 2 commits
(merged to 'next' on 2023-12-28 at aa03d9441c)
+ mem-pool: simplify alignment calculation
+ mem-pool: fix big allocations
MemPool allocator fixes.
Will merge to 'master'.
source: <fa89d269-1a23-4ed6-bebc-30c0b629f444@web.de>
* ps/refstorage-extension (2024-01-02) 13 commits
- t9500: write "extensions.refstorage" into config
- builtin/clone: introduce `--ref-format=` value flag
- builtin/init: introduce `--ref-format=` value flag
- builtin/rev-parse: introduce `--show-ref-format` flag
- t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
- setup: introduce GIT_DEFAULT_REF_FORMAT envvar
- setup: introduce "extensions.refStorage" extension
- setup: set repository's formats on init
- setup: start tracking ref storage format
- refs: refactor logic to look up storage backends
- worktree: skip reading HEAD when repairing worktrees
- t: introduce DEFAULT_REPO_FORMAT prereq
- Merge branch 'ps/clone-into-reftable-repository' into ps/refstorage-extension
Introduce a new extension "refstorage" so that we can mark a
repository that uses a non-default ref backend, like reftable.
Will merge to 'next'?
source: <cover.1703833818.git.ps@pks.im>
* ps/reftable-fixes-and-optims (2023-12-28) 9 commits
- reftable/merged: transfer ownership of records when iterating
- reftable/merged: really reuse buffers to compute record keys
- reftable/record: store "val2" hashes as static arrays
- reftable/record: store "val1" hashes as static arrays
- reftable/record: constify some parts of the interface
- reftable/writer: fix index corruption when writing multiple indices
- reftable/stack: do not auto-compact twice in `reftable_stack_add()`
- reftable/stack: do not overwrite errors when compacting
- Merge branch 'ps/reftable-fixes' into ps/reftable-fixes-and-optims
More fixes and optimizations to the reftable backend.
Expecting a (hopefully minor and final) reroll.
cf. <xmqqtto2gswa.fsf@gitster.g>
source: <cover.1703743174.git.ps@pks.im>
* tb/multi-pack-verbatim-reuse (2023-12-14) 26 commits
- t/perf: add performance tests for multi-pack reuse
- pack-bitmap: enable reuse from all bitmapped packs
- pack-objects: allow setting `pack.allowPackReuse` to "single"
- t/test-lib-functions.sh: implement `test_trace2_data` helper
- pack-objects: add tracing for various packfile metrics
- pack-bitmap: prepare to mark objects from multiple packs for reuse
- pack-revindex: implement `midx_pair_to_pack_pos()`
- pack-revindex: factor out `midx_key_to_pack_pos()` helper
- midx: implement `midx_preferred_pack()`
- git-compat-util.h: implement checked size_t to uint32_t conversion
- pack-objects: include number of packs reused in output
- pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuse
- pack-objects: prepare `write_reused_pack()` for multi-pack reuse
- pack-objects: pass `bitmapped_pack`'s to pack-reuse functions
- pack-objects: keep track of `pack_start` for each reuse pack
- pack-objects: parameterize pack-reuse routines over a single pack
- pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`
- pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signature
- ewah: implement `bitmap_is_empty()`
- pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions
- midx: implement `midx_locate_pack()`
- midx: implement `BTMP` chunk
- midx: factor out `fill_pack_info()`
- pack-bitmap: plug leak in find_objects()
- pack-bitmap-write: deep-clear the `bb_commit` slab
- pack-objects: free packing_data in more places
Streaming spans of packfile data used to be done only from a
single, primary, pack in a repository with multiple packfiles. It
has been extended to allow reuse from other packfiles, too.
Will merge to 'next'?
cf. <ZXurD1NTZ4TAs7WZ@nand.local>
source: <cover.1702592603.git.me@ttaylorr.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* en/header-cleanup (2023-12-26) 12 commits
(merged to 'next' on 2023-12-28 at 1ccddc2a10)
+ treewide: remove unnecessary includes in source files
+ treewide: add direct includes currently only pulled in transitively
+ trace2/tr2_tls.h: remove unnecessary include
+ submodule-config.h: remove unnecessary include
+ pkt-line.h: remove unnecessary include
+ line-log.h: remove unnecessary include
+ http.h: remove unnecessary include
+ fsmonitor--daemon.h: remove unnecessary includes
+ blame.h: remove unnecessary includes
+ archive.h: remove unnecessary include
+ treewide: remove unnecessary includes in source files
+ treewide: remove unnecessary includes from header files
Remove unused header "#include".
Will merge to 'master'.
source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>
* jw/builtin-objectmode-attr (2023-12-28) 1 commit
(merged to 'next' on 2024-01-02 at 4c3784b3a1)
+ attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
Will merge to 'master'.
cf. <xmqq5y0ssknj.fsf@gitster.g>
source: <20231116054437.2343549-1-jojwang@google.com>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Expecting a reroll.
cf. <20231023202212.GA5470@szeder.dev>
source: <cover.1697653929.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-12-14) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1702562879.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-12-18) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Will merge to 'next'?
source: <cover.1702823801.git.zhiyou.jx@alibaba-inc.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
--------------------------------------------------
[Discarded]
* jc/sparse-checkout-eoo (2023-12-21) 5 commits
. sparse-checkout: tighten add_patterns_from_input()
. sparse-checkout: use default patterns for 'set' only !stdin
. SQUASH??? end-of-options test
. sparse-checkout: take care of "--end-of-options" in set/add/check-rules
+ Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Superseded by the en/sparse-checkout-eoo topic.
source: <20231221065925.3234048-1-gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH V4 1/1] Replace SID with domain/username
From: Junio C Hamano @ 2024-01-03 0:43 UTC (permalink / raw)
To: Sören Krecker, Johannes Schindelin; +Cc: git, sunshine
In-Reply-To: <20240102191514.2583-2-soekkle@freenet.de>
Sören Krecker <soekkle@freenet.de> writes:
> Replace SID with domain/username in error message, if owner of repository
> and user are not equal on windows systems. Each user should have a unique
> SID (https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-identifiers#what-are-security-identifiers).
That paragraph your URL refers to does say that a SID that is used
for an account will never be reused to identify a different account.
But I am not sure if it means a user will never be assigned more
than one SID (in other words, the reverse is not necessarily true).
The paragraph also mentions that a SID can identify a non-user
entity like a computer account (as opposed to "a user account")---I
do not know what its implications are in the context of this patch,
though.
> This means that domain/username is not a loss of information.
This statement does not (grammatically) make sense, but more
importantly, loss of information may not be a bad thing in this
case. If more than one SIDs are given to a user account and
processes working for that account, these different SIDs may be
translated, by using LookupAccountSidA(), to the same string for a
single user@domain, and it would be an operation that loses
information in that sense.
But if what we *care* about is user@domain between the current
process and the owner of the directory in question being the same
(or not), then such a loss of information is a *good* thing.
So I dunno. Arguing what we care about (is that exact SID equality
between the "owner of the directory" and the "user, which the
current process is working on behalf of", or do we care about the
equality of the "accounts"?) may be a better way to justify this
change, if you ask me.
> +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
> +{
> + SID_NAME_USE pe_use;
> + DWORD len_user = 0, len_domain = 0;
> + BOOL translate_sid_to_user;
> +
> + /* returns only FALSE, because the string pointers are NULL*/
> + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
> + &pe_use);
> + /*Alloc needed space of the strings*/
> + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
> + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
> + *str, &len_domain, &pe_use);
> + if (translate_sid_to_user == FALSE) {
> + FREE_AND_NULL(*str);
> + }
Style: do not enclose a single-statement block inside {}.
> + else
> + (*str)[len_domain] = '/';
> + return translate_sid_to_user;
> +}
> @@ -2767,7 +2788,9 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
> } else if (report) {
> LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
>
> - if (ConvertSidToStringSidA(sid, &str1))
> + if (user_sid_to_user_name(sid, &str1))
> + to_free1 = str1;
> + else if (ConvertSidToStringSidA(sid, &str1))
> to_free1 = str1;
Do these two helper functions return pointers pointing into the same
kind of memory that you can free with the same function? That is ...
> ...
> "'%s' is owned by:\n"
> "\t'%s'\nbut the current user is:\n"
> "\t'%s'\n", path, str1, str2);
> - LocalFree(to_free1);
> - LocalFree(to_free2);
> + free(to_free1);
> + free(to_free2);
... the original code seems to say that the piece of memory we
obtain from ConvertSidToStringSidA() must not be freed by calling
free() but use something special called LocalFree(). I am assuing
that your user_sid_to_user_name() returns a regular piece of memory
that can be freed by calling regular free()? Do we need to keep
track of where we got the memory from and use different function to
free each variable, or something (again I do not do Windows so I'll
defer all of these to Dscho, who is CC'ed this time).
Thanks and a happy new year.
> }
> }
^ permalink raw reply
* Re: [PATCH] sideband.c: replace int with size_t for clarity
From: Taylor Blau @ 2024-01-02 22:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Chandra Pratap via GitGitGadget, git,
Chandra Pratap, Chandra Pratap
In-Reply-To: <xmqqedfejc32.fsf@gitster.g>
On Fri, Dec 22, 2023 at 11:01:37AM -0800, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
> Just this part.
>
> > Further down, we read
> > for (i = 0; i < ARRAY_SIZE(keywords); i++) {
> >
> > However, a size of an array can never be negative, so that
> > an unsigned data type is a better choice than a signed.
> > And, arrays can have more elements than an int can address,
> > at least in theory.
> > For a reader it makes more sense, to replace
> > int i;
> > with
> > size_t i;
>
> It is a very good discipline to use size_t to index into an array
> whose size is externally controled (e.g., we slurp what the end user
> or the server on the other end of the connection gave us into a
> piece of memory we allocate) to avoid integer overflows as "int" is
> often narrower than "size_t". But this particular one is a Meh; the
> keywords[] is a small hardcoded array whose size and contents are
> totally under our control.
I certainly agree in theory, though I've always erred on the side of
always using size_t for indexing into arrays, even if they're small. It
removes a potential pitfall if you are working with an
externally-controlled array and happen to forget to use size_t.
But if there is an existing index variable with type "int", and we can
easily validate that it's small, I probably wouldn't bother changing it
if I was editing nearby code.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2] mem-pool: fix big allocations
From: Taylor Blau @ 2024-01-02 22:29 UTC (permalink / raw)
To: René Scharfe
Cc: Git List, Jameson Miller, Phillip Wood, Elijah Newren,
Junio C Hamano
In-Reply-To: <1c39c0e7-05b2-4726-a90c-f78df4356a41@web.de>
Hi René,
On Thu, Dec 28, 2023 at 08:19:06PM +0100, René Scharfe wrote:
> Changes since v1:
> - simply use check() instead of a custom check_ptr() macro
> - drop unnecessary comparison of next_free and end pointers
Great find and fix. It's nice to see some examples of the testing
framework being used. I'll have to play around with it sometime :-).
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] git-clone.txt: document -4 and -6
From: Taylor Blau @ 2024-01-02 22:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Wilk, git, Eric Wong
In-Reply-To: <xmqqr0izcrfm.fsf@gitster.g>
On Tue, Jan 02, 2024 at 02:15:57PM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > Note that the 'clone' and 'fetch' versions for many of these options
> > have different wording. For example, in Documentation/git-clone.txt we
> > have:
> >
> > -j::
> > --jobs=<n>::
> > Number of parallel children to be used for all forms of fetching.
> >
> > Whereas the description in the original fetch-options.txt is more
> > verbose.
>
> Yes, so it will be impossible to unify without changing the
> resulting text. But unless one description is clearly better for
> one subcommand while the other description is also clearly better
> for the other subcommand, we should be able to pick a better one
> that would serve both subcommands, and that way we would improve
> description for one subcommand while keeping the other one the same,
> right?
Right. I meant to illustrate merely that this would probably involve
some word-smithing instead of just pure cut-and-paste, so that it may
not be worth the effort.
Thanks,
Taylor
^ permalink raw reply
* Re: [BUG] Asks for "To" even if "To" already specified in letter
From: Taylor Blau @ 2024-01-02 22:23 UTC (permalink / raw)
To: Askar Safin; +Cc: git
In-Reply-To: <CAPnZJGBbA1VLAdP5mZnbF77-x-87JjU3Ku2MhrQu0SFoJL7ggw@mail.gmail.com>
On Sat, Dec 30, 2023 at 06:20:43AM +0300, Askar Safin wrote:
> Hi. I found a bug. Steps to reproduce:
> - Create file /tmp/m with following text:
> ===
> Subject: subj
> To: example@example.com
>
> text
> ===
> - Send it using command "git send-email /tmp/m"
>
> You will see that git asks for "To". git says: "To whom should the
> emails be sent (if anyone)?"
> I don't like this. git should just use "To" from /tmp/m without asking.
>
> Seen with git 2.43.0.
>
> If I execute "git send-email --to-cmd='#' /tmp/m" or
> "git send-email --to-cmd=':' /tmp/m" or
> "git send-email --to-cmd='true' /tmp/m", then "To" is not asked.
I was going to suggest that you use the `--to-cmd=true` trick. I'm
definitely not an expert in the send-email code, but from a cursory
look, I think that this is do-able.
One thing you could do is read all of the messages ahead of time, parse
their headers, and then extract any "To:" headers that you find. This is
pretty similar to what the pre_process_file() function is already doing.
But this might not be the right approach, since FIFOs can only be read
once, and we already have some logic to handle FIFOs specially (see
3c8d3adeae (send-email: export patch counters in validate environment,
2023-04-14)).
But I think that something much simpler would work, which to avoid
asking for a "To:" value altogether, even if one isn't provided. If you
did something like:
--- 8< ---
diff --git a/git-send-email.perl b/git-send-email.perl
index 821b2b3a13..2941278315 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1018,13 +1018,6 @@ sub file_declares_8bit_cte {
my $to_whom = __("To whom should the emails be sent (if anyone)?");
my $prompting = 0;
-if (!@initial_to && !defined $to_cmd) {
- my $to = ask("$to_whom ",
- default => "",
- valid_re => qr/\@.*\./, confirm_only => 1);
- push @initial_to, parse_address_line($to) if defined $to; # sanitized/validated later
- $prompting++;
-}
sub expand_aliases {
return map { expand_one_alias($_) } @_;
--- >8 ---
I think that would more or less do the trick. send-email will happily
continue on even without an initial $to value, and validate it later
when it's actually needed.
I'm not familiar enough with the code to know if this is the right
approach, so hopefully some other send-email experts can chime in and
let me know if I'm on the right track ;-).
Thanks,
Taylor
^ permalink raw reply related
* Re: [PATCH] git-clone.txt: document -4 and -6
From: Junio C Hamano @ 2024-01-02 22:15 UTC (permalink / raw)
To: Taylor Blau; +Cc: Jakub Wilk, git, Eric Wong
In-Reply-To: <ZZRzxZNb2Aq+2feW@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> Note that the 'clone' and 'fetch' versions for many of these options
> have different wording. For example, in Documentation/git-clone.txt we
> have:
>
> -j::
> --jobs=<n>::
> Number of parallel children to be used for all forms of fetching.
>
> Whereas the description in the original fetch-options.txt is more
> verbose.
Yes, so it will be impossible to unify without changing the
resulting text. But unless one description is clearly better for
one subcommand while the other description is also clearly better
for the other subcommand, we should be able to pick a better one
that would serve both subcommands, and that way we would improve
description for one subcommand while keeping the other one the same,
right?
> In fact, the story is even more complicated than that, since even though
> the 'push' builtin would benefit from having a shared source of
> documentation for the --ipv4 and --ipv6 options, 'push' does not have a
> --jobs option.
Sure, it won't be just "write what is shared across all the transfer
commands in a single file and include it from all". The direction
of transfer is a reason why the options may differ, of course, so we
may need to have two (or three) include files if we want to go that
route.
> --progress could be shared, as could --server-option, and the two
> --ipv4/6 options. But the number of nested ifdefs necessary to share the
> other options probably dose not justify the effort to do so.
^ permalink raw reply
* [GSOC][RFC] Heed core.bare from template config file when no command line override given, as a microproject.
From: Ghanshyam Thakkar @ 2024-01-02 22:07 UTC (permalink / raw)
To: git; +Cc: newren, gitster, johannes.schindelin
Hello,
I'm currently an undergrad beginning my journey of contributing to the
Git project. I am seeking feedback on doing "Heed core.bare from
template config file when no command line override given" described
here
https://lore.kernel.org/git/5b39c530f2a0edf3b1492fa13a1132d622a0678e.1684218850.git.gitgitgadget@gmail.com/
by Elijah Newren, as a microproject. I would like to know from the
community, if the complexity and scope of the project is appropriate
for a microproject.
Approach:
As described by Elijah in commit message that fixing this cannot be
done in the create_default_files() function as it occurs too late.
This is because both clone and init have different checks and steps
for working with bare flag, like clone creates a new directory
[name].git and sets the GIT_DIR_ENVIRONMENT to it (when not provided
with an explicit dir name arg), while init sets the
GIT_DIR_ENVIRONMENT to the current working directory (when not
provided with an dir name arg). Also there are other steps like
setting no_checkout in a bare repository in builtin/clone.c. These are
all command specific steps which occur in builtin/clone.c and
builtin/init-db.c ,before we ever hit the TODO comment via
[builtin/clone.c]cmd_clone()->[setup.c]init_db()->
[setup.c]create_default_files().
Therefore, rather than centralizing the code in setup.c and adding a
bunch of if-else statements to handle different command specific
scenarios related to bare option, I propose to add a check for
template file config just after parsing of the flags and args in
builtin/init-db.c and builtin/clone.c.
e.g. in builtin/init-db.c :
static int template_bare_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if(!strcmp(var,"core.bare")) {
is_bare_repository_cfg = git_config_bool(var, value);
}
return 0;
}
int cmd_init_db(int argc, const char **argv, const char *prefix)
{
...
...
if(is_bare_repository_cfg==-1) {
if(!template_dir)
git_config_get_pathname("init.templateDir",
&template_dir);
if(template_dir) {
const char* template_config_path
= xstrfmt("%s/config",
struct stat st;
if(!stat(template_config_path, &st) &&
!S_ISDIR(st.st_mode)) {
git_config_from_file(template_bare_cfg,
template_config_path, NULL);
}
}
...
...
return init_db(git_dir, real_git_dir, template_dir, hash_algo,
initial_branch, init_shared_repository, flags);
}
I also wanted to know if the global config files should have an effect
in deciding if the repo is bare or not.
Curious to know your thoughts on, if this is the right approach or
does it require doing refactoring to bring all the logic in setup.c.
Based on your feedback, I can quickly send a patch.
p.s.: Apologies for the weird indenting, due to 70 character limit.
consider it just a prototype.
Thanks!
^ permalink raw reply
* Re: [PATCH] git-clone.txt: document -4 and -6
From: Taylor Blau @ 2024-01-02 20:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Wilk, git, Eric Wong
In-Reply-To: <ZZRqgmDycyAXCrGZ@nand.local>
On Tue, Jan 02, 2024 at 02:56:50PM -0500, Taylor Blau wrote:
> It turned up the following results:
>
> -a
> --depth
> --shallow-since
> --shallow-exclude
> --no-tags
> --recurse-submodules
> -j, --jobs
> -u, --upload-pack
> -q, --quiet
> -v, --verbose
> --progress
> -o, --server-option
Hmm. I think the story is a little more complicated. Just looking at the
ones that we agree on:
* -j, --jobs
* -u, --upload-pack
* --progress
* -o, --server-option
* --[ipv]4
* --[ipv]6
Note that the 'clone' and 'fetch' versions for many of these options
have different wording. For example, in Documentation/git-clone.txt we
have:
-j::
--jobs=<n>::
Number of parallel children to be used for all forms of fetching.
Whereas the description in the original fetch-options.txt is more
verbose.
In fact, the story is even more complicated than that, since even though
the 'push' builtin would benefit from having a shared source of
documentation for the --ipv4 and --ipv6 options, 'push' does not have a
--jobs option.
'clone', 'fetch', and 'pull' all share an '--upload-pack' option as you
note, though 'push' naturally doesn't (so we would need another ifdef
there, too). But the instances in 'clone', 'fetch', and 'pull' all
differ in their wording (e.g., the 'clone' documentation says "the
repository to clone from ..." but the others say "the repository to
fetch from ...").
--progress could be shared, as could --server-option, and the two
--ipv4/6 options. But the number of nested ifdefs necessary to share the
other options probably dose not justify the effort to do so.
Thanks,
Taylor
^ permalink raw reply
* [RFC PATCH 5/6] completion: recognize but do not complete 'view'
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
Completing it might annoy some existing users by creating completion
ambiguity on 'v' and 'vi' without adding anything useful in terms of
interface discovery/recall (because 'view' is just an alias anyway).
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a09598c5c1..3bb790220a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1589,13 +1589,16 @@ _git_bisect ()
term_good=`__git bisect terms --term-good`
fi
- local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
+ # We want to recognize 'view' but not complete it, because it overlaps
+ # with 'visualize' too much and is just an alias for it.
+ local completable_subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
+ local all_subcommands="$completable_subcommands view"
- local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local subcommand="$(__git_find_on_cmdline "$all_subcommands")"
if [ -z "$subcommand" ]; then
if [ -f "$__git_repo_path"/BISECT_START ]; then
- __gitcomp "$subcommands"
+ __gitcomp "$completable_subcommands"
else
__gitcomp "replay start"
fi
@@ -1613,7 +1616,7 @@ _git_bisect ()
;;
esac
;;
- visualize)
+ visualize|view)
case "$cur" in
-*)
__git_complete_log_opts
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 6/6] completion: add comment
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3bb790220a..7f9a626e1b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1589,8 +1589,14 @@ _git_bisect ()
term_good=`__git bisect terms --term-good`
fi
+ # We will complete any custom terms, but still always complete the
+ # more usual bad/new/good/old because git bisect gives a good error
+ # message if these are given when not in use and that's better than
+ # silent refusal to complete if the user is confused.
+ #
# We want to recognize 'view' but not complete it, because it overlaps
# with 'visualize' too much and is just an alias for it.
+ #
local completable_subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
local all_subcommands="$completable_subcommands view"
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 3/6] completion: move to maintain define-before-use
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 265 ++++++++++++-------------
1 file changed, 132 insertions(+), 133 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3472fab514..4940ad3e24 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1445,6 +1445,138 @@ _git_archive ()
__git_complete_file
}
+# Options that go well for log, shortlog and gitk
+__git_log_common_options="
+ --not --all
+ --branches --tags --remotes
+ --first-parent --merges --no-merges
+ --max-count=
+ --max-age= --since= --after=
+ --min-age= --until= --before=
+ --min-parents= --max-parents=
+ --no-min-parents --no-max-parents
+"
+# Options that go well for log and gitk (not shortlog)
+__git_log_gitk_options="
+ --dense --sparse --full-history
+ --simplify-merges --simplify-by-decoration
+ --left-right --notes --no-notes
+"
+# Options that go well for log and shortlog (not gitk)
+__git_log_shortlog_options="
+ --author= --committer= --grep=
+ --all-match --invert-grep
+"
+# Options accepted by log and show
+__git_log_show_options="
+ --diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
+"
+
+__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
+
+__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
+__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
+
+# Find only porcelain (i.e. not git-rev-list) option (not argument) and
+# selected option argument completions for git-log options and put them in
+# COMPREPLY.
+__git_complete_log_opts ()
+{
+ local merge=""
+ if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
+ merge="--merge"
+ fi
+ case "$prev,$cur" in
+ -L,:*:*)
+ return # fall back to Bash filename completion
+ ;;
+ -L,:*)
+ __git_complete_symbol --cur="${cur#:}" --sfx=":"
+ return
+ ;;
+ -G,*|-S,*)
+ __git_complete_symbol
+ return
+ ;;
+ esac
+ case "$cur" in
+ --pretty=*|--format=*)
+ __gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
+ " "" "${cur#*=}"
+ return
+ ;;
+ --date=*)
+ __gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
+ return
+ ;;
+ --decorate=*)
+ __gitcomp "full short no" "" "${cur##--decorate=}"
+ return
+ ;;
+ --diff-algorithm=*)
+ __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+ return
+ ;;
+ --submodule=*)
+ __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
+ return
+ ;;
+ --ws-error-highlight=*)
+ __gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
+ return
+ ;;
+ --no-walk=*)
+ __gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
+ return
+ ;;
+ --diff-merges=*)
+ __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
+ return
+ ;;
+ --*)
+ __gitcomp "
+ $__git_log_common_options
+ $__git_log_shortlog_options
+ $__git_log_gitk_options
+ $__git_log_show_options
+ --root --topo-order --date-order --reverse
+ --follow --full-diff
+ --abbrev-commit --no-abbrev-commit --abbrev=
+ --relative-date --date=
+ --pretty= --format= --oneline
+ --show-signature
+ --cherry-mark
+ --cherry-pick
+ --graph
+ --decorate --decorate= --no-decorate
+ --walk-reflogs
+ --no-walk --no-walk= --do-walk
+ --parents --children
+ --expand-tabs --expand-tabs= --no-expand-tabs
+ $merge
+ $__git_diff_common_options
+ "
+ return
+ ;;
+ -L:*:*)
+ return # fall back to Bash filename completion
+ ;;
+ -L:*)
+ __git_complete_symbol --cur="${cur#-L:}" --sfx=":"
+ return
+ ;;
+ -G*)
+ __git_complete_symbol --pfx="-G" --cur="${cur#-G}"
+ return
+ ;;
+ -S*)
+ __git_complete_symbol --pfx="-S" --cur="${cur#-S}"
+ return
+ ;;
+ esac
+
+}
+
_git_bisect ()
{
__git_has_doubledash && return
@@ -2052,139 +2184,6 @@ _git_ls_tree ()
__git_complete_file
}
-# Options that go well for log, shortlog and gitk
-__git_log_common_options="
- --not --all
- --branches --tags --remotes
- --first-parent --merges --no-merges
- --max-count=
- --max-age= --since= --after=
- --min-age= --until= --before=
- --min-parents= --max-parents=
- --no-min-parents --no-max-parents
-"
-# Options that go well for log and gitk (not shortlog)
-__git_log_gitk_options="
- --dense --sparse --full-history
- --simplify-merges --simplify-by-decoration
- --left-right --notes --no-notes
-"
-# Options that go well for log and shortlog (not gitk)
-__git_log_shortlog_options="
- --author= --committer= --grep=
- --all-match --invert-grep
-"
-# Options accepted by log and show
-__git_log_show_options="
- --diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
-"
-
-__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-
-__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
-__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
-
-
-# Find only porcelain (i.e. not git-rev-list) option (not argument) and
-# selected option argument completions for git-log options and put them in
-# COMPREPLY.
-__git_complete_log_opts ()
-{
- local merge=""
- if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
- merge="--merge"
- fi
- case "$prev,$cur" in
- -L,:*:*)
- return # fall back to Bash filename completion
- ;;
- -L,:*)
- __git_complete_symbol --cur="${cur#:}" --sfx=":"
- return
- ;;
- -G,*|-S,*)
- __git_complete_symbol
- return
- ;;
- esac
- case "$cur" in
- --pretty=*|--format=*)
- __gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
- " "" "${cur#*=}"
- return
- ;;
- --date=*)
- __gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
- return
- ;;
- --decorate=*)
- __gitcomp "full short no" "" "${cur##--decorate=}"
- return
- ;;
- --diff-algorithm=*)
- __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
- return
- ;;
- --submodule=*)
- __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
- return
- ;;
- --ws-error-highlight=*)
- __gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
- return
- ;;
- --no-walk=*)
- __gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
- return
- ;;
- --diff-merges=*)
- __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
- return
- ;;
- --*)
- __gitcomp "
- $__git_log_common_options
- $__git_log_shortlog_options
- $__git_log_gitk_options
- $__git_log_show_options
- --root --topo-order --date-order --reverse
- --follow --full-diff
- --abbrev-commit --no-abbrev-commit --abbrev=
- --relative-date --date=
- --pretty= --format= --oneline
- --show-signature
- --cherry-mark
- --cherry-pick
- --graph
- --decorate --decorate= --no-decorate
- --walk-reflogs
- --no-walk --no-walk= --do-walk
- --parents --children
- --expand-tabs --expand-tabs= --no-expand-tabs
- $merge
- $__git_diff_common_options
- "
- return
- ;;
- -L:*:*)
- return # fall back to Bash filename completion
- ;;
- -L:*)
- __git_complete_symbol --cur="${cur#-L:}" --sfx=":"
- return
- ;;
- -G*)
- __git_complete_symbol --pfx="-G" --cur="${cur#-G}"
- return
- ;;
- -S*)
- __git_complete_symbol --pfx="-S" --cur="${cur#-S}"
- return
- ;;
- esac
-
-}
-
_git_log ()
{
__git_has_doubledash && return
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 4/6] completion: custom git-bisect terms
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4940ad3e24..a09598c5c1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1581,10 +1581,19 @@ _git_bisect ()
{
__git_has_doubledash && return
- local subcommands="start bad new good old terms skip reset visualize replay log run help"
+ __git_find_repo_path
+
+ local term_bad term_good
+ if [ -f "$__git_repo_path"/BISECT_START ]; then
+ term_bad=`__git bisect terms --term-bad`
+ term_good=`__git bisect terms --term-good`
+ fi
+
+ local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
+
local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
if [ -z "$subcommand" ]; then
- __git_find_repo_path
if [ -f "$__git_repo_path"/BISECT_START ]; then
__gitcomp "$subcommands"
else
@@ -1617,7 +1626,7 @@ _git_bisect ()
esac
case "$subcommand" in
- bad|new|good|old|reset|skip|start)
+ bad|new|"$term_bad"|good|old|"$term_good"|reset|skip|start)
__git_complete_refs
;;
*)
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 2/6] completion: git-log opts to bisect visualize
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
To do this the majority of _git_log has been factored out into the new
__git_complete_log_opts. This is needed because the visualize command
accepts git-log options but not rev arguments (they are fixed to the
commits under bisection).
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 30 ++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 15d22ff7d9..3472fab514 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1472,6 +1472,16 @@ _git_bisect ()
;;
esac
;;
+ visualize)
+ case "$cur" in
+ -*)
+ __git_complete_log_opts
+ return
+ ;;
+ *)
+ ;;
+ esac
+ ;;
esac
case "$subcommand" in
@@ -2074,11 +2084,12 @@ __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-c
__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
-_git_log ()
-{
- __git_has_doubledash && return
- __git_find_repo_path
+# Find only porcelain (i.e. not git-rev-list) option (not argument) and
+# selected option argument completions for git-log options and put them in
+# COMPREPLY.
+__git_complete_log_opts ()
+{
local merge=""
if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
merge="--merge"
@@ -2171,6 +2182,17 @@ _git_log ()
return
;;
esac
+
+}
+
+_git_log ()
+{
+ __git_has_doubledash && return
+ __git_find_repo_path
+
+ __git_complete_log_opts
+ [ -z "$COMPREPLY" ] || return
+
__git_complete_revlist
}
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 1/6] completion: complete new old actions, start opts
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240102195744.478503-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..15d22ff7d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
{
__git_has_doubledash && return
- local subcommands="start bad good skip reset visualize replay log run"
+ local subcommands="start bad new good old terms skip reset visualize replay log run help"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_find_repo_path
@@ -1462,7 +1462,20 @@ _git_bisect ()
fi
case "$subcommand" in
- bad|good|reset|skip|start)
+ start)
+ case "$cur" in
+ --*)
+ __gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
+ return
+ ;;
+ *)
+ ;;
+ esac
+ ;;
+ esac
+
+ case "$subcommand" in
+ bad|new|good|old|reset|skip|start)
__git_complete_refs
;;
*)
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 0/6] completion: improvements for git-bisect
From: Britton Leo Kerin @ 2024-01-02 19:57 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
Bring completion for bisect up to date with current features.
This is RFC mainly because of an implementation issue in patch 2: I think
the mechanism of requiring COMPREPLY to be empty, possibly writing into it,
then checking it post-call is too fragile, but I'm not sure how people would
like to fix this. I'd prefer to just add an assert() of some sort to enforce
the empty-COMPREPLY precondition, rather than rewrite the guts of the former
_git_log() function to use some other intermediate return mechanism (this
would be overkill in this simple context IMO). But I'm not sure how to
do that in a completion context: for sure nothing that is done here should
ever have a chance of showing garbage on a user's screen, but of course for
assert() to be useful a dev would need to see it somehow. Suggestions welcome.
Other than that issue I think it's all reasonable and ready, though there
are a couple other decisions that I guess people might disagree with:
* good/old/new/bad terms are always completed (even if they're
invalild because --term-(good|bad) have been used). The idea here
is that if the user has become confused about their own terms it's
better to let git give them an error message than to have
normally-working completion not happen.
* 'view' is recognized as a subcommand, but not as a completion
candidate. This lets complete of git bisect v<TAB> keep working which
seems convenient and some poor person somewhere probably really wants :)
view is just an alias so loss of interface recall is minimal.
Britton Leo Kerin (6):
completion: complete new old actions, start opts
completion: git-log opts to bisect visualize
completion: move to maintain define-before-use
completion: custom git-bisect terms
completion: recognize but do not complete 'view'
completion: add comment
contrib/completion/git-completion.bash | 310 +++++++++++++++----------
1 file changed, 181 insertions(+), 129 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.43.0
^ permalink raw reply
* Re: [PATCH] git-clone.txt: document -4 and -6
From: Taylor Blau @ 2024-01-02 19:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Wilk, git, Eric Wong
In-Reply-To: <xmqq1qivd8d0.fsf@gitster.g>
On Thu, Jun 01, 2023 at 03:06:35PM +0900, Junio C Hamano wrote:
> Jakub Wilk <jwilk@jwilk.net> writes:
>
> > These options were added in c915f11eb4 ("connect & http: support -4 and
> > -6 switches for remote operations").
> >
> > Signed-off-by: Jakub Wilk <jwilk@jwilk.net>
> > ---
> > Documentation/git-clone.txt | 8 ++++++++
> > 1 file changed, 8 insertions(+)
>
> The patch is not _wrong_ per-se, but there are other options that
> are common among the "fetch" family of commands. I counted at least
> these should be shared between "fetch" and "clone", by splitting
> them out of "fetch-options.txt" into a new file, and including that
> new file from "fetch-options.txt" and "git-clone.txt". Those marked
> with (?) are described in different phrasing between "clone" and
> "fetch", and may fall into the "let's keep them separate, because
> they mean different things" category (later):
>
> * --jobs
> * --upload-pack
> * --quiet (?)
> * --verbose (?)
> * --progress
> * --server-option
> * --ipv[46]
>
> Note that these happen to share the same name, but to "clone" and
> "fetch" they different things, so leaving them separate is the right
> thing to do.
>
> * --no-tags
> * --recurse-submodules
I wrote this ugly shell incantation to find an exhaustive list of
potentially shareable options:
$ grep '^-.*::$' <Documentation/fetch-options.txt |
tr -d ':' | sed -e 's/\[=/=[/' -e 's/<[^>]*>//' |
grep -Eo '^[^=]+' | awk -F] '
/\[no-\]/ { print "--" $2; print "--no-" $2; next }
{ print $0 }
' |
while read arg
do
if grep -Fwq -- $arg Documentation/fetch-options.txt &&
grep -Fwq -- $arg Documentation/git-clone.txt
then
echo $arg;
fi
done
It turned up the following results:
-a
--depth
--shallow-since
--shallow-exclude
--no-tags
--recurse-submodules
-j, --jobs
-u, --upload-pack
-q, --quiet
-v, --verbose
--progress
-o, --server-option
-a is a false-positive (it comes from "you can simply run `git repack
-a`", which is in the clone documentation).
Even though depth, and the shallow options are shared by both fetch and
clone, they have different wording in each context, so they should be
kept separate.
I agree with your thinking that `--no-tags` and `--recurse-submodules`
should be kept separate.
That makes our two lists equal (modulo the --ipv[46] options, which were
previously not documented in git-clone(1) until this patch).
Thanks,
Taylor
^ permalink raw reply
* [PATCH V4 0/1] Replace SID with domain/username on Windows
From: Sören Krecker @ 2024-01-02 19:15 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <xmqqsf3feilq.fsf@gitster.g>
Hi everone,
I improve the commit message with information from Microsoft, fixes a memmory access error and the message in case of an error.
Vielen dank für die Hinweise von Junio C Hamano.
Sören Krecker (1):
Replace SID with domain/username
compat/mingw.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.39.2
^ permalink raw reply
* [PATCH V4 1/1] Replace SID with domain/username
From: Sören Krecker @ 2024-01-02 19:15 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <20240102191514.2583-1-soekkle@freenet.de>
Replace SID with domain/username in error message, if owner of repository
and user are not equal on windows systems. Each user should have a unique
SID (https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-identifiers#what-are-security-identifiers).
This means that domain/username is not a loss of information. If the translation
fails the message contains the SID as string.
Old Prompted error message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'S-1-5-21-571067702-4104414259-3379520149-500'
but the current user is:
'S-1-5-21-571067702-4104414259-3379520149-1001'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
New prompted error massage:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'DESKTOP-L78JVA6/Administrator'
but the current user is:
'DESKTOP-L78JVA6/test'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
compat/mingw.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 42053c1f65..bfd9573a29 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2684,6 +2684,27 @@ static PSID get_current_user_sid(void)
return result;
}
+static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
+{
+ SID_NAME_USE pe_use;
+ DWORD len_user = 0, len_domain = 0;
+ BOOL translate_sid_to_user;
+
+ /* returns only FALSE, because the string pointers are NULL*/
+ LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
+ &pe_use);
+ /*Alloc needed space of the strings*/
+ ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
+ translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
+ *str, &len_domain, &pe_use);
+ if (translate_sid_to_user == FALSE) {
+ FREE_AND_NULL(*str);
+ }
+ else
+ (*str)[len_domain] = '/';
+ return translate_sid_to_user;
+}
+
static int acls_supported(const char *path)
{
size_t offset = offset_1st_component(path);
@@ -2767,7 +2788,9 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
} else if (report) {
LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
- if (ConvertSidToStringSidA(sid, &str1))
+ if (user_sid_to_user_name(sid, &str1))
+ to_free1 = str1;
+ else if (ConvertSidToStringSidA(sid, &str1))
to_free1 = str1;
else
str1 = "(inconvertible)";
@@ -2776,7 +2799,10 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
str2 = "(none)";
else if (!IsValidSid(current_user_sid))
str2 = "(invalid)";
- else if (ConvertSidToStringSidA(current_user_sid, &str2))
+ else if (user_sid_to_user_name(current_user_sid, &str2))
+ to_free2 = str2;
+ else if (ConvertSidToStringSidA(current_user_sid,
+ &str2))
to_free2 = str2;
else
str2 = "(inconvertible)";
@@ -2784,8 +2810,8 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
"'%s' is owned by:\n"
"\t'%s'\nbut the current user is:\n"
"\t'%s'\n", path, str1, str2);
- LocalFree(to_free1);
- LocalFree(to_free2);
+ free(to_free1);
+ free(to_free2);
}
}
--
2.39.2
^ permalink raw reply related
* Re: [RFC PATCH] grep: default to posix digits with -P
From: Carlo Arenas @ 2024-01-02 19:02 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <55309061-5996-4f70-8e6b-b341fc632ddf@web.de>
On Mon, Jan 1, 2024 at 9:18 AM René Scharfe <l.s.r@web.de> wrote:
>
> Am 01.01.24 um 16:03 schrieb Carlo Marcelo Arenas Belón:
> > @@ -321,17 +327,22 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
> > }
> > options |= PCRE2_CASELESS;
> > }
> > - if (!opt->ignore_locale && is_utf8_locale() && !literal)
> > - options |= (PCRE2_UTF | PCRE2_UCP | PCRE2_MATCH_INVALID_UTF);
> > + if (!opt->ignore_locale && is_utf8_locale() && !literal) {
> > + options |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);
> >
> > -#ifndef GIT_PCRE2_VERSION_10_35_OR_HIGHER
> > - /*
> > - * Work around a JIT bug related to invalid Unicode character handling
> > - * fixed in 10.35:
> > - * https://github.com/PCRE2Project/pcre2/commit/c21bd977547d
> > - */
> > - options &= ~PCRE2_UCP;
> > +#ifdef GIT_PCRE2_VERSION_10_35_OR_HIGHER
> > + /*
> > + * Work around a JIT bug related to invalid Unicode character handling
> > + * fixed in 10.35:
> > + * https://github.com/PCRE2Project/pcre2/commit/c21bd977547d
> > + */
> > + options |= PCRE2_UCP;
> > +#ifdef GIT_PCRE2_VERSION_10_43_OR_HIGHER
> > + if (!opt->perl_digit)
> > + xoptions |= (PCRE2_EXTRA_ASCII_BSD | PCRE2_EXTRA_ASCII_DIGIT);
> > #endif
> > +#endif
>
> Why do we need that extra level of indentation?
I was just trying to simplify the code by including all the logic in
one single set.
The original lack of indentation that was introduced by later fixes to
the code was IMHO also misguided since the obvious "objective" as set
in the original code that added PCRE2_UCP was that it should be used
whenever UTF was also in use as shown by
acabd2048ee0ee53728100408970ab45a6dab65e.
Of course, we soon found out that the original implementation of
PCRE2_MATCH_INVALID_UTF that came with PCRE2 10.34 was buggy and so an
exception was introduced in 14b9a044798ebb3858a1f1a1377309a3d6054ac8.
Note that the problematic code is only relevant when JIT is also
enabled, but JIT is almost always enabled.
> The old code turned PCRE2_UCP on by default and turned it off for older
> versions. The new code enables PCRE2_UCP only for newer versions. The
> result should be the same, no? So why change that part at all?
Because it gets us a little closer to the real reason why we need to
disable UCP for anything older than 10.35, and that is that there is a
bug there that is ONLY relevant if we are using JIT.
My hope though is that with the release of 10.43 (currently in RC1),
10.34 will become irrelevant soon enough and this whole code could be
cleaned up further.
> But the comment is now off, isn't it? The workaround was turning
> PCRE2_UCP off for older versions (because those were broken), not
> turning it on for newer versions (because it would be required by some
> unfixed regression).
The comment was never correct, because it was turning it off, because
the combination of JIT + MATCH_INVALID_UTF (which was released in
10.34) + UCP is broken.
> Do we need to nest the checks for GIT_PCRE2_VERSION_10_35_OR_HIGHER and
> GIT_PCRE2_VERSION_10_43_OR_HIGHER? Keeping them separate would help
> keep the #ifdef parts as short as possible and maintainable
> independently.
I thought that nesting them would make it simpler to maintain, since
there are somehow connected anyway.
The additional flags that are added in PCRE 10.43 are ONLY needed and
useful on top of PCRE2_UCP, which itself only makes sense on top of
UTF.
> > @@ -27,13 +34,13 @@ do
> > if ! test_have_prereq PERF_GREP_ENGINES_THREADS
> > then
> > test_perf "grep -P '$pattern'" --prereq PCRE "
> > - git -P grep -f pat || :
> > + git grep -P -f pat || :
> > "
> > else
> > for threads in $GIT_PERF_GREP_THREADS
> > do
> > test_perf "grep -P '$pattern' with $threads threads" --prereq PTHREADS,PCRE "
> > - git -c grep.threads=$threads -P grep -f pat || :
> > + git -c grep.threads=$threads grep -P -f pat || :
>
> What's going on here? You remove the -P option of git (--no-pager) and
> add the -P option of git grep (--perl-regexp). So this perf test never
> tested PCRE despite its name? Seems worth a separate patch.
My original code was a dud. This would make that at least more obvious,
> Do the performance numbers in acabd2048e (grep: correctly identify utf-8
> characters with \{b,w} in -P, 2023-01-08) still hold up in that light?
FWIW the performance numbers still hold up, but just because I did the
tests independently using hyperfine, and indeed when I did the first
version of this patch, I had a nice reproducible description that
showed how to get 20% better performance while searching the git
repository itself for something like 4 digits (as used in Copyright
dates).
My idea was to reply to my own post with instructions on how to test
this, which basically require to also compile the recently released
10.43RC1 and build against that.
Indeed, AFAIK the test would fail if built with an older version of
PCRE, but wasn't sure if making a prerequisite that hardcoded the
version in test-tool might be the best approach to prevent that,
especially with all the libification work.
Carlo
PS. your reply got lost somehow in my SPAM folder, so I apologize for
the late reply
^ permalink raw reply
* Re: [PATCH] git-clone.txt: document -4 and -6
From: Jakub Wilk @ 2024-01-02 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <xmqq1qivd8d0.fsf@gitster.g>
* Junio C Hamano <gitster@pobox.com>, 2023-06-01 15:06:
>The patch is not _wrong_ per-se, but there are other options that are
>common among the "fetch" family of commands. I counted at least these
>should be shared between "fetch" and "clone", by splitting them out of
>"fetch-options.txt" into a new file, and including that new file from
>"fetch-options.txt" and "git-clone.txt".
Agreed such a split would be better, but I don't have energy to
implement it.
--
Jakub Wilk
^ permalink raw reply
* Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Taylor Blau @ 2024-01-02 18:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Achu Luma, christian.couder, git, Christian Couder
In-Reply-To: <xmqqsf3ohkew.fsf@gitster.g>
On Tue, Dec 26, 2023 at 10:45:59AM -0800, Junio C Hamano wrote:
> Achu Luma <ach.lumap@gmail.com> writes:
>
> > diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c
> > deleted file mode 100644
> > index e5659df40b..0000000000
> > --- a/t/helper/test-ctype.c
> > +++ /dev/null
> > @@ -1,70 +0,0 @@
> > -#include "test-tool.h"
> > -
> > -static int rc;
> > -
> > -static void report_error(const char *class, int ch)
> > -{
> > - printf("%s classifies char %d (0x%02x) wrongly\n", class, ch, ch);
> > - rc = 1;
> > -}
>
> So, if we have a is_foo() that characterises a byte that ought to
> be "foo" but gets miscategorised not to be "foo", we used to
> pinpoint exactly the byte value that was an issue. We did not do
> any early return ...
>
> > ...
> > -#define TEST_CLASS(t,s) { \
> > - int i; \
> > - for (i = 0; i < 256; i++) { \
> > - if (is_in(s, i) != t(i)) \
> > - report_error(#t, i); \
> > - } \
> > - if (t(EOF)) \
> > - report_error(#t, EOF); \
> > -}
>
> ... and reported for all errors in the "class".
>
> > diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
> > new file mode 100644
> > index 0000000000..41189ba9f9
> > --- /dev/null
> > +++ b/t/unit-tests/t-ctype.c
> > @@ -0,0 +1,76 @@
> > +#include "test-lib.h"
> > +
> > +static int is_in(const char *s, int ch)
> > +{
> > + /*
> > + * We can't find NUL using strchr. Accept it as the first
> > + * character in the spec -- there are no empty classes.
> > + */
> > + if (ch == '\0')
> > + return ch == *s;
> > + if (*s == '\0')
> > + s++;
> > + return !!strchr(s, ch);
> > +}
> > +
> > +/* Macro to test a character type */
> > +#define TEST_CTYPE_FUNC(func, string) \
> > +static void test_ctype_##func(void) \
> > +{ \
> > + int i; \
> > + for (i = 0; i < 256; i++) \
> > + check_int(func(i), ==, is_in(string, i)); \
> > +}
>
> Now, we let check_int() to do the checking for each and every byte
> value for the class. check_int() uses different reporting and shows
> the problematic value in a way that is more verbose and at the same
> time is a less specific and harder to understand:
>
> test_msg(" left: %"PRIdMAX, a);
> test_msg(" right: %"PRIdMAX, b);
>
> But that is probably the price to pay to use a more generic
> framework, I guess.
Perhaps I'm missing something here, since I haven't followed the
unit-test effort very closely, but this check_int() macro feels like it
might be overkill for what we're trying to do.
We know that the expected value is the result of is_in(string, i), so I
wonder if we might benefit from having an "assert_equals()" that looks
like:
assert_equals(is_in(string, i), func(i));
Where we follow the usual convention of treating the first argument as
the expected value, and the second as the actual value. Then we could
format our error message to be more specific, like:
test_msg("expected %d, got %d", expected, actual);
I think that this would be a little more readable, and still seems
flexible enough to support the kind of thing that check_int(..., ==,
...) is after.
> > +int cmd_main(int argc, const char **argv) {
> > + /* Run all character type tests */
> > + TEST(test_ctype_isspace(), "isspace() works as we expect");
> > + TEST(test_ctype_isdigit(), "isdigit() works as we expect");
> > + TEST(test_ctype_isalpha(), "isalpha() works as we expect");
> > + TEST(test_ctype_isalnum(), "isalnum() works as we expect");
> > + TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
> > + TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
> > + TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
> > + TEST(test_ctype_isascii(), "isascii() works as we expect");
> > + TEST(test_ctype_islower(), "islower() works as we expect");
> > + TEST(test_ctype_isupper(), "isupper() works as we expect");
> > + TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
> > + TEST(test_ctype_ispunct(), "ispunct() works as we expect");
> > + TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
> > + TEST(test_ctype_isprint(), "isprint() works as we expect");
> > +
> > + return test_done();
> > +}
>
> As a practice to use the unit-tests framework, the patch looks OK.
> helper/test-ctype.c indeed is an oddball that runs once and checks
> everything it wants to check, for which the unit tests framework is
> much more suited.
As an aside, I don't think we need the "works as we expect" suffix in
each test description. I personally would be fine with something like:
TEST(test_ctype_isspace(), "isspace()");
TEST(test_ctype_isdigit(), "isdigit()");
...
But don't feel strongly about it.
Thanks,
Taylor
^ permalink raw reply
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