* Re: [PATCH v2 1/1] config: learn the "hostname:" includeIf condition
From: Junio C Hamano @ 2024-03-10 16:59 UTC (permalink / raw)
To: Ignacio Encinas; +Cc: git
In-Reply-To: <20240309181828.45496-2-ignacio@iencinas.com>
Ignacio Encinas <ignacio@iencinas.com> writes:
> +test_expect_success 'conditional include, hostname' '
> + cat >>.git/config <<-EOF &&
> + [includeIf "hostname:$(hostname)a"]
This unconditionally runs the $(hostname) command assuming it exists
everywhere, but
$ git grep '$(hostname' t/
t/t6500-gc.sh: hostname=$(hostname || echo unknown) &&
tells us that we should be prepared to meet a platform where such a
command does not exist.
I have a feeling that it is better done with a test prerequisite
than hardcoded "unknown", as xgethostname() at C level may come up
with some random string but it is not guaranteed to be "unknown".
Perhaps have one instance of this before these added tests
test_lazy_prereq WORKING_HOSTNAME '
hostname >/dev/null 2>&1
'
and then start them with
test_expect_success WORKING_HOSTNAME 'hostname: includeIf' '
...
'
or something? Others may think of a better way to make sure this
test does not cause false failures on platforms only because they
lack working hostname(1) but have a working gethostname(2) and their
xgethostname() may be working fine.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/1] config: learn the "hostname:" includeIf condition
From: Ignacio Encinas Rubio @ 2024-03-10 18:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqy1aqvx9n.fsf@gitster.g>
On 10/3/24 17:59, Junio C Hamano wrote:
> Ignacio Encinas <ignacio@iencinas.com> writes:
>
>> +test_expect_success 'conditional include, hostname' '
>> + cat >>.git/config <<-EOF &&
>> + [includeIf "hostname:$(hostname)a"]
>
> This unconditionally runs the $(hostname) command assuming it exists
> everywhere, but
>
> $ git grep '$(hostname' t/
> t/t6500-gc.sh: hostname=$(hostname || echo unknown) &&
>
> tells us that we should be prepared to meet a platform where such a
> command does not exist.
Oops, it didn't even cross my mind. Thanks for the catch!
> I have a feeling that it is better done with a test prerequisite
> than hardcoded "unknown", as xgethostname() at C level may come up
> with some random string but it is not guaranteed to be "unknown".
I agree. Not being able to query the current hostname defeats the
purpose of the tests.
> Perhaps have one instance of this before these added tests
>
> test_lazy_prereq WORKING_HOSTNAME '
> hostname >/dev/null 2>&1
> '
>
> and then start them with
>
> test_expect_success WORKING_HOSTNAME 'hostname: includeIf' '
> ...
> '
Thanks for providing an example.
> or something? Others may think of a better way to make sure this
> test does not cause false failures on platforms only because they
> lack working hostname(1) but have a working gethostname(2) and their
> xgethostname() may be working fine.
I can't think of any room for improvement other than integrating
hostname (or a custom hostname) into git and using it in the tests, but
I doubt it is worth it.
> Thanks.
Thank you. I will wait a couple of days to post the v3 to see if anyone
has a suggestion.
^ permalink raw reply
* [PATCH v3 0/7] cherry-pick: add `--empty` for more robust redundant commit handling
From: Brian Lyles @ 2024-03-10 18:41 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
The ultimate goal of this series is to allow git-cherry-pick(1) to
automatically drop redundant commits. The mechanism chosen is an
`--empty` option that provides the same flexibility as the `--empty`
options for git-rebase(1) and git-am(1).
Some secondary goals are to improve the consistency in the values and
documentation for this option across the three commands.
See "Does extending `--empty` to git-cherry-pick make sense?" [1] for
some context for why this option is desired in git-cherry-pick(1).
[1]: https://lore.kernel.org/git/CAHPHrSevBdQF0BisR8VK=jM=wj1dTUYEVrv31gLerAzL9=Cd8Q@mail.gmail.com
Along the way, I (with some help from Elijah and Phillip) found a few
other things in the docs and related sequencer code to clean up.
The primary difference from v2 of this patch is that I no longer make
any attempt to change the behavior of `--keep-redundant-commits`
implying `--allow-empty`, and the new `--empty=keep` will likewise also
imply `--allow-empty`. See "Re: [PATCH v2 8/8] cherry-pick: add
`--empty` for more robust redundant commit handling" [2] and the
previous messages in that thread for more context. Patch 6/8 from v2 is
dropped entirely, with some adjustments to the ultimate patch in this
series as well.
[2]: https://lore.kernel.org/git/xmqqttltu7zs.fsf@gitster.g/
Brian Lyles (7):
docs: address inaccurate `--empty` default with `--exec`
docs: clean up `--empty` formatting in git-rebase(1) and git-am(1)
rebase: update `--empty=ask` to `--empty=stop`
sequencer: treat error reading HEAD as unborn branch
sequencer: do not require `allow_empty` for redundant commit options
cherry-pick: enforce `--keep-redundant-commits` incompatibility
cherry-pick: add `--empty` for more robust redundant commit handling
Documentation/git-am.txt | 20 ++++++----
Documentation/git-cherry-pick.txt | 30 +++++++++++----
Documentation/git-rebase.txt | 26 ++++++++-----
builtin/rebase.c | 16 +++++---
builtin/revert.c | 38 +++++++++++++++++-
sequencer.c | 64 ++++++++++++++++---------------
t/t3424-rebase-empty.sh | 55 ++++++++++++++++++++++++--
t/t3501-revert-cherry-pick.sh | 14 +++++--
t/t3505-cherry-pick-empty.sh | 51 +++++++++++++++++++++++-
t/t3510-cherry-pick-sequence.sh | 32 ++++++++++++++++
10 files changed, 279 insertions(+), 67 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/7] docs: address inaccurate `--empty` default with `--exec`
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster, Phillip Wood
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
The documentation for git-rebase(1) indicates that using the `--exec`
option will use `--empty=drop`. This is inaccurate: when `--interactive`
is not explicitly provided, `--exec` results in `--empty=keep`
behaviors.
Correctly indicate the behavior of `--exec` using `--empty=keep` when
`--interactive` is not specified.
Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Documentation/git-rebase.txt | 10 +++++-----
t/t3424-rebase-empty.sh | 38 ++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 06206521fc..3334e85356 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -295,11 +295,11 @@ See also INCOMPATIBLE OPTIONS below.
empty after rebasing (because they contain a subset of already
upstream changes). With drop (the default), commits that
become empty are dropped. With keep, such commits are kept.
- With ask (implied by `--interactive`), the rebase will halt when
- an empty commit is applied allowing you to choose whether to
- drop it, edit files more, or just commit the empty changes.
- Other options, like `--exec`, will use the default of drop unless
- `-i`/`--interactive` is explicitly specified.
+ With ask, the rebase will halt when an empty commit is applied
+ allowing you to choose whether to drop it, edit files more, or just
+ commit the empty changes.
+ When the `-i`/`--interactive` option is used, the default becomes ask.
+ Otherwise, when the `--exec` option is used, the default becomes keep.
+
Note that commits which start empty are kept (unless `--no-keep-empty`
is specified), and commits which are clean cherry-picks (as determined
diff --git a/t/t3424-rebase-empty.sh b/t/t3424-rebase-empty.sh
index 5e1045a0af..73ff35ced2 100755
--- a/t/t3424-rebase-empty.sh
+++ b/t/t3424-rebase-empty.sh
@@ -167,4 +167,42 @@ test_expect_success 'rebase --merge does not leave state laying around' '
test_path_is_missing .git/MERGE_MSG
'
+test_expect_success 'rebase --exec --empty=drop' '
+ git checkout -B testing localmods &&
+ git rebase --exec "true" --empty=drop upstream &&
+
+ test_write_lines D C B A >expect &&
+ git log --format=%s >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rebase --exec --empty=keep' '
+ git checkout -B testing localmods &&
+ git rebase --exec "true" --empty=keep upstream &&
+
+ test_write_lines D C2 C B A >expect &&
+ git log --format=%s >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rebase --exec uses default of --empty=keep' '
+ git checkout -B testing localmods &&
+ git rebase --exec "true" upstream &&
+
+ test_write_lines D C2 C B A >expect &&
+ git log --format=%s >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rebase --exec --empty=ask' '
+ git checkout -B testing localmods &&
+ test_must_fail git rebase --exec "true" --empty=ask upstream &&
+
+ git rebase --skip &&
+
+ test_write_lines D C B A >expect &&
+ git log --format=%s >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/7] docs: clean up `--empty` formatting in git-rebase(1) and git-am(1)
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
Both of these pages document very similar `--empty` options, but with
different styles. The exact behavior of these `--empty` options differs
somewhat, but consistent styling in the docs is still beneficial. This
commit aims to make them more consistent.
Break the possible values for `--empty` into separate sections for
readability. Alphabetical order is chosen for consistency.
In a future commit, we'll be documenting a new `--empty` option for
git-cherry-pick(1), making the consistency even more relevant.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Documentation/git-am.txt | 20 +++++++++++++-------
Documentation/git-rebase.txt | 25 ++++++++++++++++---------
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index e080458d6c..f852e0ba79 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -66,13 +66,19 @@ OPTIONS
--quoted-cr=<action>::
This flag will be passed down to 'git mailinfo' (see linkgit:git-mailinfo[1]).
---empty=(stop|drop|keep)::
- By default, or when the option is set to 'stop', the command
- errors out on an input e-mail message lacking a patch
- and stops in the middle of the current am session. When this
- option is set to 'drop', skip such an e-mail message instead.
- When this option is set to 'keep', create an empty commit,
- recording the contents of the e-mail message as its log.
+--empty=(drop|keep|stop)::
+ How to handle an e-mail message lacking a patch:
++
+--
+`drop`;;
+ The e-mail message will be skipped.
+`keep`;;
+ An empty commit will be created, with the contents of the e-mail
+ message as its log.
+`stop`;;
+ The command will fail, stopping in the middle of the current `am`
+ session. This is the default behavior.
+--
-m::
--message-id::
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 3334e85356..0b0d0ccb80 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -289,17 +289,24 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
---empty=(drop|keep|ask)::
+--empty=(ask|drop|keep)::
How to handle commits that are not empty to start and are not
clean cherry-picks of any upstream commit, but which become
empty after rebasing (because they contain a subset of already
- upstream changes). With drop (the default), commits that
- become empty are dropped. With keep, such commits are kept.
- With ask, the rebase will halt when an empty commit is applied
- allowing you to choose whether to drop it, edit files more, or just
- commit the empty changes.
- When the `-i`/`--interactive` option is used, the default becomes ask.
- Otherwise, when the `--exec` option is used, the default becomes keep.
+ upstream changes):
++
+--
+`ask`;;
+ The rebase will halt when the commit is applied, allowing you to
+ choose whether to drop it, edit files more, or just commit the empty
+ changes. This option is implied when `-i`/`--interactive` is
+ specified.
+`drop`;;
+ The commit will be dropped. This is the default behavior.
+`keep`;;
+ The commit will be kept. This option is implied when `--exec` is
+ specified unless `-i`/`--interactive` is also specified.
+--
+
Note that commits which start empty are kept (unless `--no-keep-empty`
is specified), and commits which are clean cherry-picks (as determined
@@ -704,7 +711,7 @@ be dropped automatically with `--no-keep-empty`).
Similar to the apply backend, by default the merge backend drops
commits that become empty unless `-i`/`--interactive` is specified (in
which case it stops and asks the user what to do). The merge backend
-also has an `--empty=(drop|keep|ask)` option for changing the behavior
+also has an `--empty=(ask|drop|keep)` option for changing the behavior
of handling commits that become empty.
Directory rename detection
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/7] rebase: update `--empty=ask` to `--empty=stop`
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
When git-am(1) got its own `--empty` option in 7c096b8d61 (am: support
--empty=<option> to handle empty patches, 2021-12-09), `stop` was used
instead of `ask`. `stop` is a more accurate term for describing what
really happens, and consistency is good.
Update git-rebase(1) to also use `stop`, while keeping `ask` as a
deprecated synonym. Update the tests to primarily use `stop`, but also
ensure that `ask` is still allowed.
In a future commit, we'll be adding a new `--empty` option for
git-cherry-pick(1) as well, making the consistency even more relevant.
Reported-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Documentation/git-rebase.txt | 15 ++++++++-------
builtin/rebase.c | 16 ++++++++++------
t/t3424-rebase-empty.sh | 21 ++++++++++++++++-----
3 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0b0d0ccb80..67dd0a533e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -289,23 +289,24 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
---empty=(ask|drop|keep)::
+--empty=(drop|keep|stop)::
How to handle commits that are not empty to start and are not
clean cherry-picks of any upstream commit, but which become
empty after rebasing (because they contain a subset of already
upstream changes):
+
--
-`ask`;;
- The rebase will halt when the commit is applied, allowing you to
- choose whether to drop it, edit files more, or just commit the empty
- changes. This option is implied when `-i`/`--interactive` is
- specified.
`drop`;;
The commit will be dropped. This is the default behavior.
`keep`;;
The commit will be kept. This option is implied when `--exec` is
specified unless `-i`/`--interactive` is also specified.
+`stop`;;
+`ask`;;
+ The rebase will halt when the commit is applied, allowing you to
+ choose whether to drop it, edit files more, or just commit the empty
+ changes. This option is implied when `-i`/`--interactive` is
+ specified. `ask` is a deprecated synonym of `stop`.
--
+
Note that commits which start empty are kept (unless `--no-keep-empty`
@@ -711,7 +712,7 @@ be dropped automatically with `--no-keep-empty`).
Similar to the apply backend, by default the merge backend drops
commits that become empty unless `-i`/`--interactive` is specified (in
which case it stops and asks the user what to do). The merge backend
-also has an `--empty=(ask|drop|keep)` option for changing the behavior
+also has an `--empty=(drop|keep|stop)` option for changing the behavior
of handling commits that become empty.
Directory rename detection
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b086f651a..a4916781ce 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -58,7 +58,7 @@ enum empty_type {
EMPTY_UNSPECIFIED = -1,
EMPTY_DROP,
EMPTY_KEEP,
- EMPTY_ASK
+ EMPTY_STOP
};
enum action {
@@ -951,10 +951,14 @@ static enum empty_type parse_empty_value(const char *value)
return EMPTY_DROP;
else if (!strcasecmp(value, "keep"))
return EMPTY_KEEP;
- else if (!strcasecmp(value, "ask"))
- return EMPTY_ASK;
+ else if (!strcasecmp(value, "stop"))
+ return EMPTY_STOP;
+ else if (!strcasecmp(value, "ask")) {
+ warning(_("--empty=ask is deprecated; use '--empty=stop' instead."));
+ return EMPTY_STOP;
+ }
- die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"ask\"."), value);
+ die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"stop\"."), value);
}
static int parse_opt_keep_empty(const struct option *opt, const char *arg,
@@ -1133,7 +1137,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
"instead of ignoring them"),
1, PARSE_OPT_HIDDEN),
OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
- OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|ask)",
+ OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|stop)",
N_("how to handle commits that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
OPT_CALLBACK_F('k', "keep-empty", &options, NULL,
@@ -1550,7 +1554,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.empty == EMPTY_UNSPECIFIED) {
if (options.flags & REBASE_INTERACTIVE_EXPLICIT)
- options.empty = EMPTY_ASK;
+ options.empty = EMPTY_STOP;
else if (options.exec.nr > 0)
options.empty = EMPTY_KEEP;
else
diff --git a/t/t3424-rebase-empty.sh b/t/t3424-rebase-empty.sh
index 73ff35ced2..1ee6b00fd5 100755
--- a/t/t3424-rebase-empty.sh
+++ b/t/t3424-rebase-empty.sh
@@ -72,6 +72,17 @@ test_expect_success 'rebase --merge --empty=keep' '
test_cmp expect actual
'
+test_expect_success 'rebase --merge --empty=stop' '
+ git checkout -B testing localmods &&
+ test_must_fail git rebase --merge --empty=stop upstream &&
+
+ git rebase --skip &&
+
+ test_write_lines D C B A >expect &&
+ git log --format=%s >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'rebase --merge --empty=ask' '
git checkout -B testing localmods &&
test_must_fail git rebase --merge --empty=ask upstream &&
@@ -101,9 +112,9 @@ test_expect_success 'rebase --interactive --empty=keep' '
test_cmp expect actual
'
-test_expect_success 'rebase --interactive --empty=ask' '
+test_expect_success 'rebase --interactive --empty=stop' '
git checkout -B testing localmods &&
- test_must_fail git rebase --interactive --empty=ask upstream &&
+ test_must_fail git rebase --interactive --empty=stop upstream &&
git rebase --skip &&
@@ -112,7 +123,7 @@ test_expect_success 'rebase --interactive --empty=ask' '
test_cmp expect actual
'
-test_expect_success 'rebase --interactive uses default of --empty=ask' '
+test_expect_success 'rebase --interactive uses default of --empty=stop' '
git checkout -B testing localmods &&
test_must_fail git rebase --interactive upstream &&
@@ -194,9 +205,9 @@ test_expect_success 'rebase --exec uses default of --empty=keep' '
test_cmp expect actual
'
-test_expect_success 'rebase --exec --empty=ask' '
+test_expect_success 'rebase --exec --empty=stop' '
git checkout -B testing localmods &&
- test_must_fail git rebase --exec "true" --empty=ask upstream &&
+ test_must_fail git rebase --exec "true" --empty=stop upstream &&
git rebase --skip &&
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/7] sequencer: treat error reading HEAD as unborn branch
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster, Phillip Wood
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
When using git-cherry-pick(1) with `--allow-empty` while on an unborn
branch, an error is thrown. This is inconsistent with the same
cherry-pick when `--allow-empty` is not specified.
Treat a failure reading HEAD as an unborn branch in
`is_index_unchanged`. This is consistent with other sequencer logic such
as `do_pick_commit`. When on an unborn branch, use the `empty_tree` as
the tree to compare against.
Add a new test to cover this scenario. While modelled off of the
existing 'cherry-pick on unborn branch' test, some improvements can be
made:
- Use `git switch --orphan unborn` instead of `git checkout --orphan
unborn` to avoid the need for a separate `rm -rf *` call
- Avoid using `--quiet` in the `git diff` call to make debugging easier
in the event of a failure
Make these improvements to the existing test as well as the new test.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Differences from v2:
- Minor code and test cleanup per [1] and the other replies in that
thread
[1]: https://lore.kernel.org/git/62247a1c-0249-4ce1-8626-fe97b89c23dc@gmail.com/
sequencer.c | 35 +++++++++++++++++++++--------------
t/t3501-revert-cherry-pick.sh | 14 +++++++++++---
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index f49a871ac0..a62ce244c1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -770,29 +770,36 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
static int is_index_unchanged(struct repository *r)
{
struct object_id head_oid, *cache_tree_oid;
+ const struct object_id *head_tree_oid;
struct commit *head_commit;
struct index_state *istate = r->index;
- if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL))
- return error(_("could not resolve HEAD commit"));
+ if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
+ /*
+ * Treat an error reading HEAD as an unborn branch.
+ */
+ head_tree_oid = the_hash_algo->empty_tree;
+ } else {
+ head_commit = lookup_commit(r, &head_oid);
- head_commit = lookup_commit(r, &head_oid);
+ /*
+ * If head_commit is NULL, check_commit, called from
+ * lookup_commit, would have indicated that head_commit is not
+ * a commit object already. repo_parse_commit() will return failure
+ * without further complaints in such a case. Otherwise, if
+ * the commit is invalid, repo_parse_commit() will complain. So
+ * there is nothing for us to say here. Just return failure.
+ */
+ if (repo_parse_commit(r, head_commit))
+ return -1;
- /*
- * If head_commit is NULL, check_commit, called from
- * lookup_commit, would have indicated that head_commit is not
- * a commit object already. repo_parse_commit() will return failure
- * without further complaints in such a case. Otherwise, if
- * the commit is invalid, repo_parse_commit() will complain. So
- * there is nothing for us to say here. Just return failure.
- */
- if (repo_parse_commit(r, head_commit))
- return -1;
+ head_tree_oid = get_commit_tree_oid(head_commit);
+ }
if (!(cache_tree_oid = get_cache_tree_oid(istate)))
return -1;
- return oideq(cache_tree_oid, get_commit_tree_oid(head_commit));
+ return oideq(cache_tree_oid, head_tree_oid);
}
static int write_author_script(const char *message)
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index aeab689a98..8a1d154ca6 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -104,11 +104,19 @@ test_expect_success 'revert forbidden on dirty working tree' '
'
test_expect_success 'cherry-pick on unborn branch' '
- git checkout --orphan unborn &&
+ git switch --orphan unborn &&
git rm --cached -r . &&
- rm -rf * &&
git cherry-pick initial &&
- git diff --quiet initial &&
+ git diff initial &&
+ test_cmp_rev ! initial HEAD
+'
+
+test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
+ git checkout --detach &&
+ git branch -D unborn &&
+ git switch --orphan unborn &&
+ git cherry-pick initial --allow-empty &&
+ git diff initial &&
test_cmp_rev ! initial HEAD
'
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/7] sequencer: do not require `allow_empty` for redundant commit options
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
A consumer of the sequencer that wishes to take advantage of either the
`keep_redundant_commits` or `drop_redundant_commits` feature must also
specify `allow_empty`. However, these refer to two distinct types of
empty commits:
- `allow_empty` refers specifically to commits which start empty
- `keep_redundant_commits` refers specifically to commits that do not
start empty, but become empty due to the content already existing in
the target history
Conceptually, there is no reason that the behavior for handling one of
these should be entangled with the other. It is particularly unintuitive
to require `allow_empty` in order for `drop_redundant_commits` to have
an effect: in order to prevent redundant commits automatically,
initially-empty commits would need to be kept automatically as well.
Instead, rewrite the `allow_empty()` logic to remove the over-arching
requirement that `allow_empty` be specified in order to reach any of the
keep/drop behaviors. Only if the commit was originally empty will
`allow_empty` have an effect.
Note that no behavioral changes should result from this commit -- it
merely sets the stage for future commits. In one such future commit, an
`--empty` option will be added to git-cherry-pick(1), meaning that
`drop_redundant_commits` will be used by that command.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
sequencer.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a62ce244c1..8dce175f2e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1726,34 +1726,25 @@ static int allow_empty(struct repository *r,
int index_unchanged, originally_empty;
/*
- * Four cases:
+ * For a commit that is initially empty, allow_empty determines if it
+ * should be kept or not
*
- * (1) we do not allow empty at all and error out.
- *
- * (2) we allow ones that were initially empty, and
- * just drop the ones that become empty
- *
- * (3) we allow ones that were initially empty, but
- * halt for the ones that become empty;
- *
- * (4) we allow both.
+ * For a commit that becomes empty, keep_redundant_commits and
+ * drop_redundant_commits determine whether the commit should be kept or
+ * dropped. If neither is specified, halt.
*/
- if (!opts->allow_empty)
- return 0; /* let "git commit" barf as necessary */
-
index_unchanged = is_index_unchanged(r);
if (index_unchanged < 0)
return index_unchanged;
if (!index_unchanged)
return 0; /* we do not have to say --allow-empty */
- if (opts->keep_redundant_commits)
- return 1;
-
originally_empty = is_original_commit_empty(commit);
if (originally_empty < 0)
return originally_empty;
if (originally_empty)
+ return opts->allow_empty;
+ else if (opts->keep_redundant_commits)
return 1;
else if (opts->drop_redundant_commits)
return 2;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 6/7] cherry-pick: enforce `--keep-redundant-commits` incompatibility
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
When `--keep-redundant-commits` was added in b27cfb0d8d
(git-cherry-pick: Add keep-redundant-commits option, 2012-04-20), it was
not marked as incompatible with the various operations needed to
continue or exit a cherry-pick (`--continue`, `--skip`, `--abort`, and
`--quit`).
Enforce this incompatibility via `verify_opt_compatible` like we do for
the other various options.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Differences from v2:
- New tests are added to t3505-cherry-pick-empty.sh instead of a new
test file
builtin/revert.c | 1 +
t/t3505-cherry-pick-empty.sh | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/builtin/revert.c b/builtin/revert.c
index 89821bab95..a1936ef70e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -167,6 +167,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
"--ff", opts->allow_ff,
"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
+ "--keep-redundant-commits", opts->keep_redundant_commits,
NULL);
}
diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index eba3c38d5a..61f91aaa0a 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -99,4 +99,18 @@ test_expect_success 'cherry-pick a no-op with --keep-redundant' '
test_cmp expect actual
'
+test_expect_success '--keep-redundant-commits is incompatible with operations' '
+ test_must_fail git cherry-pick HEAD 2>output &&
+ test_grep "The previous cherry-pick is now empty" output &&
+ test_must_fail git cherry-pick --keep-redundant-commits --continue 2>output &&
+ test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --continue" output &&
+ test_must_fail git cherry-pick --keep-redundant-commits --skip 2>output &&
+ test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --skip" output &&
+ test_must_fail git cherry-pick --keep-redundant-commits --abort 2>output &&
+ test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --abort" output &&
+ test_must_fail git cherry-pick --keep-redundant-commits --quit 2>output &&
+ test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --quit" output &&
+ git cherry-pick --abort
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* [PATCH v3 7/7] cherry-pick: add `--empty` for more robust redundant commit handling
From: Brian Lyles @ 2024-03-10 18:42 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
As with git-rebase(1) and git-am(1), git-cherry-pick(1) can result in a
commit being made redundant if the content from the picked commit is
already present in the target history. However, git-cherry-pick(1) does
not have the same options available that git-rebase(1) and git-am(1) have.
There are three things that can be done with these redundant commits:
drop them, keep them, or have the cherry-pick stop and wait for the user
to take an action. git-rebase(1) has the `--empty` option added in commit
e98c4269c8 (rebase (interactive-backend): fix handling of commits that
become empty, 2020-02-15), which handles all three of these scenarios.
Similarly, git-am(1) got its own `--empty` in 7c096b8d61 (am: support
--empty=<option> to handle empty patches, 2021-12-09).
git-cherry-pick(1), on the other hand, only supports two of the three
possiblities: Keep the redundant commits via `--keep-redundant-commits`,
or have the cherry-pick fail by not specifying that option. There is no
way to automatically drop redundant commits.
In order to bring git-cherry-pick(1) more in-line with git-rebase(1) and
git-am(1), this commit adds an `--empty` option to git-cherry-pick(1). It
has the same three options (keep, drop, and stop), and largely behaves
the same. The notable difference is that for git-cherry-pick(1), the
default will be `stop`, which maintains the current behavior when the
option is not specified.
Like the existing `--keep-redundant-commits`, `--empty=keep` will imply
`--allow-empty`.
The `--keep-redundant-commits` option will be documented as a deprecated
synonym of `--empty=keep`, and will be supported for backwards
compatibility for the time being.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
Differences from v2:
- `--empty=keep` will now imply `--allow-empty`, consistent with
`--keep-redundant-commits`. See [1] for more information.
- Tests for persistence of the new behaviors after `--continue`, etc.
are more focused on user-visible behaviors rather than implementation
details.
- The new empty_action enum uses -1 for unspecified instead of 0.
[1]: https://lore.kernel.org/git/xmqqttltu7zs.fsf@gitster.g/
Documentation/git-cherry-pick.txt | 30 +++++++++++++++++++------
builtin/revert.c | 37 ++++++++++++++++++++++++++++++-
sequencer.c | 6 +++++
t/t3505-cherry-pick-empty.sh | 37 ++++++++++++++++++++++++++++++-
t/t3510-cherry-pick-sequence.sh | 32 ++++++++++++++++++++++++++
5 files changed, 133 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index fdcad3d200..e6a61503c7 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -131,20 +131,36 @@ effect to your index in a row.
even without this option. Note also, that use of this option only
keeps commits that were initially empty (i.e. the commit recorded the
same tree as its parent). Commits which are made empty due to a
- previous commit are dropped. To force the inclusion of those commits
- use `--keep-redundant-commits`.
+ previous commit will cause the cherry-pick to fail. To force the
+ inclusion of those commits, use `--empty=keep`.
--allow-empty-message::
By default, cherry-picking a commit with an empty message will fail.
This option overrides that behavior, allowing commits with empty
messages to be cherry picked.
+--empty=(drop|keep|stop)::
+ How to handle commits being cherry-picked that are redundant with
+ changes already in the current history.
++
+--
+`drop`;;
+ The commit will be dropped.
+`keep`;;
+ The commit will be kept. Implies `--allow-empty`.
+`stop`;;
+ The cherry-pick will stop when the commit is applied, allowing
+ you to examine the commit. This is the default behavior.
+--
++
+Note that this option specifies how to handle a commit that was not initially
+empty, but rather became empty due to a previous commit. Commits that were
+initially empty will cause the cherry-pick to fail. To force the inclusion of
+those commits, use `--allow-empty`.
++
+
--keep-redundant-commits::
- If a commit being cherry picked duplicates a commit already in the
- current history, it will become empty. By default these
- redundant commits cause `cherry-pick` to stop so the user can
- examine the commit. This option overrides that behavior and
- creates an empty commit object. Implies `--allow-empty`.
+ Deprecated synonym for `--empty=keep`.
--strategy=<strategy>::
Use the given merge strategy. Should only be used once.
diff --git a/builtin/revert.c b/builtin/revert.c
index a1936ef70e..53935d2c68 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -43,6 +43,31 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
+enum empty_action {
+ EMPTY_COMMIT_UNSPECIFIED = -1,
+ STOP_ON_EMPTY_COMMIT, /* output errors and stop in the middle of a cherry-pick */
+ DROP_EMPTY_COMMIT, /* skip with a notice message */
+ KEEP_EMPTY_COMMIT, /* keep recording as empty commits */
+};
+
+static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
+{
+ int *opt_value = opt->value;
+
+ BUG_ON_OPT_NEG(unset);
+
+ if (!strcmp(arg, "stop"))
+ *opt_value = STOP_ON_EMPTY_COMMIT;
+ else if (!strcmp(arg, "drop"))
+ *opt_value = DROP_EMPTY_COMMIT;
+ else if (!strcmp(arg, "keep"))
+ *opt_value = KEEP_EMPTY_COMMIT;
+ else
+ return error(_("invalid value for '%s': '%s'"), "--empty", arg);
+
+ return 0;
+}
+
static int option_parse_m(const struct option *opt,
const char *arg, int unset)
{
@@ -85,6 +110,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
const char *cleanup_arg = NULL;
+ enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
int cmd = 0;
struct option base_options[] = {
OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
@@ -114,7 +140,10 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
OPT_BOOL(0, "ff", &opts->allow_ff, N_("allow fast-forward")),
OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
- OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+ OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("deprecated: use --empty=keep instead")),
+ OPT_CALLBACK_F(0, "empty", &empty_opt, "(stop|drop|keep)",
+ N_("how to handle commits that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
OPT_END(),
};
options = parse_options_concat(options, cp_extra);
@@ -134,6 +163,11 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
prepare_repo_settings(the_repository);
the_repository->settings.command_requires_full_index = 0;
+ if (opts->action == REPLAY_PICK) {
+ opts->drop_redundant_commits = (empty_opt == DROP_EMPTY_COMMIT);
+ opts->keep_redundant_commits = opts->keep_redundant_commits || (empty_opt == KEEP_EMPTY_COMMIT);
+ }
+
/* implies allow_empty */
if (opts->keep_redundant_commits)
opts->allow_empty = 1;
@@ -168,6 +202,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
"--keep-redundant-commits", opts->keep_redundant_commits,
+ "--empty", empty_opt != EMPTY_COMMIT_UNSPECIFIED,
NULL);
}
diff --git a/sequencer.c b/sequencer.c
index 8dce175f2e..a3c73ecb9f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2924,6 +2924,9 @@ static int populate_opts_cb(const char *key, const char *value,
else if (!strcmp(key, "options.allow-empty-message"))
opts->allow_empty_message =
git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
+ else if (!strcmp(key, "options.drop-redundant-commits"))
+ opts->drop_redundant_commits =
+ git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
else if (!strcmp(key, "options.keep-redundant-commits"))
opts->keep_redundant_commits =
git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
@@ -3468,6 +3471,9 @@ static int save_opts(struct replay_opts *opts)
if (opts->allow_empty_message)
res |= git_config_set_in_file_gently(opts_file,
"options.allow-empty-message", "true");
+ if (opts->drop_redundant_commits)
+ res |= git_config_set_in_file_gently(opts_file,
+ "options.drop-redundant-commits", "true");
if (opts->keep_redundant_commits)
res |= git_config_set_in_file_gently(opts_file,
"options.keep-redundant-commits", "true");
diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index 61f91aaa0a..9748443530 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -84,7 +84,7 @@ test_expect_success 'cherry-pick a commit that becomes no-op (prep)' '
git commit -m "add file2 on the side"
'
-test_expect_success 'cherry-pick a no-op without --keep-redundant' '
+test_expect_success 'cherry-pick a no-op with neither --keep-redundant nor --empty' '
git reset --hard &&
git checkout fork^0 &&
test_must_fail git cherry-pick main
@@ -113,4 +113,39 @@ test_expect_success '--keep-redundant-commits is incompatible with operations' '
git cherry-pick --abort
'
+test_expect_success '--empty is incompatible with operations' '
+ test_must_fail git cherry-pick HEAD 2>output &&
+ test_grep "The previous cherry-pick is now empty" output &&
+ test_must_fail git cherry-pick --empty=stop --continue 2>output &&
+ test_grep "fatal: cherry-pick: --empty cannot be used with --continue" output &&
+ test_must_fail git cherry-pick --empty=stop --skip 2>output &&
+ test_grep "fatal: cherry-pick: --empty cannot be used with --skip" output &&
+ test_must_fail git cherry-pick --empty=stop --abort 2>output &&
+ test_grep "fatal: cherry-pick: --empty cannot be used with --abort" output &&
+ test_must_fail git cherry-pick --empty=stop --quit 2>output &&
+ test_grep "fatal: cherry-pick: --empty cannot be used with --quit" output &&
+ git cherry-pick --abort
+'
+
+test_expect_success 'cherry-pick a no-op with --empty=stop' '
+ git reset --hard &&
+ git checkout fork^0 &&
+ test_must_fail git cherry-pick --empty=stop main 2>output &&
+ test_grep "The previous cherry-pick is now empty" output
+'
+
+test_expect_success 'cherry-pick a no-op with --empty=drop' '
+ git reset --hard &&
+ git checkout fork^0 &&
+ git cherry-pick --empty=drop main &&
+ test_commit_message HEAD -m "add file2 on the side"
+'
+
+test_expect_success 'cherry-pick a no-op with --empty=keep' '
+ git reset --hard &&
+ git checkout fork^0 &&
+ git cherry-pick --empty=keep main &&
+ test_commit_message HEAD -m "add file2 on main"
+'
+
test_done
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 72020a51c4..7eb52b12ed 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -90,6 +90,38 @@ test_expect_success 'cherry-pick persists opts correctly' '
test_cmp expect actual
'
+test_expect_success 'cherry-pick persists --empty=stop correctly' '
+ pristine_detach yetanotherpick &&
+ # Picking `anotherpick` forces a conflict so that we stop. That
+ # commit is then skipped, after which we pick `yetanotherpick`
+ # while already on `yetanotherpick` to cause an empty commit
+ test_must_fail git cherry-pick --empty=stop anotherpick yetanotherpick &&
+ test_must_fail git cherry-pick --skip 2>msg &&
+ test_grep "The previous cherry-pick is now empty" msg &&
+ rm msg &&
+ git cherry-pick --abort
+'
+
+test_expect_success 'cherry-pick persists --empty=drop correctly' '
+ pristine_detach yetanotherpick &&
+ # Picking `anotherpick` forces a conflict so that we stop. That
+ # commit is then skipped, after which we pick `yetanotherpick`
+ # while already on `yetanotherpick` to cause an empty commit
+ test_must_fail git cherry-pick --empty=drop anotherpick yetanotherpick &&
+ git cherry-pick --skip &&
+ test_cmp_rev yetanotherpick HEAD
+'
+
+test_expect_success 'cherry-pick persists --empty=keep correctly' '
+ pristine_detach yetanotherpick &&
+ # Picking `anotherpick` forces a conflict so that we stop. That
+ # commit is then skipped, after which we pick `yetanotherpick`
+ # while already on `yetanotherpick` to cause an empty commit
+ test_must_fail git cherry-pick --empty=keep anotherpick yetanotherpick &&
+ git cherry-pick --skip &&
+ test_cmp_rev yetanotherpick HEAD^
+'
+
test_expect_success 'revert persists opts correctly' '
pristine_detach initial &&
# to make sure that the session to revert a sequence
--
2.43.0
^ permalink raw reply related
* [PATCH 0/6] doc: rework doc files for git-init and git-clone
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila
Following the new formatting rules that were added, I reworked the two first
manpages of the series on git-scm.com.
This mainly entails applying placeholder and verbatim markup when it
applies. Other changes are added; if it makes sense, they can be turned into
general rules.
Jean-Noël Avila (6):
doc: git-init: format verbatim parts
doc: git-init: format placeholders
doc: git-init: rework definition lists
doc: git-init: rework config item init.templateDir
doc: git-clone: format verbatim words
doc: git-clone: format placeholders
Documentation/config/init.txt | 9 ++++--
Documentation/git-clone.txt | 30 +++++++++----------
Documentation/git-init.txt | 54 +++++++++++++++++++----------------
Documentation/urls.txt | 12 ++++----
4 files changed, 57 insertions(+), 48 deletions(-)
base-commit: e09f1254c54329773904fe25d7c545a1fb4fa920
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1687%2Fjnavila%2Fdoc_git_clone-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1687/jnavila/doc_git_clone-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1687
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/6] doc: git-init: format verbatim parts
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
Verbatim parts are all formatted as `fixed font`.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-init.txt | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index e8dc645bb59..7ff7d8e7ee0 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -33,10 +33,10 @@ If the object storage directory is specified via the
are created underneath; otherwise, the default `$GIT_DIR/objects`
directory is used.
-Running 'git init' in an existing repository is safe. It will not
+Running `git init` in an existing repository is safe. It will not
overwrite things that are already there. The primary reason for
-rerunning 'git init' is to pick up newly added templates (or to move
-the repository to another place if --separate-git-dir is given).
+rerunning `git init` is to pick up newly added templates (or to move
+the repository to another place if `--separate-git-dir` is given).
OPTIONS
-------
@@ -53,8 +53,8 @@ current working directory.
--object-format=<format>::
-Specify the given object format (hash algorithm) for the repository. The valid
-values are 'sha1' and (if enabled) 'sha256'. 'sha1' is the default.
+Specify the given object _<format>_ (hash algorithm) for the repository. The valid
+values are `sha1` and (if enabled) `sha256`. `sha1` is the default.
+
include::object-format-disclaimer.txt[]
@@ -90,12 +90,12 @@ customized via the `init.defaultBranch` configuration variable).
Specify that the Git repository is to be shared amongst several users. This
allows users belonging to the same group to push into that
-repository. When specified, the config variable "core.sharedRepository" is
+repository. When specified, the config variable `core.sharedRepository` is
set so that files and directories under `$GIT_DIR` are created with the
requested permissions. When not specified, Git will use permissions reported
-by umask(2).
+by `umask(2)`.
+
-The option can have the following values, defaulting to 'group' if no value
+The option can have the following values, defaulting to `group` if no value
is given:
+
--
@@ -109,21 +109,21 @@ specified.
Make the repository group-writable, (and g+sx, since the git group may not be
the primary group of all users). This is used to loosen the permissions of an
otherwise safe umask(2) value. Note that the umask still applies to the other
-permission bits (e.g. if umask is '0022', using 'group' will not remove read
-privileges from other (non-group) users). See '0xxx' for how to exactly specify
+permission bits (e.g. if umask is `0022`, using `group` will not remove read
+privileges from other (non-group) users). See `0xxx` for how to exactly specify
the repository permissions.
'all' (or 'world' or 'everybody')::
-Same as 'group', but make the repository readable by all users.
+Same as `group`, but make the repository readable by all users.
'<perm>'::
'<perm>' is a 3-digit octal number prefixed with `0` and each file
-will have mode '<perm>'. '<perm>' will override users' umask(2)
-value (and not only loosen permissions as 'group' and 'all'
-do). '0640' will create a repository which is group-readable, but
-not group-writable or accessible to others. '0660' will create a repo
+will have mode '<perm>'. '<perm>' will override users'`umask(2)`
+value (and not only loosen permissions as `group` and `all`
+do). `0640` will create a repository which is group-readable, but
+not group-writable or accessible to others. `0660` will create a repo
that is readable and writable to the current user and group, but
inaccessible to others (directories and executable files get their
`x` bit from the `r` bit for corresponding classes of users).
@@ -172,7 +172,7 @@ $ git add . <2>
$ git commit <3>
----------------
+
-<1> Create a /path/to/my/codebase/.git directory.
+<1> Create a `/path/to/my/codebase/.git` directory.
<2> Add all existing files to the index.
<3> Record the pristine state as the first commit in the history.
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/6] doc: git-init: format placeholders
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
With the new doc format conventions, we use _<placeholders>_.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-init.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 7ff7d8e7ee0..e8fe72861dc 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -60,7 +60,7 @@ include::object-format-disclaimer.txt[]
--ref-format=<format>::
-Specify the given ref storage format for the repository. The valid values are:
+Specify the given ref storage _<format>_ for the repository. The valid values are:
+
include::ref-storage-format.txt[]
@@ -81,7 +81,7 @@ If this is a reinitialization, the repository will be moved to the specified pat
-b <branch-name>::
--initial-branch=<branch-name>::
-Use the specified name for the initial branch in the newly created
+Use _<branch-name>_ for the initial branch in the newly created
repository. If not specified, fall back to the default name (currently
`master`, but this is subject to change in the future; the name can be
customized via the `init.defaultBranch` configuration variable).
@@ -119,8 +119,8 @@ Same as `group`, but make the repository readable by all users.
'<perm>'::
-'<perm>' is a 3-digit octal number prefixed with `0` and each file
-will have mode '<perm>'. '<perm>' will override users'`umask(2)`
+_<perm>_ is a 3-digit octal number prefixed with `0` and each file
+will have mode _<perm>_. _<perm>_ will override users'`umask(2)`
value (and not only loosen permissions as `group` and `all`
do). `0640` will create a repository which is group-readable, but
not group-writable or accessible to others. `0660` will create a repo
@@ -133,7 +133,7 @@ By default, the configuration flag `receive.denyNonFastForwards` is enabled
in shared repositories, so that you cannot force a non fast-forwarding push
into it.
-If you provide a 'directory', the command is run inside it. If this directory
+If you provide a _<directory>_, the command is run inside it. If this directory
does not exist, it will be created.
TEMPLATE DIRECTORY
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/6] doc: git-init: rework definition lists
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
In all cases of option description, each option is in its own
term. Use the same format here.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-init.txt | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index e8fe72861dc..5fce39040f2 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -99,12 +99,14 @@ The option can have the following values, defaulting to `group` if no value
is given:
+
--
-'umask' (or 'false')::
+umask::
+false::
Use permissions reported by umask(2). The default, when `--shared` is not
specified.
-'group' (or 'true')::
+group::
+true::
Make the repository group-writable, (and g+sx, since the git group may not be
the primary group of all users). This is used to loosen the permissions of an
@@ -113,11 +115,13 @@ permission bits (e.g. if umask is `0022`, using `group` will not remove read
privileges from other (non-group) users). See `0xxx` for how to exactly specify
the repository permissions.
-'all' (or 'world' or 'everybody')::
+all::
+world::
+everybody::
Same as `group`, but make the repository readable by all users.
-'<perm>'::
+<perm>::
_<perm>_ is a 3-digit octal number prefixed with `0` and each file
will have mode _<perm>_. _<perm>_ will override users'`umask(2)`
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/6] doc: git-init: rework config item init.templateDir
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
When included into a the manpage of git-init, the param section must
not refer to the manpage.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/config/init.txt | 9 ++++++---
Documentation/git-init.txt | 2 ++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index 79c79d66174..dd1d8332737 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -1,7 +1,10 @@
-init.templateDir::
- Specify the directory from which templates will be copied.
- (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+:see-git-init:
+ifndef::git-init[]
+:see-git-init: (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+endif::[]
+init.templateDir::
+ Specify the directory from which templates will be copied. {see-git-init}
init.defaultBranch::
Allows overriding the default branch name e.g. when initializing
a new repository.
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 5fce39040f2..2f864e11ed9 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -185,6 +185,8 @@ CONFIGURATION
include::includes/cmd-config-section-all.txt[]
+:git-init:
+
include::config/init.txt[]
GIT
--
gitgitgadget
^ permalink raw reply related
* [PATCH 5/6] doc: git-clone: format verbatim words
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
We also apply the formatting to urls.txt which is included.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-clone.txt | 4 ++--
Documentation/urls.txt | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 0c07720c6f4..6bad5641151 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -156,7 +156,7 @@ objects from the source repository into a pack in the cloned repository.
--[no-]reject-shallow::
Fail if the source repository is a shallow repository.
- The 'clone.rejectShallow' configuration variable can be used to
+ The `clone.rejectShallow` configuration variable can be used to
specify the default.
--bare::
@@ -263,7 +263,7 @@ corresponding `--mirror` and `--no-tags` options instead.
branch remote's `HEAD` points at.
Further fetches into the resulting repository will only update the
remote-tracking branch for the branch this option was used for the
- initial cloning. If the HEAD at the remote did not point at any
+ initial cloning. If the `HEAD` at the remote did not point at any
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index ce671f812d4..9af19be2075 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -44,20 +44,20 @@ syntaxes may be used:
ifndef::git-clone[]
These two syntaxes are mostly equivalent, except when cloning, when
-the former implies --local option. See linkgit:git-clone[1] for
+the former implies `--local` option. See linkgit:git-clone[1] for
details.
endif::git-clone[]
ifdef::git-clone[]
These two syntaxes are mostly equivalent, except the former implies
---local option.
+`--local` option.
endif::git-clone[]
-'git clone', 'git fetch' and 'git pull', but not 'git push', will also
+`git clone`, `git fetch` and `git pull`, but not `git push`, will also
accept a suitable bundle file. See linkgit:git-bundle[1].
When Git doesn't know how to handle a certain transport protocol, it
-attempts to use the 'remote-<transport>' remote helper, if one
+attempts to use the `remote-<transport>` remote helper, if one
exists. To explicitly request a remote helper, the following syntax
may be used:
--
gitgitgadget
^ permalink raw reply related
* [PATCH 6/6] doc: git-clone: format placeholders
From: Jean-Noël Avila via GitGitGadget @ 2024-03-10 19:10 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1687.git.1710097830.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
With the new formatting rules, we use _<placeholders>_.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-clone.txt | 26 +++++++++++++-------------
Documentation/urls.txt | 4 ++--
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bad5641151..f90977a8519 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -102,9 +102,9 @@ its source repository, you can simply run `git repack -a` to copy all
objects from the source repository into a pack in the cloned repository.
--reference[-if-able] <repository>::
- If the reference repository is on the local machine,
+ If the reference _<repository>_ is on the local machine,
automatically setup `.git/objects/info/alternates` to
- obtain objects from the reference repository. Using
+ obtain objects from the reference _<repository>_. Using
an already existing repository as an alternate will
require fewer objects to be copied from the repository
being cloned, reducing network and local storage costs.
@@ -161,8 +161,8 @@ objects from the source repository into a pack in the cloned repository.
--bare::
Make a 'bare' Git repository. That is, instead of
- creating `<directory>` and placing the administrative
- files in `<directory>/.git`, make the `<directory>`
+ creating _<directory>_ and placing the administrative
+ files in `<directory>/.git`, make the _<directory>_
itself the `$GIT_DIR`. This obviously implies the `--no-checkout`
because there is nowhere to check out the working tree.
Also the branch heads at the remote are copied directly
@@ -180,11 +180,11 @@ objects from the source repository into a pack in the cloned repository.
--filter=<filter-spec>::
Use the partial clone feature and request that the server sends
a subset of reachable objects according to a given object filter.
- When using `--filter`, the supplied `<filter-spec>` is used for
+ When using `--filter`, the supplied _<filter-spec>_ is used for
the partial clone filter. For example, `--filter=blob:none` will
filter out all blobs (file contents) until needed by Git. Also,
`--filter=blob:limit=<size>` will filter out all blobs of size
- at least `<size>`. For more details on filter specifications, see
+ at least _<size>_. For more details on filter specifications, see
the `--filter` option in linkgit:git-rev-list[1].
--also-filter-submodules::
@@ -203,13 +203,13 @@ objects from the source repository into a pack in the cloned repository.
-o <name>::
--origin <name>::
Instead of using the remote name `origin` to keep track of the upstream
- repository, use `<name>`. Overrides `clone.defaultRemoteName` from the
+ repository, use _<name>_. Overrides `clone.defaultRemoteName` from the
config.
-b <name>::
--branch <name>::
Instead of pointing the newly created HEAD to the branch pointed
- to by the cloned repository's HEAD, point to `<name>` branch
+ to by the cloned repository's HEAD, point to _<name>_ branch
instead. In a non-bare repository, this is the branch that will
be checked out.
`--branch` can also take tags and detaches the HEAD at that commit
@@ -230,7 +230,7 @@ objects from the source repository into a pack in the cloned repository.
Set a configuration variable in the newly-created repository;
this takes effect immediately after the repository is
initialized, but before the remote history is fetched or any
- files checked out. The key is in the same format as expected by
+ files checked out. The _<key>_ is in the same format as expected by
linkgit:git-config[1] (e.g., `core.eol=true`). If multiple
values are given for the same key, each value will be written to
the config file. This makes it safe, for example, to add
@@ -281,7 +281,7 @@ branch of some repository for search indexing.
--recurse-submodules[=<pathspec>]::
After the clone is created, initialize and clone submodules
- within based on the provided pathspec. If no pathspec is
+ within based on the provided _<pathspec>_. If no _=<pathspec>_ is
provided, all submodules are initialized and cloned.
This option can be given multiple times for pathspecs consisting
of multiple entries. The resulting clone has `submodule.active` set to
@@ -323,20 +323,20 @@ include::ref-storage-format.txt[]
Defaults to the `submodule.fetchJobs` option.
<repository>::
- The (possibly remote) repository to clone from. See the
+ The (possibly remote) _<repository>_ to clone from. See the
<<URLS,GIT URLS>> section below for more information on specifying
repositories.
<directory>::
The name of a new directory to clone into. The "humanish"
- part of the source repository is used if no directory is
+ part of the source repository is used if no _<directory>_ is
explicitly given (`repo` for `/path/to/repo.git` and `foo`
for `host.xz:foo/.git`). Cloning into an existing directory
is only allowed if the directory is empty.
--bundle-uri=<uri>::
Before fetching from the remote, fetch a bundle from the given
- `<uri>` and unbundle the data into the local repository. The refs
+ _<uri>_ and unbundle the data into the local repository. The refs
in the bundle will be stored under the hidden `refs/bundle/*`
namespace. This option is incompatible with `--depth`,
`--shallow-since`, and `--shallow-exclude`.
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 9af19be2075..0b9e0c4302d 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -61,9 +61,9 @@ attempts to use the `remote-<transport>` remote helper, if one
exists. To explicitly request a remote helper, the following syntax
may be used:
-- <transport>::<address>
+- _<transport>_::_<address>_
-where <address> may be a path, a server and path, or an arbitrary
+where _<address>_ may be a path, a server and path, or an arbitrary
URL-like string recognized by the specific remote helper being
invoked. See linkgit:gitremote-helpers[7] for details.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/2] Allow disabling advice shown after merge conflicts
From: Philippe Blain via GitGitGadget @ 2024-03-10 19:50 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Phillip Wood, Johannes Schindelin, ZheNing Hu,
Philippe Blain
In-Reply-To: <pull.1682.git.1709396291693.gitgitgadget@gmail.com>
This series introduces a new config 'advice.mergeConflict' and uses it to
allow disabling the advice shown when 'git rebase', 'git cherry-pick', 'git
revert', 'git rebase --apply' and 'git am' stop because of conflicts.
Changes since v1:
* renamed the new advice to 'advice.mergeConflict' to make it non-sequencer
specific
* added 2/2 which uses the advice in builtin/am, which covers 'git rebase
--apply' and 'git am'
Note that the code path where 'git rebase --apply' stops because of
conflicts is not covered by the tests but I tested it manually using this
diff:
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 47534f1062..34eac2e6f4 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -374,7 +374,7 @@ test_pull_autostash_fail ()
echo conflicting >>seq.txt &&
test_tick &&
git commit -m "Create conflict" seq.txt &&
- test_must_fail git pull --rebase . seq 2>err >out &&
+ test_must_fail git -c rebase.backend=apply pull --rebase . seq 2>err >out &&
test_grep "Resolve all conflicts manually" err
'
Philippe Blain (2):
sequencer: allow disabling conflict advice
builtin/am: allow disabling conflict advice
Documentation/config/advice.txt | 2 ++
advice.c | 1 +
advice.h | 1 +
builtin/am.c | 14 +++++++++-----
sequencer.c | 33 ++++++++++++++++++---------------
t/t3501-revert-cherry-pick.sh | 1 +
t/t3507-cherry-pick-conflict.sh | 2 ++
t/t4150-am.sh | 8 ++++----
t/t4254-am-corrupt.sh | 2 +-
9 files changed, 39 insertions(+), 25 deletions(-)
base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1682%2Fphil-blain%2Fsequencer-conflict-advice-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1682/phil-blain/sequencer-conflict-advice-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1682
Range-diff vs v1:
1: e929d3381cf ! 1: a2ce6fd24c2 sequencer: allow disabling conflict advice
@@ Commit message
sequencer: allow disabling conflict advice
Allow disabling the advice shown when a squencer operation results in a
- merge conflict through a new config 'advice.sequencerConflict'.
+ merge conflict through a new config 'advice.mergeConflict', which is
+ named generically such that it can be used by other commands eventually.
+
+ Note that we use 'advise_if_enabled' for each message in the second hunk
+ in sequencer.c, instead of using 'if (show_hints &&
+ advice_enabled(...)', because the former instructs the user how to
+ disable the advice, which is more user-friendly.
Update the tests accordingly. Note that the body of the second test in
t3507-cherry-pick-conflict.sh is enclosed in double quotes, so we must
@@ Commit message
## Documentation/config/advice.txt ##
@@ Documentation/config/advice.txt: advice.*::
- rmHints::
- In case of failure in the output of linkgit:git-rm[1],
- show directions on how to proceed from the current state.
-+ sequencerConflict::
-+ Advice shown when a sequencer operation stops because
-+ of conflicts.
- sequencerInUse::
- Advice shown when a sequencer command is already in progress.
- skippedCherryPicks::
+ Advice on how to set your identity configuration when
+ your information is guessed from the system username and
+ domain name.
++ mergeConflict::
++ Advice shown when various commands stop because of conflicts.
+ nestedTag::
+ Advice shown if a user attempts to recursively tag a tag object.
+ pushAlreadyExists::
## advice.c ##
@@ advice.c: static struct {
- [ADVICE_RESET_NO_REFRESH_WARNING] = { "resetNoRefresh" },
- [ADVICE_RESOLVE_CONFLICT] = { "resolveConflict" },
- [ADVICE_RM_HINTS] = { "rmHints" },
-+ [ADVICE_SEQUENCER_CONFLICT] = { "sequencerConflict" },
- [ADVICE_SEQUENCER_IN_USE] = { "sequencerInUse" },
- [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
- [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
+ [ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated" },
+ [ADVICE_IGNORED_HOOK] = { "ignoredHook" },
+ [ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity" },
++ [ADVICE_MERGE_CONFLICT] = { "mergeConflict" },
+ [ADVICE_NESTED_TAG] = { "nestedTag" },
+ [ADVICE_OBJECT_NAME_WARNING] = { "objectNameWarning" },
+ [ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists" },
## advice.h ##
@@ advice.h: enum advice_type {
- ADVICE_RESOLVE_CONFLICT,
- ADVICE_RM_HINTS,
- ADVICE_SEQUENCER_IN_USE,
-+ ADVICE_SEQUENCER_CONFLICT,
- ADVICE_SET_UPSTREAM_FAILURE,
- ADVICE_SKIPPED_CHERRY_PICKS,
- ADVICE_STATUS_AHEAD_BEHIND_WARNING,
+ ADVICE_IGNORED_HOOK,
+ ADVICE_IMPLICIT_IDENTITY,
+ ADVICE_NESTED_TAG,
++ ADVICE_MERGE_CONFLICT,
+ ADVICE_OBJECT_NAME_WARNING,
+ ADVICE_PUSH_ALREADY_EXISTS,
+ ADVICE_PUSH_FETCH_FIRST,
## sequencer.c ##
@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
if (msg) {
- advise("%s\n", msg);
-+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT, "%s\n", msg);
++ advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s\n", msg);
/*
* A conflict has occurred but the porcelain
* (typically rebase --interactive) wants to take care
@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
if (opts->no_commit)
- advise(_("after resolving the conflicts, mark the corrected paths\n"
- "with 'git add <paths>' or 'git rm <paths>'"));
-+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
++ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("after resolving the conflicts, mark the corrected paths\n"
+ "with 'git add <paths>' or 'git rm <paths>'"));
else if (opts->action == REPLAY_PICK)
@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
- "You can instead skip this commit with \"git cherry-pick --skip\".\n"
- "To abort and get back to the state before \"git cherry-pick\",\n"
- "run \"git cherry-pick --abort\"."));
-+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
++ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git cherry-pick --continue\".\n"
@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
- "You can instead skip this commit with \"git revert --skip\".\n"
- "To abort and get back to the state before \"git revert\",\n"
- "run \"git revert --abort\"."));
-+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
++ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git revert --continue\".\n"
@@ t/t3501-revert-cherry-pick.sh: test_expect_success 'advice from failed revert' '
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
-+ hint: Disable this message with "git config advice.sequencerConflict false"
++ hint: Disable this message with "git config advice.mergeConflict false"
EOF
test_commit --append --no-tag "double-add dream" dream dream &&
test_must_fail git revert HEAD^ 2>actual &&
@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
-+ hint: Disable this message with "git config advice.sequencerConflict false"
++ hint: Disable this message with "git config advice.mergeConflict false"
EOF
test_must_fail git cherry-pick picked 2>actual &&
@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
-+ hint: Disable this message with \"git config advice.sequencerConflict false\"
++ hint: Disable this message with \"git config advice.mergeConflict false\"
EOF
test_must_fail git cherry-pick --no-commit picked 2>actual &&
-: ----------- > 2: 3235542cc6f builtin/am: allow disabling conflict advice
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/2] sequencer: allow disabling conflict advice
From: Philippe Blain via GitGitGadget @ 2024-03-10 19:51 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Phillip Wood, Johannes Schindelin, ZheNing Hu,
Philippe Blain, Philippe Blain
In-Reply-To: <pull.1682.v2.git.1710100261.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Allow disabling the advice shown when a squencer operation results in a
merge conflict through a new config 'advice.mergeConflict', which is
named generically such that it can be used by other commands eventually.
Note that we use 'advise_if_enabled' for each message in the second hunk
in sequencer.c, instead of using 'if (show_hints &&
advice_enabled(...)', because the former instructs the user how to
disable the advice, which is more user-friendly.
Update the tests accordingly. Note that the body of the second test in
t3507-cherry-pick-conflict.sh is enclosed in double quotes, so we must
escape them in the added line.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
Documentation/config/advice.txt | 2 ++
advice.c | 1 +
advice.h | 1 +
sequencer.c | 33 ++++++++++++++++++---------------
t/t3501-revert-cherry-pick.sh | 1 +
t/t3507-cherry-pick-conflict.sh | 2 ++
6 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c7ea70f2e2e..a1178284b23 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -56,6 +56,8 @@ advice.*::
Advice on how to set your identity configuration when
your information is guessed from the system username and
domain name.
+ mergeConflict::
+ Advice shown when various commands stop because of conflicts.
nestedTag::
Advice shown if a user attempts to recursively tag a tag object.
pushAlreadyExists::
diff --git a/advice.c b/advice.c
index 6e9098ff089..ecce0f5a803 100644
--- a/advice.c
+++ b/advice.c
@@ -57,6 +57,7 @@ static struct {
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated" },
[ADVICE_IGNORED_HOOK] = { "ignoredHook" },
[ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity" },
+ [ADVICE_MERGE_CONFLICT] = { "mergeConflict" },
[ADVICE_NESTED_TAG] = { "nestedTag" },
[ADVICE_OBJECT_NAME_WARNING] = { "objectNameWarning" },
[ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists" },
diff --git a/advice.h b/advice.h
index 9d4f49ae38b..89117cdeb77 100644
--- a/advice.h
+++ b/advice.h
@@ -26,6 +26,7 @@ enum advice_type {
ADVICE_IGNORED_HOOK,
ADVICE_IMPLICIT_IDENTITY,
ADVICE_NESTED_TAG,
+ ADVICE_MERGE_CONFLICT,
ADVICE_OBJECT_NAME_WARNING,
ADVICE_PUSH_ALREADY_EXISTS,
ADVICE_PUSH_FETCH_FIRST,
diff --git a/sequencer.c b/sequencer.c
index f49a871ac06..d61bbe37c8c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -467,7 +467,7 @@ static void print_advice(struct repository *r, int show_hint,
char *msg = getenv("GIT_CHERRY_PICK_HELP");
if (msg) {
- advise("%s\n", msg);
+ advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s\n", msg);
/*
* A conflict has occurred but the porcelain
* (typically rebase --interactive) wants to take care
@@ -480,22 +480,25 @@ static void print_advice(struct repository *r, int show_hint,
if (show_hint) {
if (opts->no_commit)
- advise(_("after resolving the conflicts, mark the corrected paths\n"
- "with 'git add <paths>' or 'git rm <paths>'"));
+ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("after resolving the conflicts, mark the corrected paths\n"
+ "with 'git add <paths>' or 'git rm <paths>'"));
else if (opts->action == REPLAY_PICK)
- advise(_("After resolving the conflicts, mark them with\n"
- "\"git add/rm <pathspec>\", then run\n"
- "\"git cherry-pick --continue\".\n"
- "You can instead skip this commit with \"git cherry-pick --skip\".\n"
- "To abort and get back to the state before \"git cherry-pick\",\n"
- "run \"git cherry-pick --abort\"."));
+ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git cherry-pick --continue\".\n"
+ "You can instead skip this commit with \"git cherry-pick --skip\".\n"
+ "To abort and get back to the state before \"git cherry-pick\",\n"
+ "run \"git cherry-pick --abort\"."));
else if (opts->action == REPLAY_REVERT)
- advise(_("After resolving the conflicts, mark them with\n"
- "\"git add/rm <pathspec>\", then run\n"
- "\"git revert --continue\".\n"
- "You can instead skip this commit with \"git revert --skip\".\n"
- "To abort and get back to the state before \"git revert\",\n"
- "run \"git revert --abort\"."));
+ advise_if_enabled(ADVICE_MERGE_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git revert --continue\".\n"
+ "You can instead skip this commit with \"git revert --skip\".\n"
+ "To abort and get back to the state before \"git revert\",\n"
+ "run \"git revert --abort\"."));
else
BUG("unexpected pick action in print_advice()");
}
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index aeab689a98d..43c579ea53a 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -170,6 +170,7 @@ test_expect_success 'advice from failed revert' '
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
+ hint: Disable this message with "git config advice.mergeConflict false"
EOF
test_commit --append --no-tag "double-add dream" dream dream &&
test_must_fail git revert HEAD^ 2>actual &&
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index c88d597b126..f3947b400a3 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -60,6 +60,7 @@ test_expect_success 'advice from failed cherry-pick' '
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
+ hint: Disable this message with "git config advice.mergeConflict false"
EOF
test_must_fail git cherry-pick picked 2>actual &&
@@ -74,6 +75,7 @@ test_expect_success 'advice from failed cherry-pick --no-commit' "
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
+ hint: Disable this message with \"git config advice.mergeConflict false\"
EOF
test_must_fail git cherry-pick --no-commit picked 2>actual &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/2] builtin/am: allow disabling conflict advice
From: Philippe Blain via GitGitGadget @ 2024-03-10 19:51 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Phillip Wood, Johannes Schindelin, ZheNing Hu,
Philippe Blain, Philippe Blain
In-Reply-To: <pull.1682.v2.git.1710100261.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
When 'git am' or 'git rebase --apply' encounter a conflict, they show a
message instructing the user how to continue the operation. This message
can't be disabled.
Use ADVICE_MERGE_CONFLICT introduced in the previous commit to allow
disabling it. Update the tests accordingly, as the advice output is now
on stderr instead of stdout. In t4150, redirect stdout to 'out' and
stderr to 'err', since this is less confusing. In t4254, as we are
testing a specific failure mode of 'git am', simply disable the advice.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
builtin/am.c | 14 +++++++++-----
t/t4150-am.sh | 8 ++++----
t/t4254-am-corrupt.sh | 2 +-
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d1990d7edcb..0e97b827e4b 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1150,19 +1150,23 @@ static const char *msgnum(const struct am_state *state)
static void NORETURN die_user_resolve(const struct am_state *state)
{
if (state->resolvemsg) {
- printf_ln("%s", state->resolvemsg);
+ advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s", state->resolvemsg);
} else {
const char *cmdline = state->interactive ? "git am -i" : "git am";
+ struct strbuf sb = STRBUF_INIT;
- printf_ln(_("When you have resolved this problem, run \"%s --continue\"."), cmdline);
- printf_ln(_("If you prefer to skip this patch, run \"%s --skip\" instead."), cmdline);
+ strbuf_addf(&sb, _("When you have resolved this problem, run \"%s --continue\"."), cmdline);
+ strbuf_addf(&sb, _("If you prefer to skip this patch, run \"%s --skip\" instead."), cmdline);
if (advice_enabled(ADVICE_AM_WORK_DIR) &&
is_empty_or_missing_file(am_path(state, "patch")) &&
!repo_index_has_changes(the_repository, NULL, NULL))
- printf_ln(_("To record the empty patch as an empty commit, run \"%s --allow-empty\"."), cmdline);
+ strbuf_addf(&sb, _("To record the empty patch as an empty commit, run \"%s --allow-empty\"."), cmdline);
- printf_ln(_("To restore the original branch and stop patching, run \"%s --abort\"."), cmdline);
+ strbuf_addf(&sb, _("To restore the original branch and stop patching, run \"%s --abort\"."), cmdline);
+
+ advise_if_enabled(ADVICE_MERGE_CONFLICT, "%s", sb.buf);
+ strbuf_release(&sb);
}
exit(128);
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 3b125762694..5e2b6c80eae 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -1224,8 +1224,8 @@ test_expect_success 'record as an empty commit when meeting e-mail message that
test_expect_success 'skip an empty patch in the middle of an am session' '
git checkout empty-commit^ &&
- test_must_fail git am empty-commit.patch >err &&
- grep "Patch is empty." err &&
+ test_must_fail git am empty-commit.patch >out 2>err &&
+ grep "Patch is empty." out &&
grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
git am --skip &&
test_path_is_missing .git/rebase-apply &&
@@ -1236,8 +1236,8 @@ test_expect_success 'skip an empty patch in the middle of an am session' '
test_expect_success 'record an empty patch as an empty commit in the middle of an am session' '
git checkout empty-commit^ &&
- test_must_fail git am empty-commit.patch >err &&
- grep "Patch is empty." err &&
+ test_must_fail git am empty-commit.patch >out 2>err &&
+ grep "Patch is empty." out &&
grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
git am --allow-empty >output &&
grep "No changes - recorded it as an empty commit." output &&
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 45f1d4f95e5..661feb60709 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -59,7 +59,7 @@ test_expect_success setup '
# Also, it had the unwanted side-effect of deleting f.
test_expect_success 'try to apply corrupted patch' '
test_when_finished "git am --abort" &&
- test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual &&
+ test_must_fail git -c advice.amWorkDir=false -c advice.mergeConflict=false am bad-patch.diff 2>actual &&
echo "error: git diff header lacks filename information (line 4)" >expected &&
test_path_is_file f &&
test_cmp expected actual
--
gitgitgadget
^ permalink raw reply related
* [PATCH] ci(github): make Windows test artifacts name unique
From: Philippe Blain via GitGitGadget @ 2024-03-10 20:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Victoria Dye, Philippe Blain, Philippe Blain
From: Philippe Blain <levraiphilippeblain@gmail.com>
If several jobs in the windows-test or vs-test matrices fail, the
upload-artifact action in each job tries to upload the test directories
of the failed tests as "failed-tests-windows.zip", which fails for all
jobs except the one which finishes first with the following error:
Error: Failed to CreateArtifact: Received non-retryable error:
Failed request: (409) Conflict: an artifact with this name
already exists on the workflow run
Make the artifacts name unique by using the 'matrix.nr' token, and
disambiguate the vs-test artifacts from the windows-test ones.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
ci(github): make Windows test artifacts name unique
I noticed this while working on another series and thought it deserved a
quick fix.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1688%2Fphil-blain%2Fci-unique-windows-artifacts-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1688/phil-blain/ci-unique-windows-artifacts-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1688
.github/workflows/main.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 683a2d633ed..3428773b096 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -159,7 +159,7 @@ jobs:
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v4
with:
- name: failed-tests-windows
+ name: failed-tests-windows-${{ matrix.nr }}
path: ${{env.FAILED_TEST_ARTIFACTS}}
vs-build:
name: win+VS build
@@ -250,7 +250,7 @@ jobs:
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v4
with:
- name: failed-tests-windows
+ name: failed-tests-windows-vs-${{ matrix.nr }}
path: ${{env.FAILED_TEST_ARTIFACTS}}
regular:
name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})
base-commit: e09f1254c54329773904fe25d7c545a1fb4fa920
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3 4/7] sequencer: treat error reading HEAD as unborn branch
From: Junio C Hamano @ 2024-03-11 0:07 UTC (permalink / raw)
To: Brian Lyles; +Cc: git, newren, me, phillip.wood123, Phillip Wood
In-Reply-To: <20240310184602.539656-5-brianmlyles@gmail.com>
Brian Lyles <brianmlyles@gmail.com> writes:
> When using git-cherry-pick(1) with `--allow-empty` while on an unborn
> branch, an error is thrown. This is inconsistent with the same
> cherry-pick when `--allow-empty` is not specified.
When cherry-picking on top of an unborn branch without the option,
how does the code figure out that we are on an unborn branch? We
must be doing the detection, as we'd need to at least know the fact
that we are on an unborn in order to make the resulting commit a
root commit and also in order to apply the cherry-picked changes
against an empty tree.
> Treat a failure reading HEAD as an unborn branch in
> `is_index_unchanged`. This is consistent with other sequencer logic such
> as `do_pick_commit`. When on an unborn branch, use the `empty_tree` as
> the tree to compare against.
It is not a good code hygiene to assume that a failure to read HEAD
always means we are on an unborn branch, even if that is the most
likely cause of the failure. We may instead want to positively
determine that we are on an unborn state, by seeing if the HEAD is a
symbolic ref that points at a ref in refs/heads/* hierarchy, and
that ref does not exist.
But if the existing sequencer code is littered with such loose logic
everywhere, perhaps imitating the looseness for now with a NEEDSWORK
comment to later clean them all up would be the least we could do
right now. If we want to be more ambitious, a few clean-up patches
that refactors existing code paths that detect that we are on an
unborn branch into a call to a single helper function, and then
tightens its implementation, before doing this step would be even
better (but I offhand do not know how bad the current code is, so I
would understand it if it may turn out to be a lot more work than
you would want to invest in).
^ permalink raw reply
* [PATCH] diff: add diff.srcprefix and diff.dstprefix option support
From: Peter Hutterer @ 2024-03-11 2:32 UTC (permalink / raw)
To: git; +Cc: David Heidelberg
The git option equivalent to --src-prefix and --dst-prefix.
Both of these are of lower precedence than the diff.noprefix and
diff.mnemonicprefix option.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
As David already mentioned [1] my main motivation here is to use
a prefix of "./" because that is patch -p1 compatible *and* supports
double-click highlighting of the file path in most terminals.
The current approach of a/ and b/ fail at the latter and diff.noprefix
fails at the former.
[1] https://lore.kernel.org/git/f80aaf4a-ffea-48e6-b279-c3b7a6a53996@ixit.cz/
Documentation/config/diff.txt | 6 ++++++
diff.c | 18 ++++++++++++++++++
diff.h | 1 +
t/t4013-diff-various.sh | 20 ++++++++++++++++++++
4 files changed, 45 insertions(+)
diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..888632955b30 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
diff.noprefix::
If set, 'git diff' does not show any source or destination prefix.
+diff.srcprefix::
+ If set, 'git diff' uses this source prefix.
+
+diff.dstprefix::
+ If set, 'git diff' uses this destination prefix.
+
diff.relative::
If set to 'true', 'git diff' does not show changes outside of the directory
and show pathnames relative to the current directory.
diff --git a/diff.c b/diff.c
index e50def45383e..52a476737def 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
static int diff_no_prefix;
+static const char *diff_src_prefix;
+static const char *diff_dst_prefix;
static int diff_relative;
static int diff_stat_name_width;
static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
diff_no_prefix = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "diff.srcprefix")) {
+ return git_config_string(&diff_src_prefix, var, value);
+ }
+ if (!strcmp(var, "diff.dstprefix")) {
+ return git_config_string(&diff_dst_prefix, var, value);
+ }
if (!strcmp(var, "diff.relative")) {
diff_relative = git_config_bool(var, value);
return 0;
@@ -3429,6 +3437,14 @@ void diff_set_default_prefix(struct diff_options *options)
options->b_prefix = "b/";
}
+void diff_set_custom_prefix(struct diff_options *options, const char *src_prefix, const char *dst_prefix)
+{
+ if (src_prefix)
+ options->a_prefix = src_prefix;
+ if (dst_prefix)
+ options->b_prefix = dst_prefix;
+}
+
struct userdiff_driver *get_textconv(struct repository *r,
struct diff_filespec *one)
{
@@ -4736,6 +4752,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
diff_set_noprefix(options);
} else if (!diff_mnemonic_prefix) {
diff_set_default_prefix(options);
+ if (diff_src_prefix || diff_dst_prefix)
+ diff_set_custom_prefix(options, diff_src_prefix, diff_dst_prefix);
}
options->color_moved = diff_color_moved_default;
diff --git a/diff.h b/diff.h
index 66bd8aeb2936..ab4dd5ec70f3 100644
--- a/diff.h
+++ b/diff.h
@@ -499,6 +499,7 @@ void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev)
void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
void diff_set_noprefix(struct diff_options *options);
void diff_set_default_prefix(struct diff_options *options);
+void diff_set_custom_prefix(struct diff_options *options, const char *src_prefix, const char *dst_prefix);
int diff_can_quit_early(struct diff_options *);
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..86834186fdba 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,26 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
check_prefix actual a/file0 b/file0
'
+test_expect_success 'diff respects diff.srcprefix' '
+ git -c diff.srcprefix=x/ diff >actual &&
+ check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+ git -c diff.dstprefix=y/ diff >actual &&
+ check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
+ git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+ check_prefix actual file0 file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
+ git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+ check_prefix actual i/file0 w/file0
+'
+
test_expect_success 'diff --no-renames cannot be abbreviated' '
test_expect_code 129 git diff --no-rename >actual 2>error &&
test_must_be_empty actual &&
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] reftable/block: fix binary search over restart counter
From: Patrick Steinhardt @ 2024-03-11 5:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq34t1k12p.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]
On Thu, Mar 07, 2024 at 04:40:46PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> >> The consequence is that `binsearch()` essentially always returns 0,
> >> indicacting to us that we must start searching right at the beginning of
> >> the block. This works by chance because we now always do a linear scan
> >> from the start of the block, and thus we would still end up finding the
> >> desired record. But needless to say, this makes the optimization quite
> >> useless.
>
> >> Fix this bug by returning whether the current key is smaller than the
> >> searched key. As the current behaviour was correct it is not possible to
> >> write a test. Furthermore it is also not really possible to demonstrate
> >> in a benchmark that this fix speeds up seeking records.
> >
> > This is an amusing bug.
>
> Having said all that.
>
> I have to wonder if it is the custom implementation of binsearch()
> the reftable/basic.c file has, not this particular comparison
> callback. It makes an unusual expectation on the comparison
> function, unlike bsearch(3) whose compar(a,b) is expected to return
> an answer with the same sign as "a - b".
>
> I just checked the binary search loops we have in the core part of
> the system, like the one in hash-lookup.c (which takes advantage of
> the random and uniform nature of hashed values to converge faster
> than log2) and ones in builtin/pack-objects.c (both of which are
> absolute bog-standard). Luckily, we do not use such an unusual
> convention (well, we avoid overhead of compar callbacks to begin
> with, so it is a bit of apples-to-oranges comparison).
Very true, this behaviour cought me by surprise, as well, and I do think
it's quite easy to get wrong. Now I would've understood if `binsearch()`
were able to handle and forward errors to the caller by passing -1. And
I almost thought that was the case because `restart_key_less()` can
indeed fail, and it would return a negative value if so. But that error
return code is then not taken as an indicator of failure, but instead
will cause us to treat the current value as smaller than the comparison
key.
But we do know to bubble the error up via the pasesd-in args by setting
`args->error = -1`. Funny thing though: I just now noticed that we check
for `args.error` _before_ we call `binsearch()`. Oops.
I will send a follow-up patch that addresses these issues.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] sequencer: allow disabling conflict advice
From: Kristoffer Haugsbakk @ 2024-03-11 10:29 UTC (permalink / raw)
To: Josh Soref
Cc: Elijah Newren, Phillip Wood, Johannes Schindelin, ZheNing Hu,
Philippe Blain, git
In-Reply-To: <a2ce6fd24c270fcc89439cd7d119c701dd262ec5.1710100261.git.gitgitgadget@gmail.com>
Hi Philippe
On Sun, Mar 10, 2024, at 20:51, Philippe Blain via GitGitGadget wrote:
> diff --git a/Documentation/config/advice.txt
> b/Documentation/config/advice.txt
> index c7ea70f2e2e..a1178284b23 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -56,6 +56,8 @@ advice.*::
> Advice on how to set your identity configuration when
> your information is guessed from the system username and
> domain name.
> + mergeConflict::
> + Advice shown when various commands stop because of conflicts.
Given that topic kh/branch-ref-syntax-advice is in `next`, maybe this
should be changed to “Shown when”?[1]
🔗 1: https://lore.kernel.org/git/7017ff3fff773412e8c472d8e59a132b0e8faae7.1709670287.git.code@khaugsbakk.name/
> nestedTag::
> Advice shown if a user attempts to recursively tag a tag object.
> pushAlreadyExists::
^ 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