On Mon, Jul 23, 2018 at 11:52:49AM -0700, Stefan Beller wrote:
> > +DELTA ISLANDS
> > +-------------
> [...]
>
> I had to read all of this [background information] to understand the
> concept and I think it is misnamed, as my gut instinct first told me
> to have deltas only "within an island and no island hopping is allowed".
> (This message reads a bit like a commit message, not as documentation
> as it is long winded, too).
I'm not sure if I'm parsing your sentence correctly, but the reason I
called them "islands" is exactly that you'd have deltas within an island
and want to forbid island hopping. So I wasn't sure if you were saying
"that's what I think, but not how the feature works".
There _is_ a tricky thing, which is that a given object is going to
appear in many islands. So the rule is really "you cannot hop to a base
that is not in all of your islands".
> What about renaming this feature to
>
> [pack]
> excludePartialReach = refs/virtual/[0-9]]+/tags/
>
> "By setting `pack.excludePartialReach`, object deltafication is
> prohibited for objects that are not reachable from all
> manifestations of the given regex"
>
> Cryptic, but it explains it in my mind in a shorter, more concise way. ;-)
So I'm hopelessly biased at this point, having developed and worked with
the feature under the "island" name for several years. But I find your
name and explanation pretty confusing. :)
Worse, though, it does not have any noun to refer to the reachable set.
The regex capture and the island names are an important part of the
feature, because it lets you make a single island out of:
refs/virtual/([0-9]+)/heads
refs/virtual/([0-9]+)/tags
but exclude:
refs/virtual/([0-9]+)/(foo)
which goes into its own island ("123-foo" instead of "123"). So what's
the equivalent nomenclature to "island name"?
> > @@ -3182,6 +3225,8 @@ int cmd_pack_objects(int argc, const char **argv,
> > const char *prefix)
> > option_parse_missing_action },
> > OPT_BOOL(0, "exclude-promisor-objects",
> > &exclude_promisor_objects,
> > N_("do not pack objects in promisor packfiles")),
> > + OPT_BOOL(0, "delta-islands", &use_delta_islands,
> > + N_("enable islands for delta compression")),
>
> We enable this feature, but we disallow certain patterns to be used in
> packing,
> so it sounds weird to me as we tell Git to *not* explore the full design
> space,
> so we're not enabling it, but rather restricting it?
Yeah, perhaps "respect islands during delta compression" makes more sense.
-Peff
