* Re: Unable to install git-2.43.0 from source on macOS Big Sur 11.7.10
From: Eric Sunshine @ 2023-12-19 7:55 UTC (permalink / raw)
To: Jonathan Abrams; +Cc: git@vger.kernel.org
In-Reply-To: <IA1PR12MB60445F50DD0F844B2B779BA8B890A@IA1PR12MB6044.namprd12.prod.outlook.com>
On Mon, Dec 18, 2023 at 7:03 AM Jonathan Abrams
<jonathansabrams@outlook.com> wrote:
> The contents of "config.mak.autogen" are as follows:
> --
> CURL_LDFLAGS=-L/Users/jonathana/opt/anaconda3/lib -lcurl
> CHARSET_LIB=-liconv
So, 'configure' found a "libiconv". Okay.
> The last output of make V=1 all is as follows.
> --
> gcc -g -O2 -I. -o git-daemon daemon.o common-main.o libgit.a xdiff/lib.a reftable/libreftable.a libgit.a -lz -liconv -liconv
And it's linking against "libiconv" (the "-iconv" part). Good.
> Undefined symbols for architecture x86_64:
> "_libiconv", referenced from:
> _precompose_utf8_readdir in libgit.a(precompose_utf8.o)
> _reencode_string_iconv in libgit.a(utf8.o)
But it doesn't seem to like your "libiconv" for some reason.
I notice that you seem to have an Anaconda environment active. Do you
know if this "libiconv" happens to reside within the Anaconda
environment? If so, what version is it and from where was it installed
("conda-forge", "anaconda", some other)?
Have you tried deactivating the Anaconda environment ("conda
deactivate") before building Git? Also, I would suggest trying without
even running the configure script at all. So, for instance:
% cd git
% conda deactivate
% make distclean
% make all
^ permalink raw reply
* [PATCH] rebase: use strvec_pushf() for format-patch revisions
From: René Scharfe @ 2023-12-19 7:42 UTC (permalink / raw)
To: Git List
In run_am(), a strbuf is used to create a revision argument that is then
added to the argument list for git format-patch using strvec_push().
Use strvec_pushf() to add it directly instead, simplifying the code.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Formatted with --inter-hunk-context=14 for easier review.
builtin/rebase.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 9f8192e0a5..ddde4cbb87 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -582,7 +582,6 @@ static int run_am(struct rebase_options *opts)
{
struct child_process am = CHILD_PROCESS_INIT;
struct child_process format_patch = CHILD_PROCESS_INIT;
- struct strbuf revisions = STRBUF_INIT;
int status;
char *rebased_patches;
@@ -615,34 +614,32 @@ static int run_am(struct rebase_options *opts)
return run_command(&am);
}
- strbuf_addf(&revisions, "%s...%s",
- oid_to_hex(opts->root ?
- /* this is now equivalent to !opts->upstream */
- &opts->onto->object.oid :
- &opts->upstream->object.oid),
- oid_to_hex(&opts->orig_head->object.oid));
-
rebased_patches = xstrdup(git_path("rebased-patches"));
format_patch.out = open(rebased_patches,
O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (format_patch.out < 0) {
status = error_errno(_("could not open '%s' for writing"),
rebased_patches);
free(rebased_patches);
strvec_clear(&am.args);
return status;
}
format_patch.git_cmd = 1;
strvec_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
"--full-index", "--cherry-pick", "--right-only",
"--default-prefix", "--no-renames",
"--no-cover-letter", "--pretty=mboxrd", "--topo-order",
"--no-base", NULL);
if (opts->git_format_patch_opt.len)
strvec_split(&format_patch.args,
opts->git_format_patch_opt.buf);
- strvec_push(&format_patch.args, revisions.buf);
+ strvec_pushf(&format_patch.args, "%s...%s",
+ oid_to_hex(opts->root ?
+ /* this is now equivalent to !opts->upstream */
+ &opts->onto->object.oid :
+ &opts->upstream->object.oid),
+ oid_to_hex(&opts->orig_head->object.oid));
if (opts->restrict_revision)
strvec_pushf(&format_patch.args, "^%s",
oid_to_hex(&opts->restrict_revision->object.oid));
@@ -665,10 +662,8 @@ static int run_am(struct rebase_options *opts)
"As a result, git cannot rebase them."),
opts->revisions);
- strbuf_release(&revisions);
return status;
}
- strbuf_release(&revisions);
am.in = open(rebased_patches, O_RDONLY);
if (am.in < 0) {
--
2.43.0
^ permalink raw reply related
* Re: ps/clone-into-reftable-repository, was: What's cooking in git.git (Dec 2023, #03; Mon, 18)
From: Patrick Steinhardt @ 2023-12-19 5:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqr0jjc86e.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]
On Mon, Dec 18, 2023 at 05:06:33PM -0800, Junio C Hamano wrote:
> * ps/clone-into-reftable-repository (2023-12-12) 7 commits
> - builtin/clone: create the refdb with the correct object format
> - builtin/clone: skip reading HEAD when retrieving remote
> - builtin/clone: set up sparse checkout later
> - builtin/clone: fix bundle URIs with mismatching object formats
> - remote-curl: rediscover repository when fetching refs
> - setup: allow skipping creation of the refdb
> - setup: extract function to create the refdb
>
> "git clone" has been prepared to allow cloning a repository with
> non-default hash function into a repository that uses the reftable
> backend.
>
> Will merge to 'next'?
> source: <cover.1702361370.git.ps@pks.im>
I personally consider the series to be ready for 'next'. There are no
outstanding review comments that need to be addressed, and reviews of v1
were favorable. I don't think we need to wait for additional reviews on
v2 of the series given that it really only improved commit messages and
adapted a test.
In case you feel uncomfortable with the lack of more reviews I can try
to get some additional eyes onto this patch series though.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* What's cooking in git.git (Dec 2023, #03; Mon, 18)
From: Junio C Hamano @ 2023-12-19 1:06 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).
The 'maint' branch now points at the maintenance track of Git 2.43,
which was released earlier in the month, and the tip of 'next' has
been rewound and rebuilt on top of Git 2.43. I am planning to start
ejecting topics that have been in the "stalled" state for too long.
The RelNotes symbolic link says we are now working towards Git 2.44.
It may not be a bad idea to reflect on what technical debt and UI
warts we have accumulated so far to see if we have enough of them to
start planning for a breaking Git 3.0 release (or, of course, keep
incrementally improve the system, which is much more preferrable---
continuity and stability is good).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* ac/fuzz-show-date (2023-11-20) 1 commit
(merged to 'next' on 2023-12-11 at f36795a896)
+ fuzz: add new oss-fuzz fuzzer for date.c / date.h
Subject approxidate() and show_date() machinery to OSS-Fuzz.
source: <pull.1612.v4.git.1700243267653.gitgitgadget@gmail.com>
* ad/merge-file-diff-algo (2023-11-22) 1 commit
(merged to 'next' on 2023-12-11 at ab43a54c43)
+ merge-file: add --diff-algorithm option
"git merge-file" learned to take the "--diff-algorithm" option to
use algorithm different from the default "myers" diff.
source: <pull.1606.v2.git.git.1700507932937.gitgitgadget@gmail.com>
* cc/git-replay (2023-11-26) 14 commits
(merged to 'next' on 2023-12-11 at 6f7d123578)
+ replay: stop assuming replayed branches do not diverge
+ replay: add --contained to rebase contained branches
+ replay: add --advance or 'cherry-pick' mode
+ replay: use standard revision ranges
+ replay: make it a minimal server side command
+ replay: remove HEAD related sanity check
+ replay: remove progress and info output
+ replay: add an important FIXME comment about gpg signing
+ replay: change rev walking options
+ replay: introduce pick_regular_commit()
+ replay: die() instead of failing assert()
+ replay: start using parse_options API
+ replay: introduce new builtin
+ t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
cf. <6bfe1541-54dd-ca6b-e930-94d3038060f1@gmx.de>
source: <20231124111044.3426007-1-christian.couder@gmail.com>
* jb/reflog-expire-delete-dry-run-options (2023-11-26) 1 commit
(merged to 'next' on 2023-12-11 at c7e9846963)
+ builtin/reflog.c: fix dry-run option short name
Command line parsing fix for "git reflog".
source: <20231126000514.85509-1-josh@brob.st>
* jh/trace2-redact-auth (2023-11-23) 4 commits
(merged to 'next' on 2023-12-11 at 7e679a4c4d)
+ t0212: test URL redacting in EVENT format
+ t0211: test URL redacting in PERF format
+ trace2: redact passwords from https:// URLs by default
+ trace2: fix signature of trace2_def_param() macro
trace2 streams used to record the URLs that potentially embed
authentication material, which has been corrected.
source: <pull.1616.git.1700680717.gitgitgadget@gmail.com>
* js/packfile-h-typofix (2023-11-20) 1 commit
(merged to 'next' on 2023-12-11 at 328399439a)
+ packfile.c: fix a typo in `each_file_in_pack_dir_fn()`'s declaration
Typofix.
source: <pull.1614.git.1700226915859.gitgitgadget@gmail.com>
* js/update-urls-in-doc-and-comment (2023-11-26) 4 commits
(merged to 'next' on 2023-12-11 at 3cda3f2a03)
+ doc: refer to internet archive
+ doc: update links for andre-simon.de
+ doc: switch links to https
+ doc: update links to current pages
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
source: <pull.1589.v3.git.1700796916.gitgitgadget@gmail.com>
* ps/commit-graph-less-paranoid (2023-11-26) 1 commit
(merged to 'next' on 2023-12-11 at 618bd08fa1)
+ commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
Earlier we stopped relying on commit-graph that (still) records
information about commits that are lost from the object store,
which has negative performance implications. The default has been
flipped to disable this pessimization.
source: <17e08289cd59d20de0de9b4e18f5e6bf77987351.1700823746.git.ps@pks.im>
* ps/ref-deletion-updates (2023-11-17) 4 commits
(merged to 'next' on 2023-12-11 at ca551a0c36)
+ refs: remove `delete_refs` callback from backends
+ refs: deduplicate code to delete references
+ refs/files: use transactions to delete references
+ t5510: ensure that the packed-refs file needs locking
Simplify API implementation to delete references by eliminating
duplication.
source: <cover.1699951815.git.ps@pks.im>
* rs/column-leakfix (2023-11-27) 1 commit
(merged to 'next' on 2023-12-11 at 9ac1707337)
+ column: release strbuf and string_list after use
Leakfix.
source: <f087137d-a5aa-487e-a1cb-0ad7117b38ed@web.de>
* rs/i18n-cannot-be-used-together (2023-11-27) 1 commit
(merged to 'next' on 2023-12-11 at a44e1c84c9)
+ i18n: factorize even more 'incompatible options' messages
Clean-up code that handles combinations of incompatible options.
source: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
--------------------------------------------------
[New Topics]
* jk/mailinfo-oob-read-fix (2023-12-12) 1 commit
(merged to 'next' on 2023-12-14 at 0dcfcb0d02)
+ mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()
(this branch is used by jk/mailinfo-iterative-unquote-comment.)
OOB read fix.
Will merge to 'master'.
source: <20231212221243.GA1656116@coredump.intra.peff.net>
* ps/pseudo-refs (2023-12-14) 4 commits
- bisect: consistently write BISECT_EXPECTED_REV via the refdb
- refs: complete list of special refs
- refs: propagate errno when reading special refs fails
- wt-status: read HEAD and ORIG_HEAD via the refdb
Assorted changes around pseudoref handling.
source: <cover.1702560829.git.ps@pks.im>
* rs/t6300-compressed-size-fix (2023-12-13) 2 commits
- test-lib-functions: add object size functions
- t6300: avoid hard-coding object sizes
Test fix.
Will merge to 'next'?
source: <9feeb6cf-aabf-4002-917f-3f6c27547bc8@web.de>
source: <ff735aac-b60b-4d52-a6dc-180ab504fc8d@web.de>
* es/add-doc-list-short-form-of-all-in-synopsis (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at a4f20da2bf)
+ git-add.txt: add missing short option -A to synopsis
Doc update.
Will merge to 'master'.
source: <20231215204333.1253-1-ericsunshine@charter.net>
* jc/doc-misspelt-refs-fix (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at e7799fd5c9)
+ doc: format.notes specify a ref under refs/notes/ hierarchy
Doc update.
Will merge to 'master'.
source: <xmqqjzpfje33.fsf_-_@gitster.g>
* jc/doc-most-refs-are-not-that-special (2023-12-15) 5 commits
(merged to 'next' on 2023-12-18 at aead30fcc8)
+ docs: MERGE_AUTOSTASH is not that special
+ docs: AUTO_MERGE is not that special
+ refs.h: HEAD is not that special
+ git-bisect.txt: BISECT_HEAD is not that special
+ git.txt: HEAD is not that special
Doc updates.
Will merge to 'master'.
source: <20231215203245.3622299-1-gitster@pobox.com>
* jk/mailinfo-iterative-unquote-comment (2023-12-14) 2 commits
(merged to 'next' on 2023-12-18 at 92363605fd)
+ mailinfo: avoid recursion when unquoting From headers
+ t5100: make rfc822 comment test more careful
(this branch uses jk/mailinfo-oob-read-fix.)
The code to parse the From e-mail header has been updated to avoid
recursion.
Will merge to 'master'.
source: <20231214214444.GB2297853@coredump.intra.peff.net>
* ps/chainlint-self-check-update (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at 0de2e1807f)
+ tests: adjust whitespace in chainlint expectations
Test framework update.
Will merge to 'master'.
source: <fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im>
* tb/multi-pack-verbatim-reuse (2023-12-14) 26 commits
- t/perf: add performance tests for multi-pack reuse
- pack-bitmap: enable reuse from all bitmapped packs
- pack-objects: allow setting `pack.allowPackReuse` to "single"
- t/test-lib-functions.sh: implement `test_trace2_data` helper
- pack-objects: add tracing for various packfile metrics
- pack-bitmap: prepare to mark objects from multiple packs for reuse
- pack-revindex: implement `midx_pair_to_pack_pos()`
- pack-revindex: factor out `midx_key_to_pack_pos()` helper
- midx: implement `midx_preferred_pack()`
- git-compat-util.h: implement checked size_t to uint32_t conversion
- pack-objects: include number of packs reused in output
- pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuse
- pack-objects: prepare `write_reused_pack()` for multi-pack reuse
- pack-objects: pass `bitmapped_pack`'s to pack-reuse functions
- pack-objects: keep track of `pack_start` for each reuse pack
- pack-objects: parameterize pack-reuse routines over a single pack
- pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`
- pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signature
- ewah: implement `bitmap_is_empty()`
- pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions
- midx: implement `midx_locate_pack()`
- midx: implement `BTMP` chunk
- midx: factor out `fill_pack_info()`
- pack-bitmap: plug leak in find_objects()
- pack-bitmap-write: deep-clear the `bb_commit` slab
- pack-objects: free packing_data in more places
Streaming spans of packfile data used to be done only from a
single, primary, pack in a repository with multiple packfiles. It
has been extended to allow reuse from other packfiles, too.
Will merge to 'next'?
source: <cover.1702592603.git.me@ttaylorr.com>
* rs/c99-stdbool-test-balloon (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at 5a62aaa127)
+ git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
Test balloon to use C99 "bool" type from <stdbool.h>.
Will merge to 'master'.
source: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>
* sp/test-i18ngrep (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at d54442693a)
+ test-lib-functions.sh: fix test_grep fail message wording
Error message fix in the test framework.
Will merge to 'master'.
source: <20231203171956.771-1-shreyanshpaliwalcmsmn@gmail.com>
--------------------------------------------------
[Cooking]
* jx/fetch-atomic-error-message-fix (2023-12-18) 2 commits
(merged to 'next' on 2023-12-18 at a1988b00e5)
+ fetch: no redundant error message for atomic fetch
+ t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Will merge to 'master'.
cf. <ZX__e7VjyLXIl-uV@tanuki>
source: <cover.1702821462.git.zhiyou.jx@alibaba-inc.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* rs/show-ref-incompatible-options (2023-12-11) 1 commit
(merged to 'next' on 2023-12-18 at 5a092285f7)
+ show-ref: use die_for_incompatible_opt3()
Code clean-up for sanity checking of command line options for "git
show-ref".
Will merge to 'master'.
source: <e5304253-3347-4900-bbf2-d3c6ee3fb976@web.de>
* jp/use-diff-index-in-pre-commit-sample (2023-12-03) 1 commit
(merged to 'next' on 2023-12-12 at 4771ea61b9)
+ hooks--pre-commit: detect non-ASCII when renaming
The sample pre-commit hook that tries to catch introduction of new
paths that use potentially non-portable characters did not notice
an existing path getting renamed to such a problematic path, when
rename detection was enabled.
Will merge to 'master'.
source: <pull.1291.v2.git.git.1701360836307.gitgitgadget@gmail.com>
* mk/doc-gitfile-more (2023-12-03) 1 commit
(merged to 'next' on 2023-12-12 at 7990e4a163)
+ doc: make the gitfile syntax easier to discover
Doc update.
Will merge to 'master'.
source: <20231128065558.1061206-1-mk+copyleft@pimpmybyte.de>
* ps/ref-tests-update-more (2023-12-03) 10 commits
(merged to 'next' on 2023-12-12 at 3d4004fe3b)
+ t6301: write invalid object ID via `test-tool ref-store`
+ t5551: stop writing packed-refs directly
+ t5401: speed up creation of many branches
+ t4013: simplify magic parsing and drop "failure"
+ t3310: stop checking for reference existence via `test -f`
+ t1417: make `reflog --updateref` tests backend agnostic
+ t1410: use test-tool to create empty reflog
+ t1401: stop treating FETCH_HEAD as real reference
+ t1400: split up generic reflog tests from the reffile-specific ones
+ t0410: mark tests to require the reffiles backend
Tests update.
Will merge to 'master'.
source: <cover.1701242407.git.ps@pks.im>
* sh/completion-with-reftable (2023-12-03) 2 commits
- completion: stop checking for reference existence via `test -f`
- completion: refactor existence checks for special refs
Command line completion script (in contrib/) learned to work better
with the reftable backend.
Expecting a reroll.
source: <20231130202404.89791-1-stanhu@gmail.com>
* en/header-cleanup (2023-12-03) 12 commits
- treewide: remove unnecessary includes in source files
- treewide: add direct includes currently only pulled in transitively
- trace2/tr2_tls.h: remove unnecessary include
- submodule-config.h: remove unnecessary include
- pkt-line.h: remove unnecessary include
- line-log.h: remove unnecessary include
- http.h: remove unnecessary include
- fsmonitor--daemon.h: remove unnecessary includes
- blame.h: remove unnecessary includes
- archive.h: remove unnecessary include
- treewide: remove unnecessary includes in source files
- treewide: remove unnecessary includes from header files
Remove unused header "#include".
Has a few interactions with topics in flight.
source: <pull.1617.git.1701585682.gitgitgadget@gmail.com>
* jc/revision-parse-int (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 6209b4c97c)
+ revision: parse integer arguments to --max-count, --skip, etc., more carefully
The command line parser for the "log" family of commands was too
loose when parsing certain numbers, e.g., silently ignoring the
extra 'q' in "git log -n 1q" without complaining, which has been
tightened up.
Will merge to 'master'.
source: <xmqq5y181fx0.fsf_-_@gitster.g>
* jk/bisect-reset-fix (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 8f946eafb6)
+ bisect: always clean on reset
"git bisect reset" has been taught to clean up state files and refs
even when BISECT_START file is gone.
Will merge to 'master'.
source: <20231207065341.GA778781@coredump.intra.peff.net>
* jk/implicit-true (2023-12-09) 7 commits
(merged to 'next' on 2023-12-12 at 2a42fdc998)
+ fsck: handle NULL value when parsing message config
+ trailer: handle NULL value when parsing trailer-specific config
+ submodule: handle NULL value when parsing submodule.*.branch
+ help: handle NULL value for alias.* config
+ trace2: handle NULL values in tr2_sysenv config callback
+ setup: handle NULL value when parsing extensions
+ config: handle NULL value when parsing non-bools
(this branch is used by jk/config-cleanup.)
Some codepaths did not correctly parse configuration variables
specified with valueless "true", which has been corrected.
Will merge to 'master'.
source: <20231207071030.GA1275835@coredump.intra.peff.net>
* jk/config-cleanup (2023-12-09) 9 commits
(merged to 'next' on 2023-12-12 at 44ee006c25)
+ sequencer: simplify away extra git_config_string() call
+ gpg-interface: drop pointless config_error_nonbool() checks
+ push: drop confusing configset/callback redundancy
+ config: use git_config_string() for core.checkRoundTripEncoding
+ diff: give more detailed messages for bogus diff.* config
+ config: use config_error_nonbool() instead of custom messages
+ imap-send: don't use git_die_config() inside callback
+ git_xmerge_config(): prefer error() to die()
+ config: reject bogus values for core.checkstat
(this branch uses jk/implicit-true.)
Code clean-up around use of configuration variables.
Will merge to 'master'.
source: <20231207071030.GA1275835@coredump.intra.peff.net>
source: <20231207072338.GA1277727@coredump.intra.peff.net>
* jk/end-of-options (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 4ae454b26d)
+ parse-options: decouple "--end-of-options" and "--"
"git $cmd --end-of-options --rev -- --path" for some $cmd failed
to interpret "--rev" as a rev, and "--path" as a path. This was
fixed for many programs like "reset" and "checkout".
Will merge to 'master'.
source: <20231206222145.GA136253@coredump.intra.peff.net>
* ps/clone-into-reftable-repository (2023-12-12) 7 commits
- builtin/clone: create the refdb with the correct object format
- builtin/clone: skip reading HEAD when retrieving remote
- builtin/clone: set up sparse checkout later
- builtin/clone: fix bundle URIs with mismatching object formats
- remote-curl: rediscover repository when fetching refs
- setup: allow skipping creation of the refdb
- setup: extract function to create the refdb
"git clone" has been prepared to allow cloning a repository with
non-default hash function into a repository that uses the reftable
backend.
Will merge to 'next'?
source: <cover.1702361370.git.ps@pks.im>
* rs/incompatible-options-messages (2023-12-09) 7 commits
(merged to 'next' on 2023-12-12 at a13847a7f6)
+ worktree: simplify incompatibility message for --orphan and commit-ish
+ worktree: standardize incompatibility messages
+ clean: factorize incompatibility message
+ revision, rev-parse: factorize incompatibility messages about - -exclude-hidden
+ revision: use die_for_incompatible_opt3() for - -graph/--reverse/--walk-reflogs
+ repack: use die_for_incompatible_opt3() for -A/-k/--cruft
+ push: use die_for_incompatible_opt4() for - -delete/--tags/--all/--mirror
Clean-up code that handles combinations of incompatible options.
Will merge to 'master'.
source: <20231206115215.94467-1-l.s.r@web.de>
* jc/checkout-B-branch-in-use (2023-12-13) 2 commits
(merged to 'next' on 2023-12-14 at 0a3998619e)
+ checkout: forbid "-B <branch>" from touching a branch used elsewhere
+ checkout: refactor die_if_checked_out() caller
"git checkout -B <branch> [<start-point>]" allowed a branch that is
in use in another worktree to be updated and checked out, which
might be a bit unexpected. The rule has been tightened, which is a
breaking change. "--ignore-other-worktrees" option is required to
unbreak you, if you are used to the current behaviour that "-B"
overrides the safety.
Will merge to 'master'.
source: <xmqqjzq9cl70.fsf@gitster.g>
* ps/reftable-fixes (2023-12-11) 11 commits
(merged to 'next' on 2023-12-15 at ebba966016)
+ reftable/block: reuse buffer to compute record keys
+ reftable/block: introduce macro to initialize `struct block_iter`
+ reftable/merged: reuse buffer to compute record keys
+ reftable/stack: fix use of unseeded randomness
+ reftable/stack: fix stale lock when dying
+ reftable/stack: reuse buffers when reloading stack
+ reftable/stack: perform auto-compaction with transactional interface
+ reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
+ reftable: handle interrupted writes
+ reftable: handle interrupted reads
+ reftable: wrap EXPECT macros in do/while
Bunch of small fix-ups to the reftable code.
Will merge to 'master'.
source: <cover.1702285387.git.ps@pks.im>
* en/complete-sparse-checkout (2023-12-03) 4 commits
(merged to 'next' on 2023-12-12 at 3de75bd6af)
+ completion: avoid user confusion in non-cone mode
+ completion: avoid misleading completions in cone mode
+ completion: fix logic for determining whether cone mode is active
+ completion: squelch stray errors in sparse-checkout completion
Command line completion (in contrib/) learned to complete path
arguments to the "add/set" subcommands of "git sparse-checkout"
better.
Will merge to 'master'.
source: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>
* jc/orphan-unborn (2023-11-24) 2 commits
- orphan/unborn: fix use of 'orphan' in end-user facing messages
- orphan/unborn: add to the glossary and use them consistently
Doc updates to clarify what an "unborn branch" means.
Comments?
source: <xmqq4jhb977x.fsf@gitster.g>
* jw/builtin-objectmode-attr (2023-12-12) 2 commits
- SQUASH??? - leakfix
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
Needs to get leakfix reviewed.
source: <20231116054437.2343549-1-jojwang@google.com>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Comments?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-12-14) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1702562879.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-12-18) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Will merge to 'next'?
source: <cover.1702823801.git.zhiyou.jx@alibaba-inc.com>
* jc/fake-lstat (2023-09-15) 1 commit
(merged to 'next' on 2023-12-15 at 48e34cc0b4)
+ cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Will merge to 'master'.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqcyykig1l.fsf@gitster.g>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* ak/p4-initial-empty-commits (2023-11-23) 1 commit
- git-p4: fix fast import when empty commit is first
Expecting a reroll.
source: <pull.1609.git.git.1700639764041.gitgitgadget@gmail.com>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Perhaps discard?
cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
source: <cover.1697319294.git.code@khaugsbakk.name>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
(merged to 'next' on 2023-12-15 at 4aa7596593)
+ diff-lib: fix check_removed() when fsmonitor is active
+ Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
+ Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
Will merge to 'master'.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqr0n0h0tw.fsf@gitster.g>
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
^ permalink raw reply
* new config option "fetch.all"
From: Tamino Bauknecht @ 2023-12-18 20:15 UTC (permalink / raw)
To: git
Hi all,
when working on repositories with different remotes, I find myself
using "git fetch --all" quite often.
Thus, I thought that git might benefit from having a config option
"fetch.all" which will allow to always fetch all available remotes if
enabled (either in global or repository config).
The same already exists for "fetch.prune" and I don't really see any
downside to providing that convenience option also for "--all".
A probably necessary adjustment is that if the config is enabled and
a repository is explicitly specified, the specified repository should
be used instead of "--all".
Otherwise, the current output would be: "fatal: fetch --all does not
take a repository argument".
If no one sees any further issues, I'll gladly work on this feature
and provide a patch.
Best regards,
Tamino Bauknecht
^ permalink raw reply
* Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
From: Junio C Hamano @ 2023-12-18 20:19 UTC (permalink / raw)
To: Phillip Wood
Cc: René Scharfe, git, AtariDreams via GitGitGadget, Seija Kijin,
Jeff King, Phillip Wood
In-Reply-To: <b7a56625-46d5-4d77-b4bf-5595a6fb2aef@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> Thanks for the comprehensive commit message, I agree that we'd be
> better off avoiding adding a fallback. The patch looks good, I did
> wonder if we really need to covert all of these functions for a
> test-balloon but the patch is still pretty small overall.
I do have to wonder, though, if we want to be a bit more careful
than just blindly trusting the platform (i.e. <stdbool.h> might
exist and __STDC_VERSION__ may say C99, but under the hood their
implementation may be buggy and coerce the result of an assignment
of 2 to be different from assigning true).
In any case, this is a good starting place. Let's queue it, see
what happens, and then think about longer-term plans.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] test-lib-functions.sh: fix test_grep fail message wording
From: Eric Sunshine @ 2023-12-18 18:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shreyansh Paliwal, git, five231003
In-Reply-To: <xmqqh6kfe4am.fsf@gitster.g>
On Mon, Dec 18, 2023 at 1:47 PM Junio C Hamano <gitster@pobox.com> wrote:
> Here is the version I queued.
>
> --- >8 ---
> From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> Date: Sun, 3 Dec 2023 22:47:59 +0530
> Subject: [PATCH] test-lib-functions.sh: fix test_grep fail message wording
>
> In the recent commit 2e87fca189 (test framework: further deprecate
> test_i18ngrep, 2023-10-31), the test_i18ngrep function was
> deprecated, and all the callers were updated to call the test_grep
> function instead. But test_grep inherited an error message that
> still refers to test_i18ngrep by mistake. Correct it so that a
> broken call to the test_grep will identify itself as such.
This rewritten commit message gets directly to the point without
wasted words, making the purpose of the patch, and its justification,
easier to understand on first read. Nicely done.
> Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH v2] test-lib-functions.sh: fix test_grep fail message wording
From: Junio C Hamano @ 2023-12-18 18:47 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Shreyansh Paliwal, git, five231003
In-Reply-To: <xmqqjzpbh3kq.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> I'll see if that is the only glitch in the patch (in which case I'll
> manually adjust the authorship and apply) or respond on list
> (otherwise).
>
> Thanks for pinging and ponging.
Here is the version I queued.
Thanks, both.
--- >8 ---
From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Date: Sun, 3 Dec 2023 22:47:59 +0530
Subject: [PATCH] test-lib-functions.sh: fix test_grep fail message wording
In the recent commit 2e87fca189 (test framework: further deprecate
test_i18ngrep, 2023-10-31), the test_i18ngrep function was
deprecated, and all the callers were updated to call the test_grep
function instead. But test_grep inherited an error message that
still refers to test_i18ngrep by mistake. Correct it so that a
broken call to the test_grep will identify itself as such.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib-functions.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index c50bc18861..502f892fad 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1222,7 +1222,7 @@ test_grep () {
if test $# -lt 2 ||
{ test "x!" = "x$1" && test $# -lt 3 ; }
then
- BUG "too few parameters to test_i18ngrep"
+ BUG "too few parameters to test_grep"
fi
if test "x!" = "x$1"
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* Re: [PATCH] rebase-interactive: show revert option and add single letter shortcut
From: Junio C Hamano @ 2023-12-18 18:43 UTC (permalink / raw)
To: Michael Lohmann
Cc: phillip.wood123, Johannes.Schindelin, git, mi.al.lohmann,
phillip.wood
In-Reply-To: <20231218170912.73535-1-mi.al.lohmann@gmail.com>
Michael Lohmann <mial.lohmann@gmail.com> writes:
> A `revert` in an interactive rebase can be useful, e.g. if a faulty
> commit was pushed to the main branch already, so you can't just drop it.
But wouldn't that be typically done simply by running "git revert",
totally outside the context of "git rebase -i"?
Interactive rebase is more geared toward rearranging an already
built history *after* such a "git revert" is made and possibly other
commits are made either before or after that commit that was created
by "git revert".
And when "git rebase -i" sees such a series of commits, e.g.,
git checkout -b side-branch master
git commit -m "some work"
git commit -m "some more work"
git revert -m "revert a bad one for now" master~4
git commit -m "tentative alternative for what master~4 did"
git rebase -i master
The "revert a bad one for now" commit looks to the machinery just
like any other commit in the todo list.
> When you are already working in a feature branch you might just want to
> revert said commit right where you branched off from main, so you can
> continue working on the feature you intend while still being up-to-date
> otherwise.
Yes, I can see why sometimes you want to work on a history with
effects from certain commits removed. But that does not explain why
you want to _insert_ a revert that you do not even have in the
history anywhere before you start your interactive rebase.
> Another reason why you might not want to drop a commit is if it is a
> work in progress one and you want to properly fix it later, but for now
> need to revert the changes. That way it is a lot cleaner to structure
> your branch like this:
>
> A---B---C (B is WIP commit you cannot use as is)
> =>
> A---B---~B---C (temporarily revert B (called "~B") directly after
> it is created, until you find the time to fix it -
> at which point in time you will naturally drop the
> revert commit)
>
> This way you still have the WIP patch, but "your history is not broken
> the whole time".
A much cleaner way to structure your branch is not to muck with such
tentative changes *on* the branch you eventually want to store the
final result on. Fork another branch and rebase B away:
$ git checkout -b topic-ng topic [*]
$ git rebase -i A
to obtain
A---B---C topic
\
C topic-ng
and then you'd build on top a better version of B eventually
A---B---C
\
C---D---E---...---B*---X
And after that you may "rebase -i" to refine the result, and then
get rid of the tentative work:
$ work work work (still on topic-ng)
...
$ git commit -m "X"
$ git rebase -i A
$ git branch -M topic
Nowhere in the above two flow, there is no need to manually insert a
new "make a revert here of that other commit" in the todo list.
So I am not sure if I buy the above, especially this part:
> +To revert a commit, add a line starting with "revert" followed by the commit
> +name.
It really smells like a confusing odd man out, among all other
existing instructions that are naturally created by the
rebase/sequencer machinery and all the user needs to do is to
shuffle them, never creating a new thing.
[Footnote]
* I do this too frequently that I often do without a separate -ng
branch; once you get used to the flow, you learn to do this kind
of thing on detached HEAD instead, so this step would look more
like
$ git checkout --detach topic
and the remainder of the above procedure will not change. The
last step would become
$ git checkout -B topic
to bring me back to the target branch in a completed form.
One beauty about this "detached HEAD" approach is that output
from "git reflog topic" will show the refinement of the topic as
a single atomic event.
^ permalink raw reply
* Re: [PATCH] Teach git apply to respect core.fileMode settings
From: Junio C Hamano @ 2023-12-18 18:10 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <xmqqle9rfkvd.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
>> +test_expect_success FILEMODE 'ensure git apply respects core.fileMode' '
Forgot to point out the most important thing.
The code change in this patch is primarily about making the code
work better for folks without trustworthy filemode support.
Emulating what happens by setting core.fileMode to false on a
platform with capable filesystems may be a way to test the code, but
we should have a test specific to folks without FILEMODE
prerequisites and make sure it works well, no?
IOW, shouldn't we drom FILEMODE prerequisite from this test? How
does it break on say Windows if this test is added without FILEMODE
prerequisite?
^ permalink raw reply
* Re: [PATCH] Teach git apply to respect core.fileMode settings
From: Junio C Hamano @ 2023-12-18 18:04 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1620.git.1702908568890.gitgitgadget@gmail.com>
"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Chandra Pratap <chandrapratap3519@gmail.com>
>
This part goes in the final commit when the patch gets applied.
Everything between the three-dash line and the patch (i.e., the
first "diff --get" line) are discarded. Move what you wrote below
here to make it the proposed log message for this patch.
Assuming that gets done, let's review what will become the proposed
log message.
> CC: Johannes Schindelin <johannes.schindelin@gmail.com>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
> apply: make git apply respect core.fileMode settings
>
> When applying a patch that adds an executable file, git apply ignores
> the core.fileMode setting (core.fileMode in git config specifies whether
> the executable bit on files in the working tree
> should be honored or not) resulting in warnings like:
>
> warning: script.sh has type 100644, expected 100755
>
> even when core.fileMode is set to false, which is undesired. This is
> extra true for systems like Windows which don't rely on lsat().
"lstat()" you mean. Add "," between "Windows" and " which".
> Fix this by inferring the correct file mode from the existing index
> entry when core.filemode is set to false. The added test case helps
> verify the change and prevents future regression.
Perfect.
>
> Reviewed-by: Johannes Schindelin johannes.schindelin@gmail.com
> Signed-off-by: Chandra Pratap chandrapratap3519@gmail.com
The e-mail addresses somehow lost <angle brakets> around them.
> diff --git a/apply.c b/apply.c
> index 3d69fec836d..56790f515e0 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -3778,8 +3778,11 @@ static int check_preimage(struct apply_state *state,
> return error_errno("%s", old_name);
> }
>
> - if (!state->cached && !previous)
> - st_mode = ce_mode_from_stat(*ce, st->st_mode);
> + if (!state->cached && !previous) {
> + if (!trust_executable_bit && patch->old_mode)
> + st_mode = patch->old_mode;
> + else st_mode = ce_mode_from_stat(*ce, st->st_mode);
> + }
Write the body of the "else" clause on a separate line.
More importantly, even though we know we cannot trust st->st_mode on
such a filesystem (that is what !trust_executable_bit is about),
once we have a cache entry in the in-core index, shouldn't we trust
ce->ce_mode more than what the incoming patch says? Or is the
executable bit of a cache-entry totally hosed on a platform with
!trust_executable_bit?
I thought the way things should work was
(1) "--chmod=+x", which you used in the test, should mark the added
path executable in the index. Writing that to a tree (by
making a commit) should record script.sh as an executable
(i.e., "git ls-tree -r" should show 100755 not 100644).
(2) if you read such a tree, then the index will have the "correct"
executable bit in the cache entry (i.e., "git ls-files -s"
should show 100755 not 100644).
IOW, I am wondering if the above should look more like
if (!state->cached && !previous) {
if (!trust_executable_bit) {
if (*ce)
st_mode = (*ce)->ce_mode;
else
st_mode = patch->old_mode;
} else {
st_mode = ce_mode_from_stat(*ce, st->st_mode);
}
}
As setting patch->old_mode to st_mode is equivalent to saying "we
blindly trust the data on the patch much more than what we know
about the current repository state", which goes directly against
what "check_preimage()" wants to achieve.
>
> diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
> index e7a7295f1b6..95917fee128 100755
> --- a/t/t4129-apply-samemode.sh
> +++ b/t/t4129-apply-samemode.sh
> @@ -101,4 +101,19 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
> )
> '
>
> +test_expect_success FILEMODE 'ensure git apply respects core.fileMode' '
> + test_config core.fileMode false &&
> + echo true >script.sh &&
> + git add --chmod=+x script.sh &&
Perhaps we would want to check with "git ls-files -s script.sh" what
its mode bits are (hopefully it would be executable).
> + test_tick && git commit -m "Add script" &&
Similarly, check with "git ls-tree -r HEAD script.sh" what its mode
bits are?
> + echo true >>script.sh &&
> + test_tick && git commit -m "Modify script" script.sh &&
Ditto.
> + git format-patch -1 --stdout >patch &&
Check that the patch expects script.sh to have its executable bit
here, too?
> + git switch -c branch HEAD^ &&
> + git apply patch 2>err &&
We may also want to check "git apply --cached" and "git apply --index"
here, not just the "poor-man's GNU patch emulation" mode.
> + ! test_grep "has type 100644, expected 100755" err
If you use test_grep, the correct negation is not like that, but
test_grep ! "has type 100644, expected 100755" err
Giving a better diagnosis when the expectation is violated is the
whole point of using "test_grep" not a vanilla "grep", so we need to
tell it that we are reversing our expectations.
Thanks.
> +'
> +
> test_done
>
> base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
^ permalink raw reply
* Re: [PATCH] rebase-interactive: show revert option and add single letter shortcut
From: Michael Lohmann @ 2023-12-18 17:26 UTC (permalink / raw)
To: phillip.wood123
Cc: Johannes.Schindelin, git, mi.al.lohmann, mial.lohmann,
phillip.wood
In-Reply-To: <20231218170912.73535-1-mi.al.lohmann@gmail.com>
Hi Phillip
> Thanks for the patch, I'm wondering why you want to revert a commit
> when you're rebasing.
I know this is probably not going to be the most used command, but I
think (depending on your workflow) it can be as important as e.g.
`reset` or `update-ref`
> I think it would be helpful to explain that in the commit message. In
> particular why it is necessary to revert a commit rather than simply
> dropping it (presumably you're using rebase to do something more that
> just rework a series of commits)
I gave two examples - maybe they can give a hint on why this actually
can be a useful feature. Over the last few years I might have only
wanted to do this twice or so, but I know that I read through the help
string at least once to see how to do a revert.
Cheers!
P.S. I am sorry I missed the "v2" in the previous patch - I am still
learning how to deal with the mailing list...
^ permalink raw reply
* [PATCH] rebase-interactive: show revert option and add single letter shortcut
From: Michael Lohmann @ 2023-12-18 17:09 UTC (permalink / raw)
To: phillip.wood123; +Cc: Johannes.Schindelin, git, mi.al.lohmann, phillip.wood
In-Reply-To: <c2489476-f23b-4c03-8651-d6a8799ff67c@gmail.com>
A `revert` in an interactive rebase can be useful, e.g. if a faulty
commit was pushed to the main branch already, so you can't just drop it.
When you are already working in a feature branch you might just want to
revert said commit right where you branched off from main, so you can
continue working on the feature you intend while still being up-to-date
otherwise.
Another reason why you might not want to drop a commit is if it is a
work in progress one and you want to properly fix it later, but for now
need to revert the changes. That way it is a lot cleaner to structure
your branch like this:
A---B---C (B is WIP commit you cannot use as is)
=>
A---B---~B---C (temporarily revert B (called "~B") directly after
it is created, until you find the time to fix it -
at which point in time you will naturally drop the
revert commit)
This way you still have the WIP patch, but "your history is not broken
the whole time".
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
Documentation/git-rebase.txt | 3 +++
rebase-interactive.c | 1 +
sequencer.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1dd6555f66..75f6fe39a1 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -911,6 +911,9 @@ commit, the message from the final one is used. You can also use
"fixup -C" to get the same behavior as "fixup -c" except without opening
an editor.
+To revert a commit, add a line starting with "revert" followed by the commit
+name.
+
`git rebase` will stop when "pick" has been replaced with "edit" or
when a command fails due to merge errors. When you are done editing
and/or resolving conflicts you can continue with `git rebase --continue`.
diff --git a/rebase-interactive.c b/rebase-interactive.c
index d9718409b3..e1fd1e09e3 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -53,6 +53,7 @@ void append_todo_help(int command_count,
" commit's log message, unless -C is used, in which case\n"
" keep only this commit's message; -c is same as -C but\n"
" opens the editor\n"
+"v, revert <commit> = revert the changes introduced by that commit\n"
"x, exec <command> = run command (the rest of the line) using shell\n"
"b, break = stop here (continue rebase later with 'git rebase --continue')\n"
"d, drop <commit> = remove commit\n"
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed..3c18f71ed6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1767,7 +1767,7 @@ static struct {
const char *str;
} todo_command_info[] = {
[TODO_PICK] = { 'p', "pick" },
- [TODO_REVERT] = { 0, "revert" },
+ [TODO_REVERT] = { 'v', "revert" },
[TODO_EDIT] = { 'e', "edit" },
[TODO_REWORD] = { 'r', "reword" },
[TODO_FIXUP] = { 'f', "fixup" },
--
2.39.3 (Apple Git-145)
^ permalink raw reply related
* Re: [PATCH 0/1] revert/cherry-pick: add --show-current-patch option
From: Phillip Wood @ 2023-12-18 16:42 UTC (permalink / raw)
To: Michael Lohmann, git; +Cc: Michael Lohmann, Elijah Newren
In-Reply-To: <20231218121048.68290-1-mi.al.lohmann@gmail.com>
Hi Michael
On 18/12/2023 12:10, Michael Lohmann wrote:
> Hi,
> I am a lead developer of a small team and quite often I have to
> cherry-pick commits (and sometimes also revert them). When
> cherry-picking multiple commits at once and there is a merge conflict it
> sometimes can be hard to understand what the current patch is trying to
> do in order to resolve the conflict properly. With `rebase` there is
> `--show-current-patch` and since that is quite helpful I would suggest
> to also add this flag also to `cherry-pick` and `revert`.
Thanks for bringing this up I agree it can be very helpful to look at
the original commit when resolving cherry-pick and revert conflicts. I'm
in two minds about this change though - I wonder if it'd be better to
improve the documentation for CHERRY_PICK_HEAD and REVERT_HEAD and tell
users to run "git show CHERRY_PICK_HEAD" instead. I think the main
reason we have a "--show-current-patch" option for "rebase" is that
there are two different implementations of that command and the
patched-based one of them does not support REBASE_HEAD. That reasoning
does not apply to "cherry-pick" and "revert" and "--show-current-patch"
suggests a patch-based implementation which is also not the case for
these commands.
Best Wishes
Phillip
> Since this is my first contribution to git I am not exactly sure where
> the best place for this functionality is. From my initial understanding
> there are two places where to put the actual invocation of the `show`:
> - Duplicate the code (with the needed adaptations) of builtin/rebase.c
> in builtin/revert.c
> - Create a central function that shows the respective `*_HEAD` depending
> on the current `action`.
>
> In this first draft I went with the second option, since I felt that it
> reduces code duplication and the sequencer already has the action enum
> with exactly those three cases. On the other hand I don’t really have a
> good understanding of the role that this `sequencer` should play and if
> this adds additional coupling that is unwanted. My current impression
> is, that this would be the right place, since this looks to be the core
> of the commands where a user can apply a sequence of commits and in my
> opinion even if additional actions would be added, they could also fail
> and so it would be good to add the `--show-current-patch` option to that
> one as well.
>
> Side note: my only C(++) experience was ~10 years ago and only for a
> single university course, so my perspective is much more from a general
> architecture point of view than based on any C experience, let alone in
> this code base and so I would be very grateful for criticism!
>
>
> Side note: The check for the `REBASE_HEAD` would not be necessary, since
> that is already taken care of in the builtin/rebase.c before.
> Nevertheless I opted for this check, because I would much rather require
> the same preconditions no matter from where I call this function. The
> whole argument parsing / option struct are very different between rebase
> and revert. Maybe it would make sense to align them a bit further?
> Initial observations: `rebase_options->type` is functionally similar to
> `replay_opts->action` (as in "what general action am I performing? -
> interactive rebase / cherry-pick / revert / ...") whereas
> `rebase_options->action` is not part of the `replay_opts` struct at all.
> Instead the role is taken over in builtin/revert.c by `int cmd = 0;`.
> I am preparing a patch converting this to an enum, so that there are
> no random chars that have to be kept in sync manually in different
> places, or is that a design decision?
>
> I looked through the mailing list archive and did not find anything
> related on this topic. The only slightly related thread I could find was
> in [1] by Elijah Newren and that one was talking about a separate
> possible feature and how to get certain information if CHERRY_PICK_HEAD
> and REVERT_HEAD were to be replaced by a different construct. I hope I
> did not miss something...
>
> Cheers
> Michael
>
> [1]:
> https://lore.kernel.org/git/CABPp-BGd-W8T7EsvKYyjdi3=mfSTJ8zM-uzVsFnh1AWyV2wEzQ@mail.gmail.com
>
> Michael Lohmann (1):
> revert/cherry-pick: add --show-current-patch option
>
> Documentation/git-cherry-pick.txt | 2 +-
> Documentation/git-revert.txt | 2 +-
> Documentation/sequencer.txt | 5 +++++
> builtin/rebase.c | 7 ++----
> builtin/revert.c | 9 ++++++--
> contrib/completion/git-completion.bash | 2 +-
> sequencer.c | 24 +++++++++++++++++++++
> sequencer.h | 2 ++
> t/t3507-cherry-pick-conflict.sh | 30 ++++++++++++++++++++++++++
> 9 files changed, 73 insertions(+), 10 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2] test-lib-functions.sh: fix test_grep fail message wording
From: Junio C Hamano @ 2023-12-18 16:34 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Shreyansh Paliwal, git, five231003
In-Reply-To: <CAPig+cSJ=RcJtYKzT0Kj1-0nJT0YxA=KPYV=5H80_inJYS_Vnw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Sun, Dec 17, 2023 at 10:32 AM Shreyansh Paliwal
> <shreyanshpaliwalcmsmn@gmail.com> wrote:
>> ping.
>
> Junio was on vacation at the time[1] that this patch was submitted, so
> it's quite possible that it simply got overlooked or he hasn't gotten
> through the backlog of emails which accumulated while he was away.
It was dropped due to automated filter that noticed that the address
on its in-body From: line does not appear on any of its Signed-off-by:
line ;-)
I'll see if that is the only glitch in the patch (in which case I'll
manually adjust the authorship and apply) or respond on list
(otherwise).
Thanks for pinging and ponging.
> So,
> pinging is indeed the correct thing to do, and the patch is obviously
> an improvement, so hopefully it will be picked up soon.
>
> [1]: https://lore.kernel.org/git/xmqq34wj4e55.fsf@gitster.g/
^ permalink raw reply
* Re: [PATCH] rebase-interactive: show revert option and add single letter shortcut
From: Phillip Wood @ 2023-12-18 16:32 UTC (permalink / raw)
To: Michael Lohmann, git; +Cc: Michael Lohmann, Johannes Schindelin
In-Reply-To: <20231218152313.72896-1-mi.al.lohmann@gmail.com>
Hi Michael
Thanks for the patch, I'm wondering why you want to revert a commit when
you're rebasing. I think it would be helpful to explain that in the
commit message. In particular why it is necessary to revert a commit
rather than simply dropping it (presumably you're using rebase to do
something more that just rework a series of commits)
Best Wishes
Phillip
On 18/12/2023 15:23, Michael Lohmann wrote:
> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
> ---
> Documentation/git-rebase.txt | 3 +++
> rebase-interactive.c | 1 +
> sequencer.c | 2 +-
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 1dd6555f66..75f6fe39a1 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -911,6 +911,9 @@ commit, the message from the final one is used. You can also use
> "fixup -C" to get the same behavior as "fixup -c" except without opening
> an editor.
>
> +To revert a commit, add a line starting with "revert" followed by the commit
> +name.
> +
> `git rebase` will stop when "pick" has been replaced with "edit" or
> when a command fails due to merge errors. When you are done editing
> and/or resolving conflicts you can continue with `git rebase --continue`.
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index d9718409b3..e1fd1e09e3 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -53,6 +53,7 @@ void append_todo_help(int command_count,
> " commit's log message, unless -C is used, in which case\n"
> " keep only this commit's message; -c is same as -C but\n"
> " opens the editor\n"
> +"v, revert <commit> = revert the changes introduced by that commit\n"
> "x, exec <command> = run command (the rest of the line) using shell\n"
> "b, break = stop here (continue rebase later with 'git rebase --continue')\n"
> "d, drop <commit> = remove commit\n"
> diff --git a/sequencer.c b/sequencer.c
> index d584cac8ed..3c18f71ed6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1767,7 +1767,7 @@ static struct {
> const char *str;
> } todo_command_info[] = {
> [TODO_PICK] = { 'p', "pick" },
> - [TODO_REVERT] = { 0, "revert" },
> + [TODO_REVERT] = { 'v', "revert" },
> [TODO_EDIT] = { 'e', "edit" },
> [TODO_REWORD] = { 'r', "reword" },
> [TODO_FIXUP] = { 'f', "fixup" },
^ permalink raw reply
* Re: [PATCH 1/5] git.txt: HEAD is not that special
From: Junio C Hamano @ 2023-12-18 16:26 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ZYAGyLH4nm4TebA_@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
>> Named pointers called refs mark interesting points in history. A ref
>> -may contain the SHA-1 name of an object or the name of another ref. Refs
>> -with names beginning `ref/head/` contain the SHA-1 name of the most
>> +may contain the SHA-1 name of an object or the name of another ref (the
>> +latter is called a "symbolic ref").
>
> On a tangent: While we have a name for symbolic refs, do we also have a
> name for non-symbolic refs? I often use the term "direct ref" to clearly
> distinguish them from symbolic refs, but it's of course not defined in
> our glossary.
You may find me saying "normal ref", "regular ref", or somesuch when
it is not clear from the context if you dig the list archive.
"direct" is a nice word, especially it would give us a good pair of
terms if we are to change "symbolic" to "indirect", but since we are
not going to do so, I am not sure the contrast between "direct" and
"symbolic" would make such a good pair.
But quite honestly I rarely felt a need for a specific term, as it
is fairly clear from the context, e.g.
* "From a ref, we locate an object using the object name it
records and use the object"
A statement written from the point of view of the consumer of
object name, it does not matter if the object name is directly
found in the ref, or indirection is involved to find such a
concrete ref that records an object name by following the
original symbolic ref.
* "A ref usually stores an object name, but it can also be a
symbolic ref that points at another ref, in which case, asking
what object such a symbolic ref points at would yield the object
the other ref points at".
So I dunno.
>> +Refs with names beginning `ref/head/` contain the SHA-1 name of the most
>> recent commit (or "head") of a branch under development. SHA-1 names of
>> -tags of interest are stored under `ref/tags/`. A special ref named
>> +tags of interest are stored under `ref/tags/`. A symbolic ref named
>> `HEAD` contains the name of the currently checked-out branch.
>
> I was briefly wondering whether we also want to replace SHA-1 with
> "object hash" while at it, but it's certainly out of the scope of this
> patch series.
Yup, there still are too many reference to SHA-1 (and "sha1", which
is even worse), and it is not a focus of this series.
Thanks.
^ permalink raw reply
* Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
From: Phillip Wood @ 2023-12-18 16:23 UTC (permalink / raw)
To: René Scharfe, git
Cc: AtariDreams via GitGitGadget, Seija Kijin, Junio C Hamano,
Jeff King, Phillip Wood
In-Reply-To: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>
Hi René
On 16/12/2023 10:47, René Scharfe wrote:
> Use the data type bool and its values true and false to document the
> binary return value of skip_prefix() and friends more explicitly.
>
> This first use of stdbool.h, introduced with C99, is meant to check
> whether there are platforms that claim support for C99, as tested by
> 7bc341e21b (git-compat-util: add a test balloon for C99 support,
> 2021-12-01), but still lack that header for some reason.
>
> A fallback based on a wider type, e.g. int, would have to deal with
> comparisons somehow to emulate that any non-zero value is true:
>
> bool b1 = 1;
> bool b2 = 2;
> if (b1 == b2) puts("This is true.");
>
> int i1 = 1;
> int i2 = 2;
> if (i1 == i2) puts("Not printed.");
> #define BOOLEQ(a, b) (!(a) == !(b))
> if (BOOLEQ(i1, i2)) puts("This is true.");
>
> So we'd be better off using bool everywhere without a fallback, if
> possible. That's why this patch doesn't include any.
Thanks for the comprehensive commit message, I agree that we'd be better
off avoiding adding a fallback. The patch looks good, I did wonder if we
really need to covert all of these functions for a test-balloon but the
patch is still pretty small overall.
Best Wishes
Phillip
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> git-compat-util.h | 42 ++++++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 3e7a59b5ff..603c97e3b3 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -225,6 +225,7 @@ struct strbuf;
> #include <stddef.h>
> #include <stdlib.h>
> #include <stdarg.h>
> +#include <stdbool.h>
> #include <string.h>
> #ifdef HAVE_STRINGS_H
> #include <strings.h> /* for strcasecmp() */
> @@ -684,11 +685,11 @@ report_fn get_warn_routine(void);
> void set_die_is_recursing_routine(int (*routine)(void));
>
> /*
> - * If the string "str" begins with the string found in "prefix", return 1.
> + * If the string "str" begins with the string found in "prefix", return true.
> * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in
> * the string right after the prefix).
> *
> - * Otherwise, return 0 and leave "out" untouched.
> + * Otherwise, return false and leave "out" untouched.
> *
> * Examples:
> *
> @@ -699,57 +700,58 @@ void set_die_is_recursing_routine(int (*routine)(void));
> * [skip prefix if present, otherwise use whole string]
> * skip_prefix(name, "refs/heads/", &name);
> */
> -static inline int skip_prefix(const char *str, const char *prefix,
> - const char **out)
> +static inline bool skip_prefix(const char *str, const char *prefix,
> + const char **out)
> {
> do {
> if (!*prefix) {
> *out = str;
> - return 1;
> + return true;
> }
> } while (*str++ == *prefix++);
> - return 0;
> + return false;
> }
>
> /*
> * Like skip_prefix, but promises never to read past "len" bytes of the input
> * buffer, and returns the remaining number of bytes in "out" via "outlen".
> */
> -static inline int skip_prefix_mem(const char *buf, size_t len,
> - const char *prefix,
> - const char **out, size_t *outlen)
> +static inline bool skip_prefix_mem(const char *buf, size_t len,
> + const char *prefix,
> + const char **out, size_t *outlen)
> {
> size_t prefix_len = strlen(prefix);
> if (prefix_len <= len && !memcmp(buf, prefix, prefix_len)) {
> *out = buf + prefix_len;
> *outlen = len - prefix_len;
> - return 1;
> + return true;
> }
> - return 0;
> + return false;
> }
>
> /*
> - * If buf ends with suffix, return 1 and subtract the length of the suffix
> - * from *len. Otherwise, return 0 and leave *len untouched.
> + * If buf ends with suffix, return true and subtract the length of the suffix
> + * from *len. Otherwise, return false and leave *len untouched.
> */
> -static inline int strip_suffix_mem(const char *buf, size_t *len,
> - const char *suffix)
> +static inline bool strip_suffix_mem(const char *buf, size_t *len,
> + const char *suffix)
> {
> size_t suflen = strlen(suffix);
> if (*len < suflen || memcmp(buf + (*len - suflen), suffix, suflen))
> - return 0;
> + return false;
> *len -= suflen;
> - return 1;
> + return true;
> }
>
> /*
> - * If str ends with suffix, return 1 and set *len to the size of the string
> - * without the suffix. Otherwise, return 0 and set *len to the size of the
> + * If str ends with suffix, return true and set *len to the size of the string
> + * without the suffix. Otherwise, return false and set *len to the size of the
> * string.
> *
> * Note that we do _not_ NUL-terminate str to the new length.
> */
> -static inline int strip_suffix(const char *str, const char *suffix, size_t *len)
> +static inline bool strip_suffix(const char *str, const char *suffix,
> + size_t *len)
> {
> *len = strlen(str);
> return strip_suffix_mem(str, len, suffix);
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH] Use ^=1 to toggle between 0 and 1
From: Phillip Wood @ 2023-12-18 16:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, René Scharfe, AtariDreams via GitGitGadget, git,
Seija Kijin
In-Reply-To: <xmqqo7erl7er.fsf@gitster.g>
Hi Junio
On 15/12/2023 17:09, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> Even if it unlikely that we would directly compare a boolean variable
>> to "true" or "false" it is certainly conceivable that we'd compare two
>> boolean variables directly. For the integer fallback to be safe we'd
>> need to write
>>
>> if (!cond_a == !cond_b)
>>
>> rather than
>>
>> if (cond_a == cond_b)
>
> Eek, it defeats the benefit of using true Boolean type if we had to
> train ourselves to write the former, doesn't it?
Yes, it's horrible - if for some reason it turns out that we cannot use
"#include <stdbool.h>" everywhere I think we should drop it rather than
providing a subtly incompatible fallback
>> A weather-balloon seems like the safest route forward. We have been
>> requiring C99 for two years now [1], hopefully there aren't any
>> compilers out that claim to support C99 but don't provide
>> "<stdbool.h>" (I did check online and the compiler on NonStop does
>> support _Bool).
>>
>> Best Wishes
>>
>> Phillip
>>
>> [1] 7bc341e21b (git-compat-util: add a test balloon for C99 support,
>> 2021-12-01)
>
> Nice to be reminded of this one.
>
> The cited commit does not start to use any specific feature from
> C99, other than that we now require that the compiler claims C99
> conformance by __STDC_VERSION__ set appropriately. The commit log
> message says C99 "provides a variety of useful features, including
> ..., many of which we already use.", which implies that our wish was
> to officially allow any and all features in C99 to be used in our
> codebase after a successful flight of this test balloon.
>
> Now, I think we saw a successful flight of this test balloon by now.
> Is allowing all the C99 the next step we really want to take?
>
> I still personally have an aversion against decl-after-statement and
> //-comments, not due to portability reasons at all, but because I
> find that the code is easier to read without it. But in principle,
> it is powerful to be able to say "OK, as long as the feature is in
> C99 you can use it", instead of having to decide on individual
> features, and I am not fundamentally against going that route if it
> is where people want to go.
I'm not sure we necessarily want to say "use anything that is in C99"
for several reasons.
- Some features such as C99's variable length arrays are known to be
problematic.
- As you say above there maybe features that we think harm the
readability of our code.
- As René points out not all compilers necessarily support all
features.
I think using _Bool could be useful for the reasons Peff outlined. As
for other features I've written code that I think would have benefited
from compound literals, but off the top of my head I can't think of any
other C99 features that I personally wish we were using. I think that
decl-after-statement is occasionally useful to declare a variable near
where it is used in a long function body but it is much simpler just to
ban it altogether and encourage people to break up long functions to
make them more readable.
Best Wishes
Phillip
> Thanks.
>
>
^ permalink raw reply
* Re: [PATCH] doc: format.notes specify a ref under refs/notes/ hierarchy
From: Junio C Hamano @ 2023-12-18 16:16 UTC (permalink / raw)
To: Patrick Steinhardt, Jiang Xin; +Cc: git, Ramsay Jones
In-Reply-To: <ZX_9nRYKVq0jT0Lp@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> On Fri, Dec 15, 2023 at 02:28:00PM -0800, Junio C Hamano wrote:
>> There is no 'ref/notes/' hierarchy. '[format] notes = foo' uses notes
>> that are found in 'refs/notes/foo'.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> * According to my eyeballing "git grep refs/ Documentation" result,
>> this was the only remaining mention of "ref/" in Documentation/
>> hierarchy that misspells "refs/".
>
> This made me look for additional instances where we were referring to
> "ref/". Turns out it's only a very limited set, see the below diff.
Yup, I did the same grep, but I tend to avoid churning what we
published long ago (and kept in Documentation/RelNotes/), my patches
only covered documents that are still relevant.
> the translation changes with a big grain of salt though,
Hopefully pinging Jiang would be sufficient to ask help from the
French, Chinese, and Taiwaneese translation teams.
> diff --git a/po/fr.po b/po/fr.po
> index ee2e610ef1..744550b056 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -19773,7 +19773,7 @@ msgid ""
> "Neither worked, so we gave up. You must fully qualify the ref."
> msgstr ""
> "La destination que vous avez fournie n'est pas un nom de référence complète\n"
> -"(c'est-à-dire commençant par \"ref/\"). Essai d'approximation par :\n"
> +"(c'est-à-dire commençant par \"refs/\"). Essai d'approximation par :\n"
> "\n"
> "- Recherche d'une référence qui correspond à '%s' sur le serveur distant.\n"
> "- Vérification si la <source> en cours de poussée ('%s')\n"
> diff --git a/po/zh_CN.po b/po/zh_CN.po
> index 86402725b2..eb47e8f9b7 100644
> --- a/po/zh_CN.po
> +++ b/po/zh_CN.po
> @@ -13224,8 +13224,8 @@ msgid ""
> msgid_plural ""
> "Note: Some branches outside the refs/remotes/ hierarchy were not removed;\n"
> "to delete them, use:"
> -msgstr[0] "注意：ref/remotes 层级之外的一个分支未被移除。要删除它，使用："
> -msgstr[1] "注意：ref/remotes 层级之外的一些分支未被移除。要删除它们，使用："
> +msgstr[0] "注意：refs/remotes 层级之外的一个分支未被移除。要删除它，使用："
> +msgstr[1] "注意：refs/remotes 层级之外的一些分支未被移除。要删除它们，使用："
>
> #: builtin/remote.c
> #, c-format
> diff --git a/po/zh_TW.po b/po/zh_TW.po
> index f777a0596f..b2a79cdd93 100644
> --- a/po/zh_TW.po
> +++ b/po/zh_TW.po
> @@ -13109,7 +13109,7 @@ msgid ""
> msgid_plural ""
> "Note: Some branches outside the refs/remotes/ hierarchy were not removed;\n"
> "to delete them, use:"
> -msgstr[0] "注意：ref/remotes 層級之外的一個分支未被移除。要刪除它，使用："
> +msgstr[0] "注意：refs/remotes 層級之外的一個分支未被移除。要刪除它，使用："
>
> #: builtin/remote.c
> #, c-format
> Also, the test is
> interesting because it would fail even if we didn't pass an invalid atom
> to git-for-each-ref(1).
It is interesting but not surprising. It is not an error to use ref
patterns that do not match any ref. It is a mere pattern to filtering
what are in refs/ for the ones to be output.
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 54e2281259..e68f7bec8e 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -841,7 +841,7 @@ test_expect_success 'err on bad describe atom arg' '
> EOF
> test_must_fail git for-each-ref \
> --format="%(describe:tags,qux=1,abbrev=14)" \
> - ref/heads/master 2>actual &&
> + refs/heads/master 2>actual &&
> test_cmp expect actual
> )
> '
The "for-each-ref" family's "--format" string is first parsed and
sanity-checked before it is applied. The bogus ref pattern may not
yield any ref to apply the format string, but we do not optimize out
the parsing and checking, even though we could, as it would be
optimizing for a wrong case. So regardless of the ref pattern at
the end of the command line does not make a difference to the
outcome of this test.
^ permalink raw reply
* [PATCH] rebase-interactive: show revert option and add single letter shortcut
From: Michael Lohmann @ 2023-12-18 15:23 UTC (permalink / raw)
To: git; +Cc: Michael Lohmann, Johannes Schindelin
In-Reply-To: <3e71666c-22a0-f52b-4025-dddb096e7e6c@gmx.de>
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
Documentation/git-rebase.txt | 3 +++
rebase-interactive.c | 1 +
sequencer.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1dd6555f66..75f6fe39a1 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -911,6 +911,9 @@ commit, the message from the final one is used. You can also use
"fixup -C" to get the same behavior as "fixup -c" except without opening
an editor.
+To revert a commit, add a line starting with "revert" followed by the commit
+name.
+
`git rebase` will stop when "pick" has been replaced with "edit" or
when a command fails due to merge errors. When you are done editing
and/or resolving conflicts you can continue with `git rebase --continue`.
diff --git a/rebase-interactive.c b/rebase-interactive.c
index d9718409b3..e1fd1e09e3 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -53,6 +53,7 @@ void append_todo_help(int command_count,
" commit's log message, unless -C is used, in which case\n"
" keep only this commit's message; -c is same as -C but\n"
" opens the editor\n"
+"v, revert <commit> = revert the changes introduced by that commit\n"
"x, exec <command> = run command (the rest of the line) using shell\n"
"b, break = stop here (continue rebase later with 'git rebase --continue')\n"
"d, drop <commit> = remove commit\n"
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed..3c18f71ed6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1767,7 +1767,7 @@ static struct {
const char *str;
} todo_command_info[] = {
[TODO_PICK] = { 'p', "pick" },
- [TODO_REVERT] = { 0, "revert" },
+ [TODO_REVERT] = { 'v', "revert" },
[TODO_EDIT] = { 'e', "edit" },
[TODO_REWORD] = { 'r', "reword" },
[TODO_FIXUP] = { 'f', "fixup" },
--
2.43.0.77.g0ff82d959c
^ permalink raw reply related
* [PATCH] Teach git apply to respect core.fileMode settings
From: Chandra Pratap via GitGitGadget @ 2023-12-18 14:09 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
CC: Johannes Schindelin <johannes.schindelin@gmail.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
apply: make git apply respect core.fileMode settings
When applying a patch that adds an executable file, git apply ignores
the core.fileMode setting (core.fileMode in git config specifies whether
the executable bit on files in the working tree
should be honored or not) resulting in warnings like:
warning: script.sh has type 100644, expected 100755
even when core.fileMode is set to false, which is undesired. This is
extra true for systems like Windows which don't rely on lsat().
Fix this by inferring the correct file mode from the existing index
entry when core.filemode is set to false. The added test case helps
verify the change and prevents future regression.
Reviewed-by: Johannes Schindelin johannes.schindelin@gmail.com
Signed-off-by: Chandra Pratap chandrapratap3519@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1620%2FChand-ra%2Fdevel-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1620/Chand-ra/devel-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1620
apply.c | 7 +++++--
t/t4129-apply-samemode.sh | 15 +++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index 3d69fec836d..56790f515e0 100644
--- a/apply.c
+++ b/apply.c
@@ -3778,8 +3778,11 @@ static int check_preimage(struct apply_state *state,
return error_errno("%s", old_name);
}
- if (!state->cached && !previous)
- st_mode = ce_mode_from_stat(*ce, st->st_mode);
+ if (!state->cached && !previous) {
+ if (!trust_executable_bit && patch->old_mode)
+ st_mode = patch->old_mode;
+ else st_mode = ce_mode_from_stat(*ce, st->st_mode);
+ }
if (patch->is_new < 0)
patch->is_new = 0;
diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
index e7a7295f1b6..95917fee128 100755
--- a/t/t4129-apply-samemode.sh
+++ b/t/t4129-apply-samemode.sh
@@ -101,4 +101,19 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
)
'
+test_expect_success FILEMODE 'ensure git apply respects core.fileMode' '
+ test_config core.fileMode false &&
+ echo true >script.sh &&
+ git add --chmod=+x script.sh &&
+ test_tick && git commit -m "Add script" &&
+
+ echo true >>script.sh &&
+ test_tick && git commit -m "Modify script" script.sh &&
+ git format-patch -1 --stdout >patch &&
+
+ git switch -c branch HEAD^ &&
+ git apply patch 2>err &&
+ ! test_grep "has type 100644, expected 100755" err
+'
+
test_done
base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
--
gitgitgadget
^ permalink raw reply related
* Subtitles for git scm documentation video
From: 김민지 @ 2023-12-18 13:12 UTC (permalink / raw)
To: git
Hello, I am a college student attending Ajou University in South Korea.
My major is Cyber Security, and I recently discovered this site
(https://git-scm.com/doc) while studying open-source software,
including git.
While watching the video posted here, I found that there were no
Korean subtitles.
I'd like to work on adding Korean subtitles to the four videos posted
here so that other Koreans can study more easily, is it possible?
And if it is possible, I would really appreciate it if you could let
me know how to work on it.
Have a great day. Minji
^ permalink raw reply
* [PATCH 1/1] revert/cherry-pick: add --show-current-patch option
From: Michael Lohmann @ 2023-12-18 12:10 UTC (permalink / raw)
To: git; +Cc: Michael Lohmann, Elijah Newren
In-Reply-To: <20231218121048.68290-1-mi.al.lohmann@gmail.com>
This aligns the interface to the rebase one and allows for an easier way
of figuring out how to resolve conflicts if commits fail to apply
(especially when reverting/cherry-picking multiple commits at the same
time)
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
Documentation/git-cherry-pick.txt | 2 +-
Documentation/git-revert.txt | 2 +-
Documentation/sequencer.txt | 5 +++++
builtin/rebase.c | 7 ++----
builtin/revert.c | 9 ++++++--
contrib/completion/git-completion.bash | 2 +-
sequencer.c | 24 +++++++++++++++++++++
sequencer.h | 2 ++
t/t3507-cherry-pick-conflict.sh | 30 ++++++++++++++++++++++++++
9 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index fdcad3d200..af41903fe7 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git cherry-pick' [--edit] [-n] [-m <parent-number>] [-s] [-x] [--ff]
[-S[<keyid>]] <commit>...
-'git cherry-pick' (--continue | --skip | --abort | --quit)
+'git cherry-pick' (--continue | --skip | --abort | --quit | --show-current-patch)
DESCRIPTION
-----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index cbe0208834..5bd2ecf35a 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git revert' [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] <commit>...
-'git revert' (--continue | --skip | --abort | --quit)
+'git revert' (--continue | --skip | --abort | --quit | --show-current-patch)
DESCRIPTION
-----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 3bceb56474..e9394761bc 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -12,5 +12,10 @@
to clear the sequencer state after a failed cherry-pick or
revert.
+--show-current-patch::
+ Show the current patch when a revert or cherry-pick is
+ stopped because of conflicts. This is the equivalent of
+ `git show REVERT_HEAD` or `git show CHERRY_PICK_HEAD`.
+
--abort::
Cancel the operation and return to the pre-sequence state.
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 9f8192e0a5..8ad3cf3e90 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -360,12 +360,9 @@ static int run_sequencer_rebase(struct rebase_options *opts)
ret = edit_todo_file(flags);
break;
case ACTION_SHOW_CURRENT_PATCH: {
- struct child_process cmd = CHILD_PROCESS_INIT;
-
- cmd.git_cmd = 1;
- strvec_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL);
- ret = run_command(&cmd);
+ struct replay_opts replay_opts = get_replay_opts(opts);
+ ret = sequencer_show_current_patch(the_repository, &replay_opts);
break;
}
default:
diff --git a/builtin/revert.c b/builtin/revert.c
index e6f9a1ad26..cbcd9fdc23 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -24,14 +24,14 @@
static const char * const revert_usage[] = {
N_("git revert [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] <commit>..."),
- N_("git revert (--continue | --skip | --abort | --quit)"),
+ N_("git revert (--continue | --skip | --abort | --quit | --show-current-patch)"),
NULL
};
static const char * const cherry_pick_usage[] = {
N_("git cherry-pick [--edit] [-n] [-m <parent-number>] [-s] [-x] [--ff]\n"
" [-S[<keyid>]] <commit>..."),
- N_("git cherry-pick (--continue | --skip | --abort | --quit)"),
+ N_("git cherry-pick (--continue | --skip | --abort | --quit | --show-current-patch)"),
NULL
};
@@ -93,6 +93,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
OPT_CMDMODE(0, "skip", &cmd, N_("skip current commit and continue"), 's'),
+ OPT_CMDMODE(0, "show-current-patch", &cmd, N_("show the patch file being reverted or cherry-picked"), 'p'),
OPT_CLEANUP(&cleanup_arg),
OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
@@ -154,6 +155,8 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
this_operation = "--continue";
else if (cmd == 's')
this_operation = "--skip";
+ else if (cmd == 'p')
+ this_operation = "--show-current-patch";
else {
assert(cmd == 'a');
this_operation = "--abort";
@@ -224,6 +227,8 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
return sequencer_rollback(the_repository, opts);
if (cmd == 's')
return sequencer_skip(the_repository, opts);
+ if (cmd == 'p')
+ return sequencer_show_current_patch(the_repository, opts);
return sequencer_pick_revisions(the_repository, opts);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 13a39ebd2e..b740b7d48c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1618,7 +1618,7 @@ _git_checkout ()
esac
}
-__git_sequencer_inprogress_options="--continue --quit --abort --skip"
+__git_sequencer_inprogress_options="--continue --quit --abort --skip --show-current-patch"
__git_cherry_pick_inprogress_options=$__git_sequencer_inprogress_options
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed..3f6f9ad75c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3417,6 +3417,30 @@ int sequencer_skip(struct repository *r, struct replay_opts *opts)
return -1;
}
+int sequencer_show_current_patch(struct repository *r, struct replay_opts *opts)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ cmd.git_cmd = 1;
+ switch (opts->action) {
+ case REPLAY_REVERT:
+ if (!refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD"))
+ die(_("No revert in progress?"));
+ strvec_pushl(&cmd.args, "show", "REVERT_HEAD", "--", NULL);
+ break;
+ case REPLAY_PICK:
+ if (!refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD"))
+ die(_("No cherry-pick in progress?"));
+ strvec_pushl(&cmd.args, "show", "CHERRY_PICK_HEAD", "--", NULL);
+ break;
+ case REPLAY_INTERACTIVE_REBASE:
+ if (!refs_ref_exists(get_main_ref_store(r), "REBASE_HEAD"))
+ die(_("No rebase in progress?"));
+ strvec_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL);
+ break;
+ }
+ return run_command(&cmd);
+}
+
static int save_todo(struct todo_list *todo_list, struct replay_opts *opts,
int reschedule)
{
diff --git a/sequencer.h b/sequencer.h
index 913a0f652d..e20cb8bc56 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -162,6 +162,8 @@ int sequencer_pick_revisions(struct repository *repo,
struct replay_opts *opts);
int sequencer_continue(struct repository *repo, struct replay_opts *opts);
int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
+int sequencer_show_current_patch(struct repository *repo,
+ struct replay_opts *opts);
int sequencer_skip(struct repository *repo, struct replay_opts *opts);
void replay_opts_release(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index c88d597b12..4f50d287a6 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -566,6 +566,36 @@ test_expect_success 'cherry-pick preserves sparse-checkout' '
test_grep ! "Changes not staged for commit:" actual
'
+test_expect_success 'cherry-pick --show-current-patch fails if no cherry-pick in progress' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick --show-current-patch
+'
+
+test_expect_success 'cherry-pick --show-current-patch describes patch that failed to apply' '
+ test_when_finished "git cherry-pick --abort || :" &&
+ pristine_detach initial &&
+ git show picked >expected &&
+
+ test_must_fail git cherry-pick picked &&
+
+ git cherry-pick --show-current-patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'revert --show-current-patch fails if no revert in progress' '
+ pristine_detach initial &&
+ test_must_fail git revert --show-current-patch
+'
+
+test_expect_success 'revert --show-current-patch describes patch that failed to apply' '
+ test_when_finished "git revert --abort || :" &&
+ pristine_detach initial &&
+ git show picked >expected &&
+ test_must_fail git revert picked &&
+ git revert --show-current-patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'cherry-pick --continue remembers --keep-redundant-commits' '
test_when_finished "git cherry-pick --abort || :" &&
pristine_detach initial &&
--
2.43.0.77.gff6ea8bb74
^ permalink raw reply related
* [PATCH 0/1] revert/cherry-pick: add --show-current-patch option
From: Michael Lohmann @ 2023-12-18 12:10 UTC (permalink / raw)
To: git; +Cc: Michael Lohmann, Elijah Newren
Hi,
I am a lead developer of a small team and quite often I have to
cherry-pick commits (and sometimes also revert them). When
cherry-picking multiple commits at once and there is a merge conflict it
sometimes can be hard to understand what the current patch is trying to
do in order to resolve the conflict properly. With `rebase` there is
`--show-current-patch` and since that is quite helpful I would suggest
to also add this flag also to `cherry-pick` and `revert`.
Since this is my first contribution to git I am not exactly sure where
the best place for this functionality is. From my initial understanding
there are two places where to put the actual invocation of the `show`:
- Duplicate the code (with the needed adaptations) of builtin/rebase.c
in builtin/revert.c
- Create a central function that shows the respective `*_HEAD` depending
on the current `action`.
In this first draft I went with the second option, since I felt that it
reduces code duplication and the sequencer already has the action enum
with exactly those three cases. On the other hand I don’t really have a
good understanding of the role that this `sequencer` should play and if
this adds additional coupling that is unwanted. My current impression
is, that this would be the right place, since this looks to be the core
of the commands where a user can apply a sequence of commits and in my
opinion even if additional actions would be added, they could also fail
and so it would be good to add the `--show-current-patch` option to that
one as well.
Side note: my only C(++) experience was ~10 years ago and only for a
single university course, so my perspective is much more from a general
architecture point of view than based on any C experience, let alone in
this code base and so I would be very grateful for criticism!
Side note: The check for the `REBASE_HEAD` would not be necessary, since
that is already taken care of in the builtin/rebase.c before.
Nevertheless I opted for this check, because I would much rather require
the same preconditions no matter from where I call this function. The
whole argument parsing / option struct are very different between rebase
and revert. Maybe it would make sense to align them a bit further?
Initial observations: `rebase_options->type` is functionally similar to
`replay_opts->action` (as in "what general action am I performing? -
interactive rebase / cherry-pick / revert / ...") whereas
`rebase_options->action` is not part of the `replay_opts` struct at all.
Instead the role is taken over in builtin/revert.c by `int cmd = 0;`.
I am preparing a patch converting this to an enum, so that there are
no random chars that have to be kept in sync manually in different
places, or is that a design decision?
I looked through the mailing list archive and did not find anything
related on this topic. The only slightly related thread I could find was
in [1] by Elijah Newren and that one was talking about a separate
possible feature and how to get certain information if CHERRY_PICK_HEAD
and REVERT_HEAD were to be replaced by a different construct. I hope I
did not miss something...
Cheers
Michael
[1]:
https://lore.kernel.org/git/CABPp-BGd-W8T7EsvKYyjdi3=mfSTJ8zM-uzVsFnh1AWyV2wEzQ@mail.gmail.com
Michael Lohmann (1):
revert/cherry-pick: add --show-current-patch option
Documentation/git-cherry-pick.txt | 2 +-
Documentation/git-revert.txt | 2 +-
Documentation/sequencer.txt | 5 +++++
builtin/rebase.c | 7 ++----
builtin/revert.c | 9 ++++++--
contrib/completion/git-completion.bash | 2 +-
sequencer.c | 24 +++++++++++++++++++++
sequencer.h | 2 ++
t/t3507-cherry-pick-conflict.sh | 30 ++++++++++++++++++++++++++
9 files changed, 73 insertions(+), 10 deletions(-)
--
2.43.0.77.gff6ea8bb74
^ 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