* Seriously broken "git pack-refs"
@ 2007-01-26 0:51 Linus Torvalds
2007-01-26 3:22 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2007-01-26 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Do *NOT* try this on a repository you care about:
git pack-refs --all --prune
git pack-refs
because while the first "pack-refs" does the right thing, the second
pack-refs will totally screw you over.
Why?
The default for "git pack-refs" is to pack just tags. Which is a HORRIBLE
MISTAKE. Becuase it means that if you actually had any packed non-tags,
they now get removed entirely.
I'd call whoever made that decision a complete crack-addict and total
idiot, but it might be me, so I'll just tread carefully and call the
choice "interesting".
This should fix it.
Linus
----
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 6de7128..3de9b3e 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -37,7 +37,9 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
if ((flags & REF_ISSYMREF))
return 0;
is_tag_ref = !strncmp(path, "refs/tags/", 10);
- if (!cb->all && !is_tag_ref)
+
+ /* ALWAYS pack refs that were already packed or are tags */
+ if (!cb->all && !is_tag_ref && !(flags & REF_ISPACKED))
return 0;
fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Seriously broken "git pack-refs"
2007-01-26 0:51 Seriously broken "git pack-refs" Linus Torvalds
@ 2007-01-26 3:22 ` Junio C Hamano
2007-01-26 6:05 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2007-01-26 3:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
Linus Torvalds <torvalds@linux-foundation•org> writes:
> The default for "git pack-refs" is to pack just tags. Which is a HORRIBLE
> MISTAKE. Becuase it means that if you actually had any packed non-tags,
> they now get removed entirely.
>
> I'd call whoever made that decision a complete crack-addict and total
> idiot, but it might be me, so I'll just tread carefully and call the
> choice "interesting".
Guilty as charged, I guess (I haven't asked 'blame' about it but
I am reasonably sure it was me). And thanks for catching it.
But I think "only packing tags" is a reasonable default, as
local branch heads are meant to be advanced, and remote tracking
branches follow whatever happens on the remote end upon every
fetch. Tags on the other hand are meant to be stationary.
What was a HORRIBLE MISTAKE was not repacking what came out of
the pack if it was pruned.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Seriously broken "git pack-refs"
2007-01-26 3:22 ` Junio C Hamano
@ 2007-01-26 6:05 ` Linus Torvalds
2007-01-26 6:08 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2007-01-26 6:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Thu, 25 Jan 2007, Junio C Hamano wrote:
>
> But I think "only packing tags" is a reasonable default
Sure, I agree. I just think that it had the unintended side effect when
the code then didn't repack stuff that had already been packed earlier.
Which is why my fix doesn't actually change the fact that we only pack
tags by default, but just fixes the thing that caused us to then totally
drop previously packed non-tags.
So now the default isn't really "pack only tags", but "pack only tags *or*
things that were already packed".
The reason, of course, is that if we don't pack stuff that used to be
packed, it will just be dropped entirely. Oops.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Seriously broken "git pack-refs"
2007-01-26 6:05 ` Linus Torvalds
@ 2007-01-26 6:08 ` Junio C Hamano
2007-01-26 6:24 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2007-01-26 6:08 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
Linus Torvalds <torvalds@linux-foundation•org> writes:
> So now the default isn't really "pack only tags", but "pack only tags *or*
> things that were already packed".
>
> The reason, of course, is that if we don't pack stuff that used to be
> packed, it will just be dropped entirely. Oops.
Sorry again.
On the other hand, I do not think it is worth unpacking non-tags
that are packed when --all is not given, so...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Seriously broken "git pack-refs"
2007-01-26 6:08 ` Junio C Hamano
@ 2007-01-26 6:24 ` Linus Torvalds
2007-01-26 6:50 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2007-01-26 6:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Thu, 25 Jan 2007, Junio C Hamano wrote:
>
> On the other hand, I do not think it is worth unpacking non-tags
> that are packed when --all is not given, so...
Exactly.
I'd much rather have
git pack-refs --all --prune
just pack everything once and for all. Afterwards, if you do just
git pack-refs
it will keep everything that was packed packed, and any loose refs (which
might be loose because they are new, but perhaps because they simply got
overridden by being changed) will not be added to the pack unless they are
tags.
Which should be the semantics with my patch applied.
The above actually makes tons of sense: operations like big imports (or
clones) might want to start out with *everything* packed, but then as we
update, commit and modify, what we actually want to do is to basically let
the branches that are actively developed "become unpacked" as they are
updated.
So the "repack refs that were already packed, or refs that are tags" is
actually a very sane default. It's not just that we don't want to lose the
refs entirely, it's also that what we actually want to do by default is to
pack the stuff we have reason to believe won't be actively changing.
Tags automatically fall under that heading (which is why it makes sense to
pack them by default in the first place!) but so does any ref that was
already packed, and hadn't become unpacked since the last repack. By
definition, such a ref isn't "actively changing" even if it isn't a tag.
No?
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Seriously broken "git pack-refs"
2007-01-26 6:24 ` Linus Torvalds
@ 2007-01-26 6:50 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2007-01-26 6:50 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
Linus Torvalds <torvalds@linux-foundation•org> writes:
> Tags automatically fall under that heading (which is why it makes sense to
> pack them by default in the first place!) but so does any ref that was
> already packed, and hadn't become unpacked since the last repack. By
> definition, such a ref isn't "actively changing" even if it isn't a tag.
>
> No?
Yes.
diff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt
index 464269f..b79b79d 100644
--- a/Documentation/git-pack-refs.txt
+++ b/Documentation/git-pack-refs.txt
@@ -34,11 +34,16 @@ OPTIONS
\--all::
-The command by default packs all tags and leaves branch tips
+The command by default packs all tags and refs that are already
+packed, and leaves other refs
alone. This is because branches are expected to be actively
developed and packing their tips does not help performance.
This option causes branch tips to be packed as well. Useful for
a repository with many branches of historical interests.
+Once packing with `\--all` is done, further repacking with the
+default will keep the already packed refs (including branch
+heads) packed, while still actively developed branch heads will
+not be packed (because they become unpacked once modified).
\--no-prune::
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-01-26 6:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-26 0:51 Seriously broken "git pack-refs" Linus Torvalds
2007-01-26 3:22 ` Junio C Hamano
2007-01-26 6:05 ` Linus Torvalds
2007-01-26 6:08 ` Junio C Hamano
2007-01-26 6:24 ` Linus Torvalds
2007-01-26 6:50 ` 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