by the way, since I'm on the list now anyway:
is there any reason why you guys are calling
mark_parents_uninteresting() after add_parents_to_list() in
limit_list() (revision.c), considering the latter does everything the
former does? sorta trivial, but it's been bugging me...
On Sun, Jun 7, 2009 at 3:25 PM, Nick Edelen<sirnot@xxxxxxxxx> wrote:
> hi guys,
>
> I wasn't aware that Sam was going to upload this, otherwise I would've
> cleaned it up a bit. not a problem though; thanks for taking the time
> to do this Sam.
>
> @Johannes:
> the real reason I'm using start_async at all is because of shallow
> commit grafts. we could potentially add an interface to pack-objects
> to allow it to accept those types of commit grafts if you wanted.
> like, in addition to the --not flag we could add a --shallow flag or
> something... it'd actually be pretty easy to implement, but I don't
> know if you guys'd want that in pack-objects.
>
> @Nicolas:
> I'm using the --revs flag in pack-objects, which causes it to use
> get_object_list(). you'll notice, regardless of whether --thin is
> set, this function still calls
> mark_edges_uninteresting(revs.commits, &revs, show_edge);
> which sets uninteresting objects as preferred bases, which I'd think
> would create a thin pack. I could be wrong though...
>
> as I mentioned in the comment and above, it's an easy fix, but even
> then I wasn't sure what to do with commit grafts. as use_thin_pack
> seemed to be predominantly set on shallow interactions, I just didn't
> bother seperating the cases 'normal but thick pack' and 'shallow
> stuff'.
>
> (btw, I have a really cool idea for shallow/narrow/lazy interaction if
> you have the time. it basically uses 'fantom' placeholder objects to
> cover for missing blobs, so a clone/fetch would get all commits/trees
> but only retrieve blobs when a user specifies. I'll get a proof of
> concept done after this rev-cache project).
>
> thank you both for looking over this though.
>
> - Nick
>
> On Fri, Jun 5, 2009 at 6:51 PM, Nicolas Pitre<nico@xxxxxxx> wrote:
>> On Fri, 5 Jun 2009, sam@xxxxxxxxxx wrote:
>>
>>> instead of using the internal revision walker and piping object refs
>>> to pack-objects this patch passes only the revs to pack-objects, which
>>> in turn handles both enumeration and packing.
>>>
>>> Signed-off-by: Sam Vilain <sam@xxxxxxxxxx>
>>> ---
>>> Submitted on behalf of Nick in order to get wider feedback on this.
>>> This version passes the test suite.
>>>
>>> upload-pack.c | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
>>> 1 files changed, 45 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/upload-pack.c b/upload-pack.c
>>> index edc7861..7eda8fd 100644
>>> --- a/upload-pack.c
>>> +++ b/upload-pack.c
>>> @@ -155,13 +155,27 @@ static void create_pack_file(void)
>>> const char *argv[10];
>>> int arg = 0;
>>>
>>> - rev_list.proc = do_rev_list;
>>> - /* .data is just a boolean: any non-NULL value will do */
>>> - rev_list.data = create_full_pack ? &rev_list : NULL;
>>> - if (start_async(&rev_list))
>>> - die("git upload-pack: unable to fork git-rev-list");
>>> -
>>> - argv[arg++] = "pack-objects";
>>> + /* sending rev params to pack-objects directly is great, but unfortunately pack-objects
>>> + * has no way of turning off thin pack generation. this would be a relatively simple
>>> + * addition, but as we also have to deal with shallow grafts and all it's simplest to
>>> + * just resort to piping object refs.
>>> + */
>>
>> What's that? Where did you get that?
>>
>> The way to not generate a thin pack is to not specify --thin to
>> pack-objects. If you get a thin pack without specifying --thin then
>> this is a bug that needs to be fixed first.
>>
>>> + if (!use_thin_pack) {
>>> + rev_list.proc = do_rev_list;
>>> + /* .data is just a boolean: any non-NULL value will do */
>>> + rev_list.data = create_full_pack ? &rev_list : NULL;
>>> + if (start_async(&rev_list))
>>> + die("git upload-pack: unable to fork git-rev-list");
>>> +
>>> + argv[arg++] = "pack-objects";
>>> + } else {
>>> + argv[arg++] = "pack-objects";
>>> + argv[arg++] = "--revs";
>>> + argv[arg++] = "--include-tag";
>>
>> Shouldn't this be specified only if corresponding capability was
>> provided by the client?
>>
>>
>> Nicolas
>>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html