git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Recommend "git gc --prune=now" instead of "git prune"
@ 2020-06-08  2:02 John Lin via GitGitGadget
  2020-06-08 10:17 ` Denton Liu
  2020-06-09  0:23 ` [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune` John Lin via GitGitGadget
  0 siblings, 2 replies; 9+ messages in thread
From: John Lin via GitGitGadget @ 2020-06-08  2:02 UTC (permalink / raw)
  To: git; +Cc: John Lin, John Lin

From: John Lin <johnlinp@gmail.com>

Signed-off-by: John Lin <johnlinp@gmail.com>
---
    Recommend "git gc --prune=now" instead of "git prune"
    
    Signed-off-by: John Lin johnlinp@gmail.com [johnlinp@gmail.com]
    
    Fix according to #642.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-652%2Fjohnlinp%2Ffix-git-gc-warning-message-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-652/johnlinp/fix-git-gc-warning-message-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/652

 builtin/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8e0b9cf41b3..74185eac917 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -692,7 +692,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 
 	if (auto_gc && too_many_loose_objects())
 		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
+			"run 'git gc --prune=now' to remove them."));
 
 	if (!daemonized)
 		unlink(git_path("gc.log"));

base-commit: 20514004ddf1a3528de8933bc32f284e175e1012
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Recommend "git gc --prune=now" instead of "git prune"
  2020-06-08  2:02 [PATCH] Recommend "git gc --prune=now" instead of "git prune" John Lin via GitGitGadget
@ 2020-06-08 10:17 ` Denton Liu
  2020-06-09  0:23 ` [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune` John Lin via GitGitGadget
  1 sibling, 0 replies; 9+ messages in thread
From: Denton Liu @ 2020-06-08 10:17 UTC (permalink / raw)
  To: John Lin via GitGitGadget; +Cc: git, John Lin

Hi John,

Thanks for working on this topic.

On Mon, Jun 08, 2020 at 02:02:18AM +0000, John Lin via GitGitGadget wrote:
> Subject: [PATCH] Recommend "git gc --prune=now" instead of "git prune"

The format for subjects is generally "<area>: <description>" so perhaps
something like "git gc: recommend use of `git gc --prune=now`"?

> From: John Lin <johnlinp@gmail.com>
> 
> Signed-off-by: John Lin <johnlinp@gmail.com>

This commit is missing some justification. In the commit message, we
should mention that `git prune` is a plumbing command and in the
documentation for it, it mentions that the porcelain `git gc` command
should be used instead.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-08  2:02 [PATCH] Recommend "git gc --prune=now" instead of "git prune" John Lin via GitGitGadget
  2020-06-08 10:17 ` Denton Liu
@ 2020-06-09  0:23 ` John Lin via GitGitGadget
  2020-06-09  5:07   ` Junio C Hamano
  2020-06-09 11:06   ` Ævar Arnfjörð Bjarmason
  1 sibling, 2 replies; 9+ messages in thread
From: John Lin via GitGitGadget @ 2020-06-09  0:23 UTC (permalink / raw)
  To: git; +Cc: John Lin, John Lin

From: John Lin <johnlinp@gmail.com>

`git prune` is a plumbing command and should not be run directly by
users. The corresponding porcelain command is `git gc`, which is
mentioned in the man page of `git prune`.

Signed-off-by: John Lin <johnlinp@gmail.com>
---
    Recommend "git gc --prune=now" instead of "git prune"
    
    Signed-off-by: John Lin johnlinp@gmail.com [johnlinp@gmail.com]
    
    Fix according to #642.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-652%2Fjohnlinp%2Ffix-git-gc-warning-message-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-652/johnlinp/fix-git-gc-warning-message-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/652

Range-diff vs v1:

 1:  83b7137abfd ! 1:  42aa6380067 Recommend "git gc --prune=now" instead of "git prune"
     @@ Metadata
      Author: John Lin <johnlinp@gmail.com>
      
       ## Commit message ##
     -    Recommend "git gc --prune=now" instead of "git prune"
     +    gc: recommend `git gc --prune=now` instead of `git prune`
     +
     +    `git prune` is a plumbing command and should not be run directly by
     +    users. The corresponding porcelain command is `git gc`, which is
     +    mentioned in the man page of `git prune`.
      
          Signed-off-by: John Lin <johnlinp@gmail.com>
      


 builtin/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8e0b9cf41b3..74185eac917 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -692,7 +692,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 
 	if (auto_gc && too_many_loose_objects())
 		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
+			"run 'git gc --prune=now' to remove them."));
 
 	if (!daemonized)
 		unlink(git_path("gc.log"));

base-commit: 20514004ddf1a3528de8933bc32f284e175e1012
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-09  0:23 ` [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune` John Lin via GitGitGadget
@ 2020-06-09  5:07   ` Junio C Hamano
  2020-06-09 11:06   ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2020-06-09  5:07 UTC (permalink / raw)
  To: John Lin via GitGitGadget; +Cc: git, John Lin

"John Lin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Lin <johnlinp@gmail.com>
>
> `git prune` is a plumbing command and should not be run directly by
> users. The corresponding porcelain command is `git gc`, which is
> mentioned in the man page of `git prune`.

Much better than v1 that came without any justification ;-)

I however wouldn't say "should not"---that feels a bit too strong (I
personally use "prune" from the command line once or twice every
week and do not see why I should be forbidden from doing so [*1*]),
but the users who see this message would not need such a precise
control afforded by use of "git prune", so "gc --prune=now" is a
better recommendation.

Thanks.

[Footnote]

*1* The command does not even produce a machine readable output.
    The reason it is not recommended (but that is different from
    "should not") to casual users over "git gc" is because it is a
    very focused tool (it is only about removing loose objects and
    does not touch other things) and applicable to a very specific
    condition.  Those who can recognize that they are in that
    situation should not be forbidden from using it with a dogmatic
    "should not".


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-09  0:23 ` [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune` John Lin via GitGitGadget
  2020-06-09  5:07   ` Junio C Hamano
@ 2020-06-09 11:06   ` Ævar Arnfjörð Bjarmason
  2020-06-09 16:03     ` Junio C Hamano
  1 sibling, 1 reply; 9+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2020-06-09 11:06 UTC (permalink / raw)
  To: John Lin via GitGitGadget; +Cc: git, John Lin, Jeff King, Denton Liu


On Tue, Jun 09 2020, John Lin via GitGitGadget wrote:

> From: John Lin <johnlinp@gmail.com>
>
> `git prune` is a plumbing command and should not be run directly by
> users. The corresponding porcelain command is `git gc`, which is
> mentioned in the man page of `git prune`.

This change feels incomplete without a change to git-prune's
documentation, see 8d308b3540 ("Documentation: point git-prune users to
git-gc", 2008-04-29).

I.e. it still talks about "in most cases you shouldn't run this", but
here we are removing a case where it would otherwise make sense because
the user shouldn't use it directly.

I think instead the small change that makes the most sense here is to
just add "prune" to completions, i.e. I disagree with Denton Liu in
https://github.com/gitgitgadget/git/issues/642#issuecomment-640202110

The "right" change is much harder, there's numerous odd edge cases that
"gc" can get into with these loose objects (for the curious [1] and [2]
are a nice entry to that rabbit hole).

Fixing that is non-trivial, but in the meantime I don't think it's good
to:

 a) Recommend an option to "gc" which implies "repack -a", which as
    noted in gc's docs can produce corruption in a live repository.

    I.e. I don't think "let's not recommend this plumbing" should lead
    us to "let's recommend this thing that could corrupt the repository
    instead".

 b) Even if you don't get the corruption with "a" above "gc --prune=now"
    will produce a very differently packed repository. This might have
    unexpected performance etc. trade-offs the user wasn't expecting
    from a "run this for a quick fix" command.

 c) In the cases where "prune" here would help, e.g. on some large repo
    where something else added a lot of loose unreachable objects during
    the "gc" for some reason recommending a full "gc" might take a lot
    more time than just the "prune".

1. https://public-inbox.org/git/87eflvmovg.fsf@evledraar.gmail.com/
2. https://public-inbox.org/git/8736lnxlig.fsf@evledraar.gmail.com/

> Signed-off-by: John Lin <johnlinp@gmail.com>
> ---
>     Recommend "git gc --prune=now" instead of "git prune"
>
>     Signed-off-by: John Lin johnlinp@gmail.com [johnlinp@gmail.com]
>
>     Fix according to #642.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-652%2Fjohnlinp%2Ffix-git-gc-warning-message-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-652/johnlinp/fix-git-gc-warning-message-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/652
>
> Range-diff vs v1:
>
>  1:  83b7137abfd ! 1:  42aa6380067 Recommend "git gc --prune=now" instead of "git prune"
>      @@ Metadata
>       Author: John Lin <johnlinp@gmail.com>
>
>        ## Commit message ##
>      -    Recommend "git gc --prune=now" instead of "git prune"
>      +    gc: recommend `git gc --prune=now` instead of `git prune`
>      +
>      +    `git prune` is a plumbing command and should not be run directly by
>      +    users. The corresponding porcelain command is `git gc`, which is
>      +    mentioned in the man page of `git prune`.
>
>           Signed-off-by: John Lin <johnlinp@gmail.com>
>
>
>
>  builtin/gc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 8e0b9cf41b3..74185eac917 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -692,7 +692,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>
>  	if (auto_gc && too_many_loose_objects())
>  		warning(_("There are too many unreachable loose objects; "
> -			"run 'git prune' to remove them."));
> +			"run 'git gc --prune=now' to remove them."));
>
>  	if (!daemonized)
>  		unlink(git_path("gc.log"));
>
> base-commit: 20514004ddf1a3528de8933bc32f284e175e1012

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-09 11:06   ` Ævar Arnfjörð Bjarmason
@ 2020-06-09 16:03     ` Junio C Hamano
  2020-06-10  3:43       ` 林自均
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2020-06-09 16:03 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: John Lin via GitGitGadget, git, John Lin, Jeff King, Denton Liu

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Jun 09 2020, John Lin via GitGitGadget wrote:
>
>> From: John Lin <johnlinp@gmail.com>
>>
>> `git prune` is a plumbing command and should not be run directly by
>> users. The corresponding porcelain command is `git gc`, which is
>> mentioned in the man page of `git prune`.
>
> This change feels incomplete without a change to git-prune's
> documentation, see 8d308b3540 ("Documentation: point git-prune users to
> git-gc", 2008-04-29).
>
> I.e. it still talks about "in most cases you shouldn't run this", but
> here we are removing a case where it would otherwise make sense because
> the user shouldn't use it directly.
>
> I think instead the small change that makes the most sense here is to
> just add "prune" to completions,...

That's perfectly reasonable stance to take.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-09 16:03     ` Junio C Hamano
@ 2020-06-10  3:43       ` 林自均
  2020-06-10  3:47         ` Denton Liu
  0 siblings, 1 reply; 9+ messages in thread
From: 林自均 @ 2020-06-10  3:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason,
	John Lin via GitGitGadget, Git, Jeff King, Denton Liu

Hi all,

Thank you Bjarmason & Junio. If Denton doesn't have other concerns,
I'll start working on adding bash completion for "git prune".

Best,
John Lin

Junio C Hamano <gitster@pobox.com> 於 2020年6月10日 週三 上午12:03寫道:
>
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
> > On Tue, Jun 09 2020, John Lin via GitGitGadget wrote:
> >
> >> From: John Lin <johnlinp@gmail.com>
> >>
> >> `git prune` is a plumbing command and should not be run directly by
> >> users. The corresponding porcelain command is `git gc`, which is
> >> mentioned in the man page of `git prune`.
> >
> > This change feels incomplete without a change to git-prune's
> > documentation, see 8d308b3540 ("Documentation: point git-prune users to
> > git-gc", 2008-04-29).
> >
> > I.e. it still talks about "in most cases you shouldn't run this", but
> > here we are removing a case where it would otherwise make sense because
> > the user shouldn't use it directly.
> >
> > I think instead the small change that makes the most sense here is to
> > just add "prune" to completions,...
>
> That's perfectly reasonable stance to take.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-10  3:43       ` 林自均
@ 2020-06-10  3:47         ` Denton Liu
  2020-06-10  7:38           ` 林自均
  0 siblings, 1 reply; 9+ messages in thread
From: Denton Liu @ 2020-06-10  3:47 UTC (permalink / raw)
  To: 林自均
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	John Lin via GitGitGadget, Git, Jeff King

On Wed, Jun 10, 2020 at 11:43:09AM +0800, 林自均 wrote:
> Hi all,
> 
> Thank you Bjarmason & Junio. If Denton doesn't have other concerns,
> I'll start working on adding bash completion for "git prune".

No concerns. Thanks for contributing!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune`
  2020-06-10  3:47         ` Denton Liu
@ 2020-06-10  7:38           ` 林自均
  0 siblings, 0 replies; 9+ messages in thread
From: 林自均 @ 2020-06-10  7:38 UTC (permalink / raw)
  To: Denton Liu
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	John Lin via GitGitGadget, Git, Jeff King

Thanks Denton!

Best,
John Lin

Denton Liu <liu.denton@gmail.com> 於 2020年6月10日 週三 上午11:48寫道:
>
> On Wed, Jun 10, 2020 at 11:43:09AM +0800, 林自均 wrote:
> > Hi all,
> >
> > Thank you Bjarmason & Junio. If Denton doesn't have other concerns,
> > I'll start working on adding bash completion for "git prune".
>
> No concerns. Thanks for contributing!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-10  7:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08  2:02 [PATCH] Recommend "git gc --prune=now" instead of "git prune" John Lin via GitGitGadget
2020-06-08 10:17 ` Denton Liu
2020-06-09  0:23 ` [PATCH v2] gc: recommend `git gc --prune=now` instead of `git prune` John Lin via GitGitGadget
2020-06-09  5:07   ` Junio C Hamano
2020-06-09 11:06   ` Ævar Arnfjörð Bjarmason
2020-06-09 16:03     ` Junio C Hamano
2020-06-10  3:43       ` 林自均
2020-06-10  3:47         ` Denton Liu
2020-06-10  7:38           ` 林自均

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).