Re: [PATCH v5 13/40] external odb: add 'put_raw_obj' support

From: Christian Couder <christian.couder@gmail.com>
Date: Sep 14, 2017, 09:17 UTC
Message-ID: <CAP8UFD2+-0DYRO+PN+Ertn3TvoXEpghWfv=-He9+0kVSA=TNxA@mail.gmail.com>
In-Reply-To: <xmqqbmnwxwfw.fsf@gitster.mtv.corp.google.com>

On Thu, Aug 3, 2017 at 9:50 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> Add support for a 'put_raw_obj' capability/instruction to send new
>> objects to an external odb. Objects will be sent as they are (in
>> their 'raw' format). They will not be converted to Git objects.
>>
>> For now any new Git object (blob, tree, commit, ...) would be sent
>> if 'put_raw_obj' is supported by an odb helper. This is not a great
>> default, but let's leave it to following commits to tweak that.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>
> I thought in an earlier step that I saw this thing initialized in
> the codepath that adds alternate object stores, which are read-only
> places we "borrow" from.  Being able to write into it is good, but
> conceptually it no longer feels correct to initialize it from the
> alternate object database initialization codepath.
>
> Another way to say it is that an object store, whether it is local
> or external, is not "alt" if it will result in storing new objects
> we locally create.  It's just an extension of our local object
> store.

I guess you are talking about the following code in "[PATCH v5 10/40] Add initial external odb support":

+void prepare_external_alt_odb(void)
+{
+       static int linked_external;
+       const char *path;
+
+       if (linked_external)
+               return;
+
+       path = external_odb_root();
+       if (!access(path, F_OK)) {
+               link_alt_odb_entry(path, NULL, 0, "");
+               linked_external = 1;
+       }
+}
+
 void prepare_alt_odb(void)
 {
        const char *alt;

@@ -650,6 +666,7 @@ void prepare_alt_odb(void)
        link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);

        read_info_alternates(get_object_directory(), 0);
+       prepare_external_alt_odb();
 }

Would it be ok if I do the following:

- rename prepare_external_alt_odb() to just prepare_external_odb(), as
this would avoid confusion between alt_odbs and external odbs
- remove the call to prepare_external_odb() in prepare_alt_odb()
- add a prepare_alt_and_external_odb() that just calls
prepare_alt_odb() and then prepare_external_odb()
- replace all the calls to prepare_alt_odb() with calls to
prepare_alt_and_external_odb()

?