Re: [PATCH 4/4] fast-import: only store commit objects

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 4, 2013, 00:01 UTC
Message-ID: <CAMP44s3CPNyrWQoXoNDGaKfH6qniosfFMdjWVD_8FKD+Vge_8Q@mail.gmail.com>
In-Reply-To: <7vr4hnwru7.fsf@alter.siamese.dyndns.org>

On Fri, May 3, 2013 at 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> A safe and sane approach may be to teach these an option to tell
>>> them to omit non-commits or to emit all kinds, and make remote-bzr
>>> use that to exclude non-commits.
>>
>> This has nothing to do with remote-bzr, or any remote helper. These
>> objects are not useful, not even to 'git fast-export'.
>>
>>> If the defaults is matched to the
>>> current behaviour, nobody will get hurt
>>
>> Changing nothing always ensures that nobody will get hurt, but that
>> doesn't improve anything either.
>
> I do not quite follow.  Allowing existing code to depend on old
> behaviour, while letting new code that knows it does not need
> anything but commits, will get the best of both worlds. The new code
> will definitely improve (otherwise you won't be writing these
> patches), and the old code will not get hurt. Where is this "doesn't
> improve anything" coming from?

So let's suppose we add a new 'feature no-blob-store' to 'git fast-import', and some clients of 'git fast-import' enable it, and benefit from it.

How does that benefit the rest of fast-import clients? You are worrying about clients that most likely don't exist, and don't let existing real clients benefit for free. This is like premature optimization.

But whatever, let's keep writing and discarding these blobs, at least the previous patches do make such operations fast.

You can drop this patch, because I'm not going to write code for clients that don't exist. And it seems clear that even if I investigate client apps of 'git fast-import' and I'm unable to find a single one that utilizes blobs, you still wouldn't apply this patch. So there's no point in investigating what are the *actual* users, if all we are every going to do is worry about hypothetical ones.

My main interest is the previous patches, if anybody has any issues with the way this patch is handled, they can either work on the patch itself, or start a new thread in which I will not participate, I will rather concentrate on issues that affect *real* users, and have real fixes reachable today, and the previous patches in this series fit that bill.

Cheers.

-- 
Felipe Contreras