Re: [PATCH] rebase --merge: optionally skip upstreamed commits

From: Jonathan Tan <jonathantanmy@google.com>
Date: Mar 17, 2020, 03:03 UTC
Message-ID: <20200317030324.190751-1-jonathantanmy@google.com>
In-Reply-To: <CABPp-BGcQHT5PkqYw3YRWOv8kbiSumJ+YQ3owkd_7viKWs3q7A@mail.gmail.com>

Show 9 quoted lines

> > We've made changes in other places (e.g. opening an editor for merge
> > or rebase, push.default, etc.); is there any reason a similar change
> > wouldn't be justified here?
> 
> After another day of thought, and my attempt to figure out the reason
> above, perhaps my assumptions about the reason behind the original
> behavior, any my assumptions about the sanity of switching the default
> might not be as grounded as I thought.  Thus, my worries about yet
> another flag may be overstated as well, at least in this case.

Thanks for the further investigation. For what it's worth, I do agree that we should think of the cost of introducing an option before introducing it. Admittedly in my write-up [1], I mentioned my investigation into speeding up existing behavior enough to not need my new feature, but didn't mention the possibility of just changing the existing behavior. (But it seemed to me that this existing behavior is presented as a desirable feature, so I didn't think of changing it.)

[1] https://lore.kernel.org/git/20200312180427.192096-1-jonathantanmy@google.com/

This question (from your other email [2]) is probably moot if we're going to introduce this option anyway, but just to answer it:

Show 14 quoted lines

> >  If we
> > move in a direction where not only blobs but also trees (or even
> > commits) are omitted, we'll definitely want this new option.
> 
> Why would this new option be needed if we omitted trees?   If trees
> are omitted based on something like sparse-checkouts, then they are
> omitted based on path; shouldn't we be able to avoid walking trees
> just by noting they modified some path outside a requested sparse
> checkout?
> 
> I want grep, log, etc. to behave within the cone of a sparse checkout,
> which means that I need trees of upstream branches within the relevant
> paths anyway.  But theoretically I should certainly be able to avoid
> walking trees outside those paths.

I haven't given much thought to it, but the diffing mechanism will need to receive a whitelist of paths and, if it ever needs to traverse outside those, will need to abort with "there's a difference outside this whitelist". I don't know if it supports such a thing now.

[2] https://lore.kernel.org/git/CABPp-BE83ZhezkgmwatxAhqh4rptMUggcjSwBeiSByyPTUi6Lw@mail.gmail.com/

I'll give some time for others to respond, and will send out a v2 with your and Taylor's suggestions implemented.