* [PATCH v3 1/2] index-pack: test and document --strict=<msg-id>=<severity>...
From: John Cai via GitGitGadget @ 2024-01-26 20:59 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1658.v3.git.git.1706302749.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
--strict. However this is a hidden argument that was not documented nor
tested. Though it is true that most users would not call this option
directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-index-pack.txt | 9 +++++++--
builtin/index-pack.c | 2 +-
t/t5300-pack-object.sh | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 6486620c3d8..694bb9409bf 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -79,8 +79,13 @@ OPTIONS
to force the version for the generated pack index, and to force
64-bit index entries on objects located above the given offset.
---strict::
- Die, if the pack contains broken objects or links.
+--strict[=<msg-id>=<severity>...]::
+ Die, if the pack contains broken objects or links. An optional
+ comma-separated list of `<msg-id>=<severity>` can be passed to change
+ the severity of some possible issues, e.g.,
+ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
+ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
+ information on the possible values of `<msg-id>` and `<severity>`.
--progress-title::
For internal use only.
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1ea87e01f29..240c7021168 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -24,7 +24,7 @@
#include "setup.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index d402ec18b79..496fffa0f8a 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,6 +441,28 @@ test_expect_success 'index-pack with --strict' '
)
'
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+ test_when_finished rm -rf strict &&
+ git init strict &&
+ (
+ cd strict &&
+ test_commit first hello &&
+ cat >commit <<-EOF &&
+ tree $(git rev-parse HEAD^{tree})
+ parent $(git rev-parse HEAD)
+ author A U Thor
+ committer A U Thor
+
+ commit: this is a commit with bad emails
+
+ EOF
+ git hash-object --literally -t commit -w --stdin <commit >commit_list &&
+ PACK=$(git pack-objects test <commit_list) &&
+ test_must_fail git index-pack --strict "test-$PACK.pack" &&
+ git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
+ )
+'
+
test_expect_success 'honor pack.packSizeLimit' '
git config pack.packSizeLimit 3m &&
packname_10=$(git pack-objects test-10 <obj-list) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] index-pack: fsck honor checks
From: John Cai via GitGitGadget @ 2024-01-26 20:59 UTC (permalink / raw)
To: git; +Cc: John Cai
In-Reply-To: <pull.1658.v2.git.git.1706289180.gitgitgadget@gmail.com>
git-index-pack has a --strict mode that can take an optional argument to
provide a list of fsck issues to change their severity. --fsck-objects does
not have such a utility, which would be useful if one would like to be more
lenient or strict on data integrity in a repository.
Like --strict, Allow --fsck-objects to also take a list of fsck msgs to
change the severity.
Changes since V2:
* fixed some typos in the documentation
* added commit trailers
Change since V1:
* edited commit messages
* clarified formatting in documentation for --strict= and --fsck-objects=
John Cai (2):
index-pack: test and document --strict=<msg-id>=<severity>...
index-pack: --fsck-objects to take an optional argument for fsck msgs
Documentation/git-index-pack.txt | 26 +++++++++++++-------
builtin/index-pack.c | 5 ++--
t/t5300-pack-object.sh | 41 ++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 10 deletions(-)
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1658%2Fjohn-cai%2Fjc%2Findex-pack-fsck-honor-checks-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1658/john-cai/jc/index-pack-fsck-honor-checks-v3
Pull-Request: https://github.com/git/git/pull/1658
Range-diff vs v2:
1: b3b3e8bd0bf ! 1: cdf7fc7fe8a index-pack: test and document --strict=<msg>
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- index-pack: test and document --strict=<msg>
+ index-pack: test and document --strict=<msg-id>=<severity>...
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
@@ Commit message
directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
+ Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
## Documentation/git-index-pack.txt ##
@@ Documentation/git-index-pack.txt: OPTIONS
---strict::
- Die, if the pack contains broken objects or links.
+--strict[=<msg-id>=<severity>...]::
-+ Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
-+ it should be a comma-separated list of `<msg-id>=<severity>` elements where
-+ `<msg-id>` and `<severity>` are used to change the severity of some possible
-+ issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry
-+ for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
-+ more information on the possible values of `<msg-id>` and `<severity>`.
++ Die, if the pack contains broken objects or links. An optional
++ comma-separated list of `<msg-id>=<severity>` can be passed to change
++ the severity of some possible issues, e.g.,
++ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
++ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
++ information on the possible values of `<msg-id>` and `<severity>`.
--progress-title::
For internal use only.
@@ builtin/index-pack.c
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
-+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
++"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
2: cce63c6465f ! 2: a2b9adb93d8 index-pack: --fsck-objects to take an optional argument for fsck msgs
@@ Commit message
the option. This won't often be used by the normal end user, but it
turns out it is useful for Git forges like GitLab.
+ Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
## Documentation/git-index-pack.txt ##
@@ Documentation/git-index-pack.txt: default and "Indexing objects" when `--stdin`
---fsck-objects::
- For internal use only.
-+--fsck-objects[=<msg-ids>=<severity>...]::
-+ Instructs index-pack to check for broken objects, but unlike `--strict`,
-+ does not choke on broken links. If `<msg-ids>` is passed, it should be
-+ a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
-+ `<severity>` are used to change the severity of `fsck` errors e.g.,
-+ `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
-+ the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
-+ more information on the possible values of `<msg-id>` and `<severity>`.
++--fsck-objects[=<msg-id>=<severity>...]::
++ Die if the pack contains broken objects. If the pack contains a tree
++ pointing to a .gitmodules blob that does not exist, prints the hash of
++ that blob (for the caller to check) after the hash that goes into the
++ name of the pack/idx file (see "Notes").
+
- Die if the pack contains broken objects. If the pack contains a tree
- pointing to a .gitmodules blob that does not exist, prints the hash of
+-Die if the pack contains broken objects. If the pack contains a tree
+-pointing to a .gitmodules blob that does not exist, prints the hash of
+-that blob (for the caller to check) after the hash that goes into the
+-name of the pack/idx file (see "Notes").
++Unlike `--strict` however, don't choke on broken links. An optional
++comma-separated list of `<msg-id>=<severity>` can be passed to change the
++severity of some possible issues, e.g.,
++`--fsck-objects="missingEmail=ignore,badTagName=ignore"`. See the entry for the
++`fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
++information on the possible values of `<msg-id>` and `<severity>`.
+
+ --threads=<n>::
+ Specifies the number of threads to spawn when resolving
## builtin/index-pack.c ##
@@
#include "setup.h"
static const char index_pack_usage[] =
--"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
-+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] [--fsck-objects[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
++"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 1/4] completion: introduce __gitcomp_subcommand
From: Junio C Hamano @ 2024-01-26 20:34 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
In-Reply-To: <48717a57-42ad-4c00-bdd5-c23c0f3ccbe9@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> On 26-ene-2024 09:26:44, Junio C Hamano wrote:
>> Rubén Justo <rjusto@gmail.com> writes:
>>
>> > +# Completion for subcommands in commands that follow the syntax:
>> > +#
>> > +# git <command> <subcommand>
>> > +#
>> > +# 1: List of possible completion words.
>> > +# Returns false if the current word is not a possible subcommand
>> > +# (possitioned after the command), or if no option is found in
>> > +# the list provided.
>> > +__gitcomp_subcommand ()
>> > +{
>> > + local subcommands="$1"
>> > +
>> > + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then
>> > + __gitcomp "$subcommands"
>> > +
>> > + test -n "$COMPREPLY"
>> > + else
>> > + false
>> > + fi
>> > +}
>>
>>
>> I am not at all familiar with the code in this file, so treat this
>> as a question from somebody who do not know the calling convention
>> used around here.
>>
>> This helper function clobbers what was in COMPREPLY[@] before
>> calling it, from a caller's point of view. Is it a pattern that
>> potential callers in this file should already find familiar, and
>> they do not have to be reminded that they cannot rely on what they
>> prepared in COMPREPLY to be preserved across a call into this
>> function? Otherwise I would suggest mentioning it in the helpful
>> comment before the function, but I cannot tell if such a comment is
>> even needed by the intended audience, so...
>
> Maybe adding such a comment might suggest at first glance that we're
> working different here than in the rest of the __gitcomp_* family of
> functions, which is not the intention ... I dunno.
Exactly. That is why I asked. If it is a norm for all these helper
functions to stomp on COMPREPLY and if it is accepted as a common
pattern by developers who touch this file, then I agree it would be
misleading to have such a comment only to this function.
THanks.
^ permalink raw reply
* Re: [PATCH 3/4] completion: reflog with implicit "show"
From: Rubén Justo @ 2024-01-26 20:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqq8r4cnfju.fsf@gitster.g>
On 26-ene-2024 09:57:25, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > When no subcommand is specified to "reflog", we assume "show" [1]:
> >
> > $ git reflog -h
> > usage: git reflog [show] [<log-options>] [<ref>]
> > ...
> >
> > We are not completing correctly this implicit uses of "show":
> >
> > With ...
> >
> > $ git checkout -b default
> >
> > ... we are not completing "default":
> >
> > $ git reflog def<TAB><TAB>
> >
> > And we are incorrectly returning the "subcommands" when:
> >
> > $ git reflog default <TAB><TAB>
> > delete expire show
> >
> > Let's use __gitcomp_subcommand to correctly handle implicit
> > subcommands.
>
> As with a good bug report, if you are showing an "incorrect"
> behaviour, you should also illustrate what you think is the
> "correct" behaviour (see below).
>
> > 1. cf39f54efc (git reflog show, 2007-02-08)
> >
> > Signed-off-by: Rubén Justo <rjusto@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 9 ++++-----
> > t/t9902-completion.sh | 8 ++++++++
> > 2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 5f2e904b56..c41f25aa80 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -2448,13 +2448,12 @@ _git_rebase ()
> > _git_reflog ()
> > {
> > local subcommands="show delete expire"
> > - local subcommand="$(__git_find_on_cmdline "$subcommands")"
> >
> > - if [ -z "$subcommand" ]; then
> > - __gitcomp "$subcommands"
> > - else
> > - __git_complete_refs
> > + if __gitcomp_subcommand "$subcommands"; then
> > + return
> > fi
> > +
> > + __git_complete_refs
> > }
>
> So, when we see something that could be a subcommand we complete it
> as a subcommand and return. In your example, how should
>
> $ git reflog def<TAB>
>
> work? We try to see if there is a subcommand that begins with
> "def", we see nothing matching, and then run __git_complete_refs?
> What if the branch you created earlier were not named "default" but,
> say, "delmonte", and you did "git reflog del<TAB>"? Shouldn't the
> user be offered "delete" and "delmonte" as potential completions?
>
> > __git_send_email_confirm_options="always never auto cc compose"
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > index aa9a614de3..231e17f378 100755
> > --- a/t/t9902-completion.sh
> > +++ b/t/t9902-completion.sh
> > @@ -2618,6 +2618,14 @@ test_expect_success 'git clone --config= - value' '
> > EOF
> > '
> >
> > +test_expect_success 'git reflog show' '
> > + test_when_finished "git checkout -" &&
> > + git checkout -b shown &&
> > + test_completion "git reflog sho" "show " &&
>
> IOW, shouldn't this offer both show and shown?
What should we do?
When the user have a branch "show":
$ git checkout -b show
$ git reflog sho<TAB><TAB>
And with:
$ git reflog <TAB><TAB>
Should we suggest the subcommands alongside the branches?
I thought about this, and it's a can of worms. I concluded that a sane
an instructive for the user approach is to first try the subcommands and
if no option is found, then try the implicit "show".
Maybe I'm overthinking it ...
>
> > + test_completion "git reflog show sho" "shown " &&
> > + test_completion "git reflog shown sho" "shown "
> > +'
> > +
> > test_expect_success 'options with value' '
> > test_completion "git merge -X diff-algorithm=" <<-\EOF
Thanks.
^ permalink raw reply
* Re: [PATCH v2 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
From: John Cai @ 2024-01-26 20:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git
In-Reply-To: <xmqqwmrwm08t.fsf@gitster.g>
Hi Junio,
On 26 Jan 2024, at 13:13, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +--fsck-objects[=<msg-ids>=<severity>...]::
>> + Instructs index-pack to check for broken objects, but unlike `--strict`,
>> + does not choke on broken links. If `<msg-ids>` is passed, it should be
>> + a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
>> + `<severity>` are used to change the severity of `fsck` errors e.g.,
>> + `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
>
> In addition to the comment I made in my reply to 1/2, this should
> be `--fsck-objects="missingEmail=ignore,badTagName=ignore"`, I
> think. Will treak locally.
Good catch. I might as well re-roll this since I forgot to add a Reviewed-by:
Christian Couder <christian.couder@gmail.com> trailer to the commits.
>
> Thanks.
^ permalink raw reply
* git-for-windows: Rarely, checking out a branch will fast-forward that branch to the branch that was just switched from
From: Stan Stanislaus @ 2024-01-26 20:17 UTC (permalink / raw)
To: git
- [x] I was not able to find an open or closed issue matching what I'm seeing
Setup
Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options
git version 2.40.0.windows.1
C:\Users\StanStanislaus> git --version --build-options
git version 2.40.0.windows.1
cpu: x86_64
built from commit: 1d90ca2906dd4b7ddaf0669a13c173ec579d794a
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
Which version of Windows are you running? Vista, 7, 8, 10? Is it
32-bit or 64-bit?
git version 2.40.0.windows.1 Since it's in the Program Files folder
and not the Program Files (x86) folder I'm going to say 64-bit.
$ cmd.exe /c ver
** Microsoft Windows [Version 10.0.22631.2861] **
What options did you set as part of the installation? Or did you choose the
defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
> type "$env:USERPROFILE\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt
** Editor Option: VIM
Custom Editor Path:
Default Branch Option:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Enabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled
**
Any other interesting things about your environment that might be related
to the issue you're seeing?
** Using poshgit for PS **
Details
Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other
** PS **
What commands did you run to trigger this issue? If you can provide a
Minimal, Complete, and Verifiable example
this will help us understand the issue.
# m => git stash; git checkout main; git pull
function Invoke-Git-Stash-Pull-Main {
$ErrorActionPreference = "Stop"
& git stash
& git checkout main
& git pull
Write-Host Ran $MyInvocation.MyCommand from `$profile
}
New-Alias -Name m -Value Invoke-Git-Stash-Pull-Main
I'm on: git version 2.40.0.windows.1**
What did you expect to occur after running these commands?
** I expected the main branch to be checked out in the same state I left it.**
What actually happened instead?
Very intermittently and not consistently reproducibly, main is checked
out but fast-forwarded to the state of the branch I just left by
checking main out. This issue seems to be the main issue here, too:
mhutchie/vscode-git-graph#284
I also posted this on the github.com/git-for-windows repo:
https://github.com/git-for-windows/git/issues/4778
═══════════════════
Stan (David) G. Stanislaus
═══════════════════
^ permalink raw reply
* Re: [PATCH 1/4] completion: introduce __gitcomp_subcommand
From: Rubén Justo @ 2024-01-26 20:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqmsssngyz.fsf@gitster.g>
On 26-ene-2024 09:26:44, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > +# Completion for subcommands in commands that follow the syntax:
> > +#
> > +# git <command> <subcommand>
> > +#
> > +# 1: List of possible completion words.
> > +# Returns false if the current word is not a possible subcommand
> > +# (possitioned after the command), or if no option is found in
> > +# the list provided.
> > +__gitcomp_subcommand ()
> > +{
> > + local subcommands="$1"
> > +
> > + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then
> > + __gitcomp "$subcommands"
> > +
> > + test -n "$COMPREPLY"
> > + else
> > + false
> > + fi
> > +}
>
>
> I am not at all familiar with the code in this file, so treat this
> as a question from somebody who do not know the calling convention
> used around here.
>
> This helper function clobbers what was in COMPREPLY[@] before
> calling it, from a caller's point of view. Is it a pattern that
> potential callers in this file should already find familiar, and
> they do not have to be reminded that they cannot rely on what they
> prepared in COMPREPLY to be preserved across a call into this
> function? Otherwise I would suggest mentioning it in the helpful
> comment before the function, but I cannot tell if such a comment is
> even needed by the intended audience, so...
Maybe adding such a comment might suggest at first glance that we're
working different here than in the rest of the __gitcomp_* family of
functions, which is not the intention ... I dunno.
^ permalink raw reply
* Re: [PATCH v2 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
From: Junio C Hamano @ 2024-01-26 18:13 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <cce63c6465fb1e29252d7e0918e03ff0f08d37f4.1706289180.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +--fsck-objects[=<msg-ids>=<severity>...]::
> + Instructs index-pack to check for broken objects, but unlike `--strict`,
> + does not choke on broken links. If `<msg-ids>` is passed, it should be
> + a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
> + `<severity>` are used to change the severity of `fsck` errors e.g.,
> + `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
In addition to the comment I made in my reply to 1/2, this should
be `--fsck-objects="missingEmail=ignore,badTagName=ignore"`, I
think. Will treak locally.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/2] index-pack: test and document --strict=<msg>
From: Junio C Hamano @ 2024-01-26 18:03 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <b3b3e8bd0bf2c83b57debef81edc39970beaf05b.1706289180.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: John Cai <johncai86@gmail.com>
>
> 5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
> 2015-06-22) allowed a list of fsck msg to downgrade to be passed to
> --strict. However this is a hidden argument that was not documented nor
> tested. Though it is true that most users would not call this option
> directly, (nor use index-pack for that matter) it is still useful to
> document and test this feature.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> Documentation/git-index-pack.txt | 9 +++++++--
> builtin/index-pack.c | 2 +-
> t/t5300-pack-object.sh | 22 ++++++++++++++++++++++
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
> index 6486620c3d8..f7a98bbf9c8 100644
> --- a/Documentation/git-index-pack.txt
> +++ b/Documentation/git-index-pack.txt
> @@ -79,8 +79,13 @@ OPTIONS
> to force the version for the generated pack index, and to force
> 64-bit index entries on objects located above the given offset.
>
> ---strict::
> - Die, if the pack contains broken objects or links.
> +--strict[=<msg-id>=<severity>...]::
> + Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
> + it should be a comma-separated list of `<msg-id>=<severity>` elements where
> + `<msg-id>` and `<severity>` are used to change the severity of some possible
> + issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry
There no longer is <msg-ids>, so I'll tweak the text perhaps like so:
An optional value that is a comma-separated list of '<msg-id>=<severity>'
can be passed to change the severity of some possible issues, ...
while queueing. Will probably do the same for the --fsck-objects
side in the next patch.
Other than that, thanks for a pleasant read.
> + for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
> + more information on the possible values of `<msg-id>` and `<severity>`.
>
> --progress-title::
> For internal use only.
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 1ea87e01f29..1e53ca23775 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -24,7 +24,7 @@
> #include "setup.h"
>
> static const char index_pack_usage[] =
> -"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
> +"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
>
> struct object_entry {
> struct pack_idx_entry idx;
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index d402ec18b79..496fffa0f8a 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -441,6 +441,28 @@ test_expect_success 'index-pack with --strict' '
> )
> '
>
> +test_expect_success 'index-pack with --strict downgrading fsck msgs' '
> + test_when_finished rm -rf strict &&
> + git init strict &&
> + (
> + cd strict &&
> + test_commit first hello &&
> + cat >commit <<-EOF &&
> + tree $(git rev-parse HEAD^{tree})
> + parent $(git rev-parse HEAD)
> + author A U Thor
> + committer A U Thor
> +
> + commit: this is a commit with bad emails
> +
> + EOF
> + git hash-object --literally -t commit -w --stdin <commit >commit_list &&
> + PACK=$(git pack-objects test <commit_list) &&
> + test_must_fail git index-pack --strict "test-$PACK.pack" &&
> + git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
> + )
> +'
> +
> test_expect_success 'honor pack.packSizeLimit' '
> git config pack.packSizeLimit 3m &&
> packname_10=$(git pack-objects test-10 <obj-list) &&
^ permalink raw reply
* Re: [PATCH 3/4] completion: reflog with implicit "show"
From: Junio C Hamano @ 2024-01-26 17:57 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
In-Reply-To: <5991b58c-770c-4aaa-bce5-f396d9f7f16f@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> When no subcommand is specified to "reflog", we assume "show" [1]:
>
> $ git reflog -h
> usage: git reflog [show] [<log-options>] [<ref>]
> ...
>
> We are not completing correctly this implicit uses of "show":
>
> With ...
>
> $ git checkout -b default
>
> ... we are not completing "default":
>
> $ git reflog def<TAB><TAB>
>
> And we are incorrectly returning the "subcommands" when:
>
> $ git reflog default <TAB><TAB>
> delete expire show
>
> Let's use __gitcomp_subcommand to correctly handle implicit
> subcommands.
As with a good bug report, if you are showing an "incorrect"
behaviour, you should also illustrate what you think is the
"correct" behaviour (see below).
> 1. cf39f54efc (git reflog show, 2007-02-08)
>
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
> contrib/completion/git-completion.bash | 9 ++++-----
> t/t9902-completion.sh | 8 ++++++++
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5f2e904b56..c41f25aa80 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2448,13 +2448,12 @@ _git_rebase ()
> _git_reflog ()
> {
> local subcommands="show delete expire"
> - local subcommand="$(__git_find_on_cmdline "$subcommands")"
>
> - if [ -z "$subcommand" ]; then
> - __gitcomp "$subcommands"
> - else
> - __git_complete_refs
> + if __gitcomp_subcommand "$subcommands"; then
> + return
> fi
> +
> + __git_complete_refs
> }
So, when we see something that could be a subcommand we complete it
as a subcommand and return. In your example, how should
$ git reflog def<TAB>
work? We try to see if there is a subcommand that begins with
"def", we see nothing matching, and then run __git_complete_refs?
What if the branch you created earlier were not named "default" but,
say, "delmonte", and you did "git reflog del<TAB>"? Shouldn't the
user be offered "delete" and "delmonte" as potential completions?
> __git_send_email_confirm_options="always never auto cc compose"
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index aa9a614de3..231e17f378 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -2618,6 +2618,14 @@ test_expect_success 'git clone --config= - value' '
> EOF
> '
>
> +test_expect_success 'git reflog show' '
> + test_when_finished "git checkout -" &&
> + git checkout -b shown &&
> + test_completion "git reflog sho" "show " &&
IOW, shouldn't this offer both show and shown?
> + test_completion "git reflog show sho" "shown " &&
> + test_completion "git reflog shown sho" "shown "
> +'
> +
> test_expect_success 'options with value' '
> test_completion "git merge -X diff-algorithm=" <<-\EOF
^ permalink raw reply
* Re: [PATCH 2/4] completion: introduce __git_find_subcommand
From: Junio C Hamano @ 2024-01-26 17:30 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
In-Reply-To: <221f88b9-fc91-479f-8d08-f530796e2d13@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> Let's have a function to get the current subcommand when completing
> commands that follow the syntax:
>
> git <command> <subcommand>
>
> As a convenience, let's allow an optional "default subcommand" to be
> returned if none is found.
>
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
> contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 916e137021..5f2e904b56 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -575,6 +575,26 @@ __gitcomp_subcommand ()
> fi
> }
>
> +# Find the current subcommand for commands that follow the syntax:
> +#
> +# git <command> <subcommand>
> +#
> +# 1: List of possible subcommands.
> +# 2: Optional subcommand to return when none is found.
> +__git_find_subcommand ()
> +{
> + local subcommand subcommands="$1" default_subcommand="$2"
Are the callers expected to form "$1" by concatenating known tokens
with a space?
I am just wondering if we can avoid looping, e.g.
local nextword=${words[__git_cmd_idx+1]}
case " $subcommands " in
*" $nextword "*)
echo "$nextword"
return
;;
esac
It hopefully should not be a huge deal either way, though.
> +
> + for subcommand in $subcommands; do
> + if [ "$subcommand" = "${words[__git_cmd_idx+1]}" ]; then
> + echo $subcommand
> + return
> + fi
> + done
> +
> + echo $default_subcommand
> +}
> +
> # Execute 'git ls-files', unless the --committable option is specified, in
> # which case it runs 'git diff-index' to find out the files that can be
> # committed. It return paths relative to the directory specified in the first
^ permalink raw reply
* Re: [PATCH 1/4] completion: introduce __gitcomp_subcommand
From: Junio C Hamano @ 2024-01-26 17:26 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
In-Reply-To: <8c902c53-815d-43c2-8ba9-8144d8884804@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> +# Completion for subcommands in commands that follow the syntax:
> +#
> +# git <command> <subcommand>
> +#
> +# 1: List of possible completion words.
> +# Returns false if the current word is not a possible subcommand
> +# (possitioned after the command), or if no option is found in
> +# the list provided.
> +__gitcomp_subcommand ()
> +{
> + local subcommands="$1"
> +
> + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then
> + __gitcomp "$subcommands"
> +
> + test -n "$COMPREPLY"
> + else
> + false
> + fi
> +}
I am not at all familiar with the code in this file, so treat this
as a question from somebody who do not know the calling convention
used around here.
This helper function clobbers what was in COMPREPLY[@] before
calling it, from a caller's point of view. Is it a pattern that
potential callers in this file should already find familiar, and
they do not have to be reminded that they cannot rely on what they
prepared in COMPREPLY to be preserved across a call into this
function? Otherwise I would suggest mentioning it in the helpful
comment before the function, but I cannot tell if such a comment is
even needed by the intended audience, so...
^ permalink raw reply
* [PATCH v2 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
From: John Cai via GitGitGadget @ 2024-01-26 17:13 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1658.v2.git.git.1706289180.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
git-index-pack has a --strict option that can take an optional argument
to provide a list of fsck issues to change their severity.
--fsck-objects does not have such a utility, which would be useful if
one would like to be more lenient or strict on data integrity in a
repository.
Like --strict, allow --fsck-objects to also take a list of fsck msgs to
change the severity.
Remove the "For internal use only" note for --fsck-objects, and document
the option. This won't often be used by the normal end user, but it
turns out it is useful for Git forges like GitLab.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-index-pack.txt | 10 ++++++++--
builtin/index-pack.c | 5 +++--
t/t5300-pack-object.sh | 29 ++++++++++++++++++++++++-----
3 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index f7a98bbf9c8..916652d3b1b 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -96,8 +96,14 @@ default and "Indexing objects" when `--stdin` is specified.
--check-self-contained-and-connected::
Die if the pack contains broken links. For internal use only.
---fsck-objects::
- For internal use only.
+--fsck-objects[=<msg-ids>=<severity>...]::
+ Instructs index-pack to check for broken objects, but unlike `--strict`,
+ does not choke on broken links. If `<msg-ids>` is passed, it should be
+ a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
+ `<severity>` are used to change the severity of `fsck` errors e.g.,
+ `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
+ the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
+ more information on the possible values of `<msg-id>` and `<severity>`.
+
Die if the pack contains broken objects. If the pack contains a tree
pointing to a .gitmodules blob that does not exist, prints the hash of
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1e53ca23775..519162f5b91 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -24,7 +24,7 @@
#include "setup.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] [--fsck-objects[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
@@ -1785,8 +1785,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
} else if (!strcmp(arg, "--check-self-contained-and-connected")) {
strict = 1;
check_self_contained_and_connected = 1;
- } else if (!strcmp(arg, "--fsck-objects")) {
+ } else if (skip_to_optional_arg(arg, "--fsck-objects", &arg)) {
do_fsck_object = 1;
+ fsck_set_msg_types(&fsck_options, arg);
} else if (!strcmp(arg, "--verify")) {
verify = 1;
} else if (!strcmp(arg, "--verify-stat")) {
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 496fffa0f8a..a58f91035d1 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,8 +441,7 @@ test_expect_success 'index-pack with --strict' '
)
'
-test_expect_success 'index-pack with --strict downgrading fsck msgs' '
- test_when_finished rm -rf strict &&
+test_expect_success 'setup for --strict and --fsck-objects downgrading fsck msgs' '
git init strict &&
(
cd strict &&
@@ -457,12 +456,32 @@ test_expect_success 'index-pack with --strict downgrading fsck msgs' '
EOF
git hash-object --literally -t commit -w --stdin <commit >commit_list &&
- PACK=$(git pack-objects test <commit_list) &&
- test_must_fail git index-pack --strict "test-$PACK.pack" &&
- git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
+ git pack-objects test <commit_list >pack-name
)
'
+test_with_bad_commit () {
+ must_fail_arg="$1" &&
+ must_pass_arg="$2" &&
+ (
+ cd strict &&
+ test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack"
+ git index-pack "$must_pass_arg" "test-$(cat pack-name).pack"
+ )
+}
+
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+ test_with_bad_commit --strict --strict="missingEmail=ignore"
+'
+
+test_expect_success 'index-pack with --fsck-objects downgrading fsck msgs' '
+ test_with_bad_commit --fsck-objects --fsck-objects="missingEmail=ignore"
+'
+
+test_expect_success 'cleanup for --strict and --fsck-objects downgrading fsck msgs' '
+ rm -rf strict
+'
+
test_expect_success 'honor pack.packSizeLimit' '
git config pack.packSizeLimit 3m &&
packname_10=$(git pack-objects test-10 <obj-list) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/2] index-pack: test and document --strict=<msg>
From: John Cai via GitGitGadget @ 2024-01-26 17:12 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1658.v2.git.git.1706289180.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
--strict. However this is a hidden argument that was not documented nor
tested. Though it is true that most users would not call this option
directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-index-pack.txt | 9 +++++++--
builtin/index-pack.c | 2 +-
t/t5300-pack-object.sh | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 6486620c3d8..f7a98bbf9c8 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -79,8 +79,13 @@ OPTIONS
to force the version for the generated pack index, and to force
64-bit index entries on objects located above the given offset.
---strict::
- Die, if the pack contains broken objects or links.
+--strict[=<msg-id>=<severity>...]::
+ Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
+ it should be a comma-separated list of `<msg-id>=<severity>` elements where
+ `<msg-id>` and `<severity>` are used to change the severity of some possible
+ issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry
+ for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
+ more information on the possible values of `<msg-id>` and `<severity>`.
--progress-title::
For internal use only.
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1ea87e01f29..1e53ca23775 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -24,7 +24,7 @@
#include "setup.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index d402ec18b79..496fffa0f8a 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,6 +441,28 @@ test_expect_success 'index-pack with --strict' '
)
'
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+ test_when_finished rm -rf strict &&
+ git init strict &&
+ (
+ cd strict &&
+ test_commit first hello &&
+ cat >commit <<-EOF &&
+ tree $(git rev-parse HEAD^{tree})
+ parent $(git rev-parse HEAD)
+ author A U Thor
+ committer A U Thor
+
+ commit: this is a commit with bad emails
+
+ EOF
+ git hash-object --literally -t commit -w --stdin <commit >commit_list &&
+ PACK=$(git pack-objects test <commit_list) &&
+ test_must_fail git index-pack --strict "test-$PACK.pack" &&
+ git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
+ )
+'
+
test_expect_success 'honor pack.packSizeLimit' '
git config pack.packSizeLimit 3m &&
packname_10=$(git pack-objects test-10 <obj-list) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/2] index-pack: fsck honor checks
From: John Cai via GitGitGadget @ 2024-01-26 17:12 UTC (permalink / raw)
To: git; +Cc: John Cai
In-Reply-To: <pull.1658.git.git.1706215884.gitgitgadget@gmail.com>
git-index-pack has a --strict mode that can take an optional argument to
provide a list of fsck issues to change their severity. --fsck-objects does
not have such a utility, which would be useful if one would like to be more
lenient or strict on data integrity in a repository.
Like --strict, Allow --fsck-objects to also take a list of fsck msgs to
change the severity.
Change since V1:
* edited commit messages
* clarified formatting in documentation for --strict= and --fsck-objects=
John Cai (2):
index-pack: test and document --strict=<msg>
index-pack: --fsck-objects to take an optional argument for fsck msgs
Documentation/git-index-pack.txt | 19 +++++++++++----
builtin/index-pack.c | 5 ++--
t/t5300-pack-object.sh | 41 ++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 6 deletions(-)
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1658%2Fjohn-cai%2Fjc%2Findex-pack-fsck-honor-checks-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1658/john-cai/jc/index-pack-fsck-honor-checks-v2
Pull-Request: https://github.com/git/git/pull/1658
Range-diff vs v1:
1: 9b353aff73d ! 1: b3b3e8bd0bf index-pack: test and document --strict=<msg>
@@ Commit message
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
--strict. However this is a hidden argument that was not documented nor
- tested. Though true that most users would not call this option
- direction, (nor use index-pack for that matter) it is still useful to
+ tested. Though it is true that most users would not call this option
+ directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
Signed-off-by: John Cai <johncai86@gmail.com>
@@ Documentation/git-index-pack.txt: OPTIONS
---strict::
- Die, if the pack contains broken objects or links.
-+--strict[=<msg-ids>]::
++--strict[=<msg-id>=<severity>...]::
+ Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
+ it should be a comma-separated list of `<msg-id>=<severity>` elements where
+ `<msg-id>` and `<severity>` are used to change the severity of some possible
-+ issues, eg: `--strict="missingEmail=ignore,badTagName=error"`. See the entry
++ issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry
+ for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
+ more information on the possible values of `<msg-id>` and `<severity>`.
@@ t/t5300-pack-object.sh: test_expect_success 'index-pack with --strict' '
+ author A U Thor
+ committer A U Thor
+
-+ commit: this is a commit wit bad emails
++ commit: this is a commit with bad emails
+
+ EOF
+ git hash-object --literally -t commit -w --stdin <commit >commit_list &&
2: 074e0c7ab92 ! 2: cce63c6465f index-pack: --fsck-objects to take an optional argument for fsck msgs
@@ Metadata
## Commit message ##
index-pack: --fsck-objects to take an optional argument for fsck msgs
- git-index-pack has a --strict mode that can take an optional argument to
- provide a list of fsck issues to change their severity. --fsck-objects
- does not have such a utility, which would be useful if one would like to
- be more lenient or strict on data integrity in a repository.
+ git-index-pack has a --strict option that can take an optional argument
+ to provide a list of fsck issues to change their severity.
+ --fsck-objects does not have such a utility, which would be useful if
+ one would like to be more lenient or strict on data integrity in a
+ repository.
- Like --strict, Allow --fsck-objects to also take a list of fsck msgs to
+ Like --strict, allow --fsck-objects to also take a list of fsck msgs to
change the severity.
- This commit also removes the "For internal use only" note for
- --fsck-objects, and documents the option. This won't often be used by
- the normal end user, but it turns out it is useful for Git forges like
- GitLab.
+ Remove the "For internal use only" note for --fsck-objects, and document
+ the option. This won't often be used by the normal end user, but it
+ turns out it is useful for Git forges like GitLab.
Signed-off-by: John Cai <johncai86@gmail.com>
@@ Documentation/git-index-pack.txt: default and "Indexing objects" when `--stdin`
---fsck-objects::
- For internal use only.
-+--fsck-objects[=<msg-ids>]::
-+ Instructs index-pack to check for broken objects instead of broken
-+ links. If `<msg-ids>` is passed, it should be a comma-separated list of
-+ `<msg-id>=<severity>` where `<msg-id>` and `<severity>` are used to
-+ change the severity of `fsck` errors, eg: `--strict="missingEmail=ignore,badTagName=ignore"`.
-+ See the entry for the `fsck.<msg-id>` configuration options in
-+ `linkgit:git-fsck[1] for more information on the possible values of
-+ `<msg-id>` and `<severity>`.
++--fsck-objects[=<msg-ids>=<severity>...]::
++ Instructs index-pack to check for broken objects, but unlike `--strict`,
++ does not choke on broken links. If `<msg-ids>` is passed, it should be
++ a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
++ `<severity>` are used to change the severity of `fsck` errors e.g.,
++ `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
++ the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
++ more information on the possible values of `<msg-id>` and `<severity>`.
+
Die if the pack contains broken objects. If the pack contains a tree
pointing to a .gitmodules blob that does not exist, prints the hash of
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/5] reftable: fix writing multi-level indices
From: Junio C Hamano @ 2024-01-26 16:26 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1706263918.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> yesterday I noticed that writing reftables with more than 250,000 refs
> led to the last block of refs in the table not being seekable anymore.
> As it turns out, 250k refs was the boundary at which we start to write
> multi-level indices for refs.
Obviously one of the less exercised corners of the code ;-)
> The topic depends on Junio's jc/reftable-core-fsync at 1df18a1c9a
> (reftable: honor core.fsync, 2024-01-23) due to a semantic merge
> conflict in the newly added test.
Thanks for the note for the base. John Cai's work is well done and
has been reviewed, so let's merge it down to 'next' soonish.
Thanks.
^ permalink raw reply
* Re: [PATCH] merge-tree: accept 3 trees as arguments
From: Junio C Hamano @ 2024-01-26 16:01 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1647.git.1706277694231.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When specifying a merge base explicitly, there is actually no good
> reason why the inputs need to be commits: that's only needed if the
> merge base has to be deduced from the commit graph.
yes, Yes, YES, YEAHHHHH!
> I was asked to implement this at $dayjob and it seems like a feature
> that might be useful to other users, too.
Yup, I think it is an obvious building block for machinery to
perform any mergy operation to grow history. Many of the time you
may have a commit, but requiring them to be commits when you know
you will not do a virtual ancestor synthesis smells fundamentally
wrong. Thanks for fixing it.
> ---merge-base=<commit>::
> +--merge-base=<tree-ish>::
> Instead of finding the merge-bases for <branch1> and <branch2>,
> specify a merge-base for the merge, and specifying multiple bases is
> currently not supported. This option is incompatible with `--stdin`.
> ++
> +As the merge-base is provided directly, <branch1> and <branch2> do not need
> +o specify commits; it is sufficient if they specify trees.
"... do not need to specify commits; trees are enough"?
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 3bdec53fbe5..cbd8e15af6d 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -429,35 +429,43 @@ static int real_merge(struct merge_tree_options *o,
> struct merge_options opt;
>
> copy_merge_options(&opt, &o->merge_options);
> - parent1 = get_merge_parent(branch1);
> - if (!parent1)
> - help_unknown_ref(branch1, "merge-tree",
> - _("not something we can merge"));
> -
> - parent2 = get_merge_parent(branch2);
> - if (!parent2)
> - help_unknown_ref(branch2, "merge-tree",
> - _("not something we can merge"));
> -
> opt.show_rename_progress = 0;
>
> opt.branch1 = branch1;
> opt.branch2 = branch2;
>
> if (merge_base) {
> - struct commit *base_commit;
> struct tree *base_tree, *parent1_tree, *parent2_tree;
>
> - base_commit = lookup_commit_reference_by_name(merge_base);
> - if (!base_commit)
> - die(_("could not lookup commit '%s'"), merge_base);
> + /*
> + * We actually only need the trees because we already
> + * have a merge base.
> + */
> + struct object_id base_oid, head_oid, merge_oid;
> +
> + if (repo_get_oid_treeish(the_repository, merge_base, &base_oid))
> + die(_("could not parse as tree '%s'"), merge_base);
> + base_tree = parse_tree_indirect(&base_oid);
> + if (repo_get_oid_treeish(the_repository, branch1, &head_oid))
> + die(_("could not parse as tree '%s'"), branch1);
> + parent1_tree = parse_tree_indirect(&head_oid);
> + if (repo_get_oid_treeish(the_repository, branch2, &merge_oid))
> + die(_("could not parse as tree '%s'"), branch2);
> + parent2_tree = parse_tree_indirect(&merge_oid);
>
> opt.ancestor = merge_base;
> - base_tree = repo_get_commit_tree(the_repository, base_commit);
> - parent1_tree = repo_get_commit_tree(the_repository, parent1);
> - parent2_tree = repo_get_commit_tree(the_repository, parent2);
> merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);
> } else {
> + parent1 = get_merge_parent(branch1);
> + if (!parent1)
> + help_unknown_ref(branch1, "merge-tree",
> + _("not something we can merge"));
> +
> + parent2 = get_merge_parent(branch2);
> + if (!parent2)
> + help_unknown_ref(branch2, "merge-tree",
> + _("not something we can merge"));
> +
> /*
> * Get the merge bases, in reverse order; see comment above
> * merge_incore_recursive in merge-ort.h
OK. The changes here look quite straight-forward.
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index 12ac4368736..71f21bb834f 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -945,4 +945,12 @@ test_expect_success 'check the input format when --stdin is passed' '
> test_cmp expect actual
> '
>
> +test_expect_success '--merge-base with tree OIDs' '
> + git merge-tree --merge-base=side1^ side1 side3 >tree &&
> + tree=$(cat tree) &&
> + git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >tree2 &&
> + tree2=$(cat tree2) &&
> + test $tree = $tree2
> +'
You do not need $tree and $tree2 variables that would make it harder
to diagnose a failure case when we break merge-tree. You have tree
and tree2 as files, and I think it is sufficient to do
git merge-tree ... >result-from-commits &&
git merge-tree ... >result-from-trees &&
test_cmp result-from-commits result-from-trees
(no, I am not suggesting to rename these tree and tree2 files; I
just needed them to be descriptive in my illustration to show what
is going on to myself).
Thanks.
^ permalink raw reply
* Re: [PATCH] merge-tree: accept 3 trees as arguments
From: Eric Sunshine @ 2024-01-26 14:15 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1647.git.1706277694231.gitgitgadget@gmail.com>
On Fri, Jan 26, 2024 at 9:01 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> When specifying a merge base explicitly, there is actually no good
> reason why the inputs need to be commits: that's only needed if the
> merge base has to be deduced from the commit graph.
>
> This commit is best viewed with `--color-moved
> --color-moved-ws=allow-indentation-change`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
> @@ -64,10 +64,13 @@ OPTIONS
> +--merge-base=<tree-ish>::
> Instead of finding the merge-bases for <branch1> and <branch2>,
> specify a merge-base for the merge, and specifying multiple bases is
> currently not supported. This option is incompatible with `--stdin`.
> ++
> +As the merge-base is provided directly, <branch1> and <branch2> do not need
> +o specify commits; it is sufficient if they specify trees.
presumably: s/o/to/
^ permalink raw reply
* [PATCH] merge-tree: accept 3 trees as arguments
From: Johannes Schindelin via GitGitGadget @ 2024-01-26 14:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When specifying a merge base explicitly, there is actually no good
reason why the inputs need to be commits: that's only needed if the
merge base has to be deduced from the commit graph.
This commit is best viewed with `--color-moved
--color-moved-ws=allow-indentation-change`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
merge-tree: accept 3 trees as arguments
I was asked to implement this at $dayjob and it seems like a feature
that might be useful to other users, too.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1647%2Fdscho%2Fallow-merge-tree-to-accept-3-trees-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1647/dscho/allow-merge-tree-to-accept-3-trees-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1647
Documentation/git-merge-tree.txt | 5 +++-
builtin/merge-tree.c | 42 +++++++++++++++++++-------------
t/t4301-merge-tree-write-tree.sh | 8 ++++++
3 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index b50acace3bc..214e30c70ba 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -64,10 +64,13 @@ OPTIONS
share no common history. This flag can be given to override that
check and make the merge proceed anyway.
---merge-base=<commit>::
+--merge-base=<tree-ish>::
Instead of finding the merge-bases for <branch1> and <branch2>,
specify a merge-base for the merge, and specifying multiple bases is
currently not supported. This option is incompatible with `--stdin`.
++
+As the merge-base is provided directly, <branch1> and <branch2> do not need
+o specify commits; it is sufficient if they specify trees.
[[OUTPUT]]
OUTPUT
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 3bdec53fbe5..cbd8e15af6d 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -429,35 +429,43 @@ static int real_merge(struct merge_tree_options *o,
struct merge_options opt;
copy_merge_options(&opt, &o->merge_options);
- parent1 = get_merge_parent(branch1);
- if (!parent1)
- help_unknown_ref(branch1, "merge-tree",
- _("not something we can merge"));
-
- parent2 = get_merge_parent(branch2);
- if (!parent2)
- help_unknown_ref(branch2, "merge-tree",
- _("not something we can merge"));
-
opt.show_rename_progress = 0;
opt.branch1 = branch1;
opt.branch2 = branch2;
if (merge_base) {
- struct commit *base_commit;
struct tree *base_tree, *parent1_tree, *parent2_tree;
- base_commit = lookup_commit_reference_by_name(merge_base);
- if (!base_commit)
- die(_("could not lookup commit '%s'"), merge_base);
+ /*
+ * We actually only need the trees because we already
+ * have a merge base.
+ */
+ struct object_id base_oid, head_oid, merge_oid;
+
+ if (repo_get_oid_treeish(the_repository, merge_base, &base_oid))
+ die(_("could not parse as tree '%s'"), merge_base);
+ base_tree = parse_tree_indirect(&base_oid);
+ if (repo_get_oid_treeish(the_repository, branch1, &head_oid))
+ die(_("could not parse as tree '%s'"), branch1);
+ parent1_tree = parse_tree_indirect(&head_oid);
+ if (repo_get_oid_treeish(the_repository, branch2, &merge_oid))
+ die(_("could not parse as tree '%s'"), branch2);
+ parent2_tree = parse_tree_indirect(&merge_oid);
opt.ancestor = merge_base;
- base_tree = repo_get_commit_tree(the_repository, base_commit);
- parent1_tree = repo_get_commit_tree(the_repository, parent1);
- parent2_tree = repo_get_commit_tree(the_repository, parent2);
merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);
} else {
+ parent1 = get_merge_parent(branch1);
+ if (!parent1)
+ help_unknown_ref(branch1, "merge-tree",
+ _("not something we can merge"));
+
+ parent2 = get_merge_parent(branch2);
+ if (!parent2)
+ help_unknown_ref(branch2, "merge-tree",
+ _("not something we can merge"));
+
/*
* Get the merge bases, in reverse order; see comment above
* merge_incore_recursive in merge-ort.h
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 12ac4368736..71f21bb834f 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -945,4 +945,12 @@ test_expect_success 'check the input format when --stdin is passed' '
test_cmp expect actual
'
+test_expect_success '--merge-base with tree OIDs' '
+ git merge-tree --merge-base=side1^ side1 side3 >tree &&
+ tree=$(cat tree) &&
+ git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >tree2 &&
+ tree2=$(cat tree2) &&
+ test $tree = $tree2
+'
+
test_done
base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/4] completion: reflog show <log-options>
From: Rubén Justo @ 2024-01-26 12:53 UTC (permalink / raw)
To: Git List
In-Reply-To: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
Let's add completion for <log-options> in "reflog show" so that the user
can easily discover uses like:
$ git reflog --since=1.day.ago
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 13 ++++++++++++-
t/t9902-completion.sh | 5 ++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c41f25aa80..3deb98389c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2447,12 +2447,23 @@ _git_rebase ()
_git_reflog ()
{
- local subcommands="show delete expire"
+ local subcommand subcommands="show delete expire"
if __gitcomp_subcommand "$subcommands"; then
return
fi
+ subcommand="$(__git_find_subcommand "$subcommands" "show")"
+
+ case "$subcommand,$cur" in
+ show,--*)
+ __gitcomp "
+ $__git_log_common_options
+ "
+ return
+ ;;
+ esac
+
__git_complete_refs
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 231e17f378..a74d774168 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2623,7 +2623,10 @@ test_expect_success 'git reflog show' '
git checkout -b shown &&
test_completion "git reflog sho" "show " &&
test_completion "git reflog show sho" "shown " &&
- test_completion "git reflog shown sho" "shown "
+ test_completion "git reflog shown sho" "shown " &&
+ test_completion "git reflog --unt" "--until=" &&
+ test_completion "git reflog show --unt" "--until=" &&
+ test_completion "git reflog shown --unt" "--until="
'
test_expect_success 'options with value' '
--
2.43.0
^ permalink raw reply related
* [PATCH 3/4] completion: reflog with implicit "show"
From: Rubén Justo @ 2024-01-26 12:53 UTC (permalink / raw)
To: Git List
In-Reply-To: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
When no subcommand is specified to "reflog", we assume "show" [1]:
$ git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
...
We are not completing correctly this implicit uses of "show":
With ...
$ git checkout -b default
... we are not completing "default":
$ git reflog def<TAB><TAB>
And we are incorrectly returning the "subcommands" when:
$ git reflog default <TAB><TAB>
delete expire show
Let's use __gitcomp_subcommand to correctly handle implicit
subcommands.
1. cf39f54efc (git reflog show, 2007-02-08)
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 9 ++++-----
t/t9902-completion.sh | 8 ++++++++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5f2e904b56..c41f25aa80 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2448,13 +2448,12 @@ _git_rebase ()
_git_reflog ()
{
local subcommands="show delete expire"
- local subcommand="$(__git_find_on_cmdline "$subcommands")"
- if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
- else
- __git_complete_refs
+ if __gitcomp_subcommand "$subcommands"; then
+ return
fi
+
+ __git_complete_refs
}
__git_send_email_confirm_options="always never auto cc compose"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..231e17f378 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2618,6 +2618,14 @@ test_expect_success 'git clone --config= - value' '
EOF
'
+test_expect_success 'git reflog show' '
+ test_when_finished "git checkout -" &&
+ git checkout -b shown &&
+ test_completion "git reflog sho" "show " &&
+ test_completion "git reflog show sho" "shown " &&
+ test_completion "git reflog shown sho" "shown "
+'
+
test_expect_success 'options with value' '
test_completion "git merge -X diff-algorithm=" <<-\EOF
--
2.43.0
^ permalink raw reply related
* [PATCH 2/4] completion: introduce __git_find_subcommand
From: Rubén Justo @ 2024-01-26 12:51 UTC (permalink / raw)
To: Git List
In-Reply-To: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
Let's have a function to get the current subcommand when completing
commands that follow the syntax:
git <command> <subcommand>
As a convenience, let's allow an optional "default subcommand" to be
returned if none is found.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 916e137021..5f2e904b56 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -575,6 +575,26 @@ __gitcomp_subcommand ()
fi
}
+# Find the current subcommand for commands that follow the syntax:
+#
+# git <command> <subcommand>
+#
+# 1: List of possible subcommands.
+# 2: Optional subcommand to return when none is found.
+__git_find_subcommand ()
+{
+ local subcommand subcommands="$1" default_subcommand="$2"
+
+ for subcommand in $subcommands; do
+ if [ "$subcommand" = "${words[__git_cmd_idx+1]}" ]; then
+ echo $subcommand
+ return
+ fi
+ done
+
+ echo $default_subcommand
+}
+
# Execute 'git ls-files', unless the --committable option is specified, in
# which case it runs 'git diff-index' to find out the files that can be
# committed. It return paths relative to the directory specified in the first
--
2.43.0
^ permalink raw reply related
* [PATCH 1/4] completion: introduce __gitcomp_subcommand
From: Rubén Justo @ 2024-01-26 12:51 UTC (permalink / raw)
To: Git List
In-Reply-To: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
Let's have a function to complete "subcommands" only in the correct
position (right after the command), in commands that follow the syntax:
git <command> <subcommand>
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c40ade494..916e137021 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -554,6 +554,27 @@ __gitcomp_file ()
true
}
+# Completion for subcommands in commands that follow the syntax:
+#
+# git <command> <subcommand>
+#
+# 1: List of possible completion words.
+# Returns false if the current word is not a possible subcommand
+# (possitioned after the command), or if no option is found in
+# the list provided.
+__gitcomp_subcommand ()
+{
+ local subcommands="$1"
+
+ if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then
+ __gitcomp "$subcommands"
+
+ test -n "$COMPREPLY"
+ else
+ false
+ fi
+}
+
# Execute 'git ls-files', unless the --committable option is specified, in
# which case it runs 'git diff-index' to find out the files that can be
# committed. It return paths relative to the directory specified in the first
--
2.43.0
^ permalink raw reply related
* [PATCH 0/4] completion for git-reflog show
From: Rubén Justo @ 2024-01-26 12:46 UTC (permalink / raw)
To: Git List
When no subcommand is specified to "reflog" we assume "show" [1]:
$ git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
...
We are not completing correctly this implicit uses of "show":
With ...
$ git checkout -b default
... we are not completing "default":
$ git reflog def<TAB><TAB>
And we are incorrectly returning the "subcommands" when:
$ git reflog default <TAB><TAB>
delete expire show
This series fixes this and also adds completion for <log-options> in
"reflog show", so that the user can easily discover uses like:
$ git reflog --since=1.day.ago
1. cf39f54efc (git reflog show, 2007-02-08)
Rubén Justo (4):
completion: introduce __gitcomp_subcommand
completion: introduce __git_find_subcommand
completion: reflog with implicit "show"
completion: reflog show <log-options>
contrib/completion/git-completion.bash | 63 +++++++++++++++++++++++---
t/t9902-completion.sh | 11 +++++
2 files changed, 68 insertions(+), 6 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: what should "git clean -n -f [-d] [-x] <pattern>" do?
From: Sergey Organov @ 2024-01-26 12:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, git
In-Reply-To: <xmqqzfwspmh0.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>> ..
>>>> ... If the original semantics
>>>> were "you must force with -f to do anything useful", instead of "you
>>>> must choose either forcing with -f or not doing with -n", then it
>>>> would have led to the above behaviour.
>>> ...
>>> If we agree on the behavior above for sane "dry run"...
>
> Not so fast. I said "if the original semantics were ... then it
> would have led to the above behaviour". As the original semantics
> were not, that conclusion does not stand.
OK, fine, then my point is that the original semantics if flawed.
>
> The "-n" option here were not added primarily as a dry-run option,
> and haven't been treated as such forever. As can be seen by the
> "you must give either -f or -n option, and it is an error to give
> neither" rule, from the end-user's point of view, it is a way to say
> "between do-it (-f) and do-not-do-it (-n), I choose the latter for
> this invocation".
Yep, and in my opinion this is even more a mistake than "-f -f".
> And in that context, an attempt to make "-f -f"
> mean a stronger form of forcing than "-f" was a mistake, because it
> makes your "I want to see what happens if I tried that opration that
> requires the stronger force" request impossible.
I believe this just emphasizes the original mistake of "-n" design
meaning something else than simple "dry run".
>
> And there are two equally valid ways to deal with this misfeature.
I rather see two almost independent misfeatures here, so I believe both
are to be addressed.
>
> One is to admit that "-f -f" was a mistake (which I already said),
> and a natural consequence of that admission is to introduce a more
> specific "in addition to what you do usually, this riskier operation
> is allowed" option (e.g., --nested-repo).
This addresses one of the two deficiencies I see, yes.
> This leads to a design that matches real world usage better, even if
> we did not have the "how to ask dry-run?" issue, because in the real
> world, when there are multiple "risky" things you may have to
> explicitly ask to enable, these things do not necessarily form a nice
> linear "riskiness levels" that you can express your risk tolerance
> with the number of "-f" options. When you need to add special
> protection for a new case other than "nested repo", for example, the
> "riskiness levels" design may need to place it above the "nested repo"
> level of riskiness and may require the user to give three "-f"
> options, but that would make it impossible to protect against nuking
> of nested repos while allowing only that newly added case. By having
> more specific "this particular risky operation is allowed", "-f" can
> still be "between do-it and do-not-do-it, I choose the former",
Yep, makes sense.
> and the "--nested-repo" (and other options to allow specific risky
> operations we add in the future) would not have to have funny
> interactions with "-n".
Yep, but it still leaves "-n" being defective, as it for whatever reason
surprisingly clashes with "-f". I believe it shouldn't.
> The other valid way is to treat the use of the "riskiness levels" to
> specify what is forced still as a good idea. If one comes from that
> position, the resulting UI would be consistent with what you have
> been advocating for. One or more "-f" will specify what kind of
> risky stuff are allowed, and "-n" will say whether the operation
> gets carried out or merely shown what would happen if "-n" weren't
> there.
I'm not arguing in favor of "-f -f". My point is that even if you fix
"-f -f", "-n" deficiency will still cry for fixing.
>
> It is just that I think "riskiness levels" I did in a0f4afbe (clean:
> require double -f options to nuke nested git repository and work
> tree, 2009-06-30) was an utter mistake, and that is why I feel very
> hesitant to agree with the design that still promotes it.
Again, I'm not arguing in favor of "-f -f", I'm rather neutral about it.
I'm still arguing in favor of fixing "-n", and I believe a fix is needed
independently from decision about "-f -f".
Thanks,
-- Sergey Organov
^ 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