* [PATCH] Re: git-http-pull broken in latest git
2005-08-11 23:21 ` Junio C Hamano
@ 2005-08-11 23:38 ` Daniel Barkalow
2005-08-12 2:01 ` Junio C Hamano
2005-08-11 23:57 ` Junio C Hamano
2005-08-12 2:45 ` Petr Baudis
2 siblings, 1 reply; 11+ messages in thread
From: Daniel Barkalow @ 2005-08-11 23:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, git
On Thu, 11 Aug 2005, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
>
> > $ git-cat-file commit bf570303153902ec3d85570ed24515bcf8948848 | grep tree
> > tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > $ git-read-tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > fatal: failed to unpack tree object 41f10531f1799bbb31a1e0f7652363154ce96f45
>
> > Kaboom. I think the issue might be that the reference dependency tree
> > building is broken and it should've pulled the other pack as well.
>
> Last time I checked, git-http-pull did not utilize the pack
> dependency information, which indeed is wrong.
Is there documentation on the format?
> When it decides to fetch a pack instead of an asked-for object, it
> should check which commits the pack expects to have in your local
> repository and add them to its list of things to slurp.
It should work anyway, except that I messed up some logic in the parallel
pull stuff; when it finds it has something already, it ignores it
entirely, rather than processing it. The following patch fixes this.
---
[PATCH] Fix parallel pull dependancy tracking.
It didn't refetch an object it already had (good), but didn't process
it, either (bad). Synchronously process anything you already have.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
pull.c | 57 ++++++++++++++++++++++++++++++++-------------------------
1 files changed, 32 insertions(+), 25 deletions(-)
9b6b4b259c6b00d5b2502c158bc800d7623352bc
diff --git a/pull.c b/pull.c
--- a/pull.c
+++ b/pull.c
@@ -98,12 +98,38 @@ static int process_tag(struct tag *tag)
static struct object_list *process_queue = NULL;
static struct object_list **process_queue_end = &process_queue;
-static int process(unsigned char *sha1, const char *type)
+static int process_object(struct object *obj)
{
- struct object *obj;
- if (has_sha1_file(sha1))
+ if (obj->type == commit_type) {
+ if (process_commit((struct commit *)obj))
+ return -1;
+ return 0;
+ }
+ if (obj->type == tree_type) {
+ if (process_tree((struct tree *)obj))
+ return -1;
return 0;
- obj = lookup_object_type(sha1, type);
+ }
+ if (obj->type == blob_type) {
+ return 0;
+ }
+ if (obj->type == tag_type) {
+ if (process_tag((struct tag *)obj))
+ return -1;
+ return 0;
+ }
+ return error("Unable to determine requirements "
+ "of type %s for %s",
+ obj->type, sha1_to_hex(obj->sha1));
+}
+
+static int process(unsigned char *sha1, const char *type)
+{
+ struct object *obj = lookup_object_type(sha1, type);
+ if (has_sha1_file(sha1)) {
+ /* We already have it, so we should scan it now. */
+ return process_object(obj);
+ }
if (object_list_contains(process_queue, obj))
return 0;
object_list_insert(obj, process_queue_end);
@@ -134,27 +160,8 @@ static int loop(void)
return -1;
if (!obj->type)
parse_object(obj->sha1);
- if (obj->type == commit_type) {
- if (process_commit((struct commit *)obj))
- return -1;
- continue;
- }
- if (obj->type == tree_type) {
- if (process_tree((struct tree *)obj))
- return -1;
- continue;
- }
- if (obj->type == blob_type) {
- continue;
- }
- if (obj->type == tag_type) {
- if (process_tag((struct tag *)obj))
- return -1;
- continue;
- }
- return error("Unable to determine requirements "
- "of type %s for %s",
- obj->type, sha1_to_hex(obj->sha1));
+ if (process_object(obj))
+ return -1;
}
return 0;
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Re: git-http-pull broken in latest git
2005-08-11 23:38 ` [PATCH] " Daniel Barkalow
@ 2005-08-12 2:01 ` Junio C Hamano
2005-08-12 2:12 ` Daniel Barkalow
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2005-08-12 2:01 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Petr Baudis, git
Daniel Barkalow <barkalow@iabervon.org> writes:
> It should work anyway,...
That is true. Please forget about the "recommendation" to slurp
packs and not falling back on commit walker.
Thanks for the patch.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Re: git-http-pull broken in latest git
2005-08-12 2:01 ` Junio C Hamano
@ 2005-08-12 2:12 ` Daniel Barkalow
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Barkalow @ 2005-08-12 2:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, git
On Thu, 11 Aug 2005, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > It should work anyway,...
>
> That is true. Please forget about the "recommendation" to slurp
> packs and not falling back on commit walker.
>
> Thanks for the patch.
No problem; I had been wondering what the rest of those lines were about
anyway.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: git-http-pull broken in latest git
2005-08-11 23:21 ` Junio C Hamano
2005-08-11 23:38 ` [PATCH] " Daniel Barkalow
@ 2005-08-11 23:57 ` Junio C Hamano
2005-08-12 2:45 ` Petr Baudis
2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2005-08-11 23:57 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
Junio C Hamano <junkio@cox.net> writes:
> Last time I checked, git-http-pull did not utilize the pack
> dependency information, which indeed is wrong. When it decides
> to fetch a pack instead of an asked-for object, it should check
> which commits the pack expects to have in your local repository
> and add them to its list of things to slurp.
Oh, well, the above makes it sound as if I am blaming Daniel for
this, but the blame lies on me who did not document properly
what is going on, in except the comments in the source. So here
is an explanation.
A $GIT_DIR/objects/info/packs file looks like this:
P pack-3c5133604508466855453f3e609428f4bbba9131.pack
P pack-37cba29d3df65b160afabe769470f7857f98d729.pack
D 0
D 1 0
T 0 c1c774e7965ba08061c3fc7bc57aebc7eeb6b40f commit
T 0 d6602ec5194c87b0fc87103ca4d67251c76f233a tag
T 1 0918385dbd9656cab0d1d81ba7453d49bbc16250 tag
T 1 7ceca275d047c90c0c7d5afb13ab97efdf51bd6e tag
T 1 b3e9704ecdf48869f635f0aa99ddfb513f885aff tag
T 1 c5db5456ae3b0873fc659c19fafdde22313cc441 tag
T 1 f25a265a342aed6041ab0cc484224d9ca54b6f41 tag
P lines are list of packs, and they are implicitly numbered
starting from #0. 3c5133 pack is pack #0 and 37cba2 pack is
pack #1 in the above example.
D lines are pack dependencies. "D 0" says pack #0 does not
depend on any, "D 1 0" says pack #1 depends on pack #0.
T lines are what I call "pack edges". They are objects that are
not reachable from any other object contained in the same pack.
What this means is that if you have all of the listed objects
for a pack, downloading that pack is useless for you. This of
course requires that your local repository is not partial.
A D line says that pack #1 depends on pack #0. So if you decide
to slurp pack #1 because you wanted to have one object that is
not available as a plain file under objects/??/, you had better
make sure that you have all the objects available in pack #0.
One way to do so is to look at T lines for pack #0 and make sure
you have those "pack edge" objects in the local repository. If
you discover you do not have them, you either need to slurp pack
#0 as well, or start walking the commits from these pack edges.
If http-pull slurped pack #0, which does not depend on anything
else, this would obviously complete the process. However, even
if http-pull chose to walk the commits, if the remote repository
is fully packed, it would end up slurping pack #0. So either
way would work fine in theory, and the choice of which approach
to take really depends on "which one is more efficient".
The only case when walking the commits from pack edges could be
a win is when your local repository have most but not all of the
objects that are in pack #0 on the remote side, and the remote
side has those needed objects lying around unpacked, in addition
to having them in the pack #0. This is very unlikely because
(1) the remote side says pack #1 depends on pack #0, which means
pack #0 is older than pack #1, and (2) we ended up slurping pack
#1, which means objects in pack #1 have already been removed by
"git prune-packed" on the remote side. These two makes it very
likely that objects in pack #0 are already prune-packed on the
remote side. So my recommendation is to just slurp the depended
on pack, pack #0, in this case instead of adding the pack edge
objects to "to be commit-walked" list.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: git-http-pull broken in latest git
2005-08-11 23:21 ` Junio C Hamano
2005-08-11 23:38 ` [PATCH] " Daniel Barkalow
2005-08-11 23:57 ` Junio C Hamano
@ 2005-08-12 2:45 ` Petr Baudis
2005-08-12 3:17 ` [PATCH] " Daniel Barkalow
2 siblings, 1 reply; 11+ messages in thread
From: Petr Baudis @ 2005-08-12 2:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Dear diary, on Fri, Aug 12, 2005 at 01:21:46AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Petr Baudis <pasky@suse.cz> writes:
>
> > $ git-cat-file commit bf570303153902ec3d85570ed24515bcf8948848 | grep tree
> > tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > $ git-read-tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > fatal: failed to unpack tree object 41f10531f1799bbb31a1e0f7652363154ce96f45
>
> > Kaboom. I think the issue might be that the reference dependency tree
> > building is broken and it should've pulled the other pack as well.
>
> Last time I checked, git-http-pull did not utilize the pack
> dependency information, which indeed is wrong. When it decides
> to fetch a pack instead of an asked-for object, it should check
> which commits the pack expects to have in your local repository
> and add them to its list of things to slurp.
>
> A good news is that "git clone" as a whole works fine.
Yes, but cg-clone doesn't - it naively depended on the core git tools
actually, er.. working. ;-)
This became a nightmare to me by now - on two machines I tried to pull
to over HTTP, that failed miserably, and I got stuck until I applied
Daniel's patch there (and cleaned up after previous git-http-pulls).
So I have this packless git-pb repository and suspecting no evil, I pull
from you (thankfully I have .git/objects/pack there from some historical
pulls). I do a merge commit:
packed
... J
packed \
> M
/
... P
Now I want to pull on another machine. That pulls M and then fails since
I have no .git/objects/pack there, bummer. So I mkdir it, but get no
further w/o Daniel's patch - for git-*-pull, J is missing and that's it.
So I apply the patch, and get friendly
error: Unable to determine requirements of type (null) for M
and only after I delete M from the database, I finally succeed with
git-http-pull. (That was with --repair.) That's not good since this
might occur even naturally when the pull is interrupted.
With git-ssh-pull, the situation is even more vexing - it refuses to
fetch the packs for some reason yet unknown to me (I will debug it
tomorrow).
The git-*-pull tools appear yet rather fragile. :/
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone. -- Alan Cox
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Re: git-http-pull broken in latest git
2005-08-12 2:45 ` Petr Baudis
@ 2005-08-12 3:17 ` Daniel Barkalow
2005-08-12 5:20 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Barkalow @ 2005-08-12 3:17 UTC (permalink / raw)
To: Petr Baudis; +Cc: Junio C Hamano, git
On Fri, 12 Aug 2005, Petr Baudis wrote:
> Dear diary, on Fri, Aug 12, 2005 at 01:21:46AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> told me that...
> > Petr Baudis <pasky@suse.cz> writes:
> >
> > > $ git-cat-file commit bf570303153902ec3d85570ed24515bcf8948848 | grep tree
> > > tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > > $ git-read-tree 41f10531f1799bbb31a1e0f7652363154ce96f45
> > > fatal: failed to unpack tree object 41f10531f1799bbb31a1e0f7652363154ce96f45
> >
> > > Kaboom. I think the issue might be that the reference dependency tree
> > > building is broken and it should've pulled the other pack as well.
> >
> > Last time I checked, git-http-pull did not utilize the pack
> > dependency information, which indeed is wrong. When it decides
> > to fetch a pack instead of an asked-for object, it should check
> > which commits the pack expects to have in your local repository
> > and add them to its list of things to slurp.
> >
> > A good news is that "git clone" as a whole works fine.
>
> Yes, but cg-clone doesn't - it naively depended on the core git tools
> actually, er.. working. ;-)
>
> This became a nightmare to me by now - on two machines I tried to pull
> to over HTTP, that failed miserably, and I got stuck until I applied
> Daniel's patch there (and cleaned up after previous git-http-pulls).
>
> So I have this packless git-pb repository and suspecting no evil, I pull
> from you (thankfully I have .git/objects/pack there from some historical
> pulls). I do a merge commit:
>
> packed
> ... J
> packed \
> > M
> /
> ... P
>
> Now I want to pull on another machine. That pulls M and then fails since
> I have no .git/objects/pack there, bummer. So I mkdir it, but get no
> further w/o Daniel's patch - for git-*-pull, J is missing and that's it.
> So I apply the patch, and get friendly
>
> error: Unable to determine requirements of type (null) for M
>
> and only after I delete M from the database, I finally succeed with
> git-http-pull. (That was with --repair.) That's not good since this
> might occur even naturally when the pull is interrupted.
Insufficient testing on my part; patch at the end.
> With git-ssh-pull, the situation is even more vexing - it refuses to
> fetch the packs for some reason yet unknown to me (I will debug it
> tomorrow).
git-ssh-pull doesn't deal in packs; it gets individual objects out of
packs, which git-ssh-push (on the remote side) should be extracting.
Perhaps you have a git-ssh-push on the remote side that's before I make
packs work (it used to need to have the files for objects it was sending).
At some point, I have to revisit getting git-ssh-* to generate exactly the
required pack and transfer that, but that's an efficiency issue, not a
correctness one, and shouldn't be relevant to the problem you're having.
---
[PATCH] Also parse objects we already have
In the case where we don't know from context what type an object is, but
we don't have to fetch it, we need to parse it to determine the type
before processing it.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
pull.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
b8c382e76da25f45ff86176a6a6affdd9a28d60b
diff --git a/pull.c b/pull.c
--- a/pull.c
+++ b/pull.c
@@ -127,6 +127,7 @@ static int process(unsigned char *sha1,
{
struct object *obj = lookup_object_type(sha1, type);
if (has_sha1_file(sha1)) {
+ parse_object(sha1);
/* We already have it, so we should scan it now. */
return process_object(obj);
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Re: git-http-pull broken in latest git
2005-08-12 3:17 ` [PATCH] " Daniel Barkalow
@ 2005-08-12 5:20 ` Junio C Hamano
2005-08-12 15:37 ` Daniel Barkalow
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2005-08-12 5:20 UTC (permalink / raw)
To: Petr Baudis; +Cc: Daniel Barkalow, git
Daniel Barkalow <barkalow@iabervon.org> writes:
> Petr Baudis <pasky@suse.cz> writes:
>> Yes, but cg-clone doesn't - it naively depended on the core git tools
>> actually, er.. working. ;-)
Sorry about that. I used to have a wrapper to deal with packs
around http-pull before Daniel's pack enhancement, and yanking
it before really checking that enhanced http-pull actually
worked was my fault as well.
>> This became a nightmare to me by now - on two machines I tried to pull
>> to over HTTP, that failed miserably, and I got stuck until I applied
>> Daniel's patch there (and cleaned up after previous git-http-pulls).
I'll push one out with two patches from Daniel today in short
order. Currently running the final "make test" round.
> At some point, I have to revisit getting git-ssh-* to generate exactly the
> required pack and transfer that, but that's an efficiency issue, not a
> correctness one, and shouldn't be relevant to the problem you're having.
Wouldn't enhancing ssh-push to generate packs on the fly involve
reinventing send-pack and/or upload-pack?
If send-pack/receive-pack pair for the push side, and/or
fetch&clone-pack/upload-pack pair for the pull side does not
work as well as you would want, then ssh-push/pull pair may
still be a useful tool, at the same time that means send-pack
and upload-pack should be fixed to address the problem you have
with them. But if that is not the case, then it might be better
to declare that ssh-pull/push pair has outlived its usefulness.
The same thing can be said about local-pull to a lesser degree.
Lesser because people, including Pasky who said so on the list
recently, would like its hard-linking behaviour, and its not
exploding the existing packs, which send-pack and upload-pack
would not give. So I would rate local-pull higher than
ssh-push/pull on the priority scale if I were doing them.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Re: git-http-pull broken in latest git
2005-08-12 5:20 ` Junio C Hamano
@ 2005-08-12 15:37 ` Daniel Barkalow
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Barkalow @ 2005-08-12 15:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, git
On Thu, 11 Aug 2005, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > Petr Baudis <pasky@suse.cz> writes:
> >> Yes, but cg-clone doesn't - it naively depended on the core git tools
> >> actually, er.. working. ;-)
>
> Sorry about that. I used to have a wrapper to deal with packs
> around http-pull before Daniel's pack enhancement, and yanking
> it before really checking that enhanced http-pull actually
> worked was my fault as well.
It was actually the patches after the http-pull fixes (the ones for
parallelizing pull.c) that broke things; one advantage to fixing
local-pull would be that you can set up tests for it reasonably
effectively, which would have caught the regression.
> > At some point, I have to revisit getting git-ssh-* to generate exactly the
> > required pack and transfer that, but that's an efficiency issue, not a
> > correctness one, and shouldn't be relevant to the problem you're having.
>
> Wouldn't enhancing ssh-push to generate packs on the fly involve
> reinventing send-pack and/or upload-pack?
The idea is that you wouldn't have to identify what situation applied
yourself; you could just invoke git-ssh-pull/git-ssh-push, and it would
happen faster due to the compression benefits. The point is that scripts
can just pick which git-*-pull to use based on the format of the remote
branch address, without variation in behavior.
> The same thing can be said about local-pull to a lesser degree.
> Lesser because people, including Pasky who said so on the list
> recently, would like its hard-linking behaviour, and its not
> exploding the existing packs, which send-pack and upload-pack
> would not give. So I would rate local-pull higher than
> ssh-push/pull on the priority scale if I were doing them.
This is a higher priority, but writing more than bugfixes is unpleasent at
the moment due to my home workstation's monitor dying, so it'll probably
be next week that I'll get to it. The git-ssh-* stuff is longer-term,
since it works now, and isn't even all that slow with the overlapping
requests.
You could, actually, probably do the local-pull fix if you wanted. I seem
to recall that being your code originally; you just need to have fetch()
identify that an object is in a pack, copy/link/symlink the index and
pack instead of the object file, and add the pack to the list of
registered packs. I've mostly been failing to deal with reading an index
file that is in some directory that hasn't been registered as somewhere to
read from (i.e. the source repository).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 11+ messages in thread