From: Jeff Hostetler Date: Thu, 26 Oct 2017 18:11:25 GMT Subject: Re: [PATCH 01/13] dir: allow exclusions from blob in addition to file Message-ID: In-Reply-To: On 10/25/2017 11:47 PM, Junio C Hamano wrote: > Jeff Hostetler writes: > >> 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. I'm going to extract this commit, refactor it to try to share more code with the existing read_skip_worktree_file_from_index() and submit it as a separate patch series so that we can discuss it in isolation without the rest of the partial-clone code getting in the way. The call to skip_utf8_bom() was captured in the new add_excludes_from_buffer() routine that both add_excludes() and my new add_excludes_from_blob_to_list() call. > >> 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. >