git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fsck: do not print dangling objects by default
@ 2012-02-26 20:43 Clemens Buchacher
  2012-02-26 21:57 ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Clemens Buchacher @ 2012-02-26 20:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Every reference to git fsck that I could find in the documentation or on
the web is followed by the some kind of assurance that dangling objects
are "not a problem", "nothing to worry about" or "not at all
interesting".

Instead of telling the user everywhere to ignore those messages, let's
not print them in the first place. The --dangling flag can be used to
explicitly enable printing dangling objects.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 Documentation/git-fsck.txt    |    3 +++
 Documentation/git-repack.txt  |    2 +-
 Documentation/user-manual.txt |   20 ++------------------
 builtin/fsck.c                |    7 +++++--
 t/t1410-reflog.sh             |    2 +-
 t/t1450-fsck.sh               |    6 +-----
 t/t6050-replace.sh            |    2 +-
 7 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 6c47395..199c5a0 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -30,6 +30,9 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
 	Print out objects that exist but that aren't reachable from any
 	of the reference nodes.
 
+--dangling::
+	Print objects that exist but that are never 'directly' used.
+
 --root::
 	Report root nodes.
 
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 40af321..4c1aff6 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -34,7 +34,7 @@ OPTIONS
 	Especially useful when packing a repository that is used
 	for private development. Use
 	with '-d'.  This will clean up the objects that `git prune`
-	leaves behind, but `git fsck --full` shows as
+	leaves behind, but `git fsck --full --dangling` shows as
 	dangling.
 +
 Note that users fetching over dumb protocols will have to fetch the
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index f13a846..09ffbcc 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1582,25 +1582,12 @@ Checking the repository for corruption
 
 The linkgit:git-fsck[1] command runs a number of self-consistency checks
 on the repository, and reports on any problems.  This may take some
-time.  The most common warning by far is about "dangling" objects:
+time.
 
 -------------------------------------------------
 $ git fsck
-dangling commit 7281251ddd2a61e38657c827739c57015671a6b3
-dangling commit 2706a059f258c6b245f298dc4ff2ccd30ec21a63
-dangling commit 13472b7c4b80851a1bc551779171dcb03655e9b5
-dangling blob 218761f9d90712d37a9c5e36f406f92202db07eb
-dangling commit bf093535a34a4d35731aa2bd90fe6b176302f14f
-dangling commit 8e4bec7f2ddaa268bef999853c25755452100f8e
-dangling tree d50bb86186bf27b681d25af89d3b5b68382e4085
-dangling tree b24c2473f1fd3d91352a624795be026d64c8841f
-...
 -------------------------------------------------
 
-Dangling objects are not a problem.  At worst they may take up a little
-extra disk space.  They can sometimes provide a last-resort method for
-recovering lost work--see <<dangling-objects>> for details.
-
 [[recovering-lost-changes]]
 Recovering lost changes
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -1665,7 +1652,7 @@ commits in the dangling objects that `git fsck` reports.  See
 <<dangling-objects>> for the details.
 
 -------------------------------------------------
-$ git fsck
+$ git fsck --dangling
 dangling commit 7281251ddd2a61e38657c827739c57015671a6b3
 dangling commit 2706a059f258c6b245f298dc4ff2ccd30ec21a63
 dangling commit 13472b7c4b80851a1bc551779171dcb03655e9b5
@@ -3301,9 +3288,6 @@ broken link from    tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
 missing blob 4b9458b3786228369c63936db65827de3cc06200
 ------------------------------------------------
 
-(Typically there will be some "dangling object" messages too, but they
-aren't interesting.)
-
 Now you know that blob 4b9458b3 is missing, and that the tree 2d9263c6
 points to it.  If you could find just one copy of that missing blob
 object, possibly in some other repository, you could move it into
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8c479a7..0745535 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -29,6 +29,7 @@ static int errors_found;
 static int write_lost_and_found;
 static int verbose;
 static int show_progress = -1;
+static int show_dangling;
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 #define ERROR_PACK 04
@@ -221,8 +222,9 @@ static void check_unreachable_object(struct object *obj)
 	 * start looking at, for example.
 	 */
 	if (!obj->used) {
-		printf("dangling %s %s\n", typename(obj->type),
-		       sha1_to_hex(obj->sha1));
+		if (show_dangling)
+			printf("dangling %s %s\n", typename(obj->type),
+			       sha1_to_hex(obj->sha1));
 		if (write_lost_and_found) {
 			char *filename = git_path("lost-found/%s/%s",
 				obj->type == OBJ_COMMIT ? "commit" : "other",
@@ -614,6 +616,7 @@ static char const * const fsck_usage[] = {
 static struct option fsck_opts[] = {
 	OPT__VERBOSE(&verbose, "be verbose"),
 	OPT_BOOLEAN(0, "unreachable", &show_unreachable, "show unreachable objects"),
+	OPT_BOOLEAN(0, "dangling", &show_dangling, "show dangling objects"),
 	OPT_BOOLEAN(0, "tags", &show_tags, "report tags"),
 	OPT_BOOLEAN(0, "root", &show_root, "report root nodes"),
 	OPT_BOOLEAN(0, "cache", &keep_cache_objects, "make index objects head nodes"),
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 252fc82..12441c5 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -20,7 +20,7 @@ check_have () {
 }
 
 check_fsck () {
-	output=$(git fsck --full)
+	output=$(git fsck --full --dangling)
 	case "$1" in
 	'')
 		test -z "$output" ;;
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 523ce9c..78bfbc3 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -27,12 +27,8 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 		git init &&
 		echo ../../../.git/objects >.git/objects/info/alternates &&
 		test_commit C fileC one &&
-		git fsck >../out 2>&1
+		git fsck >../actual 2>&1
 	) &&
-	{
-		grep -v dangling out >actual ||
-		:
-	} &&
 	test_cmp empty actual
 '
 
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c87f28..5b36ee3 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -95,7 +95,7 @@ test_expect_success 'tag replaced commit' '
 '
 
 test_expect_success '"git fsck" works' '
-     git fsck master > fsck_master.out &&
+     git fsck --dangling master > fsck_master.out &&
      grep "dangling commit $R" fsck_master.out &&
      grep "dangling tag $(cat .git/refs/tags/mytag)" fsck_master.out &&
      test -z "$(git fsck)"
-- 
1.7.9.1

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-26 20:43 [PATCH] fsck: do not print dangling objects by default Clemens Buchacher
@ 2012-02-26 21:57 ` Junio C Hamano
  2012-02-26 22:46   ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2012-02-26 21:57 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: git

Clemens Buchacher <drizzd@aon.at> writes:

> Every reference to git fsck that I could find in the documentation or on
> the web is followed by the some kind of assurance that dangling objects
> are "not a problem", "nothing to worry about" or "not at all
> interesting".
>
> Instead of telling the user everywhere to ignore those messages, let's
> not print them in the first place. The --dangling flag can be used to
> explicitly enable printing dangling objects.
>
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>

Thanks.

I think that both the ultimate goal explained above, and the direction in
which the documentation updates tries to move us, are good.  I only gave a
cursory look at the code changes, but what they implement seems to match
the intention.

Of course I may be missing something, so objections from others to argue
why we shouldn't do this is very much welcomed to stop me and Clemens ;-).

> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index f13a846..09ffbcc 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -1582,25 +1582,12 @@ Checking the repository for corruption
>  
>  The linkgit:git-fsck[1] command runs a number of self-consistency checks
>  on the repository, and reports on any problems.  This may take some
> +time.
>  ...
> -Dangling objects are not a problem.  At worst they may take up a little
> -extra disk space.  They can sometimes provide a last-resort method for
> -recovering lost work--see <<dangling-objects>> for details.

Losing this description is not a problem and is indeed an improvement in
this section in the user manual, but let's make a mental note that the
user needs to also learn what the third sentence says elsewhere in this
document.

 << reads on >>

> @@ -1665,7 +1652,7 @@ commits in the dangling objects that `git fsck` reports.  See
>  <<dangling-objects>> for the details.
>  
>  -------------------------------------------------
> -$ git fsck
> +$ git fsck --dangling
>  dangling commit 7281251ddd2a61e38657c827739c57015671a6b3
>  dangling commit 2706a059f258c6b245f298dc4ff2ccd30ec21a63
>  dangling commit 13472b7c4b80851a1bc551779171dcb03655e9b5
> @@ -3301,9 +3288,6 @@ broken link from    tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
>  missing blob 4b9458b3786228369c63936db65827de3cc06200
>  ------------------------------------------------
>  
> -(Typically there will be some "dangling object" messages too, but they
> -aren't interesting.)
> -

The old sentence becomes stale as the example _explicitly_ asks the
command to show the dangling ones.

>  Now you know that blob 4b9458b3 is missing, and that the tree 2d9263c6
>  points to it.  If you could find just one copy of that missing blob
>  object, possibly in some other repository, you could move it into

And this part, together with the introductory text of the section (not
shown in the context), says what the "third sentence" above wanted to say.

So overall I think this is a vast improvement without losing any
information or clarity.

Nice.

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-26 21:57 ` Junio C Hamano
@ 2012-02-26 22:46   ` Junio C Hamano
  2012-02-27  6:42     ` Zbigniew Jędrzejewski-Szmek
  2012-02-27 19:18     ` Jeff King
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2012-02-26 22:46 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: git

Junio C Hamano <gitster@pobox.com> writes:

> I think that both the ultimate goal explained above, and the direction in
> which the documentation updates tries to move us, are good.  I only gave a
> cursory look at the code changes, but what they implement seems to match
> the intention.
>
> Of course I may be missing something, so objections from others to argue
> why we shouldn't do this is very much welcomed to stop me and Clemens ;-).

Let's start with the obvious.

It is much easier for a user to use a new option on the command line when
he wants to use an improved behaviour when he runs the command manually.
Having to update scripts that run the command to act on its output, on the
other hand, is much more painful to the users.

And the intended audience for this change clearly is interactive users
that follow the user-manual to try things out.

Given that, isn't it not just sufficient but actually better to instead
add a new --no-dangling option and keep the default unchanged?

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-26 22:46   ` Junio C Hamano
@ 2012-02-27  6:42     ` Zbigniew Jędrzejewski-Szmek
  2012-02-27 19:18     ` Jeff King
  1 sibling, 0 replies; 11+ messages in thread
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-27  6:42 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Clemens Buchacher

On 02/26/2012 11:46 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> I think that both the ultimate goal explained above, and the direction in
>> which the documentation updates tries to move us, are good.  I only gave a
>> cursory look at the code changes, but what they implement seems to match
>> the intention.
>>
>> Of course I may be missing something, so objections from others to argue
>> why we shouldn't do this is very much welcomed to stop me and Clemens ;-).
>
> Let's start with the obvious.
>
> It is much easier for a user to use a new option on the command line when
> he wants to use an improved behaviour when he runs the command manually.
> Having to update scripts that run the command to act on its output, on the
> other hand, is much more painful to the users.
>
> And the intended audience for this change clearly is interactive users
> that follow the user-manual to try things out.
>
> Given that, isn't it not just sufficient but actually better to instead
> add a new --no-dangling option and keep the default unchanged?

I understood the goal of this change as "modify fsck output to not show 
confusing 'dangling' messages by default. Not when running the tutorial 
and having the output explained in parallel, but when someone runs 
git-fsck to clean up the repository. In that situation, if somebody 
knows enough to run --no-dangling, than they know enough to ignore the 
'dangling' messages in the output.

For the knowledgeable user, --no-dangling could be useful to avoid 
uninteresting messages which usually dwarf the rest of output, but this 
would be less important.

Zbyszek

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-26 22:46   ` Junio C Hamano
  2012-02-27  6:42     ` Zbigniew Jędrzejewski-Szmek
@ 2012-02-27 19:18     ` Jeff King
  2012-02-27 19:29       ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff King @ 2012-02-27 19:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Clemens Buchacher, git

On Sun, Feb 26, 2012 at 02:46:46PM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I think that both the ultimate goal explained above, and the direction in
> > which the documentation updates tries to move us, are good.  I only gave a
> > cursory look at the code changes, but what they implement seems to match
> > the intention.
> >
> > Of course I may be missing something, so objections from others to argue
> > why we shouldn't do this is very much welcomed to stop me and Clemens ;-).
> 
> Let's start with the obvious.
> 
> It is much easier for a user to use a new option on the command line when
> he wants to use an improved behaviour when he runs the command manually.
> Having to update scripts that run the command to act on its output, on the
> other hand, is much more painful to the users.
> 
> And the intended audience for this change clearly is interactive users
> that follow the user-manual to try things out.
> 
> Given that, isn't it not just sufficient but actually better to instead
> add a new --no-dangling option and keep the default unchanged?

But if your intended audience is users who are confused by the dangling
warnings, explaining to them to use --no-dangling is not really
improving the situation. Of course, it is fsck, so I wonder how often
clueless people are really running it in the first place (i.e., it is
not and should not be part of most users' typical workflows). If it is
simply the case that they are being told to run "git fsck" by more
expert users without understanding what it does, then I could buy the
argument that those expert users could just as easily say "git fsck
--no-dangling".

-Peff

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-27 19:18     ` Jeff King
@ 2012-02-27 19:29       ` Junio C Hamano
  2012-02-27 21:13         ` Clemens Buchacher
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2012-02-27 19:29 UTC (permalink / raw)
  To: Jeff King; +Cc: Clemens Buchacher, git

Jeff King <peff@peff.net> writes:

>> Given that, isn't it not just sufficient but actually better to instead
>> add a new --no-dangling option and keep the default unchanged?
>
> ... Of course, it is fsck, so I wonder how often clueless people are
> really running it in the first place (i.e., it is not and should not be
> part of most users' typical workflows). If it is simply the case that
> they are being told to run "git fsck" by more expert users without
> understanding what it does, then I could buy the argument that those
> expert users could just as easily say "git fsck --no-dangling".

Yes, that was certainly part of my pros-and-cons analysis.  If you run
"git fsck" without "--no-dangling" without reading the manual, you may
get confused, but that is *not* the primary audience.  People who are
curious can read the manual and figure it out, and the need for "fsck" is
much rarer these days, compared to 2005 ;-)

In that context, only large downsides of potentially breaking and having
to adjust existing scripts remains without much upsides, if we were to
switch the default.

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-27 19:29       ` Junio C Hamano
@ 2012-02-27 21:13         ` Clemens Buchacher
  2012-02-27 21:33           ` Jeff King
  2012-02-27 21:34           ` Junio C Hamano
  0 siblings, 2 replies; 11+ messages in thread
From: Clemens Buchacher @ 2012-02-27 21:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Mon, Feb 27, 2012 at 11:29:53AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> >> Given that, isn't it not just sufficient but actually better to instead
> >> add a new --no-dangling option and keep the default unchanged?
> >
> > ... Of course, it is fsck, so I wonder how often clueless people are
> > really running it in the first place (i.e., it is not and should not be
> > part of most users' typical workflows). If it is simply the case that
> > they are being told to run "git fsck" by more expert users without
> > understanding what it does, then I could buy the argument that those
> > expert users could just as easily say "git fsck --no-dangling".
> 
> Yes, that was certainly part of my pros-and-cons analysis.  If you run
> "git fsck" without "--no-dangling" without reading the manual, you may
> get confused, but that is *not* the primary audience.

It is not my only concern that users might be confused. I believe the
command prints a lot of useless messages, which is by itself a UI
deficiency. But even worse, those numerous messages tend to hide an
actual problem in a long scrollback buffer. Sometimes my scrollback
buffer is not even large enough and I have to re-run fsck (which is not
exactly a fast command), just so I can grep out the dangling blobs.

> People who are curious can read the manual and figure it out, and the
> need for "fsck" is much rarer these days, compared to 2005 ;-)

In my opinion, the need for fsck is much more common these days. With
the alternates feature, it happens all the time that a repository breaks
if one is not extremely careful.

> In that context, only large downsides of potentially breaking and having
> to adjust existing scripts remains without much upsides, if we were to
> switch the default.

There is something wrong with weighting a UI improvement against
convenient use in scripts. If that were the issue, then we should add a
plumbing version for all commands, like we do for git status
--porcelain. Otherwise we can never change anything any more.

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-27 21:13         ` Clemens Buchacher
@ 2012-02-27 21:33           ` Jeff King
  2012-02-27 22:18             ` Clemens Buchacher
  2012-02-27 21:34           ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff King @ 2012-02-27 21:33 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Junio C Hamano, git

On Mon, Feb 27, 2012 at 10:13:16PM +0100, Clemens Buchacher wrote:

> > Yes, that was certainly part of my pros-and-cons analysis.  If you run
> > "git fsck" without "--no-dangling" without reading the manual, you may
> > get confused, but that is *not* the primary audience.
> 
> It is not my only concern that users might be confused. I believe the
> command prints a lot of useless messages, which is by itself a UI
> deficiency. But even worse, those numerous messages tend to hide an
> actual problem in a long scrollback buffer. Sometimes my scrollback
> buffer is not even large enough and I have to re-run fsck (which is not
> exactly a fast command), just so I can grep out the dangling blobs.

Yeah, but doesn't adding "--no-dangling" solve that issue?

> > People who are curious can read the manual and figure it out, and the
> > need for "fsck" is much rarer these days, compared to 2005 ;-)
> 
> In my opinion, the need for fsck is much more common these days. With
> the alternates feature, it happens all the time that a repository breaks
> if one is not extremely careful.

Interesting. I can't remember the last time I ran fsck. But I am not the
only git user, and my assumptions are only based on my own experience,
not other data.

> > In that context, only large downsides of potentially breaking and having
> > to adjust existing scripts remains without much upsides, if we were to
> > switch the default.
> 
> There is something wrong with weighting a UI improvement against
> convenient use in scripts. If that were the issue, then we should add a
> plumbing version for all commands, like we do for git status
> --porcelain. Otherwise we can never change anything any more.

I don't think it's convenience as much as scripts which try to detect
dangling by using "git fsck" will now be broken with a really bad
failure mode (i.e., they used to run "git fsck" and parse the dangling
lines on stderr, but now they will erroneously think that nothing is
dangling!).

If we really care about that, then the right answer is our usual:

  1. Introduce --dangling/--no-dangling, leaving the default.

  2. Wait a while until the features are well-established.

  3. Scripts adapt to use --dangling if they want it.

  4. Wait a while for scripts to be updated.

  5. Flip the default to --no-dangling.

I wrote but didn't send in my last email a long analysis of why it is
wrong to combine fsck's default mode of operation (which is really about
"is this repository corrupted or broken") with checking for dangling
objects (which is something you do when you are looking for lost work).
But I don't think that is even really up for debate. If we were
designing fsck from scratch, I think everybody in the thread has the
feeling that reporting dangling by default is wrong. And we should
definitely add "--no-dangling" to address that.

But flipping the default is a different story, and that has to weigh the
potential breakage above versus having a good UI. I'm on the fence about
whether the above is a big enough problem to care about (mostly because
I'm not sure anybody has actually written such a script).

-Peff

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-27 21:13         ` Clemens Buchacher
  2012-02-27 21:33           ` Jeff King
@ 2012-02-27 21:34           ` Junio C Hamano
  2012-02-28 23:25             ` [PATCH] fsck: --no-dangling omits "dangling object" information Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2012-02-27 21:34 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, git

Clemens Buchacher <drizzd@aon.at> writes:

> There is something wrong with weighting a UI improvement against
> convenient use in scripts. If that were the issue, then ...

But that is not the issue.  Nobody said anything about convenience for
script writers.

I am talking about regressions to already written long time ago and used
by many people _without_ them having to worry about how it internally
works.  By switching the default, you are suddenly making them worry about
it again.

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

* Re: [PATCH] fsck: do not print dangling objects by default
  2012-02-27 21:33           ` Jeff King
@ 2012-02-27 22:18             ` Clemens Buchacher
  0 siblings, 0 replies; 11+ messages in thread
From: Clemens Buchacher @ 2012-02-27 22:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Mon, Feb 27, 2012 at 04:33:04PM -0500, Jeff King wrote:
> On Mon, Feb 27, 2012 at 10:13:16PM +0100, Clemens Buchacher wrote:
> 
> > > Yes, that was certainly part of my pros-and-cons analysis.  If you run
> > > "git fsck" without "--no-dangling" without reading the manual, you may
> > > get confused, but that is *not* the primary audience.
> > 
> > It is not my only concern that users might be confused. I believe the
> > command prints a lot of useless messages, which is by itself a UI
> > deficiency. But even worse, those numerous messages tend to hide an
> > actual problem in a long scrollback buffer. Sometimes my scrollback
> > buffer is not even large enough and I have to re-run fsck (which is not
> > exactly a fast command), just so I can grep out the dangling blobs.
> 
> Yeah, but doesn't adding "--no-dangling" solve that issue?

I can just as well use grep -v ^dangling.

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

* [PATCH] fsck: --no-dangling omits "dangling object" information
  2012-02-27 21:34           ` Junio C Hamano
@ 2012-02-28 23:25             ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2012-02-28 23:25 UTC (permalink / raw)
  To: git; +Cc: Clemens Buchacher, Jeff King

The default output from "fsck" is often overwhelmed by informational
message on dangling objects, especially if you do not repack often, and a
real error can easily be buried.

Add "--no-dangling" option to omit them, and update the user manual to
demonstrate its use.

Based on a patch by Clemens Buchacher, but reverted the part to change
the default to --no-dangling, which is unsuitable for the first patch.
The usual three-step procedure to break the backward compatibility over
time needs to happen on top of this, if we were to go in that direction.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I butchered Clemens's patch and added "--no-dangling", and reverted the
   part that flips the default.  I also took the authorship blame, as I do
   not share the final goal of fliping the default (yet), but our first
   steps should be in the same direction, which is this patch.

 Documentation/git-fsck.txt    |    7 ++++++-
 Documentation/git-repack.txt  |    2 +-
 Documentation/user-manual.txt |   15 +++++++--------
 builtin/fsck.c                |    7 +++++--
 t/t1450-fsck.sh               |    6 +-----
 5 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 6c47395..47e2f19 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
 	 [--[no-]full] [--strict] [--verbose] [--lost-found]
-	 [--[no-]progress] [<object>*]
+	 [--[no-]dangling] [--[no-]progress] [<object>*]
 
 DESCRIPTION
 -----------
@@ -30,6 +30,11 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
 	Print out objects that exist but that aren't reachable from any
 	of the reference nodes.
 
+--dangling::
+--no-dangling::
+	Print objects that exist but that are never 'directly' used (default).
+	`--no-dangling` can be used to squech this information from the output.
+
 --root::
 	Report root nodes.
 
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 40af321..4c1aff6 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -34,7 +34,7 @@ OPTIONS
 	Especially useful when packing a repository that is used
 	for private development. Use
 	with '-d'.  This will clean up the objects that `git prune`
-	leaves behind, but `git fsck --full` shows as
+	leaves behind, but `git fsck --full --dangling` shows as
 	dangling.
 +
 Note that users fetching over dumb protocols will have to fetch the
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index f13a846..6c7fee7 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1582,7 +1582,7 @@ Checking the repository for corruption
 
 The linkgit:git-fsck[1] command runs a number of self-consistency checks
 on the repository, and reports on any problems.  This may take some
-time.  The most common warning by far is about "dangling" objects:
+time.
 
 -------------------------------------------------
 $ git fsck
@@ -1597,9 +1597,11 @@ dangling tree b24c2473f1fd3d91352a624795be026d64c8841f
 ...
 -------------------------------------------------
 
-Dangling objects are not a problem.  At worst they may take up a little
-extra disk space.  They can sometimes provide a last-resort method for
-recovering lost work--see <<dangling-objects>> for details.
+You will see informational messages on dangling objects. They are objects
+that still exist in the repository but are no longer referenced by any of
+your branches, and can (and will) be removed after a while with "gc".
+You can run `git fsck --no-dangling` to supress these messages, and still
+view real errors.
 
 [[recovering-lost-changes]]
 Recovering lost changes
@@ -3295,15 +3297,12 @@ it is with linkgit:git-fsck[1]; this may be time-consuming.
 Assume the output looks like this:
 
 ------------------------------------------------
-$ git fsck --full
+$ git fsck --full --no-dangling
 broken link from    tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
               to    blob 4b9458b3786228369c63936db65827de3cc06200
 missing blob 4b9458b3786228369c63936db65827de3cc06200
 ------------------------------------------------
 
-(Typically there will be some "dangling object" messages too, but they
-aren't interesting.)
-
 Now you know that blob 4b9458b3 is missing, and that the tree 2d9263c6
 points to it.  If you could find just one copy of that missing blob
 object, possibly in some other repository, you could move it into
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8c479a7..67eb553 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -29,6 +29,7 @@ static int errors_found;
 static int write_lost_and_found;
 static int verbose;
 static int show_progress = -1;
+static int show_dangling = 1;
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 #define ERROR_PACK 04
@@ -221,8 +222,9 @@ static void check_unreachable_object(struct object *obj)
 	 * start looking at, for example.
 	 */
 	if (!obj->used) {
-		printf("dangling %s %s\n", typename(obj->type),
-		       sha1_to_hex(obj->sha1));
+		if (show_dangling)
+			printf("dangling %s %s\n", typename(obj->type),
+			       sha1_to_hex(obj->sha1));
 		if (write_lost_and_found) {
 			char *filename = git_path("lost-found/%s/%s",
 				obj->type == OBJ_COMMIT ? "commit" : "other",
@@ -614,6 +616,7 @@ static char const * const fsck_usage[] = {
 static struct option fsck_opts[] = {
 	OPT__VERBOSE(&verbose, "be verbose"),
 	OPT_BOOLEAN(0, "unreachable", &show_unreachable, "show unreachable objects"),
+	OPT_BOOL(0, "dangling", &show_dangling, "show dangling objects"),
 	OPT_BOOLEAN(0, "tags", &show_tags, "report tags"),
 	OPT_BOOLEAN(0, "root", &show_root, "report root nodes"),
 	OPT_BOOLEAN(0, "cache", &keep_cache_objects, "make index objects head nodes"),
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 5b8ebd8..5b79c51 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -27,12 +27,8 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 		git init &&
 		echo ../../../.git/objects >.git/objects/info/alternates &&
 		test_commit C fileC one &&
-		git fsck >../out 2>&1
+		git fsck --no-dangling >../actual 2>&1
 	) &&
-	{
-		grep -v dangling out >actual ||
-		:
-	} &&
 	test_cmp empty actual
 '
 
-- 
1.7.9.2.344.g3e8c86

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

end of thread, other threads:[~2012-02-28 23:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-26 20:43 [PATCH] fsck: do not print dangling objects by default Clemens Buchacher
2012-02-26 21:57 ` Junio C Hamano
2012-02-26 22:46   ` Junio C Hamano
2012-02-27  6:42     ` Zbigniew Jędrzejewski-Szmek
2012-02-27 19:18     ` Jeff King
2012-02-27 19:29       ` Junio C Hamano
2012-02-27 21:13         ` Clemens Buchacher
2012-02-27 21:33           ` Jeff King
2012-02-27 22:18             ` Clemens Buchacher
2012-02-27 21:34           ` Junio C Hamano
2012-02-28 23:25             ` [PATCH] fsck: --no-dangling omits "dangling object" information Junio C Hamano

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).