* What's cooking in git.git (Sep 2023, #04; Tue, 12)
From: Junio C Hamano @ 2023-09-12 17:07 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[New Topics]
* js/systemd-timers-wsl-fix (2023-09-11) 1 commit
- maintenance(systemd): support the Windows Subsystem for Linux
Update "git maintainance" timers' implementation based on systemd
timers to work with WSL.
Will merge to 'next'.
source: <pull.1586.git.1694334620260.gitgitgadget@gmail.com>
* kh/range-diff-notes (2023-09-11) 1 commit
- range-diff: treat notes like `log`
"git range-diff --notes=foo" compared "log --notes=foo --notes" of
the two ranges, instead of using just the specified notes tree.
source: <a37dfb3748e23b4f5081bc9a3c80a5c546101f1d.1694383248.git.code@khaugsbakk.name>
* pw/diff-no-index-from-named-pipes (2023-09-11) 1 commit
- diff --no-index: fix -R with stdin
"git diff --no-index -R <(one) <(two)" did not work correctly,
which has been corrected.
Will merge to 'next'.
source: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de>
* rs/parse-options-value-int (2023-09-11) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* so/diff-merges-d (2023-09-11) 2 commits
- diff-merges: introduce '-d' option
- diff-merges: improve --diff-merges documentation
Teach a new "-d" option that shows the patch against the first
parent for merge commits (which is "--diff-merges=first-parent -p").
source: <20230909125446.142715-1-sorganov@gmail.com>
* js/diff-cached-fsmonitor-fix (2023-09-11) 1 commit
- diff-lib: fix check_removed when fsmonitor is on
"git diff --cached" codepath did not fill the necessary stat
information for a file when fsmonitor knows it is clean and ended
up behaving as if it is not clean, which has been corrected.
Will merge to 'next'.
source: <20230911170901.49050-2-sokcevic@google.com>
* pb/completion-aliases-doc (2023-09-11) 1 commit
- completion: improve doc for complex aliases
Clarify how "alias.foo = : git cmd ; aliased-command-string" should
be spelled with necessary whitespaces around punctuation marks to
work.
Will merge to 'next'.
source: <pull.1585.git.1694274592854.gitgitgadget@gmail.com>
--------------------------------------------------
[Stalled]
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
--------------------------------------------------
[Cooking]
* cc/repack-sift-filtered-objects-to-separate-pack (2023-09-11) 9 commits
. gc: add `gc.repackFilterTo` config option
. repack: implement `--filter-to` for storing filtered out objects
. gc: add `gc.repackFilter` config option
. repack: add `--filter=<filter-spec>` option
. pack-bitmap-write: rebuild using new bitmap when remapping
. repack: refactor finding pack prefix
. repack: refactor finishing pack-objects command
. t/helper: add 'find-pack' test-tool
. pack-objects: allow `--filter` without `--stdout`
"git repack" machinery learns to pay attention to the "--filter="
option.
May need to wait until tb/repack-existing-packs-cleanup stablizes.
source: <20230911150618.129737-1-christian.couder@gmail.com>
* la/trailer-cleanups (2023-09-11) 6 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: rename *_DEFAULT enums to *_UNSPECIFIED
- trailer: teach find_patch_start about --no-divider
- trailer: split process_command_line_args into separate functions
- trailer: split process_input_file into separate pieces
- trailer: separate public from internal portion of trailer_iterator
Code clean-up.
Will merge to 'next'.
source: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>
* so/diff-doc-for-patch-update (2023-09-06) 1 commit
(merged to 'next' on 2023-09-07 at 6da5e9defd)
+ doc/diff-options: fix link to generating patch section
References from description of the `--patch` option in various
manual pages have been simplified and improved.
Will merge to 'master'.
source: <87msxzpybo.fsf_-_@osv.gnss.ru>
* tb/repack-existing-packs-cleanup (2023-09-06) 8 commits
- builtin/repack.c: extract common cruft pack loop
- builtin/repack.c: drop `DELETE_PACK` macro
- builtin/repack.c: store existing cruft packs separately
- builtin/repack.c: extract `has_existing_non_kept_packs()`
- builtin/repack.c: extract redundant pack cleanup for existing packs
- builtin/repack.c: extract redundant pack cleanup for --geometric
- builtin/repack.c: extract marking packs for deletion
- builtin/repack.c: extract structure to store existing packs
The code to keep track of existing packs in the repository while
repacking has been refactored.
Will merge to 'next'?
source: <cover.1693946195.git.me@ttaylorr.com>
* pb/complete-commit-trailers (2023-09-07) 1 commit
(merged to 'next' on 2023-09-08 at 842587016d)
+ completion: commit: complete configured trailer tokens
The command-line complation support (in contrib/) learned to
complete "git commit --trailer=" for possible trailer keys.
Will merge to 'master'.
source: <pull.1583.git.1694108551683.gitgitgadget@gmail.com>
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Will merge to 'next'?
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* rs/grep-no-no-or (2023-09-07) 1 commit
(merged to 'next' on 2023-09-08 at 33849032bc)
+ grep: reject --no-or
"git grep -e A --no-or -e B" is accepted, even though the negation
of "or" did not mean anything, which has been tightened.
Will merge to 'master'.
source: <6aeb0ebe-0fea-ccd3-089a-ee0b5b5baf10@web.de>
* js/complete-checkout-t (2023-09-08) 1 commit
(merged to 'next' on 2023-09-08 at 461bb28fbd)
+ completion(switch/checkout): treat --track and -t the same
The completion script (in contrib/) has been taught to treat the
"-t" option to "git checkout" and "git switch" just like the
"--track" option, to complete remote-tracking branches.
Will merge to 'master'.
source: <pull.1584.git.1694176123471.gitgitgadget@gmail.com>
* cw/git-std-lib (2023-09-11) 7 commits
- SQUASH???
- git-std-lib: add test file to call git-std-lib.a functions
- git-std-lib: introduce git standard library
- parse: create new library for parsing strings and env values
- config: correct bad boolean env value error message
- wrapper: remove dependency to Git-specific internal file
- hex-ll: split out functionality from hex
Another libification effort.
Needs more work.
cf. <xmqqy1hfrk6p.fsf@gitster.g>
source: <20230908174134.1026823-1-calvinwan@google.com>
* cc/git-replay (2023-09-07) 15 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: disallow revision specific options and pathspecs
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: don't simplify history
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
source: <20230907092521.733746-1-christian.couder@gmail.com>
* la/trailer-test-and-doc-updates (2023-09-07) 13 commits
- trailer doc: <token> is a <key> or <keyAlias>, not both
- trailer doc: separator within key suppresses default separator
- trailer doc: emphasize the effect of configuration variables
- trailer --unfold help: prefer "reformat" over "join"
- trailer --parse docs: add explanation for its usefulness
- trailer --only-input: prefer "configuration variables" over "rules"
- trailer --parse help: expose aliased options
- trailer --no-divider help: describe usual "---" meaning
- trailer: trailer location is a place, not an action
- trailer doc: narrow down scope of --where and related flags
- trailer: add tests to check defaulting behavior with --no-* flags
- trailer test description: this tests --where=after, not --where=before
- trailer tests: make test cases self-contained
Test coverage for trailers has been improved.
source: <pull.1564.v3.git.1694125209.gitgitgadget@gmail.com>
* ak/pretty-decorate-more (2023-08-21) 8 commits
(merged to 'next' on 2023-09-07 at 6f5e14cef2)
+ decorate: use commit color for HEAD arrow
+ pretty: add pointer and tag options to %(decorate)
+ pretty: add %(decorate[:<options>]) format
+ decorate: color each token separately
+ decorate: avoid some unnecessary color overhead
+ decorate: refactor format_decorations()
+ pretty-formats: enclose options in angle brackets
+ pretty-formats: define "literal formatting code"
"git log --format" has been taught the %(decorate) placeholder.
Will merge to 'master'.
source: <20230820185009.20095-1-andy.koppe@gmail.com>
* bc/more-git-var (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at d8af32874c)
+ var: avoid a segmentation fault when `HOME` is unset
Fix-up for a topic that already has graduated.
Will merge to 'master'.
source: <pull.1580.git.1693808487058.gitgitgadget@gmail.com>
* ks/ref-filter-sort-numerically (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at aa4d156366)
+ ref-filter: sort numerically when ":size" is used
"git for-each-ref --sort='contents:size'" sorts the refs according
to size numerically, giving a ref that points at a blob twelve-byte
(12) long before showing a blob hundred-byte (100) long.
Will merge to 'master'.
source: <20230902090155.8978-1-five231003@gmail.com>
* ob/sequencer-reword-error-message (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at c5154b7aa2)
+ sequencer: fix error message on failure to copy SQUASH_MSG
Update an error message (which would probably never been seen).
Will merge to 'master'.
source: <20230903151132.739136-1-oswald.buddenhagen@gmx.de>
* rs/grep-parseopt-simplify (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at 249b69cfd2)
+ grep: use OPT_INTEGER_F for --max-depth
Simplify use of parse-options API a bit.
Will merge to 'master'.
source: <4d2eb736-4f34-18f8-2eb7-20e7f7b8c2f8@web.de>
* rs/name-rev-use-opt-hidden-bool (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at 9b251a5392)
+ name-rev: use OPT_HIDDEN_BOOL for --peel-tag
Simplify use of parse-options API a bit.
Will merge to 'master'.
source: <5a86c8f8-fcdc-fee9-8af5-aa5ecb036d2e@web.de>
* jk/unused-post-2.42-part2 (2023-09-05) 10 commits
(merged to 'next' on 2023-09-05 at 308ca3a052)
+ parse-options: mark unused parameters in noop callback
+ interpret-trailers: mark unused "unset" parameters in option callbacks
+ parse-options: add more BUG_ON() annotations
+ merge: do not pass unused opt->value parameter
+ parse-options: mark unused "opt" parameter in callbacks
+ parse-options: prefer opt->value to globals in callbacks
+ checkout-index: delay automatic setting of to_tempfile
+ format-patch: use OPT_STRING_LIST for to/cc options
+ merge: simplify parsing of "-n" option
+ merge: make xopts a strvec
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.
Will merge to 'master'.
source: <20230831211637.GA949188@coredump.intra.peff.net>
* jk/tree-name-and-depth-limit (2023-08-31) 10 commits
(merged to 'next' on 2023-09-07 at 7ad767ab0d)
+ lower core.maxTreeDepth default to 2048
+ tree-diff: respect max_allowed_tree_depth
+ list-objects: respect max_allowed_tree_depth
+ read_tree(): respect max_allowed_tree_depth
+ traverse_trees(): respect max_allowed_tree_depth
+ add core.maxTreeDepth config
+ fsck: detect very large tree pathnames
+ tree-walk: rename "error" variable
+ tree-walk: drop MAX_TRAVERSE_TREES macro
+ tree-walk: reduce stack size for recursive functions
We now limit depth of the tree objects and maximum length of
pathnames recorded in tree objects.
Will merge to 'master'.
source: <20230831061735.GA2702156@coredump.intra.peff.net>
* js/doc-unit-tests (2023-08-17) 3 commits
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: Add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Comments?
source: <cover.1692297001.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-08-31) 4 commits
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
source: <pull.1579.git.1693462532.gitgitgadget@gmail.com>
* ew/hash-with-openssl-evp (2023-08-31) 1 commit
(merged to 'next' on 2023-09-05 at 1ddc0078c8)
+ treewide: fix various bugs w/ OpenSSL 3+ EVP API
Fix-up new-ish code to support OpenSSL EVP API.
Will merge to 'master'.
source: <20230901020928.M610756@dcvr>
* tb/path-filter-fix (2023-08-30) 15 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
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
- 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
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs more work?
cf. <20230830200218.GA5147@szeder.dev>
source: <cover.1693413637.git.jonathantanmy@google.com>
* js/config-parse (2023-08-23) 4 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1692827403.git.steadmon@google.com>
* jc/update-index-show-index-version (2023-08-18) 3 commits
- test-tool: retire "index-version"
- update-index: add --show-index-version
- update-index doc: v4 is OK with JGit and libgit2
"git update-index" learns "--show-index-version" to inspect
the index format version used by the on-disk index file.
Needs review.
source: <20230818233729.2766281-1-gitster@pobox.com>
* ob/revert-of-revert-is-reapply (2023-09-02) 2 commits
(merged to 'next' on 2023-09-07 at 9a54f66511)
+ git-revert.txt: add discussion
+ sequencer: beautify subject of reverts of reverts
The default log message created by "git revert", when reverting a
commit that records a revert, has been tweaked.
Will merge to 'master'.
source: <20230821170720.577850-1-oswald.buddenhagen@gmx.de>
source: <20230902072035.652549-1-oswald.buddenhagen@gmx.de>
* 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
(this branch uses jc/unresolve-removal.)
Code clean-up.
Not ready to be reviewed yet.
source: <20230731224409.4181277-1-gitster@pobox.com>
* pw/rebase-i-after-failure (2023-09-06) 7 commits
(merged to 'next' on 2023-09-07 at 3cbc3c4d63)
+ rebase -i: fix adding failed command to the todo list
+ rebase --continue: refuse to commit after failed command
+ rebase: fix rewritten list for failed pick
+ sequencer: factor out part of pick_commits()
+ sequencer: use rebase_path_message()
+ rebase -i: remove patch file after conflict resolution
+ rebase -i: move unlink() calls
Various fixes to the behaviour of "rebase -i" when the command got
interrupted by conflicting changes.
Will merge to 'master'.
cf. <6b927687-cf6e-d73e-78fb-bd4f46736928@gmx.de>
source: <pull.1492.v4.git.1694013771.gitgitgadget@gmail.com>
* jc/unresolve-removal (2023-07-31) 7 commits
- checkout: allow "checkout -m path" to unmerge removed paths
- checkout/restore: add basic tests for --merge
- checkout/restore: refuse unmerging paths unless checking out of the index
- update-index: remove stale fallback code for "--unresolve"
- update-index: use unmerge_index_entry() to support removal
- resolve-undo: allow resurrecting conflicted state that resolved to deletion
- update-index: do not read HEAD and MERGE_HEAD unconditionally
(this branch is used by jc/rerere-cleanup.)
"checkout --merge -- path" and "update-index --unresolve path" did
not resurrect conflicted state that was resolved to remove path,
but now they do.
Needs review.
source: <20230731224409.4181277-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-08-01) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
cf. <xmqqtttia3vn.fsf@gitster.g>
source: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>
^ permalink raw reply
* [PATCH v2] completion: improve doc for complex aliases
From: Philippe Blain via GitGitGadget @ 2023-09-12 17:02 UTC (permalink / raw)
To: git
Cc: Steffen Prohaska, Eric Sunshine, Linus Arver, Philippe Blain,
Philippe Blain
In-Reply-To: <pull.1585.git.1694274592854.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
The completion code can be told to use a particular completion for
aliases that shell out by using ': git <cmd> ;' as the first command of
the alias. This only works if <cmd> and the semicolon are separated by a
space, since if the space is missing __git_aliased_command returns (for
example) 'checkout;' instead of just 'checkout', and then
__git_complete_command fails to find a completion for 'checkout;'.
The examples have that space but it's not clear if it's just for
style or if it's mandatory. Explicitly mention it.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
completion: improve doc for complex aliases
Changes since v1:
* fixed the typo pointed out by Eric
* added an explanation of why the space is mandatory, as suggested by
Linus
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1585%2Fphil-blain%2Fcompletion-shell-aliases-doc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1585/phil-blain/completion-shell-aliases-doc-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1585
Range-diff vs v1:
1: b3621ed25f0 ! 1: 96a0867c5ad completion: improve doc for complex aliases
@@ Commit message
The completion code can be told to use a particular completion for
aliases that shell out by using ': git <cmd> ;' as the first command of
the alias. This only works if <cmd> and the semicolon are separated by a
- space. The examples have that space but it's not clear if it's just for
- style or if it's mandatory.
+ space, since if the space is missing __git_aliased_command returns (for
+ example) 'checkout;' instead of just 'checkout', and then
+ __git_complete_command fails to find a completion for 'checkout;'.
- Explicitely mention it.
+ The examples have that space but it's not clear if it's just for
+ style or if it's mandatory. Explicitly mention it.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc95c34cc85..659df570496 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -28,6 +28,7 @@
# completion style. For example '!f() { : git commit ; ... }; f' will
# tell the completion to use commit completion. This also works with aliases
# of form "!sh -c '...'". For example, "!sh -c ': git commit ; ... '".
+# Be sure to add a space between the command name and the ';'.
#
# If you have a command that is not part of git, but you would still
# like completion, you can use __git_complete:
base-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6
--
gitgitgadget
^ permalink raw reply related
* What's cooking in git.git (Sep 2023, #04; Tue, 12)
From: Junio C Hamano @ 2023-09-12 16:57 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[New Topics]
* js/systemd-timers-wsl-fix (2023-09-11) 1 commit
- maintenance(systemd): support the Windows Subsystem for Linux
Update "git maintainance" timers' implementation based on systemd
timers to work with WSL.
Will merge to 'next'.
source: <pull.1586.git.1694334620260.gitgitgadget@gmail.com>
* kh/range-diff-notes (2023-09-11) 1 commit
- range-diff: treat notes like `log`
"git range-diff --notes=foo" compared "log --notes=foo --notes" of
the two ranges, instead of using just the specified notes tree.
source: <a37dfb3748e23b4f5081bc9a3c80a5c546101f1d.1694383248.git.code@khaugsbakk.name>
* pw/diff-no-index-from-named-pipes (2023-09-11) 1 commit
- diff --no-index: fix -R with stdin
"git diff --no-index -R <(one) <(two)" did not work correctly,
which has been corrected.
Will merge to 'next'.
source: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de>
* rs/parse-options-value-int (2023-09-11) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* so/diff-merges-d (2023-09-11) 2 commits
- diff-merges: introduce '-d' option
- diff-merges: improve --diff-merges documentation
Teach a new "-d" option that shows the patch against the first
parent for merge commits (which is "--diff-merges=first-parent -p").
source: <20230909125446.142715-1-sorganov@gmail.com>
* js/diff-cached-fsmonitor-fix (2023-09-11) 1 commit
- diff-lib: fix check_removed when fsmonitor is on
"git diff --cached" codepath did not fill the necessary stat
information for a file when fsmonitor knows it is clean and ended
up behaving as if it is not clean, which has been corrected.
Will merge to 'next'.
source: <20230911170901.49050-2-sokcevic@google.com>
* pb/completion-aliases-doc (2023-09-11) 1 commit
- completion: improve doc for complex aliases
Clarify how "alias.foo = : git cmd ; aliased-command-string" should
be spelled with necessary whitespaces around punctuation marks to
work.
Will merge to 'next'.
source: <pull.1585.git.1694274592854.gitgitgadget@gmail.com>
--------------------------------------------------
[Stalled]
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
--------------------------------------------------
[Cooking]
* cc/repack-sift-filtered-objects-to-separate-pack (2023-09-11) 9 commits
. gc: add `gc.repackFilterTo` config option
. repack: implement `--filter-to` for storing filtered out objects
. gc: add `gc.repackFilter` config option
. repack: add `--filter=<filter-spec>` option
. pack-bitmap-write: rebuild using new bitmap when remapping
. repack: refactor finding pack prefix
. repack: refactor finishing pack-objects command
. t/helper: add 'find-pack' test-tool
. pack-objects: allow `--filter` without `--stdout`
"git repack" machinery learns to pay attention to the "--filter="
option.
May need to wait until tb/repack-existing-packs-cleanup stablizes.
source: <20230911150618.129737-1-christian.couder@gmail.com>
* la/trailer-cleanups (2023-09-11) 6 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: rename *_DEFAULT enums to *_UNSPECIFIED
- trailer: teach find_patch_start about --no-divider
- trailer: split process_command_line_args into separate functions
- trailer: split process_input_file into separate pieces
- trailer: separate public from internal portion of trailer_iterator
Code clean-up.
Will merge to 'next'.
source: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>
* so/diff-doc-for-patch-update (2023-09-06) 1 commit
(merged to 'next' on 2023-09-07 at 6da5e9defd)
+ doc/diff-options: fix link to generating patch section
References from description of the `--patch` option in various
manual pages have been simplified and improved.
Will merge to 'master'.
source: <87msxzpybo.fsf_-_@osv.gnss.ru>
* tb/repack-existing-packs-cleanup (2023-09-06) 8 commits
- builtin/repack.c: extract common cruft pack loop
- builtin/repack.c: drop `DELETE_PACK` macro
- builtin/repack.c: store existing cruft packs separately
- builtin/repack.c: extract `has_existing_non_kept_packs()`
- builtin/repack.c: extract redundant pack cleanup for existing packs
- builtin/repack.c: extract redundant pack cleanup for --geometric
- builtin/repack.c: extract marking packs for deletion
- builtin/repack.c: extract structure to store existing packs
The code to keep track of existing packs in the repository while
repacking has been refactored.
Will merge to 'next'?
source: <cover.1693946195.git.me@ttaylorr.com>
* pb/complete-commit-trailers (2023-09-07) 1 commit
(merged to 'next' on 2023-09-08 at 842587016d)
+ completion: commit: complete configured trailer tokens
The command-line complation support (in contrib/) learned to
complete "git commit --trailer=" for possible trailer keys.
Will merge to 'master'.
source: <pull.1583.git.1694108551683.gitgitgadget@gmail.com>
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Will merge to 'next'?
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* rs/grep-no-no-or (2023-09-07) 1 commit
(merged to 'next' on 2023-09-08 at 33849032bc)
+ grep: reject --no-or
"git grep -e A --no-or -e B" is accepted, even though the negation
of "or" did not mean anything, which has been tightened.
Will merge to 'master'.
source: <6aeb0ebe-0fea-ccd3-089a-ee0b5b5baf10@web.de>
* js/complete-checkout-t (2023-09-08) 1 commit
(merged to 'next' on 2023-09-08 at 461bb28fbd)
+ completion(switch/checkout): treat --track and -t the same
The completion script (in contrib/) has been taught to treat the
"-t" option to "git checkout" and "git switch" just like the
"--track" option, to complete remote-tracking branches.
Will merge to 'master'.
source: <pull.1584.git.1694176123471.gitgitgadget@gmail.com>
* cw/git-std-lib (2023-09-11) 7 commits
- SQUASH???
- git-std-lib: add test file to call git-std-lib.a functions
- git-std-lib: introduce git standard library
- parse: create new library for parsing strings and env values
- config: correct bad boolean env value error message
- wrapper: remove dependency to Git-specific internal file
- hex-ll: split out functionality from hex
Another libification effort.
Needs more work.
cf. <xmqqy1hfrk6p.fsf@gitster.g>
source: <20230908174134.1026823-1-calvinwan@google.com>
* cc/git-replay (2023-09-07) 15 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: disallow revision specific options and pathspecs
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: don't simplify history
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
source: <20230907092521.733746-1-christian.couder@gmail.com>
* la/trailer-test-and-doc-updates (2023-09-07) 13 commits
- trailer doc: <token> is a <key> or <keyAlias>, not both
- trailer doc: separator within key suppresses default separator
- trailer doc: emphasize the effect of configuration variables
- trailer --unfold help: prefer "reformat" over "join"
- trailer --parse docs: add explanation for its usefulness
- trailer --only-input: prefer "configuration variables" over "rules"
- trailer --parse help: expose aliased options
- trailer --no-divider help: describe usual "---" meaning
- trailer: trailer location is a place, not an action
- trailer doc: narrow down scope of --where and related flags
- trailer: add tests to check defaulting behavior with --no-* flags
- trailer test description: this tests --where=after, not --where=before
- trailer tests: make test cases self-contained
Test coverage for trailers has been improved.
source: <pull.1564.v3.git.1694125209.gitgitgadget@gmail.com>
* ak/pretty-decorate-more (2023-08-21) 8 commits
(merged to 'next' on 2023-09-07 at 6f5e14cef2)
+ decorate: use commit color for HEAD arrow
+ pretty: add pointer and tag options to %(decorate)
+ pretty: add %(decorate[:<options>]) format
+ decorate: color each token separately
+ decorate: avoid some unnecessary color overhead
+ decorate: refactor format_decorations()
+ pretty-formats: enclose options in angle brackets
+ pretty-formats: define "literal formatting code"
"git log --format" has been taught the %(decorate) placeholder.
Will merge to 'master'.
source: <20230820185009.20095-1-andy.koppe@gmail.com>
* bc/more-git-var (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at d8af32874c)
+ var: avoid a segmentation fault when `HOME` is unset
Fix-up for a topic that already has graduated.
Will merge to 'master'.
source: <pull.1580.git.1693808487058.gitgitgadget@gmail.com>
* ks/ref-filter-sort-numerically (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at aa4d156366)
+ ref-filter: sort numerically when ":size" is used
"git for-each-ref --sort='contents:size'" sorts the refs according
to size numerically, giving a ref that points at a blob twelve-byte
(12) long before showing a blob hundred-byte (100) long.
Will merge to 'master'.
source: <20230902090155.8978-1-five231003@gmail.com>
* ob/sequencer-reword-error-message (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at c5154b7aa2)
+ sequencer: fix error message on failure to copy SQUASH_MSG
Update an error message (which would probably never been seen).
Will merge to 'master'.
source: <20230903151132.739136-1-oswald.buddenhagen@gmx.de>
* rs/grep-parseopt-simplify (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at 249b69cfd2)
+ grep: use OPT_INTEGER_F for --max-depth
Simplify use of parse-options API a bit.
Will merge to 'master'.
source: <4d2eb736-4f34-18f8-2eb7-20e7f7b8c2f8@web.de>
* rs/name-rev-use-opt-hidden-bool (2023-09-05) 1 commit
(merged to 'next' on 2023-09-06 at 9b251a5392)
+ name-rev: use OPT_HIDDEN_BOOL for --peel-tag
Simplify use of parse-options API a bit.
Will merge to 'master'.
source: <5a86c8f8-fcdc-fee9-8af5-aa5ecb036d2e@web.de>
* jk/unused-post-2.42-part2 (2023-09-05) 10 commits
(merged to 'next' on 2023-09-05 at 308ca3a052)
+ parse-options: mark unused parameters in noop callback
+ interpret-trailers: mark unused "unset" parameters in option callbacks
+ parse-options: add more BUG_ON() annotations
+ merge: do not pass unused opt->value parameter
+ parse-options: mark unused "opt" parameter in callbacks
+ parse-options: prefer opt->value to globals in callbacks
+ checkout-index: delay automatic setting of to_tempfile
+ format-patch: use OPT_STRING_LIST for to/cc options
+ merge: simplify parsing of "-n" option
+ merge: make xopts a strvec
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.
Will merge to 'master'.
source: <20230831211637.GA949188@coredump.intra.peff.net>
* jk/tree-name-and-depth-limit (2023-08-31) 10 commits
(merged to 'next' on 2023-09-07 at 7ad767ab0d)
+ lower core.maxTreeDepth default to 2048
+ tree-diff: respect max_allowed_tree_depth
+ list-objects: respect max_allowed_tree_depth
+ read_tree(): respect max_allowed_tree_depth
+ traverse_trees(): respect max_allowed_tree_depth
+ add core.maxTreeDepth config
+ fsck: detect very large tree pathnames
+ tree-walk: rename "error" variable
+ tree-walk: drop MAX_TRAVERSE_TREES macro
+ tree-walk: reduce stack size for recursive functions
We now limit depth of the tree objects and maximum length of
pathnames recorded in tree objects.
Will merge to 'master'.
source: <20230831061735.GA2702156@coredump.intra.peff.net>
* js/doc-unit-tests (2023-08-17) 3 commits
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: Add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Comments?
source: <cover.1692297001.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-08-31) 4 commits
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
source: <pull.1579.git.1693462532.gitgitgadget@gmail.com>
* ew/hash-with-openssl-evp (2023-08-31) 1 commit
(merged to 'next' on 2023-09-05 at 1ddc0078c8)
+ treewide: fix various bugs w/ OpenSSL 3+ EVP API
Fix-up new-ish code to support OpenSSL EVP API.
Will merge to 'master'.
source: <20230901020928.M610756@dcvr>
* tb/path-filter-fix (2023-08-30) 15 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
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
- 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
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs more work?
cf. <20230830200218.GA5147@szeder.dev>
source: <cover.1693413637.git.jonathantanmy@google.com>
* js/config-parse (2023-08-23) 4 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1692827403.git.steadmon@google.com>
* jc/update-index-show-index-version (2023-08-18) 3 commits
- test-tool: retire "index-version"
- update-index: add --show-index-version
- update-index doc: v4 is OK with JGit and libgit2
"git update-index" learns "--show-index-version" to inspect
the index format version used by the on-disk index file.
Needs review.
source: <20230818233729.2766281-1-gitster@pobox.com>
* ob/revert-of-revert-is-reapply (2023-09-02) 2 commits
(merged to 'next' on 2023-09-07 at 9a54f66511)
+ git-revert.txt: add discussion
+ sequencer: beautify subject of reverts of reverts
The default log message created by "git revert", when reverting a
commit that records a revert, has been tweaked.
Will merge to 'master'.
source: <20230821170720.577850-1-oswald.buddenhagen@gmx.de>
source: <20230902072035.652549-1-oswald.buddenhagen@gmx.de>
* 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
(this branch uses jc/unresolve-removal.)
Code clean-up.
Not ready to be reviewed yet.
source: <20230731224409.4181277-1-gitster@pobox.com>
* pw/rebase-i-after-failure (2023-09-06) 7 commits
(merged to 'next' on 2023-09-07 at 3cbc3c4d63)
+ rebase -i: fix adding failed command to the todo list
+ rebase --continue: refuse to commit after failed command
+ rebase: fix rewritten list for failed pick
+ sequencer: factor out part of pick_commits()
+ sequencer: use rebase_path_message()
+ rebase -i: remove patch file after conflict resolution
+ rebase -i: move unlink() calls
Various fixes to the behaviour of "rebase -i" when the command got
interrupted by conflicting changes.
Will merge to 'master'.
cf. <6b927687-cf6e-d73e-78fb-bd4f46736928@gmx.de>
source: <pull.1492.v4.git.1694013771.gitgitgadget@gmail.com>
* jc/unresolve-removal (2023-07-31) 7 commits
- checkout: allow "checkout -m path" to unmerge removed paths
- checkout/restore: add basic tests for --merge
- checkout/restore: refuse unmerging paths unless checking out of the index
- update-index: remove stale fallback code for "--unresolve"
- update-index: use unmerge_index_entry() to support removal
- resolve-undo: allow resurrecting conflicted state that resolved to deletion
- update-index: do not read HEAD and MERGE_HEAD unconditionally
(this branch is used by jc/rerere-cleanup.)
"checkout --merge -- path" and "update-index --unresolve path" did
not resurrect conflicted state that was resolved to remove path,
but now they do.
Needs review.
source: <20230731224409.4181277-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-08-01) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
cf. <xmqqtttia3vn.fsf@gitster.g>
source: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>
^ permalink raw reply
* Re: [RFC][PATCH 0/32] SHA256 and SHA1 interoperability
From: Junio C Hamano @ 2023-09-12 16:20 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: git, brian m. carlson
In-Reply-To: <87cyyoeli0.fsf@email.froward.int.ebiederm.org>
"Eric W. Biederman" <ebiederm@xmission.com> writes:
> I am not worried about what it will take time to get the changes I
> posted into the integration. I had only envisioned them as good enough
> to get the technical ideas across, and had never envisioned them as
> being accepted as is.
Ah, no worries. By "integration" I did not mean "patches considered
perfect, they are accepted, and are now part of the Git codebase".
All that happens when the patches become part of the 'master'
branch, but before that, patches that prove testable and worthy of
getting tested will be merged to the 'next' branch and spend about a
week there. What I meant to refer to is a step _before_ that, i.e.
before the patches probe to be testable. New patches first appear
on the 'seen' branch that merges "everything else" to see the
interaction with all the topics "in flight" (i.e. not yet in
'master'). The 'seen' branch is reassembled from the latest
iteration of the patches twice of thrice per day, and some patches
are merged to 'next' and down to 'master', these "merging to prepare
'master', 'next' and 'seen' branches for publishing" was what I
meant by "integration". In short, being queued on 'seen' does not
mean all that much. It gives project participants an easy access to
view how topics look in the larger picture, potentially interacting
with other topics in flight, but the patches in there can be
replaced wholesale or even dropped if they do not turn out to be
desirable.
I resolved textual conflicts and also compiler detectable semantic
conflicts (e.g. some in-flight topics may have added callsites to a
function your topic changes the function sigunature, or vice versa)
to the point that the result compiles while merging this topic to
'seen', but tests are broken the big time, it seems, even though the
topic by itself seems to pass the tests standalone.
> What I am envisioning as my future directions are:
> ...
> Does that sound like a reasonable plan?
Nice.
^ permalink raw reply
* [PATCH v2] revision: add `--ignore-missing-links` user option
From: Karthik Nayak @ 2023-09-12 15:58 UTC (permalink / raw)
To: git; +Cc: gitster, Karthik Nayak
In-Reply-To: <20230908174208.249184-1-karthik.188@gmail.com>
The revision backend is used by multiple porcelain commands such as
git-rev-list(1) and git-log(1). The backend currently supports ignoring
missing links by setting the `ignore_missing_links` bit. This allows the
revision walk to skip any objects links which are missing. Expose this
bit via an `--ignore-missing-links` user option.
A scenario where this option would be used is to find the boundary
objects between different object directories. Consider a repository with
a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
repository, enabling this option along with the `--boundary` option for
while disabling the alternate object directory allows us to find the
boundary objects between the main and alternate object directory.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes from v1:
1. Changes in the commit message and option description to be more specific
and list why and what the changes are.
2. Ensure the new option also works with the existing `--objects` options.
3. More specific testing for boundary commit.
Range diff against v1:
1: c0a4dca9b0 ! 1: e3f4d85732 revision: add `--ignore-missing-links` user option
@@ Commit message
The revision backend is used by multiple porcelain commands such as
git-rev-list(1) and git-log(1). The backend currently supports ignoring
missing links by setting the `ignore_missing_links` bit. This allows the
- revision walk to skip any objects links which are missing.
+ revision walk to skip any objects links which are missing. Expose this
+ bit via an `--ignore-missing-links` user option.
- Currently there is no way to use git-rev-list(1) to traverse the objects
- of the main object directory (GIT_OBJECT_DIRECTORY) and print the
- boundary objects when moving from the main object directory to the
- alternate object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES).
-
- By exposing this new flag `--ignore-missing-links`, users can set the
- required env variables (GIT_OBJECT_DIRECTORY and
- GIT_ALTERNATE_OBJECT_DIRECTORIES) along with the `--boundary` flag to
- find the boundary objects between object directories.
+ A scenario where this option would be used is to find the boundary
+ objects between different object directories. Consider a repository with
+ a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
+ object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
+ repository, enabling this option along with the `--boundary` option for
+ while disabling the alternate object directory allows us to find the
+ boundary objects between the main and alternate object directory.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
@@ Documentation/rev-list-options.txt: explicitly.
the bad input was not given.
+--ignore-missing-links::
-+ When an object points to another object that is missing, pretend as if the
-+ link did not exist. These missing links are not written to stdout unless
-+ the --boundary flag is passed.
++ During traversal, if an object that is referenced does not
++ exist, instead of dying of a repository corruption, pretend as
++ if the reference itself does not exist. Running the command
++ with the `--boundary` option makes these missing commits,
++ together with the commits on the edge of revision ranges
++ (i.e. true boundary objects), appear on the output, prefixed
++ with '-'.
+
ifndef::git-rev-list[]
--bisect::
Pretend as if the bad bisection ref `refs/bisect/bad`
+ ## builtin/rev-list.c ##
+@@ builtin/rev-list.c: static int finish_object(struct object *obj, const char *name UNUSED,
+ {
+ struct rev_list_info *info = cb_data;
+ if (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {
+- finish_object__ma(obj);
++ if (!info->revs->ignore_missing_links)
++ finish_object__ma(obj);
+ return 1;
+ }
+ if (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)
+
## revision.c ##
@@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->limited = 1;
@@ t/t6022-rev-list-alternates.sh (new)
+test_expect_success 'create repository and alternate directory' '
+ git init main &&
+ test_commit_bulk -C main 5 &&
++ BOUNDARY_COMMIT=$(git -C main rev-parse HEAD) &&
+ mkdir alt &&
+ mv main/.git/objects/* alt &&
+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt test_commit_bulk --start=6 -C main 5
+'
+
-+# When the alternate odb is provided, all commits are listed.
++# when the alternate odb is provided, all commits are listed along with the boundary
++# commit.
+test_expect_success 'rev-list passes with alternate object directory' '
-+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt test_stdout_line_count = 10 git -C main rev-list HEAD
++ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main rev-list HEAD >actual &&
++ test_stdout_line_count = 10 cat actual &&
++ grep $BOUNDARY_COMMIT actual
+'
+
+# When the alternate odb is not provided, rev-list fails since the 5th commit's
@@ t/t6022-rev-list-alternates.sh (new)
+'
+
+# With `--ignore-missing-links`, we stop the traversal when we encounter a
-+# missing link.
++# missing link. The boundary commit is not listed as we haven't used the
++# `--boundary` options.
+test_expect_success 'rev-list only prints main odb commits with --ignore-missing-links' '
-+ test_stdout_line_count = 5 git -C main rev-list --ignore-missing-links HEAD
++ git -C main rev-list --ignore-missing-links HEAD >actual &&
++ test_stdout_line_count = 5 cat actual &&
++ ! grep -$BOUNDARY_COMMIT actual
+'
+
+# With `--ignore-missing-links` and `--boundary`, we can even print those boundary
+# commits.
+test_expect_success 'rev-list prints boundary commit with --ignore-missing-links' '
-+ git -C main rev-list --ignore-missing-links --boundary HEAD >list-output &&
-+ test_stdout_line_count = 6 cat list-output &&
-+ test_stdout_line_count = 1 cat list-output | grep "^-"
++ git -C main rev-list --ignore-missing-links --boundary HEAD >actual &&
++ test_stdout_line_count = 6 cat actual &&
++ grep -$BOUNDARY_COMMIT actual
++'
++
++# The `--ignore-missing-links` option should ensure that git-rev-list(1) doesn't
++# fail when used alongside `--objects` when a tree is missing.
++test_expect_success 'rev-list --ignore-missing-links works with missing tree' '
++ echo "foo" >main/file &&
++ git -C main add file &&
++ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main commit -m"commit 11" &&
++ TREE_OID=$(git -C main rev-parse HEAD^{tree}) &&
++ mkdir alt/${TREE_OID:0:2} &&
++ mv main/.git/objects/${TREE_OID:0:2}/${TREE_OID:2} alt/${TREE_OID:0:2}/ &&
++ git -C main rev-list --ignore-missing-links --objects HEAD >actual &&
++ ! grep $TREE_OID actual
++'
++
++# Similar to above, it should also work when a blob is missing.
++test_expect_success 'rev-list --ignore-missing-links works with missing blob' '
++ echo "bar" >main/file &&
++ git -C main add file &&
++ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main commit -m"commit 12" &&
++ BLOB_OID=$(git -C main rev-parse HEAD:file) &&
++ mkdir alt/${BLOB_OID:0:2} &&
++ mv main/.git/objects/${BLOB_OID:0:2}/${BLOB_OID:2} alt/${BLOB_OID:0:2}/ &&
++ git -C main rev-list --ignore-missing-links --objects HEAD >actual &&
++ ! grep $BLOB_OID actual
+'
+
+test_done
Documentation/rev-list-options.txt | 9 ++++
builtin/rev-list.c | 3 +-
revision.c | 2 +
t/t6022-rev-list-alternates.sh | 75 ++++++++++++++++++++++++++++++
4 files changed, 88 insertions(+), 1 deletion(-)
create mode 100755 t/t6022-rev-list-alternates.sh
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index a4a0cb93b2..8ee713db3d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -227,6 +227,15 @@ explicitly.
Upon seeing an invalid object name in the input, pretend as if
the bad input was not given.
+--ignore-missing-links::
+ During traversal, if an object that is referenced does not
+ exist, instead of dying of a repository corruption, pretend as
+ if the reference itself does not exist. Running the command
+ with the `--boundary` option makes these missing commits,
+ together with the commits on the edge of revision ranges
+ (i.e. true boundary objects), appear on the output, prefixed
+ with '-'.
+
ifndef::git-rev-list[]
--bisect::
Pretend as if the bad bisection ref `refs/bisect/bad`
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff715d6918..5239d83c76 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -266,7 +266,8 @@ static int finish_object(struct object *obj, const char *name UNUSED,
{
struct rev_list_info *info = cb_data;
if (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {
- finish_object__ma(obj);
+ if (!info->revs->ignore_missing_links)
+ finish_object__ma(obj);
return 1;
}
if (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)
diff --git a/revision.c b/revision.c
index 2f4c53ea20..cbfcbf6e28 100644
--- a/revision.c
+++ b/revision.c
@@ -2595,6 +2595,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->limited = 1;
} else if (!strcmp(arg, "--ignore-missing")) {
revs->ignore_missing = 1;
+ } else if (!strcmp(arg, "--ignore-missing-links")) {
+ revs->ignore_missing_links = 1;
} else if (opt && opt->allow_exclude_promisor_objects &&
!strcmp(arg, "--exclude-promisor-objects")) {
if (fetch_if_missing)
diff --git a/t/t6022-rev-list-alternates.sh b/t/t6022-rev-list-alternates.sh
new file mode 100755
index 0000000000..08d9ffde5f
--- /dev/null
+++ b/t/t6022-rev-list-alternates.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='handling of alternates in rev-list'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# We create 5 commits and move them to the alt directory and
+# create 5 more commits which will stay in the main odb.
+test_expect_success 'create repository and alternate directory' '
+ git init main &&
+ test_commit_bulk -C main 5 &&
+ BOUNDARY_COMMIT=$(git -C main rev-parse HEAD) &&
+ mkdir alt &&
+ mv main/.git/objects/* alt &&
+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt test_commit_bulk --start=6 -C main 5
+'
+
+# when the alternate odb is provided, all commits are listed along with the boundary
+# commit.
+test_expect_success 'rev-list passes with alternate object directory' '
+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main rev-list HEAD >actual &&
+ test_stdout_line_count = 10 cat actual &&
+ grep $BOUNDARY_COMMIT actual
+'
+
+# When the alternate odb is not provided, rev-list fails since the 5th commit's
+# parent is not present in the main odb.
+test_expect_success 'rev-list fails without alternate object directory' '
+ test_must_fail git -C main rev-list HEAD
+'
+
+# With `--ignore-missing-links`, we stop the traversal when we encounter a
+# missing link. The boundary commit is not listed as we haven't used the
+# `--boundary` options.
+test_expect_success 'rev-list only prints main odb commits with --ignore-missing-links' '
+ git -C main rev-list --ignore-missing-links HEAD >actual &&
+ test_stdout_line_count = 5 cat actual &&
+ ! grep -$BOUNDARY_COMMIT actual
+'
+
+# With `--ignore-missing-links` and `--boundary`, we can even print those boundary
+# commits.
+test_expect_success 'rev-list prints boundary commit with --ignore-missing-links' '
+ git -C main rev-list --ignore-missing-links --boundary HEAD >actual &&
+ test_stdout_line_count = 6 cat actual &&
+ grep -$BOUNDARY_COMMIT actual
+'
+
+# The `--ignore-missing-links` option should ensure that git-rev-list(1) doesn't
+# fail when used alongside `--objects` when a tree is missing.
+test_expect_success 'rev-list --ignore-missing-links works with missing tree' '
+ echo "foo" >main/file &&
+ git -C main add file &&
+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main commit -m"commit 11" &&
+ TREE_OID=$(git -C main rev-parse HEAD^{tree}) &&
+ mkdir alt/${TREE_OID:0:2} &&
+ mv main/.git/objects/${TREE_OID:0:2}/${TREE_OID:2} alt/${TREE_OID:0:2}/ &&
+ git -C main rev-list --ignore-missing-links --objects HEAD >actual &&
+ ! grep $TREE_OID actual
+'
+
+# Similar to above, it should also work when a blob is missing.
+test_expect_success 'rev-list --ignore-missing-links works with missing blob' '
+ echo "bar" >main/file &&
+ git -C main add file &&
+ GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main commit -m"commit 12" &&
+ BLOB_OID=$(git -C main rev-parse HEAD:file) &&
+ mkdir alt/${BLOB_OID:0:2} &&
+ mv main/.git/objects/${BLOB_OID:0:2}/${BLOB_OID:2} alt/${BLOB_OID:0:2}/ &&
+ git -C main rev-list --ignore-missing-links --objects HEAD >actual &&
+ ! grep $BLOB_OID actual
+'
+
+test_done
--
2.41.0
^ permalink raw reply related
* Re: [PATCH] This fixes a minor memory leak (detected by LeakSanitizer) in git merge.
From: Elijah Newren @ 2023-09-12 15:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Backhouse via GitGitGadget, git, Kevin Backhouse
In-Reply-To: <xmqqo7j4t4dm.fsf@gitster.g>
On Fri, Aug 18, 2023 at 2:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kevin Backhouse via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Kevin Backhouse <kevinbackhouse@github.com>
> >
> > To reproduce (with an ASAN build):
> >
> > ```
> > mkdir test
> > cd test
> > git init
> > echo x > x.txt
> > git add .
> > git commit -m "WIP"
> > git checkout -b dev
> > echo y > x.txt
> > git add .
> > git commit -m "WIP"
> > git checkout main
> > echo z > x.txt
> > git add .
> > git commit -m "WIP"
> > echo a > x.txt
> > git add .
> > git merge dev
> > ```
>
> We'd rather not to see the above in the proposed log message; can't
> we add (a variation of) it to our test suite?
>
> > The fix is to call free_commit_list(merge_bases) when an error occurs.
>
> We usually have the description of what the problem is and give an
> analysis on why/how it happens, before presenting a solution. Write
> it more like:
>
> The caller of merge_ort_recursive() expects the commit list
> passed in as the merge_bases parameter to be fully consumed by
> the function and does not free it when the function returns. In
> normal cases, the commit list does get consumed, but when the
> function returns early upon encountering an error, it forgets to
> clean it up.
>
> Fix this by freeing the list in the code paths for error returns.
>
> > merge-ort-wrappers.c | 4 +++-
> > merge-ort.c | 4 +++-
>
> These two places and their fixes seem OK, but I have to wonder if
> these are complete fixes.
>
> > diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c
> > index 4acedf3c338..aeb56c9970c 100644
> > --- a/merge-ort-wrappers.c
> > +++ b/merge-ort-wrappers.c
> > @@ -54,8 +54,10 @@ int merge_ort_recursive(struct merge_options *opt,
> > struct tree *head = repo_get_commit_tree(opt->repo, side1);
> > struct merge_result tmp;
> >
> > - if (unclean(opt, head))
> > + if (unclean(opt, head)) {
> > + free_commit_list(merge_bases);
> > return -1;
> > + }
> >
> > memset(&tmp, 0, sizeof(tmp));
> > merge_incore_recursive(opt, merge_bases, side1, side2, &tmp);
>
> The function before this hunk appears to have very similar code
> structure. Does it need the same fix, or if not why not?
>
> > diff --git a/merge-ort.c b/merge-ort.c
> > index 8631c997002..a0eb91fb011 100644
> > --- a/merge-ort.c
> > +++ b/merge-ort.c
> > @@ -5070,8 +5070,10 @@ static void merge_ort_internal(struct merge_options *opt,
> > opt->branch1 = "Temporary merge branch 1";
> > opt->branch2 = "Temporary merge branch 2";
> > merge_ort_internal(opt, NULL, prev, next, result);
> > - if (result->clean < 0)
> > + if (result->clean < 0) {
> > + free_commit_list(merge_bases);
> > return;
> > + }
>
> Before this function, there is a comment that this came from another
> function and it seems to still have a very similar code structure.
> Does the other function need the same fix, or if not why not?
The other function would need a more involved fix, which would
basically involve porting a59b8dd94f (merge-ort: fix memory leak in
merge_ort_internal(), 2022-01-20) to merge-recursive as a preparatory
step. This particular cleanup cannot be ported in its current form to
merge-recursive.c until then.
^ permalink raw reply
* Re: [PATCH v2] merge-tree: add -X strategy option
From: Elijah Newren @ 2023-09-12 15:03 UTC (permalink / raw)
To: Izzy via GitGitGadget; +Cc: git, winglovet
In-Reply-To: <pull.1565.v2.git.1691818386345.gitgitgadget@gmail.com>
Hi,
Sorry for the delay in responding; I've been busy with non-git things...
On Sat, Aug 12, 2023 at 12:19 AM Izzy via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: winglovet <winglovet@gmail.com>
>
> Add merge strategy option to produce more customizable merge result such
> as automatically solve conflicts.
>
> Signed-off-by: winglovet <winglovet@gmail.com>
Junio already flagged this line in your v1; you either need to correct
it or respond to his comments about it.
> ---
> merge-tree: add -X strategy option
>
> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1565
>
> Range-diff vs v1:
>
> 1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option
> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char
> /* Handle --stdin */
> if (o.use_stdin) {
> struct strbuf buf = STRBUF_INIT;
> +
> + ## t/t4301-merge-tree-write-tree.sh ##
> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
> + git branch side1 &&
> + git branch side2 &&
> + git branch side3 &&
> ++ git branch side4 &&
> +
> + git checkout side1 &&
> + test_write_lines 1 2 3 4 5 6 >numbers &&
> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
> + test_tick &&
> + git commit -m rename-numbers &&
> +
> ++ git checkout side4 &&
> ++ test_write_lines 0 1 2 3 4 5 >numbers &&
> ++ echo yo >greeting &&
> ++ git add numbers greeting &&
> ++ test_tick &&
> ++ git commit -m other-content-modifications &&
> ++
> + git switch --orphan unrelated &&
> + >something-else &&
> + git add something-else &&
> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '
> + test_cmp expect actual
> + '
> +
> ++test_expect_success 'Auto resolve conflicts by "ours" stragety option' '
> ++ git checkout side1^0 &&
> ++
> ++ # make sure merge conflict exists
> ++ test_must_fail git merge side4 &&
> ++ git merge --abort &&
> ++
> ++ git merge -X ours side4 &&
> ++ git rev-parse HEAD^{tree} > expected &&
> ++
> ++ git merge-tree -X ours side1 side4 > actual &&
> ++
> ++ test_cmp expected actual
> ++'
> ++
> + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
> + # Mis-spell with single "s" instead of double "s"
> + test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&
>
>
> builtin/merge-tree.c | 24 ++++++++++++++++++++++++
> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 0de42aecf4b..2ec6ec0d39a 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -19,6 +19,8 @@
> #include "tree.h"
> #include "config.h"
>
> +static const char **xopts;
> +static size_t xopts_nr, xopts_alloc;
I know we already have lots of globals, and I'm partially to blame
since I also copied this style from elsewhere into this file, but it'd
sure be nice to get rid of these needless globals rather than add
more. But, certainly not a blocker....
> static int line_termination = '\n';
>
> struct merge_list {
> @@ -414,6 +416,7 @@ struct merge_tree_options {
> int show_messages;
> int name_only;
> int use_stdin;
> + struct merge_options merge_options;
> };
Probably the best we can do now, but feels a little icky to me that
parse_merge_opt() works on all of struct merge_options rather than
having a dedicated struct for the particular options it can set. Made
sense back in the merge-recursive.[ch] days, but it provides too many
extra unrelated and unnecessary fields now.
This isn't something you need to fix in this patch, though, just
something else I'll keep in mind when I get back to working on
removing merge-recursive.[ch].
> static int real_merge(struct merge_tree_options *o,
> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,
>
> init_merge_options(&opt, the_repository);
>
> + opt.recursive_variant = o->merge_options.recursive_variant;
> +
> opt.show_rename_progress = 0;
>
> opt.branch1 = branch1;
> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,
> return !result.clean; /* result.clean < 0 handled above */
> }
>
> +static int option_parse_x(const struct option *opt,
> + const char *arg, int unset)
> +{
> + if (unset)
> + return 0;
> +
> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);
> + xopts[xopts_nr++] = xstrdup(arg);
> + return 0;
> +}
> +
> int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> {
> struct merge_tree_options o = { .show_messages = -1 };
> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> &merge_base,
> N_("commit"),
> N_("specify a merge-base for the merge")),
> + OPT_CALLBACK('X', "strategy-option", &xopts,
> + N_("option=value"),
> + N_("option for selected merge strategy"),
> + option_parse_x),
> OPT_END()
> };
>
> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> argc = parse_options(argc, argv, prefix, mt_options,
> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);
>
> + for (int x = 0; x < xopts_nr; x++)
> + if (parse_merge_opt(&o.merge_options, xopts[x]))
> + die(_("unknown strategy option: -X%s"), xopts[x]);
> +
> /* Handle --stdin */
> if (o.use_stdin) {
> struct strbuf buf = STRBUF_INIT;
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index 250f721795b..2718817628c 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -22,6 +22,7 @@ test_expect_success setup '
> git branch side1 &&
> git branch side2 &&
> git branch side3 &&
> + git branch side4 &&
>
> git checkout side1 &&
> test_write_lines 1 2 3 4 5 6 >numbers &&
> @@ -46,6 +47,13 @@ test_expect_success setup '
> test_tick &&
> git commit -m rename-numbers &&
>
> + git checkout side4 &&
> + test_write_lines 0 1 2 3 4 5 >numbers &&
> + echo yo >greeting &&
> + git add numbers greeting &&
> + test_tick &&
> + git commit -m other-content-modifications &&
> +
> git switch --orphan unrelated &&
> >something-else &&
> git add something-else &&
> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
> test_cmp expect actual
> '
>
> +test_expect_success 'Auto resolve conflicts by "ours" stragety option' '
stragety -> strategy
> + git checkout side1^0 &&
> +
> + # make sure merge conflict exists
> + test_must_fail git merge side4 &&
> + git merge --abort &&
> +
> + git merge -X ours side4 &&
> + git rev-parse HEAD^{tree} > expected &&
Style: '>filename', not '> filename'.
> +
> + git merge-tree -X ours side1 side4 > actual &&
Same style issue with redirect.
Also, why is the leading spacing inconsistent with the surrounding lines?
> +
> + test_cmp expected actual
> +'
> +
> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
> # Mis-spell with single "s" instead of double "s"
> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&
I personally don't see the value in passing `-X ours` or `-X theirs`
to merges. I've never really been a fan of either. Granted, I
understand the value in making merge-tree support the same things that
existing merges support, so I'm fine with supporting it, but I'd
rather have a useful example for the testcase such as
-Xignore-space-change or something (or maybe use it as an additional
example?) Not a blocker on its own, but just a personal preference.
Anyway, the general direction of the patch seems reasonable. Not
everything I commented on needs to be fixed, as I noted, but there are
a few small things to fix up.
^ permalink raw reply
* Re: [PATCH] revision: add `--ignore-missing-links` user option
From: Karthik Nayak @ 2023-09-12 14:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwmx0sca3.fsf@gitster.g>
On Fri, Sep 8, 2023 at 9:19 PM Junio C Hamano <gitster@pobox.com> wrote:
> The above description needs tightened up a bit, I think.
>
> What is left unsaid is that you arranged a repository to borrow from
> an alternate object directory (or two), and plan to walk objects
> with this bit on in the repository, while leaving the alternates
> disabled. Without stating that you plan to disable the alternates
> while this mode of operation happens, nothing would happen when the
> traversal goes from the main to the alternate because no links are
> broken, no?
>
Fair enough, I agree with your points. I'll amend the message to highlight this
scenario.
> > By exposing this new flag `--ignore-missing-links`, users can set the
> > required env variables (GIT_OBJECT_DIRECTORY and
> > GIT_ALTERNATE_OBJECT_DIRECTORIES) along with the `--boundary` flag to
> > find the boundary objects between object directories.
>
> This command being a plumbing, there is not much reason to object to
> surfacing features that already internally exist to the command line
> option. Having said that,
>
> * Suppose your traversal with --ignore-missing-links from the tip
> of a branch reaches a tree object A, and the tree object A has a
> link to a blob B and a blob C. But B is in a separate object
> store that you usually access via the alternate mechanism.
> Instead of barfing "The repository is corrupt---object A points
> at object B that does not exist", we pretend that A does not have
> the link to B and keep traversing, discovering C and other
> objects.
>
> That much we can read from the above and also the documentation
> part of the patch. The interaction with --boundary needs to be
> clarified in this description and the documentation, though. It
> is unclear if you show 'A' or 'B' in this scenario.
Do note that the `--boundary` option only works with commits. Keeping this in
mind `--ignore-missing-links` when used with `--boundary` doesn't even traverse
non-commit objects. Which means trees/blobs being corrupted shouldn't matter.
But I did realize that `--ignore-missing-links` as this patch stands
is broken when
used alongside the `--objects` flag (`--boundary` doesn't work with
`--objects` at the
moment, this is something I plan to tackle soon after with a
`--boundary-objects` flag).
The second version of this patch will have a fix to ensure that even
non-commit objects
are ignored during traversal if `--objects` option is used.
>
> * Some traversals use the ignore-missing-links bit implicitly and
> currently there is no way to turn it off. Is it plausible that
> user may want to explicitly toggle it off, with the option
> negated, i.e. --no-ignore-missing-links? I do not immediately
> see the utility of such an option, but that is only due to my
> lack of imagination. For now, I think it makes sense not to
> allow negating this option, until somebody comes up with a useful
> use case.
>
Agreed!
> > +--ignore-missing-links::
> > + When an object points to another object that is missing, pretend as if the
> > + link did not exist. These missing links are not written to stdout unless
> > + the --boundary flag is passed.
>
> Does "git rev-list" ever writes "links"? I thought not.
>
> "These missing objects are not written" would be more sensible, but
> we never write missing objects with or without the option, so it
> is not even worth saying.
>
> When "--boundary" is passed, do they appear as if they are
> available? If not, then the above description is very misleading.
>
> During traversal, if an object that is referenced does not
> exist, pretend as if the reference itself does not exist,
> instead of dying of a repository corruption. Running the
> command with the "--boundary" option makes these missing
> objects, together with the objects on the edge of revision
> ranges (i.e. true boundary objects), appear on the output,
> prefixed with '-'.
>
> or something like that, perhaps?
>
This indeed is better, I've copied and modified it as needed.
> > +# With `--ignore-missing-links`, we stop the traversal when we encounter a
> > +# missing link.
> > +test_expect_success 'rev-list only prints main odb commits with --ignore-missing-links' '
> > + test_stdout_line_count = 5 git -C main rev-list --ignore-missing-links HEAD
> > +'
> > +
> > +# With `--ignore-missing-links` and `--boundary`, we can even print those boundary
> > +# commits.
> > +test_expect_success 'rev-list prints boundary commit with --ignore-missing-links' '
> > + git -C main rev-list --ignore-missing-links --boundary HEAD >list-output &&
> > + test_stdout_line_count = 6 cat list-output &&
> > + test_stdout_line_count = 1 cat list-output | grep "^-"
> > +'
>
> These tests are way too loose. Not only you want to see certain
> number of boundary objects, you _know_ exactly which object should
> be on the boundary, and you should check that instead. That will
> allow you to find a mistake to write commit 'A' that refers to a
> missing commit 'B', when they wanted to write the missing comit 'B',
> as a boundary object, for example.
>
Fair enough, I will make them more specific and add some tests for
missing trees/blobs.
> Thanks.
Thank you for the review. Will send the next version of the patch soon :)
^ permalink raw reply
* Re: Commit dates on conflict markers
From: Elijah Newren @ 2023-09-12 14:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Roger Light, git
In-Reply-To: <xmqq8r9cs2x1.fsf@gitster.g>
On Mon, Sep 11, 2023 at 4:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Roger Light <roger@atchoo.org> writes:
>
> > When I carry out a merge with conflicts, it's not always clear when
> > resolving the conflicts which is the correct part of code to use. I
> > sometimes use git blame to guide me as to the age of the different
> > chunks of code and hence what to choose.
> >
> > I was wondering if there might be a way to help include that sort of
> > information directly into the conflict.
> >
> > If you had a single line conflict it would be straightforward to
> > display by including the date the line was last modified alongside the
> > conflict marker:
> >
> > <<<<<<< HEAD date:yesterday
> > print("please")
> > ======= date:10 years ago
> > print("help")
> > >>>>>>> main
> >
> > With a more realistic change with multiple lines and context from
> > different commits, it's not immediately obvious to me that it's
> > possible to do in a way that isn't completely horrible.
>
> Our conflict marker lines do get human readable labels but the
> format used by merge_3way() both in merge-ort and merge-recursive
> backends is hardcoded to be <branchname> ':' <pathname> and it is
> sufficient to let you tell which commit involved in the merge and
> which path in that commit the contents came from.
>
> A change that only shows the commit date without allowing end user
> configuration will *not* be worth doing, but allowing them to use
> placeholders like '%h %s' in "git log --format='%h %s'" (check
> pretty.c for the catalog) would be a good exercise; it should not
> take somebody with an ultra-deep knowledge of how the code works.
Generalizing conflict marker annotations to use other hints may make
sense, but I am not sure that "date" is a good example or reason to
generalize it, for three reasons:
* [No date] Virtual merge bases from recursive merges do not have a
date to associate with them. Do we just make one up? Average the
range of dates of the commits being merged to create the virtual merge
base?
* [Wrong date(s)] The date Roger probably wants is the date when the
conflicting text was introduced to the given side of history, not the
date of the tip of that branch. merge-ort is pretty fundamentally a
three-way merge algorithm, meaning it only ever uses the merge-base
and the two branch tips. I don't want to see that changed either;
other than for computing the merge base, I'm very skeptical of any
movement to make merge-ort depend upon any intermediate commit(s).
Such changes would likely be better placed in an entirely new merge
algorithm, but then you have to write an entirely new merge algorithm,
which is decidedly non-trivial.
* [Too many dates] What if the conflict region had two lines on one
side and each line was added on different dates -- what then to
display for the date for that side of history? The earliest? The
latest? Some kind of weighted average? Feels like a bug report
generator to me.
So, if we generalize conflict marker annotations, we probably ought to
omit "date" from the new possibilities.
^ permalink raw reply
* Re: [PATCH 1/1] git-grep: improve the --show-function behaviour
From: Oleg Nesterov @ 2023-09-12 13:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Calvin Wan,
Carlo Marcelo Arenas Belón, Elijah Newren, Jeff King,
Linus Torvalds, Mathias Krause, René Scharfe, Taylor Blau,
git, Alexey Gladkov
In-Reply-To: <20230912130429.GA9982@redhat.com>
On 09/12, Oleg Nesterov wrote:
>
> René, Junio,
>
> I don't like the fact we can't understand each other ;) Could you
> please explain why do you think this patch should update the docs?
>
> Please forget about my patch for the moment. Lets start from the very
> beginning:
>
> -p::
> --show-function::
> Show the preceding line that contains the function name of
> the match, unless the matching line is a function name itself.
>
> and in my opinion, it is the current behaviour that doesn't match the
> documentation.
>
> -------------------------------------------------------------------------
>
> $ cat TEST1.c
> void func1()
> {
> }
> void func2()
> {
> }
>
> $ git grep --untracked -pn func2 TEST1.c
> TEST1.c=1=void func1()
> TEST1.c:4:void func2()
>
> in this case the matching line is "void func2()" and it is also a function
> name itself, in this case git-grep should not show "=void func1()" which is
> "the preceding line that contains the function name of the match.
>
> But it does. So perhaps git-grep needs another change, something like
>
> if (match_funcname(opt, gs, bol, end_of_line(...)))
> return;
>
> at the start of show_funcname_line(), but my patch does not change this
> behaviour.
>
> --------------------------------------------------------------------------
>
> $ cat TEST2.c
> void func(xxx)
> {
> use(xxx);
> }
>
> $ git grep --untracked -pn xxx TEST2.c
> TEST2.c:1:void func(xxx)
> TEST2.c:3: use(xxx)
>
> the 2nd match is use(xxx) and it is not a function name itself, in this
> case git-grep should "Show the preceding line that contains the function
> name of the match.
>
> But it doesn't. To me, this behaviour looks as
>
> Show the preceding line that contains the function name of
> the match, unless the _PREVIOUS_ matching line is a function
> name itself.
>
> Now, with my patch we have
>
> $ ./git grep --untracked -pn xxx TEST2.c
> TEST2.c:1:void func(xxx)
> TEST2.c=1=void func(xxx)
> TEST2.c:3: use(xxx);
>
> and unless I am totatlly confused this does match the documentation.
So, just in case, please see V2 below. In my opinion it _fixes_ the
current behaviour. With this patch
$ ./git grep --untracked -pn func2 TEST1.c
TEST1.c:4:void func2()
$ ./git grep --untracked -pn xxx TEST2.c
TEST2.c:1:void func(xxx)
TEST2.c=1=void func(xxx)
TEST2.c:3: use(xxx);
Or I am totally confused?
Oleg.
---
diff --git a/grep.c b/grep.c
index 0904d55b24..c240c4bfa1 100644
--- a/grep.c
+++ b/grep.c
@@ -1347,15 +1347,19 @@ static int match_funcname(struct grep_opt *opt, struct grep_source *gs,
static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,
const char *bol, unsigned lno)
{
+ unsigned long left = bol - gs->buf;
+
+ if (match_funcname(opt, gs, bol, end_of_line(bol, &left)))
+ return;
+
while (bol > gs->buf) {
const char *eol = --bol;
+ if (--lno < opt->last_shown)
+ break;
+
while (bol > gs->buf && bol[-1] != '\n')
bol--;
- lno--;
-
- if (lno <= opt->last_shown)
- break;
if (match_funcname(opt, gs, bol, eol)) {
show_line(opt, bol, eol, gs->name, lno, 0, '=');
^ permalink raw reply related
* Re: [PATCH 01/32] doc hash-file-transition: A map file for mapping between sha1 and sha256
From: Eric W. Biederman @ 2023-09-12 13:36 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano
In-Reply-To: <ZP+tTFK3Ly4sqlsq@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On 2023-09-08 at 23:10:18, Eric W. Biederman wrote:
>> The v3 pack index file as documented has a lot of complexity making it
>> difficult to implement correctly. I worked with bryan's preliminary
>> implementation and it took several passes to get the bugs out.
>>
>> The complexity also requires multiple table look-ups to find all of
>> the information that is needed to translate from one kind of oid to
>> another. Which can't be good for cache locality.
>>
>> Even worse coming up with a new index file version requires making
>> changes that have the potentialy to break anything that uses the index
>> of a pack file.
>>
>> Instead of continuing to deal with the chance of braking things
>> besides the oid mapping functionality, the additional complexity in
>> the file format, and worry if the performance would be reasonable I
>> stripped down the problem to it's fundamental complexity and came up
>> with a file format that is exactly about mapping one kind of oid to
>> another, and only supports two kinds of oids.
>>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> ---
>> .../technical/hash-function-transition.txt | 40 +++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/technical/hash-function-transition.txt b/Documentation/technical/hash-function-transition.txt
>> index ed574810891c..4b937480848a 100644
>> --- a/Documentation/technical/hash-function-transition.txt
>> +++ b/Documentation/technical/hash-function-transition.txt
>> @@ -209,6 +209,46 @@ format described in linkgit:gitformat-pack[5], just like
>> today. The content that is compressed and stored uses SHA-256 content
>> instead of SHA-1 content.
>>
>> +Per Pack Mapping Table
>> +~~~~~~~~~~~~~~~~~~~~~~
>> +A pack compat map file (.compat) files have the following format:
>> +
>> +HEADER:
>> + 4-byte signature:
>> + The signature is: {'C', 'M', 'A', 'P'}
>> + 1-byte version number:
>> + Git only writes or recognizes version 1.
>> + 1-byte First Object Id Version
>> + We infer the length of object IDs (OIDs) from this value:
>> + 1 => SHA-1
>> + 2 => SHA-256
>
> One thing I forgot to mention here, is that we have 32-bit format IDs
> for these in the structure, so we should use them here and below. These
> are GIT_SHA1_FORMAT_ID and GIT_SHA256_FORMAT_ID.
>
> Not that I would encourage distributing such software, but it makes it
> much easier for people to experiment with additional hash algorithms (in
> terms of performance, etc.) if we make the space a little sparser.
Unfortunately that ship has already sailed. If you look at pack reverse
indices, pack mtime files, multi-pack-index files, they all use an
oid_version field. So to experiment with a new hash function a new
number has to be picked.
The only use I can find of your 4 byte format_id's is in the reftable
code.
Using a 4 byte magic number in this case also conflicts with basic
simplicity. With a one byte field I can specify it easily, and read it
back with no special tools, and understand what it means at a glance.
I admit I can only understand what a oid version field means at a glance
because the variation of object id's is low, but that is fundamental.
We require global agreement on names. Fundamentally git can not
support many object id transitions. Names are just too expensive.
When I come to how the map file is specified a single byte has real
advantages. A single byte never needs byte swapping. So it won't
be misread. Using a single byte for each format allows me to
keep the header for the file at 16 bytes. Which guarantees good
alignment of everything in the file without having to be clever.
All of this is for a file that is strictly local and the entire function
of the bytes is a sanity check to make certain that something weird is
not going on, or to assist recover if something bad happens.
So in this case I don't see any additional agility provided by longer
names helping.
>> + 1-byte Second Object Id Version
>> + We infer the length of object IDs (OIDs) from this value:
>> + 1 => SHA-1
>> + 2 => SHA-256
>
> In your new patch for the next part, you consider that there might be
> multiple compatibility hash algorithms. I had anticipated only one at
> a time in my series, but I'm not opposed to multiple if you want to
> support that.
>
> However, here you're making the assumption that there are only two. If
> you want to support multiple values, we need to explicitly consider that
> both here (where we need a count of object ID version and multiple
> tables, one for each algorithm), and in the follow-up series.
>
> I had not considered more than two algorithms because it substantially
> complicates the code and requires us to develop n*(n-1) tables, but I'm
> not the one volunteering to do most of the work here, so I'll defer to
> your preference. (I do intend to send a patch or two, though.)
>
> It's also possible we could be somewhat provident and define the on-disk
> formats for multiple algorithms and then punt on the code until later if
> you prefer that.
In the long term I anticipate people disabling compatObjectFormat and
switching to readCompatMap so they still have access to their old
objects by their original names, but they don't have the over head
of computing a compatibility hash.
In a world where there is a transition to futureHash I anticipate
the files associated with an old pack looking something like:
pack-abcdefg.compat12
pack-abcdefg.compat32
For a repository still using hash version sha256 for storage, with a
mapping to some sha1 names, and a mapping of everything to new
names for compatibility with futureHash.
After transitioning to the futureHash those files would look like:
pack-abcdefg.compat13
pack-abcdefg.compat23
I deeply and fundamentally care about having some way to look up
old names because I do that all of the time.
In my work on the linux-kernel I have found myself frequently digging
into old issues. I have on my hard drive tglx's git import of the
old bitkeeper tree. I also have an import of all of the old kernel
releases into git from before the code was stored in bitkeeper. I find
myself actually using all of those trees when digging into issues.
So I think the idea that we will ever be able to get rid of the mapping
for old converted repositories is unlikely. We have entirely too many
references out there.
Which means that for every hash format conversion a repository goes
through I am going to have another collection of old names.
I don't honestly anticipate ever needing to have multiple
compatObjectFormat entries specified for a single repository. I do
agree that if we are going to worry about forward and backward
compatibility we should be robust and have a configuration file syntax
that can handle the possibility.
I do very much anticipate needing to have multiple readCompatMap
entries, and pretty much only using them in get_short_oid in
object-name.c. It will make the loop in find_short_packed_compat_object
a little longer but that is about all that will need to be implemented
and maintained long term.
I view this compat map format a lot like the loose objects. It is
simple and good enough to get us started. If it turns out we need
to optimize it's simplicity means all of the interfaces in the code
to use it have already been built, and we can just concentrate on
optimizing.
Eric
^ permalink raw reply
* Re: [PATCH v2] sequencer: remove unreachable exit condition in pick_commits()
From: Phillip Wood @ 2023-09-12 13:27 UTC (permalink / raw)
To: Oswald Buddenhagen, git; +Cc: Johannes Schindelin, Junio C Hamano
In-Reply-To: <20230912105541.272917-1-oswald.buddenhagen@gmx.de>
On 12/09/2023 11:55, Oswald Buddenhagen wrote:
> This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
> 'edit' command", 2017-01-02), and was pointless from the get-go: all
> early exits from the loop above are returns, so todo_list->current ==
> todo_list->nr is an invariant after the loop.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Thanks for updating the commit message, I think it is clearer now
Best Wishes
Phillip
> ---
> v2:
> - improved commit message
>
> Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> ---
> sequencer.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index a66dcf8ab2..99e9c520ca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,
> struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
> struct stat st;
>
> - /* Stopped in the middle, as planned? */
> - if (todo_list->current < todo_list->nr)
> - return 0;
> -
> if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
> starts_with(head_ref.buf, "refs/")) {
> const char *msg;
^ permalink raw reply
* Re: [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test
From: Phillip Wood @ 2023-09-12 13:26 UTC (permalink / raw)
To: Oswald Buddenhagen, git; +Cc: Phillip Wood, Junio C Hamano
In-Reply-To: <20230912104237.271616-1-oswald.buddenhagen@gmx.de>
On 12/09/2023 11:42, Oswald Buddenhagen wrote:
> This test was introduced by commit 0c164ae7a ("rebase -i: add another
> reword test", 2021-08-20). I didn't quite get what it was meant to do,
> so here's an explanation from Phillip:
>
> The purpose of the test is to ensure that
>
> (i) There are no uncommited changes when the editor runs. I.e., we
> commit without running the editor and then reword by amending
> that commit. This ensures that we have the same user experience
> whether or not the commit was fast-forwarded [1].
>
> (ii) That the todo list is re-read after the commit has been reworded.
> This is to allow the user to update the todo list while the rebase
> is paused for editing the commit message.
>
> [1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
This looks good to me, Thanks
Phillip
> ---
> v2:
> - actually, it's kinda a new patch now
>
> Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
> Cc: Junio C Hamano <gitster@pobox.com>
> ---
> t/t3404-rebase-interactive.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 96a56aafbe..bf94eb28b9 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -758,7 +758,7 @@ test_expect_success 'reword' '
> git show HEAD~2 | grep "C changed"
> '
>
> -test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '
> +test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '
> git checkout E &&
> test_when_finished "git checkout @{-1}" &&
> (
^ permalink raw reply
* Re: [PATCH 1/1] git-grep: improve the --show-function behaviour
From: Oleg Nesterov @ 2023-09-12 13:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Calvin Wan,
Carlo Marcelo Arenas Belón, Elijah Newren, Jeff King,
Linus Torvalds, Mathias Krause, René Scharfe, Taylor Blau,
git, Alexey Gladkov
In-Reply-To: <20230911231756.GA2840@redhat.com>
René, Junio,
I don't like the fact we can't understand each other ;) Could you
please explain why do you think this patch should update the docs?
Please forget about my patch for the moment. Lets start from the very
beginning:
-p::
--show-function::
Show the preceding line that contains the function name of
the match, unless the matching line is a function name itself.
and in my opinion, it is the current behaviour that doesn't match the
documentation.
-------------------------------------------------------------------------
$ cat TEST1.c
void func1()
{
}
void func2()
{
}
$ git grep --untracked -pn func2 TEST1.c
TEST1.c=1=void func1()
TEST1.c:4:void func2()
in this case the matching line is "void func2()" and it is also a function
name itself, in this case git-grep should not show "=void func1()" which is
"the preceding line that contains the function name of the match.
But it does. So perhaps git-grep needs another change, something like
if (match_funcname(opt, gs, bol, end_of_line(...)))
return;
at the start of show_funcname_line(), but my patch does not change this
behaviour.
--------------------------------------------------------------------------
$ cat TEST2.c
void func(xxx)
{
use(xxx);
}
$ git grep --untracked -pn xxx TEST2.c
TEST2.c:1:void func(xxx)
TEST2.c:3: use(xxx)
the 2nd match is use(xxx) and it is not a function name itself, in this
case git-grep should "Show the preceding line that contains the function
name of the match.
But it doesn't. To me, this behaviour looks as
Show the preceding line that contains the function name of
the match, unless the _PREVIOUS_ matching line is a function
name itself.
Now, with my patch we have
$ ./git grep --untracked -pn xxx TEST2.c
TEST2.c:1:void func(xxx)
TEST2.c=1=void func(xxx)
TEST2.c:3: use(xxx);
and unless I am totatlly confused this does match the documentation.
Oleg.
^ permalink raw reply
* Re: [PATCH] completion: improve doc for complex aliases
From: Philippe Blain @ 2023-09-12 12:13 UTC (permalink / raw)
To: Linus Arver, Philippe Blain via GitGitGadget, git; +Cc: Steffen Prohaska
In-Reply-To: <owlyil8gkxrm.fsf@fine.c.googlers.com>
Hi Linus,
Le 2023-09-11 à 21:04, Linus Arver a écrit :
> Hi Philippe,
>
> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> The completion code can be told to use a particular completion for
>> aliases that shell out by using ': git <cmd> ;' as the first command of
>> the alias. This only works if <cmd> and the semicolon are separated by a
>> space. The examples have that space but it's not clear if it's just for
>> style or if it's mandatory.
>>
>> Explicitely mention it.
>
> It would be even more helpful if you explain _why_ it is mandatory in
> the commit message. Is there some Bash-specific behavior or something
> else going on here?
>
> If you are unable to explain why, then as an alternative you could
> explain the error or buggy behavior (any error messages encountered, for
> example) you observe on your system when you do not use the space (which
> is corrected by applying the suggestion you are adding in this patch).
Yeah, I guess I did not investigate why it did not work without the space,
it would be more complete if I did.
There's no error message though, it just does not work without the space
(as I wrote in the commit message) in the sense that it won't suggest completion
for the git command '<cmd>'.
I'll investigate and update the commit message.
Thanks,
Philippe.
^ permalink raw reply
* Re: [PATCH v3 02/32] doc hash-function-transition: Augment compatObjectFormat with readCompatMap
From: Eric W. Biederman @ 2023-09-12 12:11 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Junio C Hamano, brian m. carlson, git
In-Reply-To: <ZQAZ19gyvt8ab7f6@ugly>
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> On Mon, Sep 11, 2023 at 06:46:19PM -0500, Eric W. Biederman wrote:
>>+The difference between compatObjectFormat and readCompatMap would be that
>>+compatObjectFormat would ask git to read existing maps, but would not ask
>>+git to write or create them.
>>
> the argument makes sense, but the asymmetry in the naming bugs me. in particular
> "[read]compatMap" seems too non-descript.
I am open to suggestions for better names.
From a code point of view I am intending readCompatMap only supporting
the things that can be support with just the mapping functions aka
repo_oid_to_algop for the "readComatMap" case.
While the compatObjectFormat case includes what can be done with using
the compatible hash algorithm and convert_object_file.
There is quite a large variation. So there is some fundamental
asymmetry in the implementation. I am just not certain how to name it.
Eric
^ permalink raw reply
* Re: [PATCH] completion: commit: complete configured trailer tokens
From: Philippe Blain @ 2023-09-12 12:02 UTC (permalink / raw)
To: Martin Ågren, Philippe Blain via GitGitGadget; +Cc: git, ZheNing Hu
In-Reply-To: <20230911102017.1927468-1-martin.agren@gmail.com>
Hi Martin,
Le 2023-09-11 à 06:20, Martin Ågren a écrit :
> Hi Philippe,
>
>> Add a __git_trailer_tokens function to list the configured trailers
>> tokens, and use it in _git_commit to suggest the configured tokens,
>> suffixing the completion words with ':' so that the user only has to add
>> the trailer value.
>
> Makes sense.
>
> I've never dabbled in the completion scripts, so take the following with
> some salt.
>
>> +__git_trailer_tokens ()
>> +{
>> + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}'
>> +}
>
> The rest of this script uses `__git config` rather than `git config`.
> The purpose of `__git` seems to be to respect options given on the
> command line, so I think we would want to use it here.
>
> These "." in "trailer." and ".key" will match any character. We also
> don't anchor this at beginning and end. Maybe tighten this a bit and use
> '^trailer\..*\.key$' to behave better in the face of config such as
> this:
>
> [strailer]
> skeying = "s"
> [trailerx]
> keyx = "x"
>
> Another thing. Consider such a config:
>
> [trailer "q.p"]
> key = "Q-p-by"
>
> The "trailer.q.p.key" config above ends up completing as just "q"
> because of how you use `print $2`. I see that `git commit --trailer=`
> itself is fairly relaxed here, so `--trailer=q` effectively ends up
> picking up "q.p" in the end. Tightening that is obviously out of scope
> here and I have no opinion if the current behavior there is intended.
> But maybe we should be a bit less relaxed here and complete to "q.p"? At
> any rate, it gets weird when you also have "trailer.q.x.key" in your
> config but we still just suggest the one "q".
>
> I see your patch is in next, but maybe some of this tightening might be
> worthwhile doing on top of it?
>
> Martin
>
These are all good suggestions. I'll send a new version with these fixes
on top.
Thanks,
Philippe.
^ permalink raw reply
* Re: [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test
From: Oswald Buddenhagen @ 2023-09-12 11:14 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Junio C Hamano
In-Reply-To: <20230912104237.271616-1-oswald.buddenhagen@gmx.de>
On Tue, Sep 12, 2023 at 12:42:36PM +0200, Oswald Buddenhagen wrote:
>so here's an explanation from Phillip:
>
>The purpose of the test is to ensure that
>
> (i) There are no uncommited changes when the editor runs. I.e., we
^
you may want to add the missing 't' here when applying.🙃
regards
^ permalink raw reply
* [PATCH v2] sequencer: remove unreachable exit condition in pick_commits()
From: Oswald Buddenhagen @ 2023-09-12 10:55 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
In-Reply-To: <ddf8dc95-6583-4257-b48a-0115f59950ef@gmail.com>
This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
'edit' command", 2017-01-02), and was pointless from the get-go: all
early exits from the loop above are returns, so todo_list->current ==
todo_list->nr is an invariant after the loop.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v2:
- improved commit message
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Phillip Wood <phillip.wood123@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>
---
sequencer.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a66dcf8ab2..99e9c520ca 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,
struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
struct stat st;
- /* Stopped in the middle, as planned? */
- if (todo_list->current < todo_list->nr)
- return 0;
-
if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
starts_with(head_ref.buf, "refs/")) {
const char *msg;
--
2.42.0.419.g70bf8a5751
^ permalink raw reply related
* [PATCH v2] t3404-rebase-interactive.sh: fix typos in title of a rewording test
From: Oswald Buddenhagen @ 2023-09-12 10:42 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Junio C Hamano
In-Reply-To: <45ee4ad1-f7a3-43d8-99ef-329efc9fcdba@gmail.com>
This test was introduced by commit 0c164ae7a ("rebase -i: add another
reword test", 2021-08-20). I didn't quite get what it was meant to do,
so here's an explanation from Phillip:
The purpose of the test is to ensure that
(i) There are no uncommited changes when the editor runs. I.e., we
commit without running the editor and then reword by amending
that commit. This ensures that we have the same user experience
whether or not the commit was fast-forwarded [1].
(ii) That the todo list is re-read after the commit has been reworded.
This is to allow the user to update the todo list while the rebase
is paused for editing the commit message.
[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v2:
- actually, it's kinda a new patch now
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
Cc: Junio C Hamano <gitster@pobox.com>
---
t/t3404-rebase-interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 96a56aafbe..bf94eb28b9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -758,7 +758,7 @@ test_expect_success 'reword' '
git show HEAD~2 | grep "C changed"
'
-test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '
+test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '
git checkout E &&
test_when_finished "git checkout @{-1}" &&
(
--
2.42.0.419.g70bf8a5751
^ permalink raw reply related
* Re: [PATCH] sequencer: remove unreachable exit condition in pick_commits()
From: Phillip Wood @ 2023-09-12 10:18 UTC (permalink / raw)
To: Oswald Buddenhagen, git; +Cc: Johannes Schindelin
In-Reply-To: <20230903151132.739151-1-oswald.buddenhagen@gmx.de>
Hi Oswald
On 03/09/2023 16:11, Oswald Buddenhagen wrote:
> This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
> 'edit' command", 2017-01-02), and was pointless from the get-go, as the
> command causes an early return.
While I agree this code is unreachable, just because it was unused when
it was added does not mean it is unused now. It would be helpful for the
commit message to explain that there are no "break" or "goto" statements
in the loop body and therefore this code is only reachable when the loop
terminates because todo_list->current == todo_list->nr
Best Wishes
Phillip
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
> ---
> this remains valid after phillip's pending series, through it becomes
> marginally harder to prove (c.f. "sequencer: factor out part of
> pick_commits()").
>
> Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> ---
> sequencer.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index a66dcf8ab2..99e9c520ca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,
> struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
> struct stat st;
>
> - /* Stopped in the middle, as planned? */
> - if (todo_list->current < todo_list->nr)
> - return 0;
> -
> if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
> starts_with(head_ref.buf, "refs/")) {
> const char *msg;
^ permalink raw reply
* Re: [PATCH] t3404-rebase-interactive.sh: fix name of a rewording test
From: Phillip Wood @ 2023-09-12 10:15 UTC (permalink / raw)
To: Junio C Hamano, Oswald Buddenhagen; +Cc: git, Phillip Wood
In-Reply-To: <xmqqwmwws5gn.fsf@gitster.g>
On 11/09/2023 23:36, Junio C Hamano wrote:
> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
>> The given test name made no sense to me at all; it seems to be a
>> concatenation of two unrelated things. This was introduced by
>> commit 0c164ae7a ("rebase -i: add another reword test", 20-08-20).
>>
>> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>>
>> ---
>> actually, i don't understand what the test even does. shouldn't it, to
>> match the description, actually dirty the tree and verify that the
>> operation fails?
>>
>> Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> As a typofix the updated text looks more correct, but the above is a > very good point, so I'll let Phillip, whose 0c164ae7 (rebase -i: add
> another reword test, 2021-08-20) introduced this test, react first.
Thanks. There is a typo, but that typo is a missing "and", the title
should be
no uncommitted changes when rewording and the todo list is reloaded
The purpose of the test is to ensure that
(i) There are no uncommited changes when the editor runs. i.e. we
commit without running the editor and then reword by amending
that commit. This ensures that we have the same user experience
whether or not the commit was fast-forwarded [1].
(ii) That the todo list is re-read after the commit has been
reworded. This is to allow the user to update the todo list while
the rebase is paused for editing the commit message.
All of the action happens in the functions called from this test.
set_reword_editor() sets up an editor that runs "git diff --exit-code
HEAD" to ensure there are no uncommitted changes when it is run and also
updates the todo list. Then check_reworded_commits() checks that the
commits were reworded and that the todo list was updated.
Best Wishes
Phillip
[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/
> Thanks for spotting and fixing.
>
>> ---
>> t/t3404-rebase-interactive.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index 96a56aafbe..31ee5bc1f6 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -758,7 +758,7 @@ test_expect_success 'reword' '
>> git show HEAD~2 | grep "C changed"
>> '
>>
>> -test_expect_success 'no uncommited changes when rewording the todo list is reloaded' '
>> +test_expect_success 'no uncommitted changes when rewording' '
>> git checkout E &&
>> test_when_finished "git checkout @{-1}" &&
>> (
^ permalink raw reply
* Re: Commit dates on conflict markers
From: Jeff King @ 2023-09-12 8:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Roger Light, git, Elijah Newren
In-Reply-To: <xmqq8r9cs2x1.fsf@gitster.g>
On Mon, Sep 11, 2023 at 04:31:06PM -0700, Junio C Hamano wrote:
> A change that only shows the commit date without allowing end user
> configuration will *not* be worth doing, but allowing them to use
> placeholders like '%h %s' in "git log --format='%h %s'" (check
> pretty.c for the catalog) would be a good exercise; it should not
> take somebody with an ultra-deep knowledge of how the code works.
FWIW, I had the same thought. But I wasn't quite sure where we even set
these strings, so here are a few thoughts for anybody who wants to work
on it:
- the relevant strings are passed in to ll_merge(); you can grep for
callers and see that there are a number of strings which end up here
(based on what info we actually have).
The most interesting one to start with is probably the call in
merge_3way() of merge-ort.c.
- there we have three object_ids, "o", "a", and "b" representing the
three sides. But these are just blobs. We have strings "ancestor",
"branch1", and "branch2" in merge_options, but we would probably not
want to re-resolve those names. So probably some extra fields need
to go into merge_options.
- I'm not sure if each of those endpoints is always a commit. For a
regular merge, they would be. But in a recursive merge, we'd
sometimes create an intermediate virtual tree. So we'd need to
handle the case that there is no commit (and either fall back to a
more vanilla string, or make a fake commit with reasonable details).
- The current labels are based on the "original" ref names (which I
think are really "what was handed to merge"; so it might be
"master~13" etc) along with the blob path (if it is not the same for
both endpoints). So you'd want more than just passing the format
string to format_commit_message(). You would want an extra
placeholder to represent those values (either ref and pathname
individually, or possibly a single placeholder for "ref and maybe
pathname if it's interesting").
The least-bad way to do that is perhaps to expand the format twice:
once for the special placeholder (quoting any "%" found in the
filename, etc), and then feeding the whole result to the
pretty-print formatter.
I was hoping this might be only a few-line change, but I actually think
it's a bit more complicated than that. But still maybe not _too_ bad.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE
From: Jeff King @ 2023-09-12 8:40 UTC (permalink / raw)
To: René Scharfe; +Cc: Oswald Buddenhagen, Git List, Junio C Hamano
In-Reply-To: <15530a5f-8d06-24c9-bc2d-e313c895f477@web.de>
On Mon, Sep 11, 2023 at 10:11:56PM +0200, René Scharfe wrote:
> Am 10.09.23 um 12:18 schrieb Oswald Buddenhagen:
> > On Sat, Sep 09, 2023 at 11:14:20PM +0200, René Scharfe wrote:
> >> Convert the offending OPT_CMDMODE users and use the typed value_int
> >> point in the macro's definition to enforce that type for future ones.
> >>
> > that defeats -Wswitch[-enum], though.
>
> True. Though I don't fully understand these warnings (why not then
> also warn about if without else?), but taking them away is a bit rude
> to those who care.
I think losing warnings is unfortunate, but it's just one example.
We're losing the type information completely from the values. That might
be of use to the compiler (both for -Wswitch, but also for code
generation in general). But it is also of use to human readers, who see
that "foo" is of type "enum bar" and know what it's supposed to contain.
> > the pedantically correct solution would be using setter callbacks.
>
> Or to use an int to point to and then copy into a companion enum
> variable to after parsing, which would be my choice.
Yeah, I had the same thought. I'm just not sure how to do that in a way
that isn't a pain for the callers.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] test-lib: prevent misuses of --invert-exit-code
From: Jeff King @ 2023-09-12 8:35 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List, Ævar Arnfjörð Bjarmason
In-Reply-To: <1a60a1ca-0ef0-ecf5-d0aa-a28d7c148a82@gmail.com>
On Sun, Sep 10, 2023 at 01:08:11AM +0200, Rubén Justo wrote:
> GIT_TEST_PASSING_SANITIZE_LEAK=true and GIT_TEST_SANITIZE_LEAK_LOG=true
> use internnlly the --invert-exit-code machinery. Therefore if the user
> wants to use --invert-exit-code in combination with them, the result
> will be confusing.
>
> For the same reason, we are already using BAIL_OUT if the user tries to
> combine GIT_TEST_PASSING_SANITIZE_LEAK=check with --invert-exit-code.
>
> Let's do the same for GIT_TEST_PASSING_SANITIZE_LEAK=true and
> GIT_TEST_SANITIZE_LEAK_LOG=true.
OK, so we are trying to find a case where the user is triggering
--invert-exit-code themselves and complaining. But in the code...
> @@ -1557,15 +1557,25 @@ then
> say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
> invert_exit_code=t
> fi
> - elif test -z "$passes_sanitize_leak" &&
> - test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
> + elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
> then
> - skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
> - test_done
> + if test -n "$invert_exit_code"
> + then
> + BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=true"
> + elif test -z "$passes_sanitize_leak"
> + then
> + skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
> + test_done
> + fi
> fi
You can see at the top of the context that we will set
invert_exit_code=t ourselves, which will then complain here:
> if test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false
> then
> + if test -n "$invert_exit_code"
> + then
> + BAIL_OUT "cannot use --invert-exit-code and GIT_TEST_SANITIZE_LEAK_LOG=true"
> + fi
> +
> if ! mkdir -p "$TEST_RESULTS_SAN_DIR"
> then
> BAIL_OUT "cannot create $TEST_RESULTS_SAN_DIR"
That varible-set in the earlier context is from running in "check" mode.
So:
make GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true
will now always fail. But this is the main way you'd want to run it
(enabling the leak log catches more stuff, and the log-check function
you touch in patch 2 already covers check mode).
So I think you'd have to hoist your check above the if/else for setting
up PASSING_SANITIZE_LEAK modes.
-Peff
^ 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