* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Jeff King @ 2023-11-05 5:34 UTC (permalink / raw)
To: Eric Sunshine; +Cc: rsbecker, Git List
In-Reply-To: <CAPig+cTy7Mq1xhTtssoUDpwrCNB_65q4VjK902jOpJ4469_tLw@mail.gmail.com>
On Sat, Nov 04, 2023 at 08:11:01PM -0400, Eric Sunshine wrote:
> > Hmm. With compilation, we split the audience of "developers" vs "people
> > who just want to build the program", and we crank up the number and
> > severity of warning checks for the former. We could do the same here for
> > tests. I.e., turn off test linting by default and re-enable it for
> > DEVELOPER=1.
>
> My knee-jerk reaction is that this would move us in the wrong
> direction since it is probable that most drive-by contributors won't
> have DEVELOPER=1 set, yet they are the ones who are likely to benefit
> most from test script linting (which is not to say that it doesn't
> help seasoned contributors, as well).
Yeah, that's a good point. If the linting is not causing frequent
headaches (and I don't think it is), then we are better to leave it on
by default.
-Peff
^ permalink raw reply
* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Eric Sunshine @ 2023-11-05 0:11 UTC (permalink / raw)
To: Jeff King; +Cc: rsbecker, Git List
In-Reply-To: <20231104134915.GA1492953@coredump.intra.peff.net>
On Sat, Nov 4, 2023 at 9:49 AM Jeff King <peff@peff.net> wrote:
> On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:
> > I don't see an urgent need for it. Unlike the actual tests themselves
> > run by `make test` which may catch platform-specific problems in Git
> > itself, the purpose of the "linting" checks is not to catch
> > platform-specific problems, but rather to help test authors by
> > identifying mistakes in the tests which might make them fragile. So,
> > disabling linting on a particular platform isn't going to cause `make
> > test` to miss some important Git problem specific to that platform.
>
> Hmm. With compilation, we split the audience of "developers" vs "people
> who just want to build the program", and we crank up the number and
> severity of warning checks for the former. We could do the same here for
> tests. I.e., turn off test linting by default and re-enable it for
> DEVELOPER=1.
My knee-jerk reaction is that this would move us in the wrong
direction since it is probable that most drive-by contributors won't
have DEVELOPER=1 set, yet they are the ones who are likely to benefit
most from test script linting (which is not to say that it doesn't
help seasoned contributors, as well).
> OTOH, this is the first time I think I've seen the linting cause a
> problem (whereas unexpected compile warnings are much more likely, as we
> are depending on the system compiler's behavior).
There have been a few other times when the linting scripts have needed
a tweak or two to work properly on some less-well-represented
platform, but they were minor issues[1,2,3,4].
> So consider it an idle thought for discussion, and not necessarily a
> proposal. ;)
As noted in my response to Junio[5], in the long run, we may want to
go with Ævar's idea of having `make` track changes to the test
scripts, thus only run linting on an as-needed basis.
[1]: a3c4c8841c (tests: use shorter labels in chainlint.sed for AIX
sed, 2018-08-24)
[2]: 2d9ded8acc (tests: fix comment syntax in chainlint.sed for AIX
sed, 2018-08-24)
[3]: b3b753b104 (Fit to Plan 9's ANSI/POSIX compatibility layer, 2020-09-10)
[4]: 1f51b77f4f (chainlint.pl: fix /proc/cpuinfo regexp, 2022-11-22)
[5]: https://lore.kernel.org/git/CAPig+cSC8m5a8PhMw_eJbswwNB-VgBt+n56HSTLLabV9_+y--g@mail.gmail.com/
^ permalink raw reply
* [PATCH v3 1/2] rebase: support non-interactive autosquash
From: Andy Koppe @ 2023-11-05 0:08 UTC (permalink / raw)
To: git; +Cc: gitster, newren, Andy Koppe
In-Reply-To: <20231104220330.14577-1-andy.koppe@gmail.com>
So far, the rebase --autosquash option and rebase.autoSquash=true
config setting are quietly ignored when used without --interactive,
except that they prevent fast-forward and that they trigger conflicts
with --apply and relatives, which is less than helpful particularly for
the config setting.
Since the "merge" backend used for interactive rebase also is the
default for non-interactive rebase, there doesn't appear to be a
reason not to do --autosquash without --interactive, so support that.
Turn rebase.autoSquash into a comma-separated list of flags, with
"interactive" or "i" enabling auto-squashing with --interactive, and
"no-interactive" or "no-i" enabling it without. Make boolean true mean
"interactive" for backward compatibility.
Don't prevent fast-forwards or report conflicts with --apply options
when auto-squashing is not active.
Change the git-rebase and config/rebase documentation accordingly, and
extend t3415-rebase-autosquash.sh to test the new rebase.autosquash
values and combinations with and without --interactive.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/rebase.txt | 11 +++-
Documentation/git-rebase.txt | 2 +-
builtin/rebase.c | 66 +++++++++++++++-----
t/t3415-rebase-autosquash.sh | 83 +++++++++++++++++++++-----
t/t3422-rebase-incompatible-options.sh | 2 +-
5 files changed, 132 insertions(+), 32 deletions(-)
diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index 9c248accec..68191e5673 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -9,7 +9,16 @@ rebase.stat::
rebase. False by default.
rebase.autoSquash::
- If set to true enable `--autosquash` option by default.
+ A comma-separated list of flags for when to enable auto-squashing.
+ Specifying `interactive` or `i` enables auto-squashing for rebasing with
+ `--interactive`, whereas `no-interactive` or `no-i` enables it for
+ rebasing without that option. For example, setting this to `i,no-i`
+ enables auto-squashing for both types. Setting it to true is equivalent
+ to setting it to `interactive`.
+
+ The `--autosquash` and `--no-autosquash` options of
+ linkgit:git-rebase[1] override the setting here.
+ Auto-squashing is disabled by default.
rebase.autoStash::
When set to true, automatically create a temporary stash entry
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..102ff91493 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -592,7 +592,7 @@ See also INCOMPATIBLE OPTIONS below.
When the commit log message begins with "squash! ..." or "fixup! ..."
or "amend! ...", and there is already a commit in the todo list that
matches the same `...`, automatically modify the todo list of
- `rebase -i`, so that the commit marked for squashing comes right after
+ `rebase`, so that the commit marked for squashing comes right after
the commit to be modified, and change the action of the moved commit
from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
matches the `...` if the commit subject matches, or if the `...` refers
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 261a9a61fc..26c3e5dcb4 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -131,7 +131,10 @@ struct rebase_options {
int reapply_cherry_picks;
int fork_point;
int update_refs;
- int config_autosquash;
+ enum {
+ AUTOSQUASH_INTERACTIVE = 1 << 0,
+ AUTOSQUASH_NO_INTERACTIVE = 1 << 1,
+ } config_autosquash;
int config_rebase_merges;
int config_update_refs;
};
@@ -149,7 +152,6 @@ struct rebase_options {
.reapply_cherry_picks = -1, \
.allow_empty_message = 1, \
.autosquash = -1, \
- .config_autosquash = -1, \
.rebase_merges = -1, \
.config_rebase_merges = -1, \
.update_refs = -1, \
@@ -711,10 +713,8 @@ static int run_specific_rebase(struct rebase_options *opts)
if (opts->type == REBASE_MERGE) {
/* Run sequencer-based rebase */
setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
- if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+ if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT))
setenv("GIT_SEQUENCE_EDITOR", ":", 1);
- opts->autosquash = 0;
- }
if (opts->gpg_sign_opt) {
/* remove the leading "-S" */
char *tmp = xstrdup(opts->gpg_sign_opt + 2);
@@ -748,6 +748,30 @@ static int run_specific_rebase(struct rebase_options *opts)
return status ? -1 : 0;
}
+static void parse_rebase_autosquash_value(struct rebase_options *opts,
+ const char *var, const char *value)
+{
+ struct string_list tokens = STRING_LIST_INIT_NODUP;
+ char *buf = xstrdup(value);
+
+ opts->config_autosquash = 0;
+ string_list_split_in_place(&tokens, buf, ",", -1);
+
+ for (int i = 0; i < tokens.nr; i++) {
+ const char *s = tokens.items[i].string;
+
+ if (!strcmp(s, "i") || !strcmp(s, "interactive"))
+ opts->config_autosquash |= AUTOSQUASH_INTERACTIVE;
+ else if (!strcmp(s, "no-i") || !strcmp(s, "no-interactive"))
+ opts->config_autosquash |= AUTOSQUASH_NO_INTERACTIVE;
+ else
+ die(_("invalid value for '%s': '%s'"), var, s);
+ }
+
+ string_list_clear(&tokens, 0);
+ free(buf);
+}
+
static void parse_rebase_merges_value(struct rebase_options *options, const char *value)
{
if (!strcmp("no-rebase-cousins", value))
@@ -772,8 +796,14 @@ static int rebase_config(const char *var, const char *value,
}
if (!strcmp(var, "rebase.autosquash")) {
- opts->config_autosquash = git_config_bool(var, value);
- return 0;
+ int b = git_parse_maybe_bool(value);
+
+ if (b < 0)
+ parse_rebase_autosquash_value(opts, var, value);
+ else if (b)
+ opts->config_autosquash = AUTOSQUASH_INTERACTIVE;
+ else
+ opts->config_autosquash = 0;
}
if (!strcmp(var, "commit.gpgsign")) {
@@ -1402,13 +1432,23 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
state_dir_base, cmd_live_rebase, buf.buf);
}
+ if (options.autosquash == 1) {
+ imply_merge(&options, "--autosquash");
+ } else if (options.autosquash == -1) {
+ int conf = options.config_autosquash;
+ options.autosquash =
+ (options.flags & REBASE_INTERACTIVE_EXPLICIT)
+ ? !!(conf & AUTOSQUASH_INTERACTIVE)
+ : !!(conf & AUTOSQUASH_NO_INTERACTIVE);
+ }
+
if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
(options.action != ACTION_NONE) ||
(options.exec.nr > 0) ||
- (options.autosquash == -1 && options.config_autosquash == 1) ||
- options.autosquash == 1) {
+ options.autosquash) {
allow_preemptive_ff = 0;
}
+
if (options.committer_date_is_author_date || options.ignore_date)
options.flags |= REBASE_FORCE;
@@ -1508,7 +1548,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (is_merge(&options))
die(_("apply options and merge options "
"cannot be used together"));
- else if (options.autosquash == -1 && options.config_autosquash == 1)
+ else if (options.autosquash && options.config_autosquash)
die(_("apply options are incompatible with rebase.autoSquash. Consider adding --no-autosquash"));
else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
die(_("apply options are incompatible with rebase.rebaseMerges. Consider adding --no-rebase-merges"));
@@ -1529,11 +1569,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
- if (options.autosquash == 1)
- imply_merge(&options, "--autosquash");
- options.autosquash = (options.autosquash >= 0) ? options.autosquash :
- ((options.config_autosquash >= 0) ? options.config_autosquash : 0);
-
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
options.type = REBASE_MERGE;
@@ -1858,3 +1893,4 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(keep_base_onto_name);
return !!ret;
}
+
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index a364530d76..1386eb6394 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -43,7 +43,7 @@ test_auto_fixup () {
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -61,15 +61,43 @@ test_auto_fixup () {
}
test_expect_success 'auto fixup (option)' '
- test_auto_fixup final-fixup-option --autosquash
+ test_auto_fixup fixup-option --autosquash &&
+ test_auto_fixup fixup-option-i "-i --autosquash"
'
-test_expect_success 'auto fixup (config)' '
- git config rebase.autosquash true &&
- test_auto_fixup final-fixup-config-true &&
- test_auto_fixup ! fixup-config-true-no --no-autosquash &&
+test_expect_success 'auto fixup (config false)' '
git config rebase.autosquash false &&
- test_auto_fixup ! final-fixup-config-false
+ test_auto_fixup ! fixup-config-false &&
+ test_auto_fixup ! fixup-config-false-i -i
+'
+
+test_expect_success 'auto fixup (config true)' '
+ git config rebase.autosquash true &&
+ test_auto_fixup ! fixup-config-true &&
+ test_auto_fixup fixup-config-true-i -i &&
+ test_auto_fixup ! fixup-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto fixup (config interactive)' '
+ git config rebase.autosquash interactive &&
+ test_auto_fixup ! fixup-config-interactive &&
+ test_auto_fixup fixup-config-interactive-i -i &&
+ test_auto_fixup ! fixup-config-interactive-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto fixup (config no-interactive)' '
+ git config rebase.autosquash no-interactive &&
+ test_auto_fixup fixup-config-no-interactive &&
+ test_auto_fixup ! fixup-config-no-interactive-i -i &&
+ test_auto_fixup ! fixup-config-no-interactive-no "--no-autosquash"
+'
+
+test_expect_success 'auto fixup (config always)' '
+ git config rebase.autosquash i,no-i &&
+ test_auto_fixup fixup-config-always &&
+ test_auto_fixup fixup-config-always-i -i &&
+ test_auto_fixup ! fixup-config-always-no --no-autosquash &&
+ test_auto_fixup ! fixup-config-always-i-no "-i --no-autosquash"
'
test_auto_squash () {
@@ -87,7 +115,7 @@ test_auto_squash () {
git commit -m "squash! first" -m "extra para for first" &&
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -105,15 +133,42 @@ test_auto_squash () {
}
test_expect_success 'auto squash (option)' '
- test_auto_squash final-squash --autosquash
+ test_auto_squash squash-option --autosquash &&
+ test_auto_squash squash-option-i "-i --autosquash"
'
-test_expect_success 'auto squash (config)' '
- git config rebase.autosquash true &&
- test_auto_squash final-squash-config-true &&
- test_auto_squash ! squash-config-true-no --no-autosquash &&
+test_expect_success 'auto squash (config false)' '
git config rebase.autosquash false &&
- test_auto_squash ! final-squash-config-false
+ test_auto_squash ! squash-config-false &&
+ test_auto_squash ! squash-config-false-i -i
+'
+
+test_expect_success 'auto squash (config true)' '
+ git config rebase.autosquash true &&
+ test_auto_squash ! squash-config-true &&
+ test_auto_squash squash-config-true-i -i &&
+ test_auto_squash ! squash-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto squash (config interactive)' '
+ git config rebase.autosquash i &&
+ test_auto_squash ! squash-config-interactive &&
+ test_auto_squash squash-config-interactive-i -i &&
+ test_auto_squash ! squash-config-interactive-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto squash (config no-interactive)' '
+ git config rebase.autosquash no-i &&
+ test_auto_squash squash-config-no-interactive &&
+ test_auto_squash ! squash-config-no-interactive-i -i &&
+ test_auto_squash ! squash-config-no-interactive-no "--no-autosquash"
+'
+test_expect_success 'auto squash (config always)' '
+ git config rebase.autosquash interactive,no-interactive &&
+ test_auto_squash squash-config-always &&
+ test_auto_squash squash-config-always-i -i &&
+ test_auto_squash ! squash-config-always-no --no-autosquash &&
+ test_auto_squash ! squash-config-always-i-no "-i --no-autosquash"
'
test_expect_success 'misspelled auto squash' '
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 2eba00bdf5..e5119e7371 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -102,7 +102,7 @@ test_rebase_am_only () {
test_expect_success "$opt incompatible with rebase.autosquash" "
git checkout B^0 &&
- test_must_fail git -c rebase.autosquash=true rebase $opt A 2>err &&
+ test_must_fail git -c rebase.autosquash=no-i rebase $opt A 2>err &&
grep -e --no-autosquash err
"
--
2.43.0-rc0
^ permalink raw reply related
* [PATCH v3 2/2] docs: rewrite rebase --(no-)autosquash description
From: Andy Koppe @ 2023-11-05 0:08 UTC (permalink / raw)
To: git; +Cc: gitster, newren, Andy Koppe
In-Reply-To: <20231105000808.10171-1-andy.koppe@gmail.com>
Rewrite the description of the rebase --(no-)autosquash options to try
to make it a bit clearer. Don't use "the '...'" to refer to part of a
commit message, mention how --interactive can be used to review the
todo list, and add a bit more detail on commit --squash/amend.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/git-rebase.txt | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 102ff91493..594158fcbc 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -589,21 +589,25 @@ See also INCOMPATIBLE OPTIONS below.
--autosquash::
--no-autosquash::
- When the commit log message begins with "squash! ..." or "fixup! ..."
- or "amend! ...", and there is already a commit in the todo list that
- matches the same `...`, automatically modify the todo list of
- `rebase`, so that the commit marked for squashing comes right after
- the commit to be modified, and change the action of the moved commit
- from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
- matches the `...` if the commit subject matches, or if the `...` refers
- to the commit's hash. As a fall-back, partial matches of the commit
- subject work, too. The recommended way to create fixup/amend/squash
- commits is by using the `--fixup`, `--fixup=amend:` or `--fixup=reword:`
- and `--squash` options respectively of linkgit:git-commit[1].
+ Automatically squash commits with specially formatted messages into
+ previous commits. If a commit message starts with "squash! ",
+ "fixup! " or "amend! ", the remainder of the subject line is taken
+ as a commit specifier, which matches a previous commit if it matches
+ the start of the subject line or the hash of that commit.
+
-If the `--autosquash` option is enabled by default using the
-configuration variable `rebase.autoSquash`, this option can be
-used to override and disable this setting.
+In the rebase todo list, commits marked for squashing are moved right after
+the commits they modify, and their action is changed from `pick` to `squash`,
+`fixup` or `fixup -C`, depending on the squash marker. The `--interactive`
+option can be used to review and edit the todo list before proceeding.
++
+The recommended way to create commits with squash markers is by using the
+`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
+linkgit:git-commit[1], which take the target commit as an argument and
+automatically fill in the subject line of the new commit from that.
++
+The configuration variable `rebase.autoSquash` can be used to enable
+`--autosquash` by default. See the CONFIGURATION section below for details.
+The `--no-autosquash` option overrides that setting.
+
See also INCOMPATIBLE OPTIONS below.
--
2.43.0-rc0
^ permalink raw reply related
* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Eric Sunshine @ 2023-11-04 23:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: rsbecker, Git List
In-Reply-To: <xmqq8r7egd8u.fsf@gitster.g>
On Sat, Nov 4, 2023 at 3:59 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > Also, in the longer term, as you suggested, `xargs` is likely a more
> > fruitful solution.
>
> Hmph, the list of our test scripts exceed command line limit? That
> sounds a bit nasty, as we somehow need to prepare a pipe and feed
> them into it, in order to drive xargs downstream of the pipe.
>
> Ideally if there were a GNUMake function that slices a list into
> sublists of "reasonable" lengths, we could use it to directly drive
> N invocations of check-non-portable-shell script instead of xargs,
> but I didn't find one. Here is I came up with, using foreach that
> is "slice the list into many sublists of 1 element", but it made me
> feel dirty.
Indeed, that's ugly. I hadn't even put any thought into it since there
doesn't seem to be a pressing need for it.
In the long run, Ævar's idea of having `make` notice which, if any,
test scripts have changed, and only perform linting on an as-needed
basis may be the way to go[1,2,3], thus only passing a single script
as argument to each of the linters.
[1]: https://lore.kernel.org/git/220901.86bkrzjm6e.gmgdl@evledraar.gmail.com/
[2]: https://lore.kernel.org/git/221122.86cz9fbyln.gmgdl@evledraar.gmail.com/
[3]: https://github.com/avar/git/commits/avar/t-Makefile-break-T-to-file-association
^ permalink raw reply
* Re: BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
From: Kache Hit @ 2023-11-04 22:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <CAC7ZvyaQpYiVAszu_Oe5UoKgpe48dRJ8i1O8hLNOSo3UXfPVug@mail.gmail.com>
Just reporting that I ran into this again, this time with some info to
help repro, though the issue may be fixed already.
I managed to avoid it by turning off core.splitIndex, so I'd suspected
the setting conflicts with feature.manyFiles.
It could very also be/instead conflict with fsmonitor that I also use,
as mentioned in the similar/related thread:
https://public-inbox.org/git/xmqqbkhv6dw3.fsf@gitster.g/T/#m13a5ad383f040bb3a6be7641bd04aa20424a274c
Which references a splitindex & fsmonitor bug that's since been
addressed since 2.41:
https://github.com/git/git/commit/3704fed5eae8ca2fa20bcf6adb277ee83b012ce0
On Mon, Aug 22, 2022 at 9:53 PM Kache Hit <kache.hit@gmail.com> wrote:
>
> Hi,
>
> I've not been able to successfully repro this after managing to
> recover from it by rebuilding the index:
> https://stackoverflow.com/questions/73044253
>
> I'm sorry I couldn't be more helpful.
>
> On Fri, Jul 29, 2022 at 8:59 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Hi Kache,
> >
> > On Tue, 19 Jul 2022, Kache Hit wrote:
> >
> > > A thought: the 179457 is reminiscent of something else I did just before this:
> > >
> > > I was doing some "code archeology" and was headlessly checking out
> > > some old SHAs in this large monorepo.
> > > During checkout, it said it was updating 174823 files in total.
> >
> > Do you think it would be possible to whittle this down a bit, and maybe
> > attempt to come up with a reproducible example? Something like what is
> > described in https://stackoverflow.com/help/mcve.
> >
> > If all else fails, and you _only_ manage to reproduce it in the original
> > repository, could you at least try to figure out a reliable way to get the
> > Git index into the indicated state (if I were you, I would start off by
> > switching to the pre-rebase revision, deleting `.git/index` and then
> > running `git reset --hard` and then see whether the bug can be
> > reproduced)?
> >
> > Ciao,
> > Johannes
> >
> > >
> > > On Tue, Jul 19, 2022 at 2:36 PM Kache Hit <kache.hit@gmail.com> wrote:
> > > >
> > > > Hi. Output of git bugreport:
> > > >
> > > > ---
> > > >
> > > > Thank you for filling out a Git bug report!
> > > > Please answer the following questions to help us understand your issue.
> > > >
> > > > What did you do before the bug happened? (Steps to reproduce your issue)
> > > >
> > > > Wanted to retain git tree structure when pulling latest and rebasing.
> > > > First indication of error was the `rebase -r` of the merge commit
> > > >
> > > > What did you expect to happen? (Expected behavior)
> > > >
> > > > successful --rebase-merges rebase of my commits on top of master
> > > >
> > > > What happened instead? (Actual behavior)
> > > >
> > > > ```sh
> > > > ❯ git rebase -r master
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > > zsh: abort git rebase -r master
> > > > ```
> > > >
> > > > What's different between what you expected and what actually happened?
> > > >
> > > > Anything else you want to add:
> > > >
> > > > I'm currently "stuck" in this state, not sure how to recover or repro:
> > > >
> > > > ```sh
> > > > ❯ git s
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > > error: git died of signal 6
> > > >
> > > > ❯ git log
> > > >
> > > > ❯ git d head~
> > > > error: git died of signal 6
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > >
> > > > ❯ git log # works
> > > >
> > > > ❯ git status
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > > zsh: abort git status
> > > >
> > > > ❯ git commit --amend
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > > zsh: abort git commit --amend
> > > >
> > > > ❯ git checkout head
> > > > fatal: Unable to create '/Users/XXXXX/YYYYY/.git/index.lock': File exists.
> > > >
> > > > Another git process seems to be running in this repository, e.g. #
> > > > All of this was run while git bugreport was running
> > > > an editor opened by 'git commit'. Please make sure all processes
> > > > are terminated then try again. If it still fails, a git process
> > > > may have crashed in this repository earlier:
> > > > remove the file manually to continue.
> > > >
> > > > ❯ rm /Users/XXXXX/YYYYY/.git/index.lock
> > > >
> > > > ❯ git checkout head
> > > > BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index
> > > > (179457 > 1040)
> > > > zsh: abort git checkout head
> > > >
> > > > ❯ git checkout head
> > > > fatal: Unable to create '/Users/XXXXX/YYYYY/.git/index.lock': File exists.
> > > >
> > > > Another git process seems to be running in this repository, e.g.
> > > > an editor opened by 'git commit'. Please make sure all processes
> > > > are terminated then try again. If it still fails, a git process
> > > > may have crashed in this repository earlier:
> > > > remove the file manually to continue.
> > > > ```
> > > >
> > > >
> > > > Please review the rest of the bug report below.
> > > > You can delete any lines you don't wish to share.
> > > >
> > > >
> > > > [System Info]
> > > > git version:
> > > > git version 2.37.1
> > > > cpu: x86_64
> > > > no commit associated with this build
> > > > sizeof-long: 8
> > > > sizeof-size_t: 8
> > > > shell-path: /bin/sh
> > > > feature: fsmonitor--daemon
> > > > uname: Darwin 20.6.0 Darwin Kernel Version 20.6.0: Tue Feb 22 21:10:41
> > > > PST 2022; root:xnu-7195.141.26~1/RELEASE_X86_64 x86_64
> > > > compiler info: clang: 13.0.0 (clang-1300.0.29.30)
> > > > libc info: no libc information available
> > > > $SHELL (typically, interactive shell): /bin/zsh
> > > >
> > > >
> > > > [Enabled Hooks]
> > > > pre-commit
> > > > pre-push
> > >
^ permalink raw reply
* Re: [PATCH 1/2] rebase: support non-interactive autosquash
From: Andy Koppe @ 2023-11-04 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, newren
In-Reply-To: <xmqqil6iiacq.fsf@gitster.g>
On 04/11/2023 01:19, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
>
>> rebase.autoSquash::
>> - If set to true enable `--autosquash` option by default.
>> + When set to 'interactive' or 'true', enable the `--autosquash` option
>> + for interactive rebase. When set to 'always', enable it for
>> + non-interactive rebase as well. Defaults to 'false'.
>
> I think a better and more extensible way to coax the new feature
> into the configuration system is to arrange it more like so:
>
> false - synonym for "".
> true - synonym for "interactive".
> anything else - comman separated list of rebase methods, e.g.,
> "interactive,noninteractive"
>
> possible rebase method names might include other
> stuff like "apply" or "merge", but I haven't
> thought it through, so take this part with a
> grain of salt.
>
> That way, the Boolean versions can be considered historical spelling
> of a more general system where you can exactly tell when autosquash
> takes place. When we add to a new variant on top of 'interactive'
> and 'non-interactive' variants the current rebase has, we do not
> know if it makes sense to allow it to also handle autosquash without
> knowing how that new variant's behavior appears to the end user, so
> 'always' that blindly enables autosquash for any unforseen future
> variants of 'rebase' is probably not what you want.
Thanks, done in v2.
Andy
^ permalink raw reply
* [PATCH v2 2/2] docs: rewrite rebase --(no-)autosquash description
From: Andy Koppe @ 2023-11-04 22:03 UTC (permalink / raw)
To: git; +Cc: gitster, newren, Andy Koppe
In-Reply-To: <20231104220330.14577-1-andy.koppe@gmail.com>
Rewrite the description of the rebase --(no-)autosquash options to try
to make it a bit clearer. Don't use "the '...'" to refer to part of a
commit message, mention how --interactive can be used to review the
todo list, and add a bit more detail on commit --squash/amend.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/git-rebase.txt | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 102ff91493..594158fcbc 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -589,21 +589,25 @@ See also INCOMPATIBLE OPTIONS below.
--autosquash::
--no-autosquash::
- When the commit log message begins with "squash! ..." or "fixup! ..."
- or "amend! ...", and there is already a commit in the todo list that
- matches the same `...`, automatically modify the todo list of
- `rebase`, so that the commit marked for squashing comes right after
- the commit to be modified, and change the action of the moved commit
- from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
- matches the `...` if the commit subject matches, or if the `...` refers
- to the commit's hash. As a fall-back, partial matches of the commit
- subject work, too. The recommended way to create fixup/amend/squash
- commits is by using the `--fixup`, `--fixup=amend:` or `--fixup=reword:`
- and `--squash` options respectively of linkgit:git-commit[1].
+ Automatically squash commits with specially formatted messages into
+ previous commits. If a commit message starts with "squash! ",
+ "fixup! " or "amend! ", the remainder of the subject line is taken
+ as a commit specifier, which matches a previous commit if it matches
+ the start of the subject line or the hash of that commit.
+
-If the `--autosquash` option is enabled by default using the
-configuration variable `rebase.autoSquash`, this option can be
-used to override and disable this setting.
+In the rebase todo list, commits marked for squashing are moved right after
+the commits they modify, and their action is changed from `pick` to `squash`,
+`fixup` or `fixup -C`, depending on the squash marker. The `--interactive`
+option can be used to review and edit the todo list before proceeding.
++
+The recommended way to create commits with squash markers is by using the
+`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
+linkgit:git-commit[1], which take the target commit as an argument and
+automatically fill in the subject line of the new commit from that.
++
+The configuration variable `rebase.autoSquash` can be used to enable
+`--autosquash` by default. See the CONFIGURATION section below for details.
+The `--no-autosquash` option overrides that setting.
+
See also INCOMPATIBLE OPTIONS below.
--
2.43.0-rc0
^ permalink raw reply related
* [PATCH v2 1/2] rebase: support non-interactive autosquash
From: Andy Koppe @ 2023-11-04 22:03 UTC (permalink / raw)
To: git; +Cc: gitster, newren, Andy Koppe
In-Reply-To: <20231103212958.18472-1-andy.koppe@gmail.com>
So far, the rebase --autosquash option and rebase.autoSquash=true
config setting are quietly ignored when used without --interactive,
except that they prevent fast-forward and that they trigger conflicts
with --apply and relatives, which is less than helpful particularly for
the config setting.
Since the "merge" backend used for interactive rebase also is the
default for non-interactive rebase, there doesn't appear to be a
reason not to do --autosquash without --interactive, so support that.
Turn rebase.autoSquash into a comma-separated list of flags, with
"interactive" or "i" enabling auto-squashing with --interactive, and
"no-interactive" or "no-i" enabling it without. Make boolean true mean
"interactive" for backward compatibility.
Don't prevent fast-forwards or report conflicts with --apply options
when auto-squashing is not active.
Change the git-rebase and config/rebase documentation accordingly, and
extend t3415-rebase-autosquash.sh to test the new rebase.autosquash
values and combinations with and without --interactive.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/rebase.txt | 11 +++-
Documentation/git-rebase.txt | 2 +-
builtin/rebase.c | 63 ++++++++++++++-----
t/t3415-rebase-autosquash.sh | 83 +++++++++++++++++++++-----
t/t3422-rebase-incompatible-options.sh | 2 +-
5 files changed, 129 insertions(+), 32 deletions(-)
diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index 9c248accec..68191e5673 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -9,7 +9,16 @@ rebase.stat::
rebase. False by default.
rebase.autoSquash::
- If set to true enable `--autosquash` option by default.
+ A comma-separated list of flags for when to enable auto-squashing.
+ Specifying `interactive` or `i` enables auto-squashing for rebasing with
+ `--interactive`, whereas `no-interactive` or `no-i` enables it for
+ rebasing without that option. For example, setting this to `i,no-i`
+ enables auto-squashing for both types. Setting it to true is equivalent
+ to setting it to `interactive`.
+
+ The `--autosquash` and `--no-autosquash` options of
+ linkgit:git-rebase[1] override the setting here.
+ Auto-squashing is disabled by default.
rebase.autoStash::
When set to true, automatically create a temporary stash entry
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..102ff91493 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -592,7 +592,7 @@ See also INCOMPATIBLE OPTIONS below.
When the commit log message begins with "squash! ..." or "fixup! ..."
or "amend! ...", and there is already a commit in the todo list that
matches the same `...`, automatically modify the todo list of
- `rebase -i`, so that the commit marked for squashing comes right after
+ `rebase`, so that the commit marked for squashing comes right after
the commit to be modified, and change the action of the moved commit
from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
matches the `...` if the commit subject matches, or if the `...` refers
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 261a9a61fc..0403c7415c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -131,7 +131,10 @@ struct rebase_options {
int reapply_cherry_picks;
int fork_point;
int update_refs;
- int config_autosquash;
+ enum {
+ AUTOSQUASH_INTERACTIVE = 1 << 0,
+ AUTOSQUASH_NO_INTERACTIVE = 1 << 1,
+ } config_autosquash;
int config_rebase_merges;
int config_update_refs;
};
@@ -149,7 +152,6 @@ struct rebase_options {
.reapply_cherry_picks = -1, \
.allow_empty_message = 1, \
.autosquash = -1, \
- .config_autosquash = -1, \
.rebase_merges = -1, \
.config_rebase_merges = -1, \
.update_refs = -1, \
@@ -711,10 +713,8 @@ static int run_specific_rebase(struct rebase_options *opts)
if (opts->type == REBASE_MERGE) {
/* Run sequencer-based rebase */
setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
- if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+ if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT))
setenv("GIT_SEQUENCE_EDITOR", ":", 1);
- opts->autosquash = 0;
- }
if (opts->gpg_sign_opt) {
/* remove the leading "-S" */
char *tmp = xstrdup(opts->gpg_sign_opt + 2);
@@ -748,6 +748,27 @@ static int run_specific_rebase(struct rebase_options *opts)
return status ? -1 : 0;
}
+static void parse_rebase_autosquash_value(struct rebase_options *opts,
+ const char *var, const char *value)
+{
+ struct string_list tokens = STRING_LIST_INIT_NODUP;
+ char *buf = xstrdup(value);
+
+ opts->config_autosquash = 0;
+ string_list_split_in_place(&tokens, buf, ",", -1);
+
+ for (int i = 0; i < tokens.nr; i++) {
+ const char *s = tokens.items[i].string;
+
+ if (!strcmp(s, "i") || !strcmp(s, "interactive"))
+ opts->config_autosquash |= AUTOSQUASH_INTERACTIVE;
+ else if (!strcmp(s, "no-i") || !strcmp(s, "no-interactive"))
+ opts->config_autosquash |= AUTOSQUASH_NO_INTERACTIVE;
+ else
+ die(_("invalid value for '%s': '%s'"), var, s);
+ }
+}
+
static void parse_rebase_merges_value(struct rebase_options *options, const char *value)
{
if (!strcmp("no-rebase-cousins", value))
@@ -772,8 +793,14 @@ static int rebase_config(const char *var, const char *value,
}
if (!strcmp(var, "rebase.autosquash")) {
- opts->config_autosquash = git_config_bool(var, value);
- return 0;
+ int b = git_parse_maybe_bool(value);
+
+ if (b < 0)
+ parse_rebase_autosquash_value(opts, var, value);
+ else if (b)
+ opts->config_autosquash = AUTOSQUASH_INTERACTIVE;
+ else
+ opts->config_autosquash = 0;
}
if (!strcmp(var, "commit.gpgsign")) {
@@ -1402,13 +1429,23 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
state_dir_base, cmd_live_rebase, buf.buf);
}
+ if (options.autosquash == 1) {
+ imply_merge(&options, "--autosquash");
+ } else if (options.autosquash == -1) {
+ int conf = options.config_autosquash;
+ options.autosquash =
+ (options.flags & REBASE_INTERACTIVE_EXPLICIT)
+ ? !!(conf & AUTOSQUASH_INTERACTIVE)
+ : !!(conf & AUTOSQUASH_NO_INTERACTIVE);
+ }
+
if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
(options.action != ACTION_NONE) ||
(options.exec.nr > 0) ||
- (options.autosquash == -1 && options.config_autosquash == 1) ||
- options.autosquash == 1) {
+ options.autosquash) {
allow_preemptive_ff = 0;
}
+
if (options.committer_date_is_author_date || options.ignore_date)
options.flags |= REBASE_FORCE;
@@ -1508,7 +1545,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (is_merge(&options))
die(_("apply options and merge options "
"cannot be used together"));
- else if (options.autosquash == -1 && options.config_autosquash == 1)
+ else if (options.autosquash && options.config_autosquash)
die(_("apply options are incompatible with rebase.autoSquash. Consider adding --no-autosquash"));
else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
die(_("apply options are incompatible with rebase.rebaseMerges. Consider adding --no-rebase-merges"));
@@ -1529,11 +1566,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
- if (options.autosquash == 1)
- imply_merge(&options, "--autosquash");
- options.autosquash = (options.autosquash >= 0) ? options.autosquash :
- ((options.config_autosquash >= 0) ? options.config_autosquash : 0);
-
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
options.type = REBASE_MERGE;
@@ -1858,3 +1890,4 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(keep_base_onto_name);
return !!ret;
}
+
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index a364530d76..1386eb6394 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -43,7 +43,7 @@ test_auto_fixup () {
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -61,15 +61,43 @@ test_auto_fixup () {
}
test_expect_success 'auto fixup (option)' '
- test_auto_fixup final-fixup-option --autosquash
+ test_auto_fixup fixup-option --autosquash &&
+ test_auto_fixup fixup-option-i "-i --autosquash"
'
-test_expect_success 'auto fixup (config)' '
- git config rebase.autosquash true &&
- test_auto_fixup final-fixup-config-true &&
- test_auto_fixup ! fixup-config-true-no --no-autosquash &&
+test_expect_success 'auto fixup (config false)' '
git config rebase.autosquash false &&
- test_auto_fixup ! final-fixup-config-false
+ test_auto_fixup ! fixup-config-false &&
+ test_auto_fixup ! fixup-config-false-i -i
+'
+
+test_expect_success 'auto fixup (config true)' '
+ git config rebase.autosquash true &&
+ test_auto_fixup ! fixup-config-true &&
+ test_auto_fixup fixup-config-true-i -i &&
+ test_auto_fixup ! fixup-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto fixup (config interactive)' '
+ git config rebase.autosquash interactive &&
+ test_auto_fixup ! fixup-config-interactive &&
+ test_auto_fixup fixup-config-interactive-i -i &&
+ test_auto_fixup ! fixup-config-interactive-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto fixup (config no-interactive)' '
+ git config rebase.autosquash no-interactive &&
+ test_auto_fixup fixup-config-no-interactive &&
+ test_auto_fixup ! fixup-config-no-interactive-i -i &&
+ test_auto_fixup ! fixup-config-no-interactive-no "--no-autosquash"
+'
+
+test_expect_success 'auto fixup (config always)' '
+ git config rebase.autosquash i,no-i &&
+ test_auto_fixup fixup-config-always &&
+ test_auto_fixup fixup-config-always-i -i &&
+ test_auto_fixup ! fixup-config-always-no --no-autosquash &&
+ test_auto_fixup ! fixup-config-always-i-no "-i --no-autosquash"
'
test_auto_squash () {
@@ -87,7 +115,7 @@ test_auto_squash () {
git commit -m "squash! first" -m "extra para for first" &&
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -105,15 +133,42 @@ test_auto_squash () {
}
test_expect_success 'auto squash (option)' '
- test_auto_squash final-squash --autosquash
+ test_auto_squash squash-option --autosquash &&
+ test_auto_squash squash-option-i "-i --autosquash"
'
-test_expect_success 'auto squash (config)' '
- git config rebase.autosquash true &&
- test_auto_squash final-squash-config-true &&
- test_auto_squash ! squash-config-true-no --no-autosquash &&
+test_expect_success 'auto squash (config false)' '
git config rebase.autosquash false &&
- test_auto_squash ! final-squash-config-false
+ test_auto_squash ! squash-config-false &&
+ test_auto_squash ! squash-config-false-i -i
+'
+
+test_expect_success 'auto squash (config true)' '
+ git config rebase.autosquash true &&
+ test_auto_squash ! squash-config-true &&
+ test_auto_squash squash-config-true-i -i &&
+ test_auto_squash ! squash-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto squash (config interactive)' '
+ git config rebase.autosquash i &&
+ test_auto_squash ! squash-config-interactive &&
+ test_auto_squash squash-config-interactive-i -i &&
+ test_auto_squash ! squash-config-interactive-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto squash (config no-interactive)' '
+ git config rebase.autosquash no-i &&
+ test_auto_squash squash-config-no-interactive &&
+ test_auto_squash ! squash-config-no-interactive-i -i &&
+ test_auto_squash ! squash-config-no-interactive-no "--no-autosquash"
+'
+test_expect_success 'auto squash (config always)' '
+ git config rebase.autosquash interactive,no-interactive &&
+ test_auto_squash squash-config-always &&
+ test_auto_squash squash-config-always-i -i &&
+ test_auto_squash ! squash-config-always-no --no-autosquash &&
+ test_auto_squash ! squash-config-always-i-no "-i --no-autosquash"
'
test_expect_success 'misspelled auto squash' '
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 2eba00bdf5..e5119e7371 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -102,7 +102,7 @@ test_rebase_am_only () {
test_expect_success "$opt incompatible with rebase.autosquash" "
git checkout B^0 &&
- test_must_fail git -c rebase.autosquash=true rebase $opt A 2>err &&
+ test_must_fail git -c rebase.autosquash=no-i rebase $opt A 2>err &&
grep -e --no-autosquash err
"
--
2.43.0-rc0
^ permalink raw reply related
* RE: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: rsbecker @ 2023-11-04 14:55 UTC (permalink / raw)
To: 'Jeff King', 'Eric Sunshine'; +Cc: 'Git List'
In-Reply-To: <20231104134915.GA1492953@coredump.intra.peff.net>
On November 4, 2023 9:49 AM, Jeff King wrote:
>On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:
>
>> I don't see an urgent need for it. Unlike the actual tests themselves
>> run by `make test` which may catch platform-specific problems in Git
>> itself, the purpose of the "linting" checks is not to catch
>> platform-specific problems, but rather to help test authors by
>> identifying mistakes in the tests which might make them fragile. So,
>> disabling linting on a particular platform isn't going to cause `make
>> test` to miss some important Git problem specific to that platform.
>
>Hmm. With compilation, we split the audience of "developers" vs "people who just
>want to build the program", and we crank up the number and severity of warning
>checks for the former. We could do the same here for tests. I.e., turn off test linting
>by default and re-enable it for DEVELOPER=1.
>
>OTOH, this is the first time I think I've seen the linting cause a problem (whereas
>unexpected compile warnings are much more likely, as we are depending on the
>system compiler's behavior).
>
>So consider it an idle thought for discussion, and not necessarily a proposal. ;)
In my case, I am building git for users of the platform (a.k.a. mostly a packager), although I'm hoping to be more involved in contributions soon. This involves build + full test under multiple situations on the NonStop platform.
--Randall
^ permalink raw reply
* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Jeff King @ 2023-11-04 13:49 UTC (permalink / raw)
To: Eric Sunshine; +Cc: rsbecker, Git List
In-Reply-To: <CAPig+cQpxRQnhffR8EWWRhqJPmOeOuCE3qsuMcnDyuMLLbTH8Q@mail.gmail.com>
On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:
> I don't see an urgent need for it. Unlike the actual tests themselves
> run by `make test` which may catch platform-specific problems in Git
> itself, the purpose of the "linting" checks is not to catch
> platform-specific problems, but rather to help test authors by
> identifying mistakes in the tests which might make them fragile. So,
> disabling linting on a particular platform isn't going to cause `make
> test` to miss some important Git problem specific to that platform.
Hmm. With compilation, we split the audience of "developers" vs "people
who just want to build the program", and we crank up the number and
severity of warning checks for the former. We could do the same here for
tests. I.e., turn off test linting by default and re-enable it for
DEVELOPER=1.
OTOH, this is the first time I think I've seen the linting cause a
problem (whereas unexpected compile warnings are much more likely, as we
are depending on the system compiler's behavior).
So consider it an idle thought for discussion, and not necessarily a
proposal. ;)
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Andy Koppe @ 2023-11-04 9:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kousik Sanagavarapu, Liam Beguin, git
In-Reply-To: <xmqqy1fegu5i.fsf@gitster.g>
On 04/11/2023 01:54, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
>
>> I'm not sure that this is the right way to handle a missing '@' here
>> actually, because %al already returns the whole email field in that
>> case, which makes sense as the likes of the 'mail' command would
>> interpret it as a local username.
>
> We could expand "%am" to \C-h (\010) so that "%al@%am" would end up
> displaying the same as "%al" but that would be way too cute for its
> own worth ;-)
:)
Unfortunately it also wouldn't always work, because ^H only moves the
cursor, so if the next thing is a newline, the '@' wouldn't actually get
deleted.
> It is unfortunate that "%al@%am" cannot be the same as "%ae" for
> local-only address, but giving an empty string for "%am" if "%ae" is
> local-only would be the best we could do for our users, and certainly
> much better than giving the same as "%ae", as you said above.
I suppose "%@am" could mean prepending an '@' when a domain is present,
similar to how "% am" would mean prepending a space and "%+am" would
mean prepending a newline. With that, "%al%@am" would be equivalent to
"%ae".
But that then raises the question whether it should be implemented just
for "%@[ac][mM]", or for all placeholders. In any case, I don't think it
needs to be part of the changes at hand.
Andy
^ permalink raw reply
* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Junio C Hamano @ 2023-11-04 7:59 UTC (permalink / raw)
To: Eric Sunshine; +Cc: rsbecker, Git List
In-Reply-To: <CAPig+cQpxRQnhffR8EWWRhqJPmOeOuCE3qsuMcnDyuMLLbTH8Q@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> Also, in the longer term, as you suggested, `xargs` is likely a more
> fruitful solution.
Hmph, the list of our test scripts exceed command line limit? That
sounds a bit nasty, as we somehow need to prepare a pipe and feed
them into it, in order to drive xargs downstream of the pipe.
Ideally if there were a GNUMake function that slices a list into
sublists of "reasonable" lengths, we could use it to directly drive
N invocations of check-non-portable-shell script instead of xargs,
but I didn't find one. Here is I came up with, using foreach that
is "slice the list into many sublists of 1 element", but it made me
feel dirty.
t/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git c/t/Makefile w/t/Makefile
index 3e00cdd801..f9ac6bb4eb 100644
--- c/t/Makefile
+++ w/t/Makefile
@@ -122,7 +122,8 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
+ @{ $(foreach t,$(T) $(THELPERS) $(TPERF),echo "$t";) } | \
+ xargs '$(PERL_PATH_SQ)' check-non-portable-shell.pl
test-lint-filenames:
@# We do *not* pass a glob to ls-files but use grep instead, to catch
^ permalink raw reply related
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Junio C Hamano @ 2023-11-04 7:14 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Joanna Wang, git
In-Reply-To: <CAPig+cRR0PfWP0_uoevQnctSosb-k9cp3b08-sQNV5OPuQ2tAg@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Sat, Nov 4, 2023 at 1:12 AM Junio C Hamano <gitster@pobox.com> wrote:
>> > + if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
>>
>> Overly long, with a stylo.
>>
>> Here is what I ended up queuing.
>>
>> + for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
>> + if ((magic & pathspec_magic[i].bit) &&
>> + (pathspec_magic[i].mnemonic)) {
>
> Nit: The no-value-added parentheses around
> `pathspec_magic[i].mnemonic` can also be dropped.
Yeah, thanks for spotting.
^ permalink raw reply
* Re: [RFC PATCH] status: avoid reporting worktrees as "Untracked files"
From: Junio C Hamano @ 2023-11-04 6:58 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: git
In-Reply-To: <20231104000209.916189-1-eantoranz@gmail.com>
Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:
> Given that worktrees are tracked in their own special fashion separately,
> it makes sense to _not_ report them as "untracked".
My gut feeling is that a much better solution to the unstated
problem you are solving is to make sure that "git worktree add" will
complain and not allow adding a subdirectory of any existing working
tree of a repository as a new worktree.
What problem are you trying to solve? "git add foo" where "foo" is
actually a different worktree of the repository would add it as a
submodule that causes confusion? If that is the case, I think the
right solution is not to get into such a state, i.e. not create a
worktree of the repository inside a different worktree in the first
place.
^ permalink raw reply
* Re: Request for Help - Too many perl arguments as of 2.43.0-rc0
From: Eric Sunshine @ 2023-11-04 6:36 UTC (permalink / raw)
To: rsbecker; +Cc: Git List
In-Reply-To: <00c401da0ea1$b61899c0$2249cd40$@nexbridge.com>
On Fri, Nov 3, 2023 at 6:04 PM <rsbecker@nexbridge.com> wrote:
> On November 3, 2023 5:51 PM, Eric Sunshine wrote:
> >On Fri, Nov 3, 2023 at 5:31 PM <rsbecker@nexbridge.com> wrote:
> >> /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
> >> Makefile:125: recipe for target 'test-lint-shell-syntax' failed
> >
> >This is a separate issue from chainlint (though, it too will likely suffer the same
> >problem). In this case, it's trying to run the check-non-portable-shell.pl script and
> >the command-line is too long.
> >Unfortunately, unlike chainlint for which there is a knob to disble it, there is no direct
> >knob to disable `test-lint-shell-syntax`. You may be able to skip all the shell-script
> >linting by setting TEST_LINT to an empty value.
>
> That worked. I wonder whether it might be useful to set up a knob for this.
I don't see an urgent need for it. Unlike the actual tests themselves
run by `make test` which may catch platform-specific problems in Git
itself, the purpose of the "linting" checks is not to catch
platform-specific problems, but rather to help test authors by
identifying mistakes in the tests which might make them fragile. So,
disabling linting on a particular platform isn't going to cause `make
test` to miss some important Git problem specific to that platform.
Also, in the longer term, as you suggested, `xargs` is likely a more
fruitful solution.
^ permalink raw reply
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Eric Sunshine @ 2023-11-04 6:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joanna Wang, git
In-Reply-To: <xmqqsf5mgl0r.fsf@gitster.g>
On Sat, Nov 4, 2023 at 1:12 AM Junio C Hamano <gitster@pobox.com> wrote:
> > + if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
>
> Overly long, with a stylo.
>
> Here is what I ended up queuing.
>
> + for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
> + if ((magic & pathspec_magic[i].bit) &&
> + (pathspec_magic[i].mnemonic)) {
Nit: The no-value-added parentheses around
`pathspec_magic[i].mnemonic` can also be dropped.
^ permalink raw reply
* Re: [RFC PATCH] status: avoid reporting worktrees as "Untracked files"
From: Eric Sunshine @ 2023-11-04 6:15 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: git
In-Reply-To: <20231104000209.916189-1-eantoranz@gmail.com>
On Fri, Nov 3, 2023 at 8:03 PM Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> Given that worktrees are tracked in their own special fashion separately,
> it makes sense to _not_ report them as "untracked". Also, when seeing the
> directory of a worktree listed as Untracked, it might be tempting to try
> to do operations (like 'git add') on them from the parent worktree which,
> at the moment, will silently do nothing.
>
> With this patch, we check items against the list of worktrees to add
> them into the untracked items list effectively hiding them.
>
> END OF PATCH
>
> Here are a few questions more inline with the "RFC" part of the patch.
>
> About UI
> - Would it make more sense to separate them from Untracked files instead
> of hiding them (perhaps add a --worktrees option to display them)?
> - Follow-up if the previous answer is 'yes': List a worktree only if it
> is not clean?
>
> About code:
> - If keeping the idea/patch, Would it make more sense (performance-wise) to
> fist check an item in the list of worktrees before checking it in the
> index? In other words, reverse the conditions to add an item to the
> untracked list?
I have slightly mixed feelings about this idea since I'm sympathetic
to the motivation, however, my knee-jerk reaction is that these really
_are_ untracked considering that Git is a "content tracker" and
worktrees are not project content. Git already has general mechanisms
such as .git/info/exclude and .gitignore for suppressing certain
untracked items, so introducing special-purpose code to suppress
worktrees from being considered untracked may be a case of adding
complexity for little gain.
Moreover, although your personal workflow may be to create worktrees
within your main directory:
git worktree add new-feature
other people use a workflow in which worktrees are created at other
locations, such as making them siblings:
git worktree add ../new-feature
For the former case, it's easy enough to mention worktrees in
.git/info/exclude, especially if you use a standard naming convention
for your worktrees, in which case a single wildcard pattern may allow
you to set it once and forget about it. For the latter workflow, the
extra "is untracked" checking is simply wasteful.
Having said all that, I think that someone may have recently floated
the idea on the mailing list about suppressing dirty submodules from
showing up as "dirty" in git-status. Although the underlying concepts
and mechanisms are quite distinct (especially since a submodule _is_
content), perhaps there is some sort of analogy between worktrees and
dirty submodules which invalidates my knee-jerk reaction. Also, I'm
just one person responding without having put all that much thought
into it. Others may feel differently.
Regarding the patch itself...
> diff --git a/wt-status.c b/wt-status.c
> @@ -795,9 +796,12 @@ static void wt_status_collect_untracked(struct wt_status *s)
> + worktrees = get_worktrees();
> +
> for (i = 0; i < dir.nr; i++) {
> struct dir_entry *ent = dir.entries[i];
> - if (index_name_is_other(istate, ent->name, ent->len))
> + if (index_name_is_other(istate, ent->name, ent->len) &&
> + !find_worktree_by_path(worktrees, ent->name))
> string_list_insert(&s->untracked, ent->name);
> }
This first-stab implementation unfortunately has worse than quadratic
complexity, perhaps even cubic complexity since
find_worktree_by_path() performs a linear scan through the worktree
list. So, for each path in `dir`, it's performing a
character-by-character string comparison with each path in
`worktrees`. Worse, find_worktree_by_path() calls strbuf_realpath()
which hits the filesystem for each path in `worktrees` each time it's
called.
So, a real (non-RFC) implementation would probably need to perform a
preparatory step of creating a hash-table/set in which the keys are
the realpath'd elements from `worktrees`, and then simply consult the
hash-table/set for each path in `dir`.
> @@ -809,6 +813,9 @@ static void wt_status_collect_untracked(struct wt_status *s)
> + if (worktrees)
> + free_worktrees(worktrees);
Nit: At this point, we _know_ that `worktrees` is non-NULL, so
free_worktrees() can be called unconditionally.
^ permalink raw reply
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Junio C Hamano @ 2023-11-04 5:11 UTC (permalink / raw)
To: Joanna Wang; +Cc: git
In-Reply-To: <20231103163449.1578841-1-jojwang@google.com>
Thanks. The compiled result from this version looks quite good. As
you started a new round with typofix, let me start the final
nitpicking.
Joanna Wang <jojwang@google.com> writes:
> This lets users limit files or exclude files based on file
> attributes during git-add and git-stash.
> For example, the chromium project would like to use this like
> "git add --all ':(exclude,attr:submodule)'", as submodules are managed in a
> unique way and often results in submodule changes that users do not want in
> their commits.
I would say
Allow users to limit or exclude files based on file attributes
during git-add and git-stash.
For example, the chromium project would like to use
$ git add . ':(exclude,attr:submodule)'
as submodules are managed in a unique way and often results in
submodule changes that users do not want in their commits.
except that I would prefer to see "in a unique way" rephrased.
Would "are managed outside Git using external tools" be a more
accurate description?
It may be true that the chromium project handles submodules unlike
all the other projects, but you guys being alone is not an important
part of the reason why we might want this new feature to help users.
When another project adopts the way how chromium manages the
submodules, forbidding its end-users from running "git add" to
record the changes to their submodules, you guys will no longer be
"unique", but the need for this feature will still exist (and the
demand may even be stronger). The reason why this feature helps
should be stated here.
> This does not change any attr magic implementation. It is only adding
> attr as an allowed pathspec in git-add and git-stash, which was previously
> blocked by GUARD_PATHSPEC and a pathspec mask in parse_pathspec()).
Add blank line here between paragraphs.
> However, this does fix a bug in prefix_magic() where attr values were unintentionally removed.
> This was hit whenever parse_pathspec() is called with PATHSPEC_PREFIX_ORIGIN as a flag,
> which was the case for git-stash.
> (Bug originally filed here:
Wrap overlong lines here and in the rest of the proposed log message.
> diff --git a/pathspec.c b/pathspec.c
> index bb1efe1f39..2f8721cc15 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -109,16 +109,34 @@ static struct pathspec_magic {
> { PATHSPEC_ATTR, '\0', "attr" },
> };
>
> -static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
> +static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic, const char *element)
> {
This is overly long.
> + /* No magic was found in element, just add prefix magic */
> + if (magic == 0) {
Style:
if (!magic) {
> + strbuf_addf(sb, ":(prefix:%d)", prefixlen);
> + return;
> + }
> +
> + /*
> + * At this point we known that parse_element_magic() was able to extract some pathspec
> + * magic from element. So we know element is correctly formatted in either shorthand
> + * or longhand form
> + */
Overly long, with a typo. "At this point, we know that ...".
> + if (element[1] != '(') {
> + /* Process an element in shorthand form (e.g. ":!/<match>") */
> + strbuf_addstr(sb, ":(");
> + for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
> + if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
Overly long, with a stylo.
> + if (sb->buf[sb->len - 1] != '(')
> + strbuf_addch(sb, ',');
> + strbuf_addstr(sb, pathspec_magic[i].name);
> + }
> }
> + } else {
> + /* For the longhand form, we copy everything up to the final ')' */
> + size_t len = strchr(element, ')') - element;
> + strbuf_add(sb, element, len);
> + }
> strbuf_addf(sb, ",prefix:%d)", prefixlen);
> }
Here is what I ended up queuing.
Thanks.
---- >8 -------- >8 -------- >8 -------- >8 ----
From: Joanna Wang <jojwang@google.com>
Date: Fri, 3 Nov 2023 16:34:48 +0000
Subject: [PATCH] attr: enable attr pathspec magic for git-add and git-stash
Allow users to limit or exclude files based on file attributes
during git-add and git-stash.
For example, the chromium project would like to use
$ git add . ':(exclude,attr:submodule)'
as submodules are managed by an external tool, forbidding end users
to record changes with "git add". Allowing "git add" to often
records changes that users do not want in their commits.
This commit does not change any attr magic implementation. It is
only adding attr as an allowed pathspec in git-add and git-stash,
which was previously blocked by GUARD_PATHSPEC and a pathspec mask
in parse_pathspec()).
However, we fix a bug in prefix_magic() where attr values were
unintentionally removed. This was triggerable when parse_pathspec()
is called with PATHSPEC_PREFIX_ORIGIN as a flag, which was the case
for git-stash (Bug originally filed here [*])
Furthermore, while other commands hit this code path it did not
result in unexpected behavior because this bug only impacts the
pathspec->items->original field which is NOT used to filter
paths. However, git-stash does use pathspec->items->original when
building args used to call other git commands. (See add_pathspecs()
usage and implementation in stash.c)
It is possible that when the attr pathspec feature was first added
in b0db704652 (pathspec: allow querying for attributes, 2017-03-13),
"PATHSPEC_ATTR" was just unintentionally left out of a few
GUARD_PATHSPEC() invocations.
Later, to get a more user-friendly error message when attr was used
with git-add, PATHSPEC_ATTR was added as a mask to git-add's
invocation of parse_pathspec() 84d938b732 (add: do not accept
pathspec magic 'attr', 2018-09-18). However, this user-friendly
error message was never added for git-stash.
[Reference]
* https://lore.kernel.org/git/CAMmZTi-0QKtj7Q=sbC5qhipGsQxJFOY-Qkk1jfkRYwfF5FcUVg@mail.gmail.com/)
Signed-off-by: Joanna Wang <jojwang@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/add.c | 7 ++-
dir.c | 3 +-
pathspec.c | 39 +++++++++---
t/t6135-pathspec-with-attrs.sh | 108 +++++++++++++++++++++++++++++++--
4 files changed, 139 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 5126d2ede3..d46e4d10e9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -424,7 +424,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
* Check the "pathspec '%s' did not match any files" block
* below before enabling new magic.
*/
- parse_pathspec(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, argv);
@@ -433,7 +433,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (pathspec.nr)
die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
- parse_pathspec_file(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec_file(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, pathspec_from_file, pathspec_file_nul);
@@ -504,7 +504,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
diff --git a/dir.c b/dir.c
index 16fdb03f2a..4d1cd039be 100644
--- a/dir.c
+++ b/dir.c
@@ -2179,7 +2179,8 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec->nr; i++) {
const struct pathspec_item *item = &pathspec->items[i];
diff --git a/pathspec.c b/pathspec.c
index bb1efe1f39..d132eefac2 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -109,16 +109,37 @@ static struct pathspec_magic {
{ PATHSPEC_ATTR, '\0', "attr" },
};
-static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
+static void prefix_magic(struct strbuf *sb, int prefixlen,
+ unsigned magic, const char *element)
{
- int i;
- strbuf_addstr(sb, ":(");
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
- if (magic & pathspec_magic[i].bit) {
- if (sb->buf[sb->len - 1] != '(')
- strbuf_addch(sb, ',');
- strbuf_addstr(sb, pathspec_magic[i].name);
+ /* No magic was found in element, just add prefix magic */
+ if (!magic) {
+ strbuf_addf(sb, ":(prefix:%d)", prefixlen);
+ return;
+ }
+
+ /*
+ * At this point, we know that parse_element_magic() was able
+ * to extract some pathspec magic from element. So we know
+ * element is correctly formatted in either shorthand or
+ * longhand form
+ */
+ if (element[1] != '(') {
+ /* Process an element in shorthand form (e.g. ":!/<match>") */
+ strbuf_addstr(sb, ":(");
+ for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if ((magic & pathspec_magic[i].bit) &&
+ (pathspec_magic[i].mnemonic)) {
+ if (sb->buf[sb->len - 1] != '(')
+ strbuf_addch(sb, ',');
+ strbuf_addstr(sb, pathspec_magic[i].name);
+ }
}
+ } else {
+ /* For the longhand form, we copy everything up to the final ')' */
+ size_t len = strchr(element, ')') - element;
+ strbuf_add(sb, element, len);
+ }
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
@@ -493,7 +514,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
struct strbuf sb = STRBUF_INIT;
/* Preserve the actual prefix length of each pattern */
- prefix_magic(&sb, prefixlen, element_magic);
+ prefix_magic(&sb, prefixlen, element_magic, elt);
strbuf_addstr(&sb, match);
item->original = strbuf_detach(&sb, NULL);
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index f70c395e75..e46fa176ed 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -64,12 +64,24 @@ test_expect_success 'setup .gitattributes' '
fileSetLabel label
fileValue label=foo
fileWrongLabel label☺
+ newFileA* labelA
+ newFileB* labelB
EOF
echo fileSetLabel label1 >sub/.gitattributes &&
git add .gitattributes sub/.gitattributes &&
git commit -m "add attributes"
'
+test_expect_success 'setup .gitignore' '
+ cat <<-\EOF >.gitignore &&
+ actual
+ expect
+ pathspec_file
+ EOF
+ git add .gitignore &&
+ git commit -m "add gitignore"
+'
+
test_expect_success 'check specific set attr' '
cat <<-\EOF >expect &&
fileSetLabel
@@ -150,6 +162,7 @@ test_expect_success 'check specific value attr (2)' '
test_expect_success 'check unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileA
fileAB
fileAC
@@ -175,6 +188,7 @@ test_expect_success 'check unspecified attr' '
test_expect_success 'check unspecified attr (2)' '
cat <<-\EOF >expect &&
HEAD:.gitattributes
+ HEAD:.gitignore
HEAD:fileA
HEAD:fileAB
HEAD:fileAC
@@ -200,6 +214,7 @@ test_expect_success 'check unspecified attr (2)' '
test_expect_success 'check multiple unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileC
fileNoLabel
fileWrongLabel
@@ -239,16 +254,99 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
test_i18ngrep "Only one" actual
'
-test_expect_success 'fail if attr magic is used places not implemented' '
+test_expect_success 'fail if attr magic is used in places not implemented' '
# The main purpose of this test is to check that we actually fail
# when you attempt to use attr magic in commands that do not implement
- # attr magic. This test does not advocate git-add to stay that way,
- # though, but git-add is convenient as it has its own internal pathspec
- # parsing.
- test_must_fail git add ":(attr:labelB)" 2>actual &&
+ # attr magic. This test does not advocate check-ignore to stay that way.
+ # When you teach the command to grok the pathspec, you need to find
+ # another command to replace it for the test.
+ test_must_fail git check-ignore ":(attr:labelB)" 2>actual &&
test_i18ngrep "magic not supported" actual
'
+test_expect_success 'check that attr magic works for git stash push' '
+ cat <<-\EOF >expect &&
+ A sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git stash push --include-untracked -- ":(exclude,attr:labelB)" &&
+ git stash show --include-untracked --name-status >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --all' '
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -W -S . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add -u' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add -u ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . && rm sub/new* &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add <path>' '
+ cat <<-\EOF >expect &&
+ fileA
+ fileB
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add ":(exclude,attr:labelB)sub/*" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git -add .' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ cd sub &&
+ git add . ":(exclude,attr:labelB)" &&
+ cd .. &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --pathspec-from-file' '
+ cat <<-\EOF >pathspec_file &&
+ :(exclude,attr:labelB)
+ EOF
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all --pathspec-from-file=pathspec_file &&
+ git diff --name-only --cached >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'abort on giving invalid label on the command line' '
test_must_fail git ls-files . ":(attr:☺)"
'
--
2.43.0-rc0
^ permalink raw reply related
* Re: [PATCH] t: avoid perl's pack/unpack "Q" specifier
From: Jeff King @ 2023-11-04 4:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: rsbecker, git
In-Reply-To: <xmqq5y2ii91p.fsf@gitster.g>
On Sat, Nov 04, 2023 at 10:47:30AM +0900, Junio C Hamano wrote:
> > +sub unpack_quad {
> > + my $bytes = shift;
> > + my ($n1, $n2) = unpack("NN", $bytes);
> > + die "quad value exceeds 32 bits" if $n1;
> > + return $n2;
> > +};
>
> Is this an unnecessary ';' at the end?
Oops, yes. I'm not sure how that snuck in there. (It is not breaking
anything, but if you were to remove it while applying, I would be very
happy).
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Junio C Hamano @ 2023-11-04 1:54 UTC (permalink / raw)
To: Andy Koppe; +Cc: Kousik Sanagavarapu, Liam Beguin, git
In-Reply-To: <4e3a5661-cba0-4849-8340-06d8d4094f48@gmail.com>
Andy Koppe <andy.koppe@gmail.com> writes:
> I'm not sure that this is the right way to handle a missing '@' here
> actually, because %al already returns the whole email field in that
> case, which makes sense as the likes of the 'mail' command would
> interpret it as a local username.
We could expand "%am" to \C-h (\010) so that "%al@%am" would end up
displaying the same as "%al" but that would be way too cute for its
own worth ;-)
It is unfortunate that "%al@%am" cannot be the same as "%ae" for
local-only address, but giving an empty string for "%am" if "%ae" is
local-only would be the best we could do for our users, and certainly
much better than giving the same as "%ae", as you said above.
Thanks.
^ permalink raw reply
* Re: [PATCH] t: avoid perl's pack/unpack "Q" specifier
From: Junio C Hamano @ 2023-11-04 1:47 UTC (permalink / raw)
To: Jeff King; +Cc: rsbecker, git
In-Reply-To: <20231103162019.GB1470570@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> +# Some platforms' perl builds don't support 64-bit integers, and hence do not
> +# allow packing/unpacking quadwords with "Q". The chunk format uses 64-bit file
> +# offsets to support files of any size, but in practice our test suite will
> +# only use small files. So we can fake it by asking for two 32-bit values and
> +# discarding the first (most significant) one, which is equivalent as long as
> +# it's just zero.
> +sub unpack_quad {
> + my $bytes = shift;
> + my ($n1, $n2) = unpack("NN", $bytes);
> + die "quad value exceeds 32 bits" if $n1;
> + return $n2;
> +};
Is this an unnecessary ';' at the end?
> +sub pack_quad {
> + my $n = shift;
> + my $ret = pack("NN", 0, $n);
> + # double check that our original $n did not exceed the 32-bit limit.
> + # This is presumably impossible on a 32-bit system (which would have
> + # truncated much earlier), but would still alert us on a 64-bit build
> + # of a new test that would fail on a 32-bit build (though we'd
> + # presumably see the die() from unpack_quad() in such a case).
> + die "quad round-trip failed" if unpack_quad($ret) != $n;
> + return $ret;
> +}
Nice. Both sub are done carefully.
> # read until we find table-of-contents entry for chunk;
> # note that we cheat a bit by assuming 4-byte alignment and
> # that no ToC entry will accidentally look like a header.
> #
> # If we don't find the entry, copy() will hit EOF and exit
> # (which should cause the caller to fail the test).
> while (copy(4) ne $chunk) { }
> -my $offset = unpack("Q>", copy(8));
> +my $offset = unpack_quad(copy(8));
>
> # In clear mode, our length will change. So figure out
> # the length by comparing to the offset of the next chunk, and
> @@ -38,11 +62,11 @@ sub copy {
> my $id;
> do {
> $id = copy(4);
> - my $next = unpack("Q>", get(8));
> + my $next = unpack_quad(get(8));
> if (!defined $len) {
> $len = $next - $offset;
> }
> - print pack("Q>", $next - $len + length($bytes));
> + print pack_quad($next - $len + length($bytes));
> } while (unpack("N", $id));
> }
^ permalink raw reply
* Re: [PATCH 1/2] rebase: support non-interactive autosquash
From: Junio C Hamano @ 2023-11-04 1:19 UTC (permalink / raw)
To: Andy Koppe; +Cc: git, newren
In-Reply-To: <20231103212958.18472-1-andy.koppe@gmail.com>
Andy Koppe <andy.koppe@gmail.com> writes:
> rebase.autoSquash::
> - If set to true enable `--autosquash` option by default.
> + When set to 'interactive' or 'true', enable the `--autosquash` option
> + for interactive rebase. When set to 'always', enable it for
> + non-interactive rebase as well. Defaults to 'false'.
I think a better and more extensible way to coax the new feature
into the configuration system is to arrange it more like so:
false - synonym for "".
true - synonym for "interactive".
anything else - comman separated list of rebase methods, e.g.,
"interactive,noninteractive"
possible rebase method names might include other
stuff like "apply" or "merge", but I haven't
thought it through, so take this part with a
grain of salt.
That way, the Boolean versions can be considered historical spelling
of a more general system where you can exactly tell when autosquash
takes place. When we add to a new variant on top of 'interactive'
and 'non-interactive' variants the current rebase has, we do not
know if it makes sense to allow it to also handle autosquash without
knowing how that new variant's behavior appears to the end user, so
'always' that blindly enables autosquash for any unforseen future
variants of 'rebase' is probably not what you want.
^ permalink raw reply
* [RFC PATCH] status: avoid reporting worktrees as "Untracked files"
From: Edmundo Carmona Antoranz @ 2023-11-04 0:02 UTC (permalink / raw)
To: git; +Cc: Edmundo Carmona Antoranz
Given that worktrees are tracked in their own special fashion separately,
it makes sense to _not_ report them as "untracked". Also, when seeing the
directory of a worktree listed as Untracked, it might be tempting to try
to do operations (like 'git add') on them from the parent worktree which,
at the moment, will silently do nothing.
With this patch, we check items against the list of worktrees to add
them into the untracked items list effectively hiding them.
END OF PATCH
Here are a few questions more inline with the "RFC" part of the patch.
About UI
- Would it make more sense to separate them from Untracked files instead
of hiding them (perhaps add a --worktrees option to display them)?
- Follow-up if the previous answer is 'yes': List a worktree only if it
is not clean?
About code:
- If keeping the idea/patch, Would it make more sense (performance-wise) to
fist check an item in the list of worktrees before checking it in the
index? In other words, reverse the conditions to add an item to the
untracked list?
---
wt-status.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..5fd1e6007a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -775,6 +775,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
struct dir_struct dir = DIR_INIT;
uint64_t t_begin = getnanotime();
struct index_state *istate = s->repo->index;
+ struct worktree **worktrees;
if (!s->show_untracked_files)
return;
@@ -795,9 +796,12 @@ static void wt_status_collect_untracked(struct wt_status *s)
fill_directory(&dir, istate, &s->pathspec);
+ worktrees = get_worktrees();
+
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
- if (index_name_is_other(istate, ent->name, ent->len))
+ if (index_name_is_other(istate, ent->name, ent->len) &&
+ !find_worktree_by_path(worktrees, ent->name))
string_list_insert(&s->untracked, ent->name);
}
@@ -809,6 +813,9 @@ static void wt_status_collect_untracked(struct wt_status *s)
dir_clear(&dir);
+ if (worktrees)
+ free_worktrees(worktrees);
+
if (advice_enabled(ADVICE_STATUS_U_OPTION))
s->untracked_in_ms = (getnanotime() - t_begin) / 1000000;
}
--
2.42.0
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2023, #01; Thu, 2)
From: Christian Couder @ 2023-11-03 22:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqa5rwo7u7.fsf@gitster.g>
On Thu, Nov 2, 2023 at 9:53 AM Junio C Hamano <gitster@pobox.com> wrote:
> * ps/ci-gitlab (2023-11-02) 8 commits
> - ci: add support for GitLab CI
> - ci: install test dependencies for linux-musl
> - ci: squelch warnings when testing with unusable Git repo
> - ci: unify setup of some environment variables
> - ci: split out logic to set up failed test artifacts
> - ci: group installation of Docker dependencies
> - ci: make grouping setup more generic
> - ci: reorder definitions for grouping functions
>
> Add support for GitLab CI.
>
> Comments?
> source: <cover.1698843660.git.ps@pks.im>
I just took a look at this series and I like both the idea of having a
GitLab CI configuration in the code codebase and the implementation
which makes things as easy as possible to maintain.
> * cc/git-replay (2023-10-10) 14 commits
> - replay: stop assuming replayed branches do not diverge
> - replay: add --contained to rebase contained branches
> - replay: add --advance or 'cherry-pick' mode
> - replay: use standard revision ranges
> - replay: make it a minimal server side command
> - replay: remove HEAD related sanity check
> - replay: remove progress and info output
> - replay: add an important FIXME comment about gpg signing
> - replay: change rev walking options
> - replay: introduce pick_regular_commit()
> - replay: die() instead of failing assert()
> - replay: start using parse_options API
> - replay: introduce new builtin
> - t6429: remove switching aspects of fast-rebase
>
> Introduce "git replay", a tool meant on the server side without
> working tree to recreate a history.
>
> Expectting a (hopefully final and quick) reroll.
> cf. <bd872b81-80a9-5e4e-dcb6-faebc9671848@gmx.de>
> source: <20231010123847.2777056-1-christian.couder@gmail.com>
Reroll sent yesterday
(https://lore.kernel.org/git/20231102135151.843758-1-christian.couder@gmail.com/).
A quick review of the latest changes would be nice.
> * js/doc-unit-tests (2023-11-02) 3 commits
> - ci: run unit tests in CI
> - unit tests: add TAP unit test framework
> - unit tests: add a project plan document
> (this branch is used by js/doc-unit-tests-with-cmake.)
>
> Process to add some form of low-level unit tests has started.
>
> Will merge to 'next'?
> source: <cover.1698881249.git.steadmon@google.com>
I just took a look at the v9 of the series and it looks good despite a
small nit. Also I like the idea and implementation, so I agree with
merging it to 'next' and then 'master'.
^ 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