Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 26, 2017, 03:47 UTC
Message-ID: <xmqqpo9afu3s.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <b063dec4-b9c4-7145-9b57-7df22ccbc655@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 10 quoted lines

> The existing code handles use cases where you want to read the
> exclusion list from a pathname in the worktree -- or from blob
> named in the index when the pathname is not populated (presumably
> because of the skip-worktree bit).
>
> I was wanting to add a more general case (and perhaps my commit
> message should be improved).  I want to be able to read it from
> a blob not necessarily associated with the current commit or
> not necessarily available on the local client, but yet known to
> exist.

Oh, I understand the above two paragraphs perfectly well, and I agree with you that such a helper to read from an arbitrary blob is a worthy thing to have. I was merely commenting on the fact that such a helper that is meant to be able to handle more general cases is not used to help the more specific case that we already have, which was a bit curious.

I guess the reason why it is not done is (besides expediency) because the model the new helper operates in would not fit well with the existing logic flow, where everything is loaded into core (either from the filesystem or from a blob) and then a common code parses and registers; the helper wants to do the reading (only) from the blob, the parsing and the registration all by itself, so there is not much that can be shared even if the existing code wanted to reuse what the helper offers.

The new helper mimicks the read_skip_worktree_file_from_index() codepath to massage the data it reads from the blob to buf[] but not really (e.g. even though it copies and pastes a lot, it forgets to call skip_utf8_bom(), for example). We may still want to see if we can share more so that we do not have to worry about these tiny differences between codepaths.

Show 8 quoted lines

> With my "add_excludes_from_blob_to_list()", we can request a
> blob-ish expression, such as "master:enlistments/foo".  In my
> later commits associated with clone and fetch, we can use this
> mechanism to let the client ask the server to filter using the
> blob associated with this blob-ish.  If the client has the blob
> (such as during a later fetch) and can resolve it, then it can
> and send the server the OID, but it can also send the blob-ish
> to the server and let it resolve it.

Security-minded people may want to keep an eye or two open for these later patches---extended SHA-1 expressions is a new attack surface we would want to carefully polish and protect.