* [PATCH v2 1/8] docs: address inaccurate `--empty` default with `--exec`
From: Brian Lyles @ 2024-02-10 7:43 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.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
This commit was not present in v1. It is a fix to some existing
documentation that Phillip noticed was incorrect.
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 150734fb39..9d7397b696 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 v2 2/8] docs: clean up `--empty` formatting in git-rebase(1) and git-am(1)
From: Brian Lyles @ 2024-02-10 7:43 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>
---
Changes since v1:
- Options are now listed in alphabetical order per Phillip's
recommendation.
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 9d7397b696..68cdebd2aa 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
@@ -698,7 +705,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 v2 3/8] rebase: update `--empty=ask` to `--empty=drop`
From: Brian Lyles @ 2024-02-10 7:43 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.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Reported-by: Elijah Newren <newren@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 68cdebd2aa..6f64084a95 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`
@@ -705,7 +706,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 4084a6abb8..3b9bb2fa06 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 {
@@ -954,10 +954,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,
@@ -1136,7 +1140,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,
@@ -1553,7 +1557,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 v2 4/8] sequencer: treat error reading HEAD as unborn branch
From: Brian Lyles @ 2024-02-10 7:43 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.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
This is another new commit that was not present in v1.
See this comment[1] from Phillip for context.
[1]: https://lore.kernel.org/git/b5213705-4cd6-40ef-8c5f-32b214534b8b@gmail.com/
sequencer.c | 36 ++++++++++++++++++++---------------
t/t3501-revert-cherry-pick.sh | 11 +++++++++++
2 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 3cc88d8a80..b1b19512de 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -769,30 +769,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;
+ struct object_id head_oid, *cache_tree_oid, 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..390e0ed186 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -112,6 +112,17 @@ test_expect_success 'cherry-pick on unborn branch' '
test_cmp_rev ! initial HEAD
'
+test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
+ git checkout main &&
+ git branch -D unborn &&
+ git checkout --orphan unborn &&
+ git rm --cached -r . &&
+ rm -rf * &&
+ git cherry-pick initial --allow-empty &&
+ git diff --quiet initial &&
+ test_cmp_rev ! initial HEAD
+'
+
test_expect_success 'cherry-pick "-" to pick from previous branch' '
git checkout unborn &&
test_commit to-pick actual content &&
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/8] sequencer: do not require `allow_empty` for redundant commit options
From: Brian Lyles @ 2024-02-10 7:43 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>
---
This is the first half of the first commit[1] in v1, which has now been
split up. While the next commit may be considered somewhat
controversial, this part of the change should not be.
[1]: https://lore.kernel.org/git/20240119060721.3734775-2-brianmlyles@gmail.com/
sequencer.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b1b19512de..3f41863dae 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1725,34 +1725,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 v2 6/8] cherry-pick: decouple `--allow-empty` and `--keep-redundant-commits`
From: Brian Lyles @ 2024-02-10 7:43 UTC (permalink / raw)
To: git; +Cc: Brian Lyles, newren, me, phillip.wood123, gitster
In-Reply-To: <20240119060721.3734775-2-brianmlyles@gmail.com>
As noted in the git-cherry-pick(1) docs, `--keep-redundant-commits`
implies `--allow-empty`, despite the two having distinct,
non-overlapping meanings:
- `allow_empty` refers specifically to commits which start empty, as
indicated by the documentation for `--allow-empty` within
git-cherry-pick(1):
"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."
- `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. This is indicated by the documentation for
`--keep-redundant-commits` within git-cherry-pick(1):
"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."
This implication of `--allow-empty` therefore seems incorrect: One
should be able to keep a commit that becomes empty without also being
forced to pick commits that start as empty. However, the following
series of commands results in both the commit that became empty and the
commit that started empty being picked, despite only
`--keep-redundant-commits` being specified:
git init
echo "a" >test
git add test
git commit -m "Initial commit"
echo "b" >test
git commit -am "a -> b"
git commit --allow-empty -m "empty"
git cherry-pick --keep-redundant-commits HEAD^ HEAD
The same cherry-pick with `--allow-empty` would fail on the redundant
commit, and with neither option would fail on the empty commit.
Do not imply `--allow-empty` when using `--keep-redundant-commits` with
git-cherry-pick(1).
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---
This is the second half of the first commit[1] in v1, which has now been
split up.
This commit proposes a breaking change, albeit one that seems correct
and relatively minor to me. If this change is deemed too controversial,
I am prepared to drop it from the series. See Junio's[2] and
Phillip's[3] comments on v1 for additional context.
[1]: https://lore.kernel.org/git/20240119060721.3734775-2-brianmlyles@gmail.com/
[2]: https://lore.kernel.org/git/xmqqy1cfnca7.fsf@gitster.g/
[3]: https://lore.kernel.org/git/8ff4650c-f84f-41bd-a46c-3b845ff29b70@gmail.com/
Documentation/git-cherry-pick.txt | 10 +++++++---
builtin/revert.c | 4 ----
t/t3505-cherry-pick-empty.sh | 6 ++++++
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index fdcad3d200..c88bb88822 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -131,8 +131,8 @@ 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 `--keep-redundant-commits`.
--allow-empty-message::
By default, cherry-picking a commit with an empty message will fail.
@@ -144,7 +144,11 @@ effect to your index in a row.
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`.
+ creates an empty commit object. Note that use of this option only
+ results in an empty commit when the commit 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`.
--strategy=<strategy>::
Use the given merge strategy. Should only be used once.
diff --git a/builtin/revert.c b/builtin/revert.c
index 89821bab95..d83977e36e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -134,10 +134,6 @@ 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;
- /* implies allow_empty */
- if (opts->keep_redundant_commits)
- opts->allow_empty = 1;
-
if (cleanup_arg) {
opts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);
opts->explicit_cleanup = 1;
diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index eba3c38d5a..2709cfc677 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -59,6 +59,12 @@ test_expect_success 'cherry pick an empty non-ff commit without --allow-empty' '
test_must_fail git cherry-pick empty-change-branch
'
+test_expect_success 'cherry pick an empty non-ff commit with --keep-redundant-commits' '
+ git checkout main &&
+ test_must_fail git cherry-pick --keep-redundant-commits empty-change-branch 2>output &&
+ test_grep "The previous cherry-pick is now empty" output
+'
+
test_expect_success 'cherry pick an empty non-ff commit with --allow-empty' '
git checkout main &&
git cherry-pick --allow-empty empty-change-branch
--
2.43.0
^ permalink raw reply related
* [PATCH v2 7/8] cherry-pick: enforce `--keep-redundant-commits` incompatibility
From: Brian Lyles @ 2024-02-10 7:43 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>
---
This commit was not present in v1 either. It addresses an existing issue
that I noticed after Phillip pointed out the same deficiency for my new
`--empty` option introduced in the ultimate commit in this series.
[1]: https://lore.kernel.org/git/CAHPHrSf+joHe6ikErHLgWrk-_qjSROS-dXCHagxWGDAF=2deDg@mail.gmail.com/
builtin/revert.c | 1 +
t/t3515-cherry-pick-incompatible-options.sh | 34 +++++++++++++++++++++
2 files changed, 35 insertions(+)
create mode 100755 t/t3515-cherry-pick-incompatible-options.sh
diff --git a/builtin/revert.c b/builtin/revert.c
index d83977e36e..48c426f277 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -163,6 +163,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/t3515-cherry-pick-incompatible-options.sh b/t/t3515-cherry-pick-incompatible-options.sh
new file mode 100755
index 0000000000..6100ab64fd
--- /dev/null
+++ b/t/t3515-cherry-pick-incompatible-options.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='test if cherry-pick detects and aborts on incompatible options'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ echo first > file1 &&
+ git add file1 &&
+ test_tick &&
+ git commit -m "first" &&
+
+ echo second > file1 &&
+ git add file1 &&
+ test_tick &&
+ git commit -m "second"
+'
+
+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 v2 8/8] cherry-pick: add `--empty` for more robust redundant commit handling
From: Brian Lyles @ 2024-02-10 7:43 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.
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>
---
Documentation/git-cherry-pick.txt | 32 +++++++++++------
builtin/revert.c | 37 ++++++++++++++++++-
sequencer.c | 6 ++++
t/t3505-cherry-pick-empty.sh | 23 +++++++++++-
t/t3510-cherry-pick-sequence.sh | 40 +++++++++++++++++++++
t/t3515-cherry-pick-incompatible-options.sh | 14 ++++++++
6 files changed, 140 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index c88bb88822..a444d960b1 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -132,23 +132,35 @@ effect to your index in a row.
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 will cause the cherry-pick to fail. To force the
- inclusion of those commits, use `--keep-redundant-commits`.
+ 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.
+`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 species 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. Note that use of this option only
- results in an empty commit when the commit 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`.
+ 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 48c426f277..27efb6284b 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 = 0,
+ 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);
+ }
+
if (cleanup_arg) {
opts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);
opts->explicit_cleanup = 1;
@@ -164,6 +198,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 3f41863dae..509a5244d2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2921,6 +2921,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);
@@ -3465,6 +3468,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 2709cfc677..669416c158 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -90,7 +90,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
@@ -105,4 +105,25 @@ test_expect_success 'cherry-pick a no-op with --keep-redundant' '
test_cmp expect actual
'
+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..5f6c45dfe3 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -90,6 +90,46 @@ test_expect_success 'cherry-pick persists opts correctly' '
test_cmp expect actual
'
+test_expect_success 'cherry-pick persists --empty=stop correctly' '
+ pristine_detach initial &&
+ # to make sure that the session to cherry-pick a sequence
+ # gets interrupted, use a high-enough number that is larger
+ # than the number of parents of any commit we have created
+ mainline=4 &&
+ test_expect_code 128 git cherry-pick -s -m $mainline --empty=stop initial..anotherpick &&
+ test_path_is_file .git/sequencer/opts &&
+ test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits &&
+ test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits
+'
+
+test_expect_success 'cherry-pick persists --empty=drop correctly' '
+ pristine_detach initial &&
+ # to make sure that the session to cherry-pick a sequence
+ # gets interrupted, use a high-enough number that is larger
+ # than the number of parents of any commit we have created
+ mainline=4 &&
+ test_expect_code 128 git cherry-pick -s -m $mainline --empty=drop initial..anotherpick &&
+ test_path_is_file .git/sequencer/opts &&
+ test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits &&
+ echo "true" >expect &&
+ git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick persists --empty=keep correctly' '
+ pristine_detach initial &&
+ # to make sure that the session to cherry-pick a sequence
+ # gets interrupted, use a high-enough number that is larger
+ # than the number of parents of any commit we have created
+ mainline=4 &&
+ test_expect_code 128 git cherry-pick -s -m $mainline --empty=keep initial..anotherpick &&
+ test_path_is_file .git/sequencer/opts &&
+ echo "true" >expect &&
+ git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits >actual &&
+ test_cmp expect actual &&
+ test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits
+'
+
test_expect_success 'revert persists opts correctly' '
pristine_detach initial &&
# to make sure that the session to revert a sequence
diff --git a/t/t3515-cherry-pick-incompatible-options.sh b/t/t3515-cherry-pick-incompatible-options.sh
index 6100ab64fd..b2780fdbf3 100755
--- a/t/t3515-cherry-pick-incompatible-options.sh
+++ b/t/t3515-cherry-pick-incompatible-options.sh
@@ -31,4 +31,18 @@ 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_done
--
2.43.0
^ permalink raw reply related
* Open Apple Store
From: Ashok Sharma @ 2024-02-10 9:02 UTC (permalink / raw)
To: git
Sent from my iPhone
^ permalink raw reply
* Re: Interactive rebase: using "pick" for merge commits
From: Stefan Haller @ 2024-02-10 9:23 UTC (permalink / raw)
To: phillip.wood, git
In-Reply-To: <ad561600-faf6-4d3c-80b2-34b3d1a1b99e@gmail.com>
On 09.02.24 17:24, Phillip Wood wrote:
> On 09/02/2024 15:52, Stefan Haller wrote:
>> When I do an interactive rebase, and manually enter a "pick" with the
>> commit hash of a merge commit, I get the following confusing error
>> message:
>>
>> error: commit fa1afe1 is a merge but no -m option was given.
>>
>> Is it crazy to want pick to work like this? Should it be supported?
>
> It causes problems trying to maintain the topology. In the past there
> was a "--preserve-merges" option that allowed one to "pick" merges but
> it broke if the user edited the todo list. The "--rebase-merges" option
> was introduced with the "label", "reset" and "merge" todo list
> instructions to allow the user to control the topology.
Yes, I'm familiar with all this, but that's not what I mean. I don't
want to maintain the topology here, and I'm also not suggesting that git
itself generates such "pick" entries with -mX arguments (maybe I wasn't
clear on that). What I want to do is to add such entries myself, as a
user, resulting in the equivalent of doing a "break" at that point in
the rebase and doing a "git cherry-pick -mX <hash-of-merge-commit>"
manually.
-Stefan
^ permalink raw reply
* Re: Bug: Commit fails when no global email address is set even though --author is used
From: Marcus Tillmanns @ 2024-02-10 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, git@vger.kernel.org, Phillip Wood
In-Reply-To: <xmqqfry1fm2e.fsf@gitster.g>
For me the issue was that I knew already about the global config setting, but in my case I specifically did not want to set that.
What pulled my attention away from the important part was the huge wall of text about how to setup global stuff etc, making the single word in the top left basically invisible to my eye. I was thinking "I already knew all that, I’ve set the author, so you complaining still with the same error message must be a bug”.
I think, if a user tries to commit with “—author”, and git fails to figure out the comitter, it should have a specific error message about “hey, you’ve set the author, but we still have to figure out whom to set as the committer, or you can use “—authorAndComitter” if they should be the same”.
That would make it obvious to the user what’s going on.
Cheers,
Marcus
> On 9. Feb 2024, at 20:56, Junio C Hamano <gitster@pobox.com> wrote:
>
> [You don't often get email from gitster@pobox.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
>
>> On Fri, Feb 9, 2024, at 18:30, Junio C Hamano wrote:
>>> So, now, let's be productive. When somebody who does not know much
>>> about Git tries to commit without configuring anything and hits the
>>> error, what is a more appropriate message to guide who does not know
>>> what he or she does not know?
>>>
>>> The user claims that "committer identity unknown, please tell me who
>>> you are" were not helpful enough. Would it make it more helpful if
>>> we append how to "tell who they are" after that message, perhaps
>>> with "git config" on user.email and user.name variables, or
>>> something?
>>>
>>> Or do we need three-way switch that does
>>>
>>> if (neither is known) {
>>> printf("neither author or committer is known");
>>> } else if (author is known but committer is not known) {
>>> printf("author is known but committer is not"):
>>> } else if (author is not known but committer is known) {
>>> printf("committer is known but author is not"):
>>> } else {
>>> return happy;
>>> }
>>>
>>> printf("please tell us who you are...");
>>>
>>> perhaps?
>>
>> I think a three-way switch looks good. With the amendment that it steers
>> you towards `user.*` instead of setting both `author.*` and
>> `committer.*`.
>>
>> Something like
>>
>> • Author is set, not committer
>> • Message: author is set but not committer: you might want to set
>> *user* instead (prints suggested config)
>>
>> I can try to make a patch later.
>
> Wait. I didn't realize this when I wrote the message you are
> responding to, but we *do* already suggest settig user.* variables.
>
> If the user chose to ignore that, then there isn't much we can do to
> help, is there?
>
> Puzzled, but I'll stop here.
>
^ permalink raw reply
* Re: [PATCH] column: disallow negative padding
From: Chris Torek @ 2024-02-10 9:48 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, Tiago Pascoal
In-Reply-To: <19119aa6-9a8c-44c6-af79-0ea6a8bcb181@app.fastmail.com>
On Sat, Feb 10, 2024 at 1:46 AM Kristoffer Haugsbakk
<code@khaugsbakk.name> wrote:
> I forgot tests.
You presumably also wanted the `_` here for gettext-ing:
> + die("%s: argument must be a non-negative integer", "padding");
Chris
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.44.0-rc0
From: Johannes Schindelin @ 2024-02-10 10:19 UTC (permalink / raw)
To: git-for-windows, git, git-packagers; +Cc: Johannes Schindelin
Dear Git users,
I hereby announce that Git for Windows 2.44.0-rc0 is available from:
https://github.com/git-for-windows/git/releases/tag/v2.44.0-rc0.windows.1
Changes since Git for Windows v2.43.0 (November 20th 2023)
Git for Windows will drop support for Windows 7 and for Windows 8 in
the next versions, see the announcement of MSYS2 on which Git for
Windows relies for components such as Bash and Perl.
Please also note that the 32-bit variant of Git for Windows is
deprecated; Its last official release is planned for 2025.
New Features
* Comes with Git v2.44.0-rc0.
* Comes with gnupg v2.2.42.
* Comes with libfido2 v1.14.0.
* Comes with the MSYS2 runtime (Git for Windows flavor) based on
Cygwin v3.4.10.
* Comes with Perl v5.38.2.
* Git for Windows learned to detect and use native Windows support
for ANSI sequences, which allows using 24-bit colors in terminal
windows.
* Comes with Git LFS v3.4.1.
* The repository viewer Tig that is included in Git for Windows can
now be called also directly from PowerShell/CMD.
* Comes with OpenSSH v9.6.P1.
* Comes with Bash v5.2.26.
* Comes with GNU TLS v3.8.3.
* Comes with OpenSSL v3.2.1.
* Comes with cURL v8.6.0.
* Comes with GNU Privacy Guard v2.4.4.
Bug Fixes
* The 32-bit variant of Git for Windows was missing some MSYS2
runtime updates, which was addressed; Do note 32-bit support is
phased out.
* The Git for Windows installer showed cut-off text in some setups.
This has been fixed.
* The git credential-manager --help command previously would not find
a page to display in the web browser, which has been fixed.
Git-2.44.0-rc0-64-bit.exe | b3674c6a4fc010dda9065130e06d9a3c8a4b900d2c3b2bdf8183cc47919940a4
Git-2.44.0-rc0-32-bit.exe | 344039b0f71bcd07a19747ec58944cadc175c24f37365e4282d60cc77c77687d
PortableGit-2.44.0-rc0-64-bit.7z.exe | b5b29161c2f80d7b0b9183fff274f545f0c4290e9ce1ef820107da44bf424a07
PortableGit-2.44.0-rc0-32-bit.7z.exe | d1b6c533e5ec7df93870c085b960ad2112a44e1fc9d52cd97c530984892e5cb8
MinGit-2.44.0-rc0-64-bit.zip | e3e2d8f601bb80b6472cd9d58a3ca7704f245b015e236d9b56231b4ced08169f
MinGit-2.44.0-rc0-32-bit.zip | 953c76671644727df28f1b78a80c97266a25d62857afa36e7aacc88ff365fb97
MinGit-2.44.0-rc0-busybox-64-bit.zip | c65698f812ba3090215591978bdd62523922fcaaca31d8fdcaf27c6b75ec8ec1
MinGit-2.44.0-rc0-busybox-32-bit.zip | d614f5b893c8d8784c8e78c22470540eb050946500e57aac65e08e4c403510eb
Git-2.44.0-rc0-64-bit.tar.bz2 | 4ad88d90e553cf31b90381c1ca880509c7c51c91e330ea1b9b73733f31598eee
Git-2.44.0-rc0-32-bit.tar.bz2 | 28f4fa7ff42bd210e944173417f6efcf3b055bfb0ef149941ced91f03ab1560d
Ciao,
Johannes
^ permalink raw reply
* Re: cloning the linxu kernel repo at a VPS with small RAM
From: Toralf Förster @ 2024-02-10 10:35 UTC (permalink / raw)
To: git
In-Reply-To: <2f773980-70ec-4ad0-a49c-3ac12c294a39@gmx.de>
On 2/8/24 18:32, Toralf Förster wrote:
>
> Q:
> I do wonder if Git could automatically try to deal with only 1.5 GiB
> available RAM?
Given that git fails for the linux kernel repo - where Git was used
first in the wild (?) - it should definitely works for that repo even
with a small RAM, right?
--
Toralf
^ permalink raw reply
* [L10N] Kickoff for Git 2.44.0 round #1
From: Jiang Xin @ 2024-02-10 10:43 UTC (permalink / raw)
To: Git List, Git l10n discussion group, Alexander Shopov, Jordi Mas,
Ralf Thielow, Jimmy Angelakos, Christopher Díaz,
Jean-Noël Avila, Bagas Sanjaya, Alessandro Menti,
Gwan-gyeong Mun, Arusekk, Daniel Santos, Dimitriy Ryazantcev,
Peter Krefting, Emir SARI, Arkadii Yakovets,
Trần Ngọc Quân, Teng Long, Yi-Jyun Pan
Cc: Jiang Xin, Patrick Steinhardt
Hi,
Git v2.44.0-rc0 has been released, and it's time to start new round of
git l10n. This time there are 52 updated messages need to be translated
since last release. Please send your pull request to the l10n coordinator's
repository below before this update window closes on Sun, 18 Feb 2024.
https://github.com/git-l10n/git-po/
Our l10n helper program (git-po-helper) has been upgraded to v0.7.3 to
detect typos on mismatched refspec (such as refs/ vs ref/) reported by
Patrick. See [1]. Please check translations using git-po-helper before
sending your PR.
As of git 2.37, we (git l10n contributors) have a new l10n workflow. The
following description of the new l10n workflow is from the "po/README.md"
file.
## The "po/git.pot" file is a generated file, no longer in the repository
The l10n coordinator does not need to generate the "po/git.pot" file every
time to start a new l10n workflow, and there is no "po/git.pot" file at all.
Everyone can generate the "po/git.pot" file with the command below:
make po/git.pot
But we can also forget about it. By updating our corresponding "po/XX.po"
file, the "po/git.pot" file is automatically generated.
## Update the "po/XX.po" file, and start to translate
Before updating the "po/XX.po" file, l10n contributors should pull the latest
commits from the master branch of "git.git". E.g.:
git pull --rebase git@github.com:git/git.git master
Then update the cooresponding "po/XX.po" file using the following command:
make po-update PO_FILE=po/XX.po
Translate the uptodate "po/XX.po" file, and create a new commit.
## Refine your commits, send pull requests
In the "po/XX.po" file, there are location lines in comments like below:
#: add-interactive.c:535 add-interactive.c:836 reset.c:136 sequencer.c:3505
#: sequencer.c:3970 sequencer.c:4127 builtin/rebase.c:1261
#: builtin/rebase.c:1671
These comments with file locations are useful for l10n contributors to locate
the context easily during translation. But these file locations introduce a
lot of noise and will consume a lot of repository storage. Therefore, we
should remove these file locations from the "po/XX.po" file.
To remove file locations in the "po/XX.po" file, you can use one of the
following two ways, but don't switch back and forth.
* Keep the filenames, only remove locations (need gettext 0.19 and above):
msgcat --add-location=file po/XX.po >po/XX.po.new
mv po/XX.po.new po/XX.po
* Remove both filenames and locations:
msgcat --no-location po/XX.po >po/XX.po.new
mv po/XX.po.new po/XX.po
After squashing trivial commits and removing file locations in the "po/XX.po"
file, send pull request to the l10n coordinator's repository below:
https://github.com/git-l10n/git-po/
## Resolve errors found by the l10n CI pipeline for the pull request
A helper program hosted on "https://github.com/git-l10n/git-po-helper" can
help git l10n coordinator and git l10n contributors to check the conventions
of git l10n contributions, and it is also used in GitHub actions as l10n CI
pipeline to validate each pull request in the "git-l10n/git-po" repository.
Please fix the issues found by the helper program.
** Please note: The update window will close on Sun, 18 Feb 2024. **
[1] https://lore.kernel.org/git/ZX_9nRYKVq0jT0Lp@tanuki/
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH v2 2/5] completion: complete 'submodule.*' config variables
From: Philippe Blain @ 2024-02-10 15:39 UTC (permalink / raw)
To: Patrick Steinhardt, Philippe Blain via GitGitGadget; +Cc: git
In-Reply-To: <ZcSF0Uw0xxlJXRlH@tanuki>
Hi Patrick,
Le 2024-02-08 à 02:42, Patrick Steinhardt a écrit :
> On Mon, Jan 29, 2024 at 01:27:58PM +0000, Philippe Blain via GitGitGadget wrote:
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> In the Bash completion script, function
>> __git_complete_config_variable_name completes config variables and has
>> special logic to deal with config variables involving user-defined
>> names, like branch.<name>.* and remote.<name>.*.
>>
>> This special logic is missing for submodule-related config variables.
>> Add the appropriate branches to the case statement, making use of the
>> in-tree '.gitmodules' to list relevant submodules.
>>
>> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
>> ---
>> contrib/completion/git-completion.bash | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 159a4fd8add..8af9bc3f4e1 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2803,6 +2803,19 @@ __git_complete_config_variable_name ()
>> __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
>> return
>> ;;
>> + submodule.*.*)
>> + local pfx="${cur_%.*}."
>> + cur_="${cur_##*.}"
>> + __gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
>> + return
>> + ;;
>> + submodule.*)
>> + local pfx="${cur_%.*}."
>> + cur_="${cur_#*.}"
>> + __gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
>> + __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
>> + return
>> + ;;
>
> Hm, it feels quite awkward that we have to manually massage the
> gitmodules config like this. But the closest tool I could find is
> `git submodule status`, which would also end up describing commits in
> each of the submodules and thus do needless work. And second, it prints
> submodule paths and not submodule names, so it surfaces the wrong info
> in the first place.
>
> Ideally, we would create such a tool that makes the information more
> accessible to us. But that certainly seems out of scope of this patch
> series.
>
> In any case though it would be nice to add some tests for these new
> completions.
OK, I end up testing them in 3/5 via the __git_compute_first_level_config_vars_for_section
function I'm adding. But it's true I could add the test directly
in 2/5, if it makes more sense.
Thanks for your review !
Philippe.
^ permalink raw reply
* Re: [PATCH v2 3/5] completion: add and use __git_compute_first_level_config_vars_for_section
From: Philippe Blain @ 2024-02-10 16:06 UTC (permalink / raw)
To: Patrick Steinhardt, Philippe Blain via GitGitGadget; +Cc: git
In-Reply-To: <ZcSF1mJ-JXQLmoZ5@tanuki>
Le 2024-02-08 à 02:42, Patrick Steinhardt a écrit :
> On Mon, Jan 29, 2024 at 01:27:59PM +0000, Philippe Blain via GitGitGadget wrote:
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> The function __git_complete_config_variable_name in the Bash completion
>> script hardcodes several config variable names. These variables are
>> those in config section where user-defined names can appear, such as
>> "branch.<name>". These sections are treated first by the case statement,
>> and the two last "catch all" cases are used for other sections, making
>> use of the __git_compute_config_vars and __git_compute_config_sections
>> function, which omit listing any variables containing wildcards or
>> placeholders. Having hardcoded config variables introduces the risk of
>> the completion code becoming out of sync with the actual config
>> variables accepted by Git.
>>
>> To avoid these hardcoded config variables, introduce a new function,
>> __git_compute_first_level_config_vars_for_section, making use of the
>> existing __git_config_vars variable. This function takes as argument a
>> config section name and computes the matching "first level" config
>> variables for that section, i.e. those _not_ containing any placeholder,
>> like 'branch.autoSetupMerge, 'remote.pushDefault', etc. Use this
>> function and the variables it defines in the 'branch.*', 'remote.*' and
>> 'submodule.*' switches of the case statement instead of hardcoding the
>> corresponding config variables. Note that we use indirect expansion
>> instead of associative arrays because those are not supported in Bash 3,
>> on which macOS is stuck for licensing reasons.
>>
>> Add a test to make sure the new function works correctly by verfying it
>> lists all 'submodule' config variables. This has the downside that this
>> test must be updated when new 'submodule' configuration are added, but
>> this should be a small burden since it happens infrequently.
>>
>> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
>> ---
>> contrib/completion/git-completion.bash | 24 +++++++++++++++++++++---
>> t/t9902-completion.sh | 11 +++++++++++
>> 2 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 8af9bc3f4e1..2934ceb7637 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2596,6 +2596,15 @@ __git_compute_config_vars ()
>> __git_config_vars="$(git help --config-for-completion)"
>> }
>>
>> +__git_compute_first_level_config_vars_for_section ()
>> +{
>> + section="$1"
>
> Section needs to be `local`, right?
Good eyes, I'll fix that in v3.
>> + __git_compute_config_vars
>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>> + test -n "${!this_section}" ||
>> + printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>> +}
>
> I've been wondering a bit why we store the result in a global variable.
> The value certainly isn't reused in the completion scripts here. It took
> me quite some time to realize though that it's going to end up in the
> user's shell environment even after completion finishes so that it can
> be reused the next time we invoke the completion function.
>
> While this does feel a tad weird to me to be stateful like this across
> completion calls, we use the same pattern for `__git_config_vars` and
> `__git_config_sections`. So I guess it should be fine given that there
> is precedent.
Yes, I used this pattern because it was already used for other variables in
the script, __git_config_vars and __git_config_sections are some examples,
git grep 'test -n .* ||' contrib/completion/git-completion.bash
finds also others. I think the idea is to cache these lists to avoid
computing them everytime they are needed (probably most useful on Windows
where process creation is longer). I'll mention that in the
commit message.
>> __git_config_sections=
>> __git_compute_config_sections ()
>> {
>> @@ -2749,8 +2758,11 @@ __git_complete_config_variable_name ()
>> branch.*)
>> local pfx="${cur_%.*}."
>> cur_="${cur_#*.}"
>> + local section="${pfx%.}"
>> __gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
>> - __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" "${sfx:- }"
>> + __git_compute_first_level_config_vars_for_section "${section}"
>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>> + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
>> return
>> ;;
>> guitool.*.*)
>> @@ -2799,8 +2811,11 @@ __git_complete_config_variable_name ()
>> remote.*)
>> local pfx="${cur_%.*}."
>> cur_="${cur_#*.}"
>> + local section="${pfx%.}"
>> __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
>> - __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
>> + __git_compute_first_level_config_vars_for_section "${section}"
>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>> + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
>> return
>> ;;
>> submodule.*.*)
>> @@ -2812,8 +2827,11 @@ __git_complete_config_variable_name ()
>> submodule.*)
>> local pfx="${cur_%.*}."
>> cur_="${cur_#*.}"
>> + local section="${pfx%.}"
>> __gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
>> - __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
>> + __git_compute_first_level_config_vars_for_section "${section}"
>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>> + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
>> return
>> ;;
>> url.*.*)
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index 35eb534fdda..f28d8f531b7 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -2583,6 +2583,17 @@ test_expect_success 'git config - variable name include' '
>> EOF
>> '
>>
>> +test_expect_success 'git config - variable name - __git_compute_first_level_config_vars_for_section' '
>> + test_completion "git config submodule." <<-\EOF
>> + submodule.active Z
>> + submodule.alternateErrorStrategy Z
>> + submodule.alternateLocation Z
>> + submodule.fetchJobs Z
>> + submodule.propagateBranches Z
>> + submodule.recurse Z
>> + EOF
>> +'
>> +
>
> Shouldn't we verify that we know to complete both first-level config
> vars as well as the user-specified submodule names here?
Yes that would be more complete indeed, but it would then make more
sense to add that test in 2/5 since __git_compute_first_level_config_vars_for_section
is not involved in determining submodule names.
I'll make that change, thanks.
Philippe.
^ permalink raw reply
* Re: [PATCH v2 4/5] builtin/help: add --config-all-for-completion
From: Philippe Blain @ 2024-02-10 16:13 UTC (permalink / raw)
To: Patrick Steinhardt, Philippe Blain via GitGitGadget; +Cc: git
In-Reply-To: <ZcSF2mw-zR1d38UG@tanuki>
Le 2024-02-08 à 02:42, Patrick Steinhardt a écrit :
> On Mon, Jan 29, 2024 at 01:28:00PM +0000, Philippe Blain via GitGitGadget wrote:
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> There is currently no machine-friendly way to show _all_ configuration
>> variables from the command line. 'git help --config' does show them all,
>> but it also sets up the pager. 'git help --config-for-completion' omits
>> some variables (those containing wildcards, for example) and 'git help
>> --config-section-for-completion' shows only top-level section names.
>
> You can invoke `git --no-pager help --config` so that Git does not set
> up the pager. Is there a reason why we can't use that?
I'm glad to say there is no reason we can't use that! I just did not
think of it and dived straight into the C code. I'll use that in v3 and
just drop this patch from the series.
Thanks!
Philippe.
^ permalink raw reply
* Re: [PATCH v2 5/5] completion: add an use __git_compute_second_level_config_vars_for_section
From: Philippe Blain @ 2024-02-10 16:19 UTC (permalink / raw)
To: Patrick Steinhardt, Philippe Blain via GitGitGadget; +Cc: git
In-Reply-To: <ZcSF4fv_a16Ziwyy@tanuki>
Le 2024-02-08 à 02:42, Patrick Steinhardt a écrit :
> On Mon, Jan 29, 2024 at 01:28:01PM +0000, Philippe Blain via GitGitGadget wrote:
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
> [snip]
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 2934ceb7637..0e8fd63bfdb 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2605,6 +2612,15 @@ __git_compute_first_level_config_vars_for_section ()
>> printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>> }
>>
>> +__git_compute_second_level_config_vars_for_section ()
>> +{
>> + section="$1"
>
> This should be `local section`, as well.
Thanks, fixed.
> [snip]
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index f28d8f531b7..24ff786b273 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -2593,6 +2593,16 @@ test_expect_success 'git config - variable name - __git_compute_first_level_conf
>> submodule.recurse Z
>> EOF
>> '
>
> Missing a newline.
Fixed.
Thank you again for your review Patrick, much appreciated.
Philippe.
^ permalink raw reply
* Re: git status became very slow after upgrading git
From: Sean Allred @ 2024-02-10 17:06 UTC (permalink / raw)
To: Vijay Raghavan Aravamudhan; +Cc: git
In-Reply-To: <CAK7MZG1MeeS5QNPog9oS+MbdKpkDXu61eVOszsC20Q=ik+Ng=g@mail.gmail.com>
Vijay Raghavan Aravamudhan <avijayr@gmail.com> writes:
> What did you do before the bug happened? (Steps to reproduce your issue)
> 1. brew update which pulled in latest version of git
> 2. git status in a repository (without submodules)
>
> What did you expect to happen? (Expected behavior)
> git status should have been fast
>
> What happened instead? (Actual behavior)
> git status takes almost 5s to complete.
Thanks for the report. This isn't a whole lot of information to go on.
At least, I'm not able to reproduce locally with a trivial repository:
git init
echo foo > file
git add file
git commit -mtest
git status
If you're able to reproduce, can you re-run `git status` with tracing
enabled and provide your output?
GIT_TRACE=1 GIT_TRACE_SETUP=1 GIT_TRACE_PERFORMANCE=1 git status
If you can provide reproduction instructions that start with `git init`,
that would also help. It may take some time for you, but it'll take less
time than folks on this list taking shots in the dark :-)
--
Sean Allred
^ permalink raw reply
* Re: [PATCH v2 3/5] completion: add and use __git_compute_first_level_config_vars_for_section
From: Junio C Hamano @ 2024-02-10 17:15 UTC (permalink / raw)
To: Philippe Blain; +Cc: Patrick Steinhardt, Philippe Blain via GitGitGadget, git
In-Reply-To: <e8642ad8-bdc9-00d6-39b5-81dd399e60ec@gmail.com>
Philippe Blain <levraiphilippeblain@gmail.com> writes:
>>> + __git_compute_config_vars
>>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>>> + test -n "${!this_section}" ||
>>> + printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>>> +}
A silly question (primarily because I do not much use the indirect
reference construct ${!name}). Does the assignment with printf need
to spell out the long variable name with "_${section}"? Can it be
printf -v "$this_section" ...
instead, as we already have the short-hand for it?
> finds also others. I think the idea is to cache these lists to avoid
> computing them everytime they are needed (probably most useful on Windows
> where process creation is longer). I'll mention that in the
> commit message.
Yup, as long as the contents of the list stays stable (e.g., list of
Git subcommands, list of options a Git subcommand takes, list of
configuration variable names that do not have end-user customization
part, etc.), it is a viable optimization technique. The available
<slot> for color.branch.<slot> and color.diff.<slot> do not change
(unless you talk about new version of Git adding support for more
slots) and is a good idea to cache. remote.<name>.url takes its
<name> component out of an unbound set of end-user controlled names,
so unless we somehow have a method to invalidate cached values, the
list can go stale as remotes are added and removed.
Thanks.
^ permalink raw reply
* Re: [ANNOUNCE] Git for Windows 2.44.0-rc0
From: Junio C Hamano @ 2024-02-10 17:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git-for-windows, git, git-packagers
In-Reply-To: <20240210101922.3663-1-johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Dear Git users,
>
> I hereby announce that Git for Windows 2.44.0-rc0 is available from:
>
> https://github.com/git-for-windows/git/releases/tag/v2.44.0-rc0.windows.1
>
> Changes since Git for Windows v2.43.0 (November 20th 2023)
>
> Git for Windows will drop support for Windows 7 and for Windows 8 in
> the next versions, see the announcement of MSYS2 on which Git for
> Windows relies for components such as Bash and Perl.
>
> Please also note that the 32-bit variant of Git for Windows is
> deprecated; Its last official release is planned for 2025.
>
> New Features
>
> * Comes with Git v2.44.0-rc0.
> * Comes with gnupg v2.2.42.
> * Comes with libfido2 v1.14.0.
> * Comes with the MSYS2 runtime (Git for Windows flavor) based on
> Cygwin v3.4.10.
> * Comes with Perl v5.38.2.
> * Git for Windows learned to detect and use native Windows support
> for ANSI sequences, which allows using 24-bit colors in terminal
> windows.
> * Comes with Git LFS v3.4.1.
> * The repository viewer Tig that is included in Git for Windows can
> now be called also directly from PowerShell/CMD.
> * Comes with OpenSSH v9.6.P1.
> * Comes with Bash v5.2.26.
> * Comes with GNU TLS v3.8.3.
> * Comes with OpenSSL v3.2.1.
> * Comes with cURL v8.6.0.
> * Comes with GNU Privacy Guard v2.4.4.
Impressive set of updates. Thanks.
^ permalink raw reply
* Re: What's the recommendation for forgetting all rerere's records?
From: Sean Allred @ 2024-02-10 17:18 UTC (permalink / raw)
To: Sean Allred; +Cc: git, Junio C Hamano
In-Reply-To: <m05y0au2od.fsf@epic96565.epic.com>
Sean Allred <allred.sean@gmail.com> writes:
> I've driven myself a little nuts trying to reproduce it this morning,
> but in doing so I've come to an important discovery: this bug presents
> if `core.autocrlf=true` but does *not* present if `core.autocrlf=input`.
>
> For completeness and future reference, the following script reproduces
> the issue on Windows:
>
> [clip]
>
> At the end of this script, the 'bad merge' is still the recorded
> resolution and no rerere record exists for the 'good merge'.
>
> Just in case there's another piece of config somehow relevant, here's a
> dump of the system that reproduced this:
>
> [clip]
>
> It's worth noting at this point that while I believe I reproduced on
> macOS last week, that doesn't jive with the available evidence (and I
> can't reproduce it on macOS this morning, though I suspect that has more
> to do with native use of LF over CRLF than anything else).
Is there a good way to convert this to a proper bug report without
losing the context?
--
Sean Allred
^ permalink raw reply
* Re: Bug: Commit fails when no global email address is set even though --author is used
From: Junio C Hamano @ 2024-02-10 17:21 UTC (permalink / raw)
To: Marcus Tillmanns; +Cc: Kristoffer Haugsbakk, git@vger.kernel.org, Phillip Wood
In-Reply-To: <8DDFD548-1540-4E41-8AA3-84E273372F01@qt.io>
Marcus Tillmanns <Marcus.Tillmanns@qt.io> writes:
> For me the issue was that I knew already about the global config
> setting, but in my case I specifically did not want to set that.
>
> What pulled my attention away from the important part was the huge
> wall of text about how to setup global stuff etc, making the
> single word in the top left basically invisible to my eye. I was
> thinking "I already knew all that, I’ve set the author, so you
> complaining still with the same error message must be a bug”.
>
> I think, if a user tries to commit with “—author”, and git
> fails to figure out the comitter, it should have a specific error
> message about “hey, you’ve set the author, but we still have to
> figure out whom to set as the committer, or you can use
> “—authorAndComitter” if they should be the same”.
>
> That would make it obvious to the user what’s going on.
I guess so, provided that such an additional text would not trigger
the "I missed due to the huge wall of text" again. So in short, you
are saying that the three-way message would help?
Can I get you intereseted enough to care to come up with a patch
;-)?
Thanks.
>>>> Or do we need three-way switch that does
>>>>
>>>> if (neither is known) {
>>>> printf("neither author or committer is known");
>>>> } else if (author is known but committer is not known) {
>>>> printf("author is known but committer is not"):
>>>> } else if (author is not known but committer is known) {
>>>> printf("committer is known but author is not"):
>>>> } else {
>>>> return happy;
>>>> }
>>>>
>>>> printf("please tell us who you are...");
>>>>
>>>> perhaps?
^ permalink raw reply
* Re: [PATCH v2 3/5] completion: add and use __git_compute_first_level_config_vars_for_section
From: Philippe Blain @ 2024-02-10 17:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, Philippe Blain via GitGitGadget, git
In-Reply-To: <xmqqwmrcb5q8.fsf@gitster.g>
Hi Junio,
Le 2024-02-10 à 12:15, Junio C Hamano a écrit :
> Philippe Blain <levraiphilippeblain@gmail.com> writes:
>
>>>> + __git_compute_config_vars
>>>> + local this_section="__git_first_level_config_vars_for_section_${section}"
>>>> + test -n "${!this_section}" ||
>>>> + printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>>>> +}
>
> A silly question (primarily because I do not much use the indirect
> reference construct ${!name}). Does the assignment with printf need
> to spell out the long variable name with "_${section}"? Can it be
>
> printf -v "$this_section" ...
>
> instead, as we already have the short-hand for it?
No, unfortunately neither "$this_section" nor "${!this_section}"
work, so we must use the long name.
>
>> finds also others. I think the idea is to cache these lists to avoid
>> computing them everytime they are needed (probably most useful on Windows
>> where process creation is longer). I'll mention that in the
>> commit message.
>
> Yup, as long as the contents of the list stays stable (e.g., list of
> Git subcommands, list of options a Git subcommand takes, list of
> configuration variable names that do not have end-user customization
> part, etc.), it is a viable optimization technique. The available
> <slot> for color.branch.<slot> and color.diff.<slot> do not change
> (unless you talk about new version of Git adding support for more
> slots) and is a good idea to cache. remote.<name>.url takes its
> <name> component out of an unbound set of end-user controlled names,
> so unless we somehow have a method to invalidate cached values, the
> list can go stale as remotes are added and removed.
Indeed. Here I'm caching Git config variables, not any user-defined names,
so these are stable.
Thanks,
Philippe.
^ 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