Re: [PATCHv2] grep: use slash for path delimiter, not colon

From: Jeff King <peff@peff.net>
Date: Aug 26, 2013, 19:28 UTC
Message-ID: <20130826192826.GA23598@sigill.intra.peff.net>
In-Reply-To: <1377528372-31206-1-git-send-email-hordp@cisco.com>

On Mon, Aug 26, 2013 at 10:46:12AM -0400, Phil Hord wrote:

Show 6 quoted lines

> This version is a bit more deterministic and also adds a test.
> 
> It accepts the expense of examining the path argument again to 
> determine if it is a tree-ish + path rather than just a tree (commit).
> The get_sha1 call occurs one extra time for each tree-ish argument,
> so it's not expensive.

I don't like this approach in general because it lacks atomicity. IOW, the thing you are looking up may change between the two get_sha1 calls. You're _almost_ good here because you don't actually care what the second call returns, but only which features it _would_ have used. But you may see the second call fail because the ref doesn't exist anymore, or points to a different tree, and you will erroneously use ":" instead of "/".

I admit this is not that likely, but I'd really rather avoid introducing such races if we can.

> We avoid mucking with the object_array API this way, and also do not
> rely on the object-type to tell us anything about the way the object
> name was spelled.

Changing the object_array API would be hard, but I don't think we need to do it here. Can we simply stop using object_array to pass the list, and instead just have a custom list?

I'll see how painful that is.

-Peff